RedDocMD updated this revision to Diff 326410. RedDocMD added a comment. Cleaned up tests, added test above class hierarchy
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96976/new/ https://reviews.llvm.org/D96976 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp
Index: clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp =================================================================== --- clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp +++ clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp @@ -1,11 +1,11 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s -// XFAIL: asserts 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 { +struct BaseOfBase {}; + +struct Base : public BaseOfBase { int field; }; @@ -15,12 +15,28 @@ struct Some {}; -void f() { +void analyzableCasts() { 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}} } + +void castOutsideHierarchy() { + int DoubleDerived::*ddf = &Base::field; + int Some::*sf = reinterpret_cast<int Some::*>(ddf); + Some some; + some.*sf = 14; + clang_analyzer_eval(some.*sf == 14); // expected-warning{{UNKNOWN}} +} + +void castAbove() { + int DoubleDerived::*ddf = &Base::field; + int BaseOfBase::*bbf = reinterpret_cast<int BaseOfBase::*>(ddf); + BaseOfBase bb; + bb.*bbf = 23; + clang_analyzer_eval(bb.*bbf == 23); // expected-warning{{UNKNOWN}} +} + } // namespace testReinterpretCasting Index: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -522,8 +522,7 @@ continue; } case CK_DerivedToBaseMemberPointer: - case CK_BaseToDerivedMemberPointer: - case CK_ReinterpretMemberPointer: { + case CK_BaseToDerivedMemberPointer: { SVal V = state->getSVal(Ex, LCtx); if (auto PTMSV = V.getAs<nonloc::PointerToMember>()) { SVal CastedPTMSV = @@ -537,6 +536,25 @@ state = handleLVectorSplat(state, LCtx, CastE, Bldr, Pred); continue; } + case CK_ReinterpretMemberPointer: { + SVal V = state->getSVal(Ex, LCtx); + if (auto PTMSV = V.getAs<nonloc::PointerToMember>()) { + if (const auto *ReinterpretE = + llvm::dyn_cast<CXXReinterpretCastExpr>(CastE)) { + if (const PointerToMemberData *Data = + getBasicVals().basesForReinterpretCast(ReinterpretE, + *PTMSV)) { + SVal CastedPTMSV = svalBuilder.makePointerToMember(Data); + state = state->BindExpr(CastE, LCtx, CastedPTMSV); + Bldr.generateNode(CastE, Pred, state); + continue; + } + } + } + // Explicitly proceed with default handler for this case cascade. + state = handleLVectorSplat(state, LCtx, CastE, Bldr, Pred); + continue; + } // Various C++ casts that are not handled yet. case CK_ToUnion: case CK_VectorSplat: { Index: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp +++ clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp @@ -22,8 +22,11 @@ #include "llvm/ADT/ImmutableList.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallPtrSet.h" +#include "llvm/ADT/SmallVector.h" #include <cassert> #include <cstdint> +#include <deque> +#include <memory> #include <utility> using namespace clang; @@ -239,13 +242,115 @@ 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); } +struct BFSNode { + const CXXBaseSpecifier *BaseSpec; + std::shared_ptr<BFSNode> Parent; + + BFSNode(const CXXBaseSpecifier *BaseSpec, + std::shared_ptr<BFSNode> Parent = nullptr) + : BaseSpec{BaseSpec}, Parent{Parent} {} +}; + +std::pair<llvm::SmallVector<const CXXBaseSpecifier *, 16>, bool> +findPathToBase(const Type *Start, const Type *End) { + std::deque<std::shared_ptr<BFSNode>> queue; + llvm::SmallVector<const CXXBaseSpecifier *, 16> Path; + const CXXRecordDecl *StartDecl = Start->getAsCXXRecordDecl(); + if (!StartDecl) + return {Path, false}; + + for (const CXXBaseSpecifier &Base : StartDecl->bases()) + queue.push_back(std::make_shared<BFSNode>(&Base)); + + while (!queue.empty()) { + std::shared_ptr<BFSNode> CurrNode = queue.front(); + queue.pop_front(); + const CXXBaseSpecifier *CurrBaseSpec = CurrNode->BaseSpec; + const Type *CurrType = CurrBaseSpec->getType().getTypePtr(); + if (CurrType == End) { + BFSNode *Ptr = CurrNode.get(); + while (Ptr) { + Path.push_back(Ptr->BaseSpec); + Ptr = Ptr->Parent.get(); + } + std::reverse(Path.begin(), Path.end()); + return {Path, true}; + } + const CXXRecordDecl *CurrRecordDecl = CurrType->getAsCXXRecordDecl(); + assert(CurrRecordDecl && + "Base of a CXX class/struct must be a CXX class/struct"); + for (const CXXBaseSpecifier &Base : CurrRecordDecl->bases()) + queue.push_back(std::make_shared<BFSNode>(&Base, CurrNode)); + } + + return {Path, false}; +} + +const CXXRecordDecl *retrieveCXXRecord(const NamedDecl *ND) { + if (const auto *FD = llvm::dyn_cast<FieldDecl>(ND)) { + const RecordDecl *RD = FD->getParent(); + return llvm::dyn_cast<CXXRecordDecl>(RD); + } + if (const auto *MD = llvm::dyn_cast<CXXMethodDecl>(ND)) { + return MD->getParent(); + } + if (const auto *IFD = llvm::dyn_cast<IndirectFieldDecl>(ND)) { + if (const FieldDecl *FD = IFD->getAnonField()) { + const RecordDecl *RD = FD->getParent(); + return llvm::dyn_cast<CXXRecordDecl>(RD); + } + } + return nullptr; +} + +const PointerToMemberData * +BasicValueFactory::basesForReinterpretCast(const CXXReinterpretCastExpr *CastE, + const nonloc::PointerToMember &PTM) { + const MemberPointerType *StartPTMTy = + CastE->getTypeAsWritten().getTypePtr()->getAs<MemberPointerType>(); + const CXXRecordDecl *Start = StartPTMTy->getMostRecentCXXRecordDecl(); + const Type *StartTy = Start->getTypeForDecl(); + + const NamedDecl *ND; + llvm::ImmutableList<const CXXBaseSpecifier *> BaseSpecList; + nonloc::PointerToMember::PTMDataType PTMDT = PTM.getPTMData(); + if (PTMDT.isNull() || PTMDT.is<const NamedDecl *>()) { + if (PTMDT.is<const NamedDecl *>()) + ND = PTMDT.get<const NamedDecl *>(); + } else { + const PointerToMemberData *PTMD = PTMDT.get<const PointerToMemberData *>(); + ND = PTMD->getDeclaratorDecl(); + } + + const CXXRecordDecl *Target = retrieveCXXRecord(ND); + if (!Target) + return nullptr; + const Type *TargetTy = Target->getTypeForDecl(); + + // If TargetTy and StartTy are the same, we can return an empty list + if (TargetTy == StartTy) + return getPointerToMemberData(ND, CXXBaseListFactory.getEmptyList()); + + // If Target is higher up than Start in the class hierarchy + const auto UpPath = findPathToBase(StartTy, TargetTy); + if (UpPath.second) { + for (const CXXBaseSpecifier *BaseSpec : UpPath.first) + BaseSpecList = CXXBaseListFactory.add(BaseSpec, BaseSpecList); + return getPointerToMemberData(ND, BaseSpecList); + } + + // Otherwise Target is lower in the class hierarchy as Start, in which case + // the sub-object is unreachable. Otherwise Target is unrelated to Start In + // either case, it makes no sense to continue further analysis, so we return + // nullptr + return nullptr; +} + const llvm::APSInt* BasicValueFactory::evalAPSInt(BinaryOperator::Opcode Op, const llvm::APSInt& V1, const llvm::APSInt& V2) { Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h @@ -262,6 +262,10 @@ accumCXXBase(llvm::iterator_range<CastExpr::path_const_iterator> PathRange, const nonloc::PointerToMember &PTM, const clang::CastKind &kind); + const PointerToMemberData * + basesForReinterpretCast(const CXXReinterpretCastExpr *CastE, + const nonloc::PointerToMember &PTM); + const llvm::APSInt* evalAPSInt(BinaryOperator::Opcode Op, const llvm::APSInt& V1, const llvm::APSInt& V2);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits