RedDocMD updated this revision to Diff 326414.
RedDocMD added a comment.
Added some more comments to increase clarity
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,51 @@
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 testMultipleInheritance {
+struct A {};
+struct B : public A {};
+struct C {
+ int field;
+};
+struct D : public C {};
+struct E : public B, public D {};
+struct F : public E {};
+
+void testMultiple() {
+ int F::*f = &F::field;
+ int A::*a = reinterpret_cast<int A::*>(f);
+ int C::*c = reinterpret_cast<int C::*>(f);
+ A aobj;
+ C cobj;
+ aobj.*a = 13;
+ cobj.*c = 29;
+ clang_analyzer_eval(aobj.*a == 13); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(cobj.*c == 29); // expected-warning{{TRUE}}
+}
+} // namespace testMultipleInheritance
+
} // 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,128 @@
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) {
+ // Performs BFS to find a path in the class hierarchy from Start to End
+ std::deque<std::shared_ptr<BFSNode>> queue;
+ llvm::SmallVector<const CXXBaseSpecifier *, 16> Path;
+ const CXXRecordDecl *StartDecl = Start->getAsCXXRecordDecl();
+ if (!StartDecl)
+ return {Path, false};
+
+ // We don't want Start in the final path, so add its bases as the initial
+ // nodes
+ for (const CXXBaseSpecifier &Base : StartDecl->bases())
+ queue.push_back(std::make_shared<BFSNode>(&Base));
+
+ // Standard iterative BFS algorithm
+ while (!queue.empty()) {
+ std::shared_ptr<BFSNode> CurrNode = queue.front();
+ queue.pop_front();
+ const CXXBaseSpecifier *CurrBaseSpec = CurrNode->BaseSpec;
+ const Type *CurrType = CurrBaseSpec->getType().getTypePtr();
+
+ // We have found it!
+ if (CurrType == End) {
+ BFSNode *Ptr = CurrNode.get();
+ while (Ptr) {
+ Path.push_back(Ptr->BaseSpec);
+ Ptr = Ptr->Parent.get();
+ }
+ // We want a path from Start to End, not the other way round
+ 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) {
+ // Retrieve the class/struct whose member-pointer the expression will be
+ // declared as. It will be the one written as target type of the
+ // reinterpret_cast.
+ 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();
+ }
+
+ // Retrieve the class/struct which actually contains the field/method being
+ // pointed to
+ 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
