diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h @@ -258,9 +258,9 @@ return CXXBaseListFactory.add(CBS, L); } - const PointerToMemberData *accumCXXBase( - llvm::iterator_range PathRange, - const nonloc::PointerToMember &PTM); + const PointerToMemberData * + accumCXXBase(llvm::iterator_range PathRange, + const nonloc::PointerToMember &PTM, const clang::CastKind &kind); const llvm::APSInt* evalAPSInt(BinaryOperator::Opcode Op, const llvm::APSInt& V1, diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h @@ -514,7 +514,8 @@ /// This SVal is represented by a DeclaratorDecl which can be a member function /// pointer or a member data pointer and a list of CXXBaseSpecifiers. This list /// is required to accumulate the pointer-to-member cast history to figure out -/// the correct subobject field. +/// the correct subobject field. In particular, implicit casts grow this list +/// and explicit casts like static_cast shrink this list. class PointerToMember : public NonLoc { friend class ento::SValBuilder; diff --git a/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp b/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp --- a/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp +++ b/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp @@ -21,6 +21,7 @@ #include "llvm/ADT/FoldingSet.h" #include "llvm/ADT/ImmutableList.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallPtrSet.h" #include #include #include @@ -176,28 +177,73 @@ return D; } +LLVM_ATTRIBUTE_UNUSED bool hasNoRepeatedElements( + llvm::ImmutableList BaseSpecList) { + llvm::SmallPtrSet BaseSpecSeen; + for (const CXXBaseSpecifier *BaseSpec : BaseSpecList) { + QualType BaseType = BaseSpec->getType(); + // Check whether inserted + if (!BaseSpecSeen.insert(BaseType).second) + return false; + } + return true; +} + const PointerToMemberData *BasicValueFactory::accumCXXBase( llvm::iterator_range PathRange, - const nonloc::PointerToMember &PTM) { + const nonloc::PointerToMember &PTM, const CastKind &kind) { + assert((kind == CK_DerivedToBaseMemberPointer || + kind == CK_BaseToDerivedMemberPointer || + kind == CK_ReinterpretMemberPointer) && + "accumCXXBase called with wrong CastKind"); nonloc::PointerToMember::PTMDataType PTMDT = PTM.getPTMData(); const NamedDecl *ND = nullptr; - llvm::ImmutableList PathList; + llvm::ImmutableList BaseSpecList; if (PTMDT.isNull() || PTMDT.is()) { if (PTMDT.is()) ND = PTMDT.get(); - PathList = CXXBaseListFactory.getEmptyList(); - } else { // const PointerToMemberData * + BaseSpecList = CXXBaseListFactory.getEmptyList(); + } else { const PointerToMemberData *PTMD = PTMDT.get(); ND = PTMD->getDeclaratorDecl(); - PathList = PTMD->getCXXBaseList(); + BaseSpecList = PTMD->getCXXBaseList(); } - for (const auto &I : llvm::reverse(PathRange)) - PathList = prependCXXBase(I, PathList); - return getPointerToMemberData(ND, PathList); + assert(hasNoRepeatedElements(BaseSpecList) && + "CXXBaseSpecifier list of PointerToMemberData must not have repeated " + "elements"); + + if (kind == CK_DerivedToBaseMemberPointer) { + // Here we pop off matching CXXBaseSpecifiers from BaseSpecList. + // Because, CK_DerivedToBaseMemberPointer comes from a static_cast and + // serves to remove a matching implicit cast. Note that static_cast's that + // are no-ops do not count since they produce an empty PathRange, a nice + // thing about Clang AST. + + // Now we know that there are no repetitions in BaseSpecList. + // So, popping the first element from it corresponding to each element in + // PathRange is equivalent to only including elements that are in + // BaseSpecList but not it PathRange + auto ReducedBaseSpecList = CXXBaseListFactory.getEmptyList(); + for (const CXXBaseSpecifier *BaseSpec : BaseSpecList) { + auto IsSameAsBaseSpec = [&BaseSpec](const CXXBaseSpecifier *I) -> bool { + return BaseSpec->getType() == I->getType(); + }; + if (llvm::none_of(PathRange, IsSameAsBaseSpec)) + ReducedBaseSpecList = + CXXBaseListFactory.add(BaseSpec, ReducedBaseSpecList); + } + + return getPointerToMemberData(ND, ReducedBaseSpecList); + } + // FIXME: Reinterpret casts on member-pointers are not handled properly by + // this code + for (const CXXBaseSpecifier *I : llvm::reverse(PathRange)) + BaseSpecList = prependCXXBase(I, BaseSpecList); + return getPointerToMemberData(ND, BaseSpecList); } const llvm::APSInt* diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp --- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -526,10 +526,9 @@ case CK_ReinterpretMemberPointer: { SVal V = state->getSVal(Ex, LCtx); if (auto PTMSV = V.getAs()) { - SVal CastedPTMSV = svalBuilder.makePointerToMember( - getBasicVals().accumCXXBase( - llvm::make_range( - CastE->path_begin(), CastE->path_end()), *PTMSV)); + SVal CastedPTMSV = + svalBuilder.makePointerToMember(getBasicVals().accumCXXBase( + CastE->path(), *PTMSV, CastE->getCastKind())); state = state->BindExpr(CastE, LCtx, CastedPTMSV); Bldr.generateNode(CastE, Pred, state); continue; diff --git a/clang/test/Analysis/pointer-to-member.cpp b/clang/test/Analysis/pointer-to-member.cpp --- a/clang/test/Analysis/pointer-to-member.cpp +++ b/clang/test/Analysis/pointer-to-member.cpp @@ -287,3 +287,26 @@ clang_analyzer_eval(a.*ep == 5); // expected-warning{{TRUE}} } } // namespace testAnonymousMember + +namespace testStaticCasting { +// From bug #48739 +struct Grandfather { + int field; +}; + +struct Father : public Grandfather {}; +struct Son : public Father {}; + +void test() { + int Son::*sf = &Son::field; + Grandfather grandpa; + grandpa.field = 10; + int Grandfather::*gpf1 = static_cast(sf); + int Grandfather::*gpf2 = static_cast(static_cast(sf)); + int Grandfather::*gpf3 = static_cast(static_cast(static_cast(sf))); + clang_analyzer_eval(grandpa.*gpf1 == 10); // expected-warning{{TRUE}} + clang_analyzer_eval(grandpa.*gpf2 == 10); // expected-warning{{TRUE}} + clang_analyzer_eval(grandpa.*gpf3 == 10); // expected-warning{{TRUE}} +} +} // namespace testStaticCasting + diff --git a/clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp b/clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp new file mode 100644 --- /dev/null +++ b/clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp @@ -0,0 +1,26 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s +// XFAIL: * + +void clang_analyzer_eval(bool); + +// TODO: The following test will work properly once reinterpret_cast on pointer-to-member is handled properly +namespace testReinterpretCasting { +struct Base { + int field; +}; + +struct Derived : public Base {}; + +struct DoubleDerived : public Derived {}; + +struct Some {}; + +void f() { + int DoubleDerived::*ddf = &Base::field; + int Base::*bf = reinterpret_cast(reinterpret_cast(reinterpret_cast(ddf))); + int Some::*sf = reinterpret_cast(ddf); + Base base; + base.field = 13; + clang_analyzer_eval(base.*bf == 13); // expected-warning{{TRUE}} +} +} // namespace testReinterpretCasting