RedDocMD created this revision.
RedDocMD added reviewers: vsavchenko, NoQ, xazax.hun, dcoughlin.
Herald added subscribers: martong, Charusso, rnkovacs.
RedDocMD requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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.
Repository:
rG LLVM Github Monorepo
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
@@ -23,6 +23,7 @@
#include "llvm/ADT/STLExtras.h"
#include <cassert>
#include <cstdint>
+#include <list>
#include <utility>
using namespace clang;
@@ -178,7 +179,11 @@
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;
@@ -195,8 +200,38 @@
PathList = PTMD->getCXXBaseList();
}
- for (const auto &I : llvm::reverse(PathRange))
- PathList = prependCXXBase(I, PathList);
+ if (kind == CK_DerivedToBaseMemberPointer) {
+ // Here we pop off matching CXXBaseSpecifiers from PathList.
+ // 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.
+ std::list<const CXXBaseSpecifier *> BaseList;
+ for (const auto &I : PathList)
+ BaseList.push_back(I);
+
+ auto find_first =
+ [](auto begin, auto end, const CXXBaseSpecifier *baseSpec) -> auto {
+ for (auto it = begin; it != end; ++it)
+ if ((*it)->getType() == baseSpec->getType())
+ return it;
+ return end;
+ };
+
+ for (const auto &PathBase : llvm::reverse(PathRange)) {
+ auto DelIt = find_first(BaseList.begin(), BaseList.end(), PathBase);
+ assert(DelIt != BaseList.end() && "PTM has insufficient base specifiers");
+ BaseList.erase(DelIt);
+ }
+
+ PathList = CXXBaseListFactory.getEmptyList();
+ for (const auto &I : BaseList)
+ PathList = CXXBaseListFactory.add(I, PathList);
+
+ } else {
+ for (const auto &I : llvm::reverse(PathRange))
+ PathList = prependCXXBase(I, PathList);
+ }
return getPointerToMemberData(ND, PathList);
}
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
