[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

clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h

@@ -258,9 +258,9 @@ public:
     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,

clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h

@@ -514,7 +514,8 @@ private:
 /// 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;
 

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*

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;

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
+

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