[clang] 9cbfdde - [analyzer] Fix crash with pointer to members values
Valeriy Savchenko via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 13 08:04:30 PDT 2020
Author: Valeriy Savchenko
Date: 2020-08-13T18:03:59+03:00
New Revision: 9cbfdde2ea060d7e51fd2637f63eaa74b8d92848
URL: https://github.com/llvm/llvm-project/commit/9cbfdde2ea060d7e51fd2637f63eaa74b8d92848
DIFF: https://github.com/llvm/llvm-project/commit/9cbfdde2ea060d7e51fd2637f63eaa74b8d92848.diff
LOG: [analyzer] Fix crash with pointer to members values
This fix unifies all of the different ways we handled pointer to
members into one. The crash was caused by the fact that the type
of pointer-to-member values was `void *`, and while this works
for the vast majority of cases it breaks when we actually need
to explain the path for the report.
rdar://problem/64202361
Differential Revision: https://reviews.llvm.org/D85817
Added:
clang/test/Analysis/PR46264.cpp
Modified:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
clang/lib/StaticAnalyzer/Core/SVals.cpp
clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
clang/test/Analysis/pointer-to-member.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
index a001c0dc7030..142b1ab11750 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -79,11 +79,11 @@ class LazyCompoundValData : public llvm::FoldingSetNode {
};
class PointerToMemberData : public llvm::FoldingSetNode {
- const DeclaratorDecl *D;
+ const NamedDecl *D;
llvm::ImmutableList<const CXXBaseSpecifier *> L;
public:
- PointerToMemberData(const DeclaratorDecl *D,
+ PointerToMemberData(const NamedDecl *D,
llvm::ImmutableList<const CXXBaseSpecifier *> L)
: D(D), L(L) {}
@@ -92,11 +92,11 @@ class PointerToMemberData : public llvm::FoldingSetNode {
iterator begin() const { return L.begin(); }
iterator end() const { return L.end(); }
- static void Profile(llvm::FoldingSetNodeID& ID, const DeclaratorDecl *D,
+ static void Profile(llvm::FoldingSetNodeID &ID, const NamedDecl *D,
llvm::ImmutableList<const CXXBaseSpecifier *> L);
- void Profile(llvm::FoldingSetNodeID& ID) { Profile(ID, D, L); }
- const DeclaratorDecl *getDeclaratorDecl() const {return D;}
+ void Profile(llvm::FoldingSetNodeID &ID) { Profile(ID, D, L); }
+ const NamedDecl *getDeclaratorDecl() const { return D; }
llvm::ImmutableList<const CXXBaseSpecifier *> getCXXBaseList() const {
return L;
@@ -236,9 +236,9 @@ class BasicValueFactory {
const LazyCompoundValData *getLazyCompoundValData(const StoreRef &store,
const TypedValueRegion *region);
- const PointerToMemberData *getPointerToMemberData(
- const DeclaratorDecl *DD,
- llvm::ImmutableList<const CXXBaseSpecifier *> L);
+ const PointerToMemberData *
+ getPointerToMemberData(const NamedDecl *ND,
+ llvm::ImmutableList<const CXXBaseSpecifier *> L);
llvm::ImmutableList<SVal> getEmptySValList() {
return SValListFactory.getEmptyList();
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index 35ebefdc00d6..4ea85f9730bb 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -233,7 +233,7 @@ class SValBuilder {
const LocationContext *LCtx,
unsigned count);
- DefinedSVal getMemberPointer(const DeclaratorDecl *DD);
+ DefinedSVal getMemberPointer(const NamedDecl *ND);
DefinedSVal getFunctionPointer(const FunctionDecl *func);
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
index a561ac67bf78..bb295ab591d4 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -520,7 +520,7 @@ class PointerToMember : public NonLoc {
public:
using PTMDataType =
- llvm::PointerUnion<const DeclaratorDecl *, const PointerToMemberData *>;
+ llvm::PointerUnion<const NamedDecl *, const PointerToMemberData *>;
const PTMDataType getPTMData() const {
return PTMDataType::getFromOpaqueValue(const_cast<void *>(Data));
@@ -528,7 +528,7 @@ class PointerToMember : public NonLoc {
bool isNullMemberPointer() const;
- const DeclaratorDecl *getDecl() const;
+ const NamedDecl *getDecl() const;
template<typename AdjustedDecl>
const AdjustedDecl *getDeclAs() const {
diff --git a/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp b/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
index 73f057f09550..d1f5ac02278f 100644
--- a/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -42,7 +42,7 @@ void LazyCompoundValData::Profile(llvm::FoldingSetNodeID& ID,
}
void PointerToMemberData::Profile(
- llvm::FoldingSetNodeID& ID, const DeclaratorDecl *D,
+ llvm::FoldingSetNodeID &ID, const NamedDecl *D,
llvm::ImmutableList<const CXXBaseSpecifier *> L) {
ID.AddPointer(D);
ID.AddPointer(L.getInternalPointer());
@@ -159,17 +159,17 @@ BasicValueFactory::getLazyCompoundValData(const StoreRef &store,
}
const PointerToMemberData *BasicValueFactory::getPointerToMemberData(
- const DeclaratorDecl *DD, llvm::ImmutableList<const CXXBaseSpecifier *> L) {
+ const NamedDecl *ND, llvm::ImmutableList<const CXXBaseSpecifier *> L) {
llvm::FoldingSetNodeID ID;
- PointerToMemberData::Profile(ID, DD, L);
+ PointerToMemberData::Profile(ID, ND, L);
void *InsertPos;
PointerToMemberData *D =
PointerToMemberDataSet.FindNodeOrInsertPos(ID, InsertPos);
if (!D) {
- D = (PointerToMemberData*) BPAlloc.Allocate<PointerToMemberData>();
- new (D) PointerToMemberData(DD, L);
+ D = (PointerToMemberData *)BPAlloc.Allocate<PointerToMemberData>();
+ new (D) PointerToMemberData(ND, L);
PointerToMemberDataSet.InsertNode(D, InsertPos);
}
@@ -180,25 +180,24 @@ const PointerToMemberData *BasicValueFactory::accumCXXBase(
llvm::iterator_range<CastExpr::path_const_iterator> PathRange,
const nonloc::PointerToMember &PTM) {
nonloc::PointerToMember::PTMDataType PTMDT = PTM.getPTMData();
- const DeclaratorDecl *DD = nullptr;
+ const NamedDecl *ND = nullptr;
llvm::ImmutableList<const CXXBaseSpecifier *> PathList;
- if (PTMDT.isNull() || PTMDT.is<const DeclaratorDecl *>()) {
- if (PTMDT.is<const DeclaratorDecl *>())
- DD = PTMDT.get<const DeclaratorDecl *>();
+ if (PTMDT.isNull() || PTMDT.is<const NamedDecl *>()) {
+ if (PTMDT.is<const NamedDecl *>())
+ ND = PTMDT.get<const NamedDecl *>();
PathList = CXXBaseListFactory.getEmptyList();
} else { // const PointerToMemberData *
- const PointerToMemberData *PTMD =
- PTMDT.get<const PointerToMemberData *>();
- DD = PTMD->getDeclaratorDecl();
+ const PointerToMemberData *PTMD = PTMDT.get<const PointerToMemberData *>();
+ ND = PTMD->getDeclaratorDecl();
PathList = PTMD->getCXXBaseList();
}
for (const auto &I : llvm::reverse(PathRange))
PathList = prependCXXBase(I, PathList);
- return getPointerToMemberData(DD, PathList);
+ return getPointerToMemberData(ND, PathList);
}
const llvm::APSInt*
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 7888029399f1..27b3e7ddb44e 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2530,16 +2530,8 @@ void ExprEngine::VisitCommonDeclRefExpr(const Expr *Ex, const NamedDecl *D,
return;
}
if (isa<FieldDecl>(D) || isa<IndirectFieldDecl>(D)) {
- // FIXME: Compute lvalue of field pointers-to-member.
- // Right now we just use a non-null void pointer, so that it gives proper
- // results in boolean contexts.
- // FIXME: Maybe delegate this to the surrounding operator&.
- // Note how this expression is lvalue, however pointer-to-member is NonLoc.
- SVal V = svalBuilder.conjureSymbolVal(Ex, LCtx, getContext().VoidPtrTy,
- currBldrCtx->blockCount());
- state = state->assume(V.castAs<DefinedOrUnknownSVal>(), true);
- Bldr.generateNode(Ex, Pred, state->BindExpr(Ex, LCtx, V), nullptr,
- ProgramPoint::PostLValueKind);
+ // Delegate all work related to pointer to members to the surrounding
+ // operator&.
return;
}
if (isa<BindingDecl>(D)) {
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index c5e38cc7423d..507400c705b9 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -991,10 +991,11 @@ void ExprEngine::VisitUnaryOperator(const UnaryOperator* U, ExplodedNode *Pred,
if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Ex)) {
const ValueDecl *VD = DRE->getDecl();
- if (isa<CXXMethodDecl>(VD) || isa<FieldDecl>(VD)) {
+ if (isa<CXXMethodDecl>(VD) || isa<FieldDecl>(VD) ||
+ isa<IndirectFieldDecl>(VD)) {
ProgramStateRef State = (*I)->getState();
const LocationContext *LCtx = (*I)->getLocationContext();
- SVal SV = svalBuilder.getMemberPointer(cast<DeclaratorDecl>(VD));
+ SVal SV = svalBuilder.getMemberPointer(cast<NamedDecl>(VD));
Bldr.generateNode(U, *I, State->BindExpr(U, LCtx, SV));
break;
}
diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
index 5b6b6973b310..32d2a3e30708 100644
--- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -236,10 +236,11 @@ SValBuilder::getDerivedRegionValueSymbolVal(SymbolRef parentSymbol,
return nonloc::SymbolVal(sym);
}
-DefinedSVal SValBuilder::getMemberPointer(const DeclaratorDecl *DD) {
- assert(!DD || isa<CXXMethodDecl>(DD) || isa<FieldDecl>(DD));
+DefinedSVal SValBuilder::getMemberPointer(const NamedDecl *ND) {
+ assert(!ND || isa<CXXMethodDecl>(ND) || isa<FieldDecl>(ND) ||
+ isa<IndirectFieldDecl>(ND));
- if (const auto *MD = dyn_cast_or_null<CXXMethodDecl>(DD)) {
+ if (const auto *MD = dyn_cast_or_null<CXXMethodDecl>(ND)) {
// Sema treats pointers to static member functions as have function pointer
// type, so return a function pointer for the method.
// We don't need to play a similar trick for static member fields
@@ -249,7 +250,7 @@ DefinedSVal SValBuilder::getMemberPointer(const DeclaratorDecl *DD) {
return getFunctionPointer(MD);
}
- return nonloc::PointerToMember(DD);
+ return nonloc::PointerToMember(ND);
}
DefinedSVal SValBuilder::getFunctionPointer(const FunctionDecl *func) {
diff --git a/clang/lib/StaticAnalyzer/Core/SVals.cpp b/clang/lib/StaticAnalyzer/Core/SVals.cpp
index 465800fa67fc..c4d5377f3bae 100644
--- a/clang/lib/StaticAnalyzer/Core/SVals.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SVals.cpp
@@ -157,18 +157,18 @@ bool nonloc::PointerToMember::isNullMemberPointer() const {
return getPTMData().isNull();
}
-const DeclaratorDecl *nonloc::PointerToMember::getDecl() const {
+const NamedDecl *nonloc::PointerToMember::getDecl() const {
const auto PTMD = this->getPTMData();
if (PTMD.isNull())
return nullptr;
- const DeclaratorDecl *DD = nullptr;
- if (PTMD.is<const DeclaratorDecl *>())
- DD = PTMD.get<const DeclaratorDecl *>();
+ const NamedDecl *ND = nullptr;
+ if (PTMD.is<const NamedDecl *>())
+ ND = PTMD.get<const NamedDecl *>();
else
- DD = PTMD.get<const PointerToMemberData *>()->getDeclaratorDecl();
+ ND = PTMD.get<const PointerToMemberData *>()->getDeclaratorDecl();
- return DD;
+ return ND;
}
//===----------------------------------------------------------------------===//
@@ -185,14 +185,14 @@ nonloc::CompoundVal::iterator nonloc::CompoundVal::end() const {
nonloc::PointerToMember::iterator nonloc::PointerToMember::begin() const {
const PTMDataType PTMD = getPTMData();
- if (PTMD.is<const DeclaratorDecl *>())
+ if (PTMD.is<const NamedDecl *>())
return {};
return PTMD.get<const PointerToMemberData *>()->begin();
}
nonloc::PointerToMember::iterator nonloc::PointerToMember::end() const {
const PTMDataType PTMD = getPTMData();
- if (PTMD.is<const DeclaratorDecl *>())
+ if (PTMD.is<const NamedDecl *>())
return {};
return PTMD.get<const PointerToMemberData *>()->end();
}
diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index a64ed78ac345..facadaf1225f 100644
--- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -1106,19 +1106,28 @@ SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state,
}
SVal SimpleSValBuilder::evalBinOpLN(ProgramStateRef state,
- BinaryOperator::Opcode op,
- Loc lhs, NonLoc rhs, QualType resultTy) {
+ BinaryOperator::Opcode op, Loc lhs,
+ NonLoc rhs, QualType resultTy) {
if (op >= BO_PtrMemD && op <= BO_PtrMemI) {
if (auto PTMSV = rhs.getAs<nonloc::PointerToMember>()) {
if (PTMSV->isNullMemberPointer())
return UndefinedVal();
- if (const FieldDecl *FD = PTMSV->getDeclAs<FieldDecl>()) {
+
+ auto getFieldLValue = [&](const auto *FD) -> SVal {
SVal Result = lhs;
for (const auto &I : *PTMSV)
Result = StateMgr.getStoreManager().evalDerivedToBase(
- Result, I->getType(),I->isVirtual());
+ Result, I->getType(), I->isVirtual());
+
return state->getLValue(FD, Result);
+ };
+
+ if (const auto *FD = PTMSV->getDeclAs<FieldDecl>()) {
+ return getFieldLValue(FD);
+ }
+ if (const auto *FD = PTMSV->getDeclAs<IndirectFieldDecl>()) {
+ return getFieldLValue(FD);
}
}
diff --git a/clang/test/Analysis/PR46264.cpp b/clang/test/Analysis/PR46264.cpp
new file mode 100644
index 000000000000..9f4d34f0fc29
--- /dev/null
+++ b/clang/test/Analysis/PR46264.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+// rdar://problem/64202361
+
+struct A {
+ int a;
+ struct {
+ struct {
+ int b;
+ union {
+ int c;
+ };
+ };
+ };
+};
+
+int testCrash() {
+ int *x = 0;
+ int A::*ap = &A::a;
+
+ if (ap) // no crash
+ return *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+
+ return 10;
+}
+
+int testIndirectCrash() {
+ int *x = 0;
+ int A::*cp = &A::c;
+
+ if (cp) // no crash
+ return *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+
+ return 10;
+}
diff --git a/clang/test/Analysis/pointer-to-member.cpp b/clang/test/Analysis/pointer-to-member.cpp
index 65882527d2da..27cad4ed98aa 100644
--- a/clang/test/Analysis/pointer-to-member.cpp
+++ b/clang/test/Analysis/pointer-to-member.cpp
@@ -233,39 +233,57 @@ void double_diamond() {
namespace testAnonymousMember {
struct A {
+ int a;
struct {
- int x;
+ int b;
+ int c;
};
struct {
struct {
- int y;
+ int d;
+ int e;
};
};
struct {
union {
- int z;
+ int f;
};
};
};
void test() {
- clang_analyzer_eval(&A::x); // expected-warning{{TRUE}}
- clang_analyzer_eval(&A::y); // expected-warning{{TRUE}}
- clang_analyzer_eval(&A::z); // expected-warning{{TRUE}}
+ clang_analyzer_eval(&A::a); // expected-warning{{TRUE}}
+ clang_analyzer_eval(&A::b); // expected-warning{{TRUE}}
+ clang_analyzer_eval(&A::c); // expected-warning{{TRUE}}
+ clang_analyzer_eval(&A::d); // expected-warning{{TRUE}}
+ clang_analyzer_eval(&A::e); // expected-warning{{TRUE}}
+ clang_analyzer_eval(&A::f); // expected-warning{{TRUE}}
+
+ int A::*ap = &A::a,
+ A::*bp = &A::b,
+ A::*cp = &A::c,
+ A::*dp = &A::d,
+ A::*ep = &A::e,
+ A::*fp = &A::f;
+
+ clang_analyzer_eval(ap); // expected-warning{{TRUE}}
+ clang_analyzer_eval(bp); // expected-warning{{TRUE}}
+ clang_analyzer_eval(cp); // expected-warning{{TRUE}}
+ clang_analyzer_eval(dp); // expected-warning{{TRUE}}
+ clang_analyzer_eval(ep); // expected-warning{{TRUE}}
+ clang_analyzer_eval(fp); // expected-warning{{TRUE}}
- // FIXME: These should be true.
- int A::*l = &A::x, A::*m = &A::y, A::*n = &A::z;
- clang_analyzer_eval(l); // expected-warning{{UNKNOWN}}
- clang_analyzer_eval(m); // expected-warning{{UNKNOWN}}
- clang_analyzer_eval(n); // expected-warning{{UNKNOWN}}
-
- // FIXME: These should be true as well.
A a;
- a.x = 1;
- clang_analyzer_eval(a.*l == 1); // expected-warning{{UNKNOWN}}
- a.y = 2;
- clang_analyzer_eval(a.*m == 2); // expected-warning{{UNKNOWN}}
- a.z = 3;
- clang_analyzer_eval(a.*n == 3); // expected-warning{{UNKNOWN}}
+ a.a = 1;
+ a.b = 2;
+ a.c = 3;
+ a.d = 4;
+ a.e = 5;
+
+ clang_analyzer_eval(a.*ap == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(a.*bp == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(a.*cp == 3); // expected-warning{{TRUE}}
+ clang_analyzer_eval(a.*dp == 4); // expected-warning{{TRUE}}
+ clang_analyzer_eval(a.*ep == 5); // expected-warning{{TRUE}}
}
-} // end of testAnonymousMember namespace
+} // namespace testAnonymousMember
More information about the cfe-commits
mailing list