RedDocMD updated this revision to Diff 321139.
RedDocMD added a comment.
Cleaner implementation of the popping logic, using STL/LLVM algorithms
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95877/new/
https://reviews.llvm.org/D95877
Files:
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
Index: clang/test/Analysis/pointer-to-member.cpp
===================================================================
--- clang/test/Analysis/pointer-to-member.cpp
+++ clang/test/Analysis/pointer-to-member.cpp
@@ -287,3 +287,25 @@
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
Index: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -526,10 +526,11 @@
case CK_ReinterpretMemberPointer: {
SVal V = state->getSVal(Ex, LCtx);
if (auto PTMSV = V.getAs<nonloc::PointerToMember>()) {
- SVal CastedPTMSV = svalBuilder.makePointerToMember(
- getBasicVals().accumCXXBase(
+ SVal CastedPTMSV =
+ svalBuilder.makePointerToMember(getBasicVals().accumCXXBase(
llvm::make_range<CastExpr::path_const_iterator>(
- CastE->path_begin(), CastE->path_end()), *PTMSV));
+ CastE->path_begin(), CastE->path_end()),
+ *PTMSV, CastE->getCastKind()));
state = state->BindExpr(CastE, LCtx, CastedPTMSV);
Bldr.generateNode(CastE, Pred, state);
continue;
Index: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -21,8 +21,10 @@
#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 <list>
#include <utility>
using namespace clang;
@@ -178,26 +180,63 @@
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();
+ BaseSpecList = CXXBaseListFactory.getEmptyList();
} else { // const PointerToMemberData *
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);
+ 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.
+
+ // First we need to make sure that there are no-repetitions in BaseSpecList
+ llvm::SmallPtrSet<QualType, 16> BaseSpecSeen;
+ for (const auto &BaseSpec : BaseSpecList) {
+ auto BaseType = BaseSpec->getType();
+ assert(BaseSpecSeen.find(BaseType) == BaseSpecSeen.end() &&
+ "CXXBaseSpecifier list of PointerToMemberData must not have "
+ "repeated elements");
+ BaseSpecSeen.insert(BaseType);
+ }
+
+ // 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 auto &BaseSpec : BaseSpecList)
+ if (llvm::none_of(
+ PathRange, [&BaseSpec](const auto &I) -> auto {
+ return BaseSpec->getType() == I->getType();
+ }))
+ ReducedBaseSpecList =
+ CXXBaseListFactory.add(BaseSpec, ReducedBaseSpecList);
+
+ return getPointerToMemberData(ND, ReducedBaseSpecList);
+ } else {
+ for (const auto &I : llvm::reverse(PathRange))
+ BaseSpecList = prependCXXBase(I, BaseSpecList);
+ return getPointerToMemberData(ND, BaseSpecList);
+ }
}
const llvm::APSInt*
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ 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;
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
@@ -258,9 +258,9 @@
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,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
