[clang] 21daada - [analyzer] Fix static_cast on pointer-to-member handling
Valeriy Savchenko via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 15 00:48:18 PST 2021
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
More information about the cfe-commits
mailing list