[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
This commit is contained in:
Valeriy Savchenko 2020-08-12 11:04:56 +03:00
parent 63863451d1
commit 9cbfdde2ea
11 changed files with 126 additions and 71 deletions

View File

@ -79,11 +79,11 @@ public:
};
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 @@ public:
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 @@ public:
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();

View File

@ -233,7 +233,7 @@ public:
const LocationContext *LCtx,
unsigned count);
DefinedSVal getMemberPointer(const DeclaratorDecl *DD);
DefinedSVal getMemberPointer(const NamedDecl *ND);
DefinedSVal getFunctionPointer(const FunctionDecl *func);

View File

@ -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 @@ public:
bool isNullMemberPointer() const;
const DeclaratorDecl *getDecl() const;
const NamedDecl *getDecl() const;
template<typename AdjustedDecl>
const AdjustedDecl *getDeclAs() const {

View File

@ -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*

View File

@ -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)) {

View File

@ -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;
}

View File

@ -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) {

View File

@ -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();
}

View File

@ -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);
}
}

View File

@ -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;
}

View File

@ -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}}
// 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}}
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 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