Author: Deep Majumder Date: 2021-02-15T11:44:37+03:00 New Revision: 21daada95079a37c7ca259fabfc735b6d1b362ad
URL: https://github.com/llvm/llvm-project/commit/21daada95079a37c7ca259fabfc735b6d1b362ad DIFF: https://github.com/llvm/llvm-project/commit/21daada95079a37c7ca259fabfc735b6d1b362ad.diff LOG: [analyzer] Fix static_cast on pointer-to-member handling This commit fixes bug #48739. The bug was caused by the way static_casts on pointer-to-member caused the CXXBaseSpecifier list of a MemberToPointer to grow instead of shrink. The list is now grown by implicit casts and corresponding entries are removed by static_casts. No-op static_casts cause no effect. Reviewed By: vsavchenko Differential Revision: https://reviews.llvm.org/D95877 Added: clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp Modified: clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp clang/lib/StaticAnalyzer/Core/ExprEngineC.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 142b1ab11750..9f464e82304f 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h @@ -258,9 +258,9 @@ class BasicValueFactory { return CXXBaseListFactory.add(CBS, L); } - const PointerToMemberData *accumCXXBase( - llvm::iterator_range<CastExpr::path_const_iterator> PathRange, - const nonloc::PointerToMember &PTM); + const PointerToMemberData * + accumCXXBase(llvm::iterator_range<CastExpr::path_const_iterator> 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 index bb295ab591d4..a2354ba62cdb 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h @@ -514,7 +514,8 @@ class LazyCompoundVal : public NonLoc { /// 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 index d1f5ac02278f..40cdaef1bfa7 100644 --- 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 <cassert> #include <cstdint> #include <utility> @@ -176,28 +177,73 @@ const PointerToMemberData *BasicValueFactory::getPointerToMemberData( return D; } +LLVM_ATTRIBUTE_UNUSED bool hasNoRepeatedElements( + llvm::ImmutableList<const CXXBaseSpecifier *> BaseSpecList) { + llvm::SmallPtrSet<QualType, 16> 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<CastExpr::path_const_iterator> 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<const CXXBaseSpecifier *> PathList; + llvm::ImmutableList<const CXXBaseSpecifier *> BaseSpecList; if (PTMDT.isNull() || PTMDT.is<const NamedDecl *>()) { if (PTMDT.is<const NamedDecl *>()) ND = PTMDT.get<const NamedDecl *>(); - PathList = CXXBaseListFactory.getEmptyList(); - } else { // const PointerToMemberData * + BaseSpecList = CXXBaseListFactory.getEmptyList(); + } else { const PointerToMemberData *PTMD = PTMDT.get<const PointerToMemberData *>(); 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 index 18d1b2169eed..c0aae596c702 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -526,10 +526,9 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, case CK_ReinterpretMemberPointer: { SVal V = state->getSVal(Ex, LCtx); if (auto PTMSV = V.getAs<nonloc::PointerToMember>()) { - SVal CastedPTMSV = svalBuilder.makePointerToMember( - getBasicVals().accumCXXBase( - llvm::make_range<CastExpr::path_const_iterator>( - 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 index 27cad4ed98aa..e1b4d0c11949 100644 --- a/clang/test/Analysis/pointer-to-member.cpp +++ b/clang/test/Analysis/pointer-to-member.cpp @@ -287,3 +287,26 @@ void test() { 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<int Grandfather::*>(sf); + int Grandfather::*gpf2 = static_cast<int Grandfather::*>(static_cast<int Father::*>(sf)); + int Grandfather::*gpf3 = static_cast<int Grandfather::*>(static_cast<int Son::*>(static_cast<int Father::*>(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 index 000000000000..1631be70da3e --- /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<int Base::*>(reinterpret_cast<int Derived::*>(reinterpret_cast<int Base::*>(ddf))); + int Some::*sf = reinterpret_cast<int Some::*>(ddf); + Base base; + base.field = 13; + clang_analyzer_eval(base.*bf == 13); // expected-warning{{TRUE}} +} +} // namespace testReinterpretCasting _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits