[analyzer] Support SVal::getType for pointer-to-member values

[vsavchenko]

This one is a bit more invasive, so I decided to put it into a separate patch.

[vsavchenko]

Restore behavior for nullptr declarations

[vsavchenko]

Rebase

[vsavchenko]

Rebase

[NoQ]

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

85

What prevents you from obtaining the type from D? I guess NamedDecl itself doesn't provide a getType() on its own but what specific sub-classes can D be that don't have a type?

[vsavchenko]

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

85

Because it's not a declaration of pointer-to-member, but a declaration of the actual member. It can be nullptr while the type is always known.
I'd prefer not to construct a pointer-to-member type myself. It's the work for Sema, not for the analyzer.

[martong]

This looks good to me.

[ASDenysPetrov]

@vsavchenko Is this alive?

[martong]

In D104647#3643060, @ASDenysPetrov wrote:

@vsavchenko Is this alive?

I think, we could commit this and set Valery as the author (he is no longer working on CSA).
Unless you find something wrong with the patch; in that case we can still make changes after we commandeer the patch.

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

static void Profile(llvm::FoldingSetNodeID& ID,
                       const StoreRef &store,
                       const TypedValueRegion *region);
 
   void Profile(llvm::FoldingSetNodeID& ID) { Profile(ID, store, region); }
 };
 
 class PointerToMemberData : public llvm::FoldingSetNode {
   const NamedDecl *D;
+  QualType T;
   llvm::ImmutableList<const CXXBaseSpecifier *> L;
 
 public:
-  PointerToMemberData(const NamedDecl *D,
+  PointerToMemberData(const NamedDecl *D, QualType T,
                       llvm::ImmutableList<const CXXBaseSpecifier *> L)
-      : D(D), L(L) {}
+      : D(D), T(T), L(L) {}
 
   using iterator = llvm::ImmutableList<const CXXBaseSpecifier *>::iterator;
 
   iterator begin() const { return L.begin(); }
   iterator end() const { return L.end(); }
 
   static void Profile(llvm::FoldingSetNodeID &ID, const NamedDecl *D,
+                      QualType T,
                       llvm::ImmutableList<const CXXBaseSpecifier *> L);
 
-  void Profile(llvm::FoldingSetNodeID &ID) { Profile(ID, D, L); }
+  void Profile(llvm::FoldingSetNodeID &ID) { Profile(ID, D, T, L); }
   const NamedDecl *getDeclaratorDecl() const { return D; }
 
+  QualType getType() const { return T; }
+
   llvm::ImmutableList<const CXXBaseSpecifier *> getCXXBaseList() const {
     return L;
   }
 };
 
 class BasicValueFactory {
   using APSIntSetTy =
       llvm::FoldingSet<llvm::FoldingSetNodeWrapper<llvm::APSInt>>;
public:
 
   const llvm::APSInt &getTruthValue(bool b) {
     return getTruthValue(b, Ctx.getLogicalOperationType());
   }
 
   const CompoundValData *getCompoundValData(QualType T,
                                             llvm::ImmutableList<SVal> Vals);
 
-  const LazyCompoundValData *getLazyCompoundValData(const StoreRef &store,
-                                            const TypedValueRegion *region);
+  const LazyCompoundValData *
+  getLazyCompoundValData(const StoreRef &store, const TypedValueRegion *region);
+
+  const PointerToMemberData *getPointerToMemberData(const NamedDecl *ND,
+                                                    QualType T) {
+    return getPointerToMemberData(ND, T, CXXBaseListFactory.getEmptyList());
+  }
 
   const PointerToMemberData *
-  getPointerToMemberData(const NamedDecl *ND,
+  getPointerToMemberData(const NamedDecl *ND, QualType T,
                          llvm::ImmutableList<const CXXBaseSpecifier *> L);
 
   llvm::ImmutableList<SVal> getEmptySValList() {
     return SValListFactory.getEmptyList();
   }
 
   llvm::ImmutableList<SVal> prependSVal(SVal X, llvm::ImmutableList<SVal> L) {
     return SValListFactory.add(X, L);

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

DefinedOrUnknownSVal getDerivedRegionValueSymbolVal(
       SymbolRef parentSymbol, const TypedValueRegion *region);
 
   DefinedSVal getMetadataSymbolVal(const void *symbolTag,
                                    const MemRegion *region,
                                    const Expr *expr, QualType type,
                                    const LocationContext *LCtx,
                                    unsigned count);
 
-  DefinedSVal getMemberPointer(const NamedDecl *ND);
+  DefinedSVal getMemberPointer(const NamedDecl *ND, QualType T);
 
   DefinedSVal getFunctionPointer(const FunctionDecl *func);
 
   DefinedSVal getBlockPointer(const BlockDecl *block, CanQualType locTy,
                               const LocationContext *locContext,
                               unsigned blockCount);
 
   /// Returns the value of \p E, if it can be determined in a non-path-sensitive
   /// manner.
   ///
   /// If \p E is not a constant or cannot be modeled, returns \c None.
   Optional<SVal> getConstantVal(const Expr *E);
 
   NonLoc makeCompoundVal(QualType type, llvm::ImmutableList<SVal> vals) {
     return nonloc::CompoundVal(BasicVals.getCompoundValData(type, vals));
   }
 
   NonLoc makeLazyCompoundVal(const StoreRef &store,
                              const TypedValueRegion *region) {
     return nonloc::LazyCompoundVal(
         BasicVals.getLazyCompoundValData(store, region));
   }
 
-  NonLoc makePointerToMember(const DeclaratorDecl *DD) {
-    return nonloc::PointerToMember(DD);
-  }
-
   NonLoc makePointerToMember(const PointerToMemberData *PTMD) {
     return nonloc::PointerToMember(PTMD);
   }
 
   NonLoc makeZeroArrayIndex() {
     return nonloc::ConcreteInt(BasicVals.getValue(0, ArrayIndexTy));
   }
 

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

 /// pointer or a member data pointer and an optional list of CXXBaseSpecifiers.
 /// This list is required to accumulate the pointer-to-member cast history to
 /// figure out 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;
 
 public:
-  using PTMDataType =
-      llvm::PointerUnion<const NamedDecl *, const PointerToMemberData *>;
-
-  const PTMDataType getPTMData() const {
-    return PTMDataType::getFromOpaqueValue(const_cast<void *>(Data));
+  const PointerToMemberData *getPTMData() const {
+    return static_cast<const PointerToMemberData *>(Data);
   }
 
   bool isNullMemberPointer() const;
 
   const NamedDecl *getDecl() const;
 
+  QualType getType() const;
+
   template<typename AdjustedDecl>
   const AdjustedDecl *getDeclAs() const {
     return dyn_cast_or_null<AdjustedDecl>(getDecl());
   }
 
   using iterator = llvm::ImmutableList<const CXXBaseSpecifier *>::iterator;
 
   iterator begin() const;
   iterator end() const;
 
 private:
   friend class SVal;
 
   PointerToMember() = default;
-  explicit PointerToMember(const PTMDataType D)
-      : NonLoc(PointerToMemberKind, D.getOpaqueValue()) {}
+  explicit PointerToMember(const PointerToMemberData *D)
+      : NonLoc(PointerToMemberKind, D) {}
 
   static bool isKind(const SVal& V) {
     return V.getBaseKind() == NonLocKind &&
            V.getSubKind() == PointerToMemberKind;
   }
 
   static bool isKind(const NonLoc& V) {
     return V.getSubKind() == PointerToMemberKind;

clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp

 void LazyCompoundValData::Profile(llvm::FoldingSetNodeID& ID,
                                   const StoreRef &store,
                                   const TypedValueRegion *region) {
   ID.AddPointer(store.getStore());
   ID.AddPointer(region);
 }
 
 void PointerToMemberData::Profile(
-    llvm::FoldingSetNodeID &ID, const NamedDecl *D,
+    llvm::FoldingSetNodeID &ID, const NamedDecl *D, QualType T,
     llvm::ImmutableList<const CXXBaseSpecifier *> L) {
   ID.AddPointer(D);
+  ID.AddPointer(T.getAsOpaquePtr());
   ID.AddPointer(L.getInternalPointer());
 }
 
 using SValData = std::pair<SVal, uintptr_t>;
 using SValPair = std::pair<SVal, SVal>;
 
 namespace llvm {
 
if (!D) {
     new (D) LazyCompoundValData(store, region);
     LazyCompoundValDataSet.InsertNode(D, InsertPos);
   }
 
   return D;
 }
 
 const PointerToMemberData *BasicValueFactory::getPointerToMemberData(
-    const NamedDecl *ND, llvm::ImmutableList<const CXXBaseSpecifier *> L) {
+    const NamedDecl *ND, QualType T,
+    llvm::ImmutableList<const CXXBaseSpecifier *> L) {
   llvm::FoldingSetNodeID ID;
-  PointerToMemberData::Profile(ID, ND, L);
+  PointerToMemberData::Profile(ID, ND, T, L);
   void *InsertPos;
 
   PointerToMemberData *D =
       PointerToMemberDataSet.FindNodeOrInsertPos(ID, InsertPos);
 
   if (!D) {
     D = (PointerToMemberData *)BPAlloc.Allocate<PointerToMemberData>();
-    new (D) PointerToMemberData(ND, L);
+    new (D) PointerToMemberData(ND, T, L);
     PointerToMemberDataSet.InsertNode(D, InsertPos);
   }
 
   return D;
 }
 
 LLVM_ATTRIBUTE_UNUSED bool hasNoRepeatedElements(
     llvm::ImmutableList<const CXXBaseSpecifier *> BaseSpecList) {

 
 const PointerToMemberData *BasicValueFactory::accumCXXBase(
     llvm::iterator_range<CastExpr::path_const_iterator> PathRange,
     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 *> BaseSpecList;
-
-  if (PTMDT.isNull() || PTMDT.is<const NamedDecl *>()) {
-    if (PTMDT.is<const NamedDecl *>())
-      ND = PTMDT.get<const NamedDecl *>();
-
-    BaseSpecList = CXXBaseListFactory.getEmptyList();
-  } else {
-    const PointerToMemberData *PTMD = PTMDT.get<const PointerToMemberData *>();
-    ND = PTMD->getDeclaratorDecl();
-
-    BaseSpecList = PTMD->getCXXBaseList();
-  }
+  const PointerToMemberData *PTMD = PTM.getPTMData();
+  llvm::ImmutableList<const CXXBaseSpecifier *> BaseSpecList =
+      PTMD->getCXXBaseList();
 
   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
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);
+    return getPointerToMemberData(PTMD->getDeclaratorDecl(), PTMD->getType(),
+                                  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);
+  return getPointerToMemberData(PTMD->getDeclaratorDecl(), PTMD->getType(),
+                                BaseSpecList);
 }
 
 const llvm::APSInt*
 BasicValueFactory::evalAPSInt(BinaryOperator::Opcode Op,
                              const llvm::APSInt& V1, const llvm::APSInt& V2) {
   switch (Op) {
     default:
       llvm_unreachable("Invalid Opcode.");

clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp

switch (CastE->getCastKind()) {
       }
       case CK_NullToPointer: {
         SVal V = svalBuilder.makeNull();
         state = state->BindExpr(CastE, LCtx, V);
         Bldr.generateNode(CastE, Pred, state);
         continue;
       }
       case CK_NullToMemberPointer: {
-        SVal V = svalBuilder.getMemberPointer(nullptr);
+        SVal V = svalBuilder.getMemberPointer(nullptr, CastE->getType());
         state = state->BindExpr(CastE, LCtx, V);
         Bldr.generateNode(CastE, Pred, state);
         continue;
       }
       case CK_DerivedToBaseMemberPointer:
       case CK_BaseToDerivedMemberPointer:
       case CK_ReinterpretMemberPointer: {
         SVal V = state->getSVal(Ex, LCtx);
case UO_AddrOf: {
       const Expr *Ex = U->getSubExpr()->IgnoreParens();
       if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Ex)) {
         const ValueDecl *VD = DRE->getDecl();
 
         if (isa<CXXMethodDecl>(VD) || isa<FieldDecl>(VD) ||
             isa<IndirectFieldDecl>(VD)) {
           ProgramStateRef State = (*I)->getState();
           const LocationContext *LCtx = (*I)->getLocationContext();
-          SVal SV = svalBuilder.getMemberPointer(cast<NamedDecl>(VD));
+          SVal SV =
+              svalBuilder.getMemberPointer(cast<NamedDecl>(VD), U->getType());
           Bldr.generateNode(U, *I, State->BindExpr(U, LCtx, SV));
           break;
         }
       }
       // Explicitly proceed with default handler for this case cascade.
       handleUOExtension(I, U, Bldr);
       break;
     }

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

SValBuilder::getDerivedRegionValueSymbolVal(SymbolRef parentSymbol,
   SymbolRef sym = SymMgr.getDerivedSymbol(parentSymbol, region);
 
   if (Loc::isLocType(T))
     return loc::MemRegionVal(MemMgr.getSymbolicRegion(sym));
 
   return nonloc::SymbolVal(sym);
 }
 
-DefinedSVal SValBuilder::getMemberPointer(const NamedDecl *ND) {
+DefinedSVal SValBuilder::getMemberPointer(const NamedDecl *ND, QualType T) {
   assert(!ND || isa<CXXMethodDecl>(ND) || isa<FieldDecl>(ND) ||
          isa<IndirectFieldDecl>(ND));
 
   if (const auto *MD = dyn_cast_or_null<CXXMethodDecl>(ND)) {
     // Sema treats pointers to static member functions as have function pointer
     // type, so return a function pointer for the method.
     // We don't need to play a similar trick for static member fields
     // because these are represented as plain VarDecls and not FieldDecls
     // in the AST.
     if (MD->isStatic())
       return getFunctionPointer(MD);
   }
 
-  return nonloc::PointerToMember(ND);
+  return nonloc::PointerToMember(
+      getBasicValueFactory().getPointerToMemberData(ND, T));
 }
 
 DefinedSVal SValBuilder::getFunctionPointer(const FunctionDecl *func) {
   return loc::MemRegionVal(MemMgr.getFunctionCodeRegion(func));
 }
 
 DefinedSVal SValBuilder::getBlockPointer(const BlockDecl *block,
                                          CanQualType locTy,

clang/lib/StaticAnalyzer/Core/SVals.cpp

Optional<QualType> VisitNonLocCompoundVal(nonloc::CompoundVal CV) {
     return CV.getValue()->getType();
   }
   Optional<QualType> VisitNonLocLazyCompoundVal(nonloc::LazyCompoundVal LCV) {
     return Visit(LCV.getRegion());
   }
   Optional<QualType> VisitNonLocSymbolVal(nonloc::SymbolVal SV) {
     return Visit(SV.getSymbol());
   }
+  Optional<QualType> VisitNonLocPointerToMember(nonloc::PointerToMember PTM) {
+    QualType T = PTM.getType();
+
+    if (T.isNull())
+      return None;
+
+    return T;
+  }
   Optional<QualType> VisitSymbolicRegion(const SymbolicRegion *SR) {
     return Visit(SR->getSymbol());
   }
   Optional<QualType> VisitTypedRegion(const TypedRegion *TR) {
     return TR->getLocationType();
   }
   Optional<QualType> VisitSymExpr(const SymExpr *SE) { return SE->getType(); }
 };
const void *nonloc::LazyCompoundVal::getStore() const {
   return static_cast<const LazyCompoundValData*>(Data)->getStore();
 }
 
 const TypedValueRegion *nonloc::LazyCompoundVal::getRegion() const {
   return static_cast<const LazyCompoundValData*>(Data)->getRegion();
 }
 
 bool nonloc::PointerToMember::isNullMemberPointer() const {
-  return getPTMData().isNull();
+  return getPTMData() == nullptr;
 }
 
 const NamedDecl *nonloc::PointerToMember::getDecl() const {
-  const auto PTMD = this->getPTMData();
-  if (PTMD.isNull())
+  if (!getPTMData())
     return nullptr;
 
-  const NamedDecl *ND = nullptr;
-  if (PTMD.is<const NamedDecl *>())
-    ND = PTMD.get<const NamedDecl *>();
-  else
-    ND = PTMD.get<const PointerToMemberData *>()->getDeclaratorDecl();
+  return getPTMData()->getDeclaratorDecl();
+}
+
+QualType nonloc::PointerToMember::getType() const {
+  if (!getPTMData())
+    return {};
 
-  return ND;
+  return getPTMData()->getType();
 }
 
 //===----------------------------------------------------------------------===//
 // Other Iterators.
 //===----------------------------------------------------------------------===//
 
 nonloc::CompoundVal::iterator nonloc::CompoundVal::begin() const {
   return getValue()->begin();
 }
 
 nonloc::CompoundVal::iterator nonloc::CompoundVal::end() const {
   return getValue()->end();
 }
 
 nonloc::PointerToMember::iterator nonloc::PointerToMember::begin() const {
-  const PTMDataType PTMD = getPTMData();
-  if (PTMD.is<const NamedDecl *>())
-    return {};
-  return PTMD.get<const PointerToMemberData *>()->begin();
+  return getPTMData()->begin();
 }
 
 nonloc::PointerToMember::iterator nonloc::PointerToMember::end() const {
-  const PTMDataType PTMD = getPTMData();
-  if (PTMD.is<const NamedDecl *>())
-    return {};
-  return PTMD.get<const PointerToMemberData *>()->end();
+  return getPTMData()->end();
 }
 
 //===----------------------------------------------------------------------===//
 // Useful predicates.
 //===----------------------------------------------------------------------===//
 
 bool SVal::isConstant() const {
   return getAs<nonloc::ConcreteInt>() || getAs<loc::ConcreteInt>();

clang/unittests/StaticAnalyzer/SValTest.cpp

)") {
   EXPECT_EQ(Context.VoidPtrTy, *A.getType(Context));
 
   SVal B = getByName("a");
   ASSERT_TRUE(B.getType(Context).hasValue());
   // TODO: Change to CharTy when we support symbolic casts
   EXPECT_EQ(Context.VoidPtrTy, *B.getType(Context));
 }
 
+SVAL_TEST(GetMemberPtrType, R"(
+struct A {
+  int a;
+  struct {
+    int b;
+  };
+};
+void foo(int A::*x) {
+  int A::*a = &A::a;
+  int A::*b = &A::b;
+  int A::*c = x;
+}
+)") {
+  SVal A = getByName("a");
+  ASSERT_TRUE(A.getType(Context).hasValue());
+  const auto *AMemberPtrTy = dyn_cast<MemberPointerType>(*A.getType(Context));
+  ASSERT_NE(AMemberPtrTy, nullptr);
+  EXPECT_EQ(Context.IntTy, AMemberPtrTy->getPointeeType());
+  const auto *ARecordType = dyn_cast<RecordType>(AMemberPtrTy->getClass());
+  ASSERT_NE(ARecordType, nullptr);
+  EXPECT_EQ("A", ARecordType->getDecl()->getName());
+
+  SVal B = getByName("b");
+  ASSERT_TRUE(B.getType(Context).hasValue());
+  const auto *BMemberPtrTy = dyn_cast<MemberPointerType>(*B.getType(Context));
+  ASSERT_NE(BMemberPtrTy, nullptr);
+  EXPECT_EQ(Context.IntTy, BMemberPtrTy->getPointeeType());
+  const auto *BRecordType = dyn_cast<RecordType>(BMemberPtrTy->getClass());
+  ASSERT_NE(BRecordType, nullptr);
+  EXPECT_EQ("A", BRecordType->getDecl()->getName());
+
+  SVal C = getByName("c");
+  EXPECT_TRUE(C.isUnknown());
+}
+
 } // namespace
 } // namespace ento
 } // namespace clang