[analyzer] Add a new SVal to support pointer-to-member operations.

[kromanenkov]

Summary

Add a new type of NonLoc SVal for pointer-to-member operations. This SVal supports both pointers to member functions and pointers to member data.

[NoQ]

Yay, thanks for posting this! :)

I've got a bit of concern for some assert-suppressions.

lib/StaticAnalyzer/Core/ExprEngine.cpp

2314 ↗

(On Diff #74249)

Hmm. Why would anybody try to load anything from a plain pointer-to-member, rather than from a pointer-to-member-applied-to-an-object (which would no longer be represented by a PointerToMember value)? I suspect there's something wrong above the stack (or one of the sub-expression SVals is incorrect), because otherwise i think that making PointerToMember a NonLoc is correct - we cannot store things in it or load things from it.

lib/StaticAnalyzer/Core/ExprEngineC.cpp

465

cast<>()? It seems that all dynamic casts here are asserted to succeed.

lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

69 ↗

(On Diff #74249)

Same concern: Why are we copying a NonLoc?

test/Analysis/pointer-to-member.cpp

74–172

In fact, maybe dereferencing a null pointer-to-member should produce an UndefinedVal, which could be later caught by core.uninitialized.UndefReturn. I wonder why doesn't this happen.

[kromanenkov]

lib/StaticAnalyzer/Core/ExprEngine.cpp

2314 ↗

(On Diff #74249)

Brief analysis shows that we call evalLoad of pointer-to-member SVal after ExprEngine::VisitCast call when cast kind is CK_LValueToRValue. Skipping this check leads to assertion fail in pointer-to-member.cpp test. I will investigate the other ways to supress this assertion.

test/Analysis/pointer-to-member.cpp

74–172

In fact, I plan to caught dereferencing of null pointer-to-members by the checker which I intend to commit after this patch :) So, do you think that the check for dereferencing a null pointer-to-member should be a part of an analyzer core?

[NoQ]

test/Analysis/pointer-to-member.cpp

74–172

That'd be great! However, we're doing a lot of such double-work in case a checker to find the defect is not enabled.

For example, as seen by ExprEngine, result of division by zero, or of reading past an array bound, or of reading an unitialized variable, is UndefinedVal. For division by zero and for reading past an array bound, there's a checker that immediately terminates the analysis when the error occurs, which makes it completely irrelevant how exactly ExprEngine models the erroneous operation. In case of array bound, however, this checker is alpha and disabled by default. For reading an unitialized variable, no checker immediately warns, and UndefinedVal proceeds to exist until it is actually used anywhere.

So i think this tradition is worth keeping, and the result of null pointer-to-member dereference should first be modeled as UndefinedVal.

More importantly, however, it is worth investigating why doesn't it already magically happen. I suspect it might be related to the other problem of loading from a non-loc.

[kromanenkov]

Null pointer-to-member dereference is now modeled as UndefinedVal. Fixing this also releases us from loading from nonloc SVal.
I delete an assertion suppression in ExprEngine::performTrivialCopy because I can't reproduce its violation.

[dcoughlin]

lib/StaticAnalyzer/Core/ExprEngineC.cpp

462

I don't think pattern matching on the sub expression to find the referred-to declaration is the right thing to do here. It isn't always the case that the casted expression will be a unary pointer to member operation. For example, this is perfectly fine and triggers an assertion failure on your patch:

struct B {
  int f;
};

struct D : public B {
  int g;
};

void foo() {
  D d;
  d.f = 7;

  int B::* pfb = &B::f;
  int D::* pfd = pfb;
  int v = d.*pfd;
}

Note that you can't just propagate the value already computed for the subexpression. Here is a particularly annoying example from the C++ spec:

struct B {
  int f;
};
struct L : public B { };
struct R : public B { };
struct D : public L, R { };

void foo() {
  D d;

  int B::* pb = &B::f;
  int L::* pl = pb;
  int R::* pr = pb;

  int D::* pdl = pl;
  int D::* pdr = pr;

  clang_analyzer_eval(pdl == pdr); // FALSE
  clang_analyzer_eval(pb == pl); // TRUE
}

My guess is this will require accumulating CXXBasePath s or something similar for each cast. I don't know how to do this efficiently.

[dcoughlin]

lib/StaticAnalyzer/Core/ExprEngineC.cpp

462

I don't know how to do this efficiently.

Jordan suggested storing this in a bump-pointer allocated object with a lifetime of the AnalysisDeclContext. The common case is no multiple inheritance, so that should be the fast case.

Maybe the Data could be a pointer union between a DeclaratorDecl and an immutable linked list (with sharing) of CXXBaseSpecifiers from the CastExprs in the AST. The storage for this could be managed with a new manager in SValBuilder.

[dcoughlin]

lib/StaticAnalyzer/Core/ExprEngineC.cpp

462

(The bump pointer-allocated thing would have to have the Decl as well.)

This could also probably live in BasicValueFactory. The extra data would be similar to LazyCompoundValData.

[kromanenkov]

lib/StaticAnalyzer/Core/ExprEngineC.cpp

462

My understanding is that PointerToMember SVal should be represented similar to LazyCompoundVal SVal. If I got you right, PointerToMember SVal should be constructed in VisitUnaryOperator and VisitCast (CK_BaseToDerivedMemberPointer and so on) just add CXXBaseSpecifiers to this SVal?
What do you mean by sharing of immutable linked list?

[dcoughlin]

lib/StaticAnalyzer/Core/ExprEngineC.cpp

462

SVal has very strict size limitations -- you can only store a single pointer for its Data. Since you'll need to have multiple CXXBaseSpecifiers in a single SVal (for example, consider a multiple inheritance hierarchy with a double diamond), the storage for this container will have to live elsewhere.

One way to do this is to bump-pointer allocate linked list nodes for the "path" of casts that the value has taken. If you do this then you can share the memory for the nodes for a shared path prefix for two paths with a common prefix.

[kromanenkov]

According to dcoughlin suggestion PointerToMember SVal is now modeled as a PointerUnion of DeclaratorDecl and bump pointer-allocated object stored DeclaratorDecl and immutable linked list of CXXBaseSpecifiers.

[kromanenkov]

ping

[kromanenkov]

ping

[NoQ]

This looks good to me (bikeshedded a bit), but i think Devin should have another look, because his comments were way deeper than mine.

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

217

Hmm, is it "construct"? Or "constrain"? I think a verbose name wouldn't hurt.
In fact, i suspect that consVals are rather conSVals, where con stands for "concatenate".

lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

882

Hmm, do we need to cover CXXMethodDecl here? Or is it modeled as a function pointer anyway, so no action is needed? Worth commenting, i think.

[kromanenkov]

Thanks for your comments, Artem! Make function name less ambiguous. Also I take liberty to update analogical function name.

[kromanenkov]

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

217

In fact I thought that this entails "consume" :)

lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

882

AFAIU CXXMethodDecl is processed in SVal::getAsFunctionDecl() so i'm for no action.

[dcoughlin]

PointerToMemberData looks like it is on the right track! One thing that is still missing is using the base specifier list to figure out the correct subobject field to read and write from. This is important when there is non-virtual multiple inheritance, as there will be multiple copies of the same field in the same object.

Here are is an example where the current patch is not quite right; both of these should evaluate to true:

void clang_analyzer_eval(int);

struct B {
  int f;
};
struct L1 : public B { };
struct R1 : public B { };
struct M : public L1, R1 { };
struct L2 : public M { };
struct R2 : public M { };
struct D2 : public L2, R2 { };


void diamond() {
  M m;

  static_cast<L1 *>(&m)->f = 7;
  static_cast<R1 *>(&m)->f = 16;

  int L1::* pl1 = &B::f;
  int M::* pm_via_l1 = pl1;

  int R1::* pr1 = &B::f;
  int M::* pm_via_r1 = pr1;

  clang_analyzer_eval(m.*(pm_via_l1) == 7); // expected-warning {{TRUE}}
  clang_analyzer_eval(m.*(pm_via_r1) == 16); // expected-warning {{TRUE}}
}

I suspect you'll need to do something similar to what StoreManager::evalDerivedToBase() does (or maybe just use that function) to figure out the correct location to read and write from when accessing via a pointer to member.

It would also be good to add some tests for the double diamond scenario to ensure the list of path specifiers is constructed in the right order. For example:

void double_diamond() {
  D2 d2;

  static_cast<L1 *>(static_cast<L2 *>(&d2))->f = 1;
  static_cast<L1 *>(static_cast<R2 *>(&d2))->f = 2;
  static_cast<R1 *>(static_cast<L2 *>(&d2))->f = 3;
  static_cast<R1 *>(static_cast<R2 *>(&d2))->f = 4;

  clang_analyzer_eval(d2.*(static_cast<int D2::*>(static_cast<int L2::*>(static_cast<int L1::*>(&B::f)))) == 1); // expected-warning {{TRUE}}
  clang_analyzer_eval(d2.*(static_cast<int D2::*>(static_cast<int R2::*>(static_cast<int L1::*>(&B::f)))) == 2); // expected-warning {{TRUE}}
  clang_analyzer_eval(d2.*(static_cast<int D2::*>(static_cast<int L2::*>(static_cast<int R1::*>(&B::f)))) == 3); // expected-warning {{TRUE}}
  clang_analyzer_eval(d2.*(static_cast<int D2::*>(static_cast<int R2::*>(static_cast<int R1::*>(&B::f)))) == 4); // expected-warning {{TRUE}}
}

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

217

In functional paradigms, 'cons' is used to mean prepending an item the the beginning of a linked list. https://en.wikipedia.org/wiki/Cons

In my opinion, 'prepend' is better than 'cons', which I find super-confusing. I don't think 'concatenate' is quite right, since typically that operation combines two (or more) lists.

lib/StaticAnalyzer/Core/ExprEngineC.cpp

322

These conditional fallthroughs make me very, very uncomfortable because they will broken if any of the intervening cases get special handling in the future.

I think it would safer to factor out code in the "destination" case (here 'CK_LValueBitCast') into a function, call it directly, and then continue regardless of the branch.

Another possibility is to use gotos to directly jump to the default 'destination' case.

472

I think it would be good to be explicit about this fallthrough behavior, as well.

899

Just sticking this in the middle of a fallthrough cascade seems quite brittle. For example, it relies on the sub expression of a unary deref never being a DeclRefExpr to a field. This may be guaranteed by Sema (?) but if so it is quite a non-obvious invariant to rely upon.

I think it would be better the restructure this so that the AddrOf case doesn't fall in the the middle of a fallthrough cascade. You could either factor out the default behavior into a function or use a goto.

lib/StaticAnalyzer/Core/SValBuilder.cpp

298

Can this be removed? There are no tests for it.

[kromanenkov]

lib/StaticAnalyzer/Core/ExprEngineC.cpp

899

It seems that UO_Extension case is the default behavior for this case cascade (just below UO_AddrOf). AFAIU it would be better to explicitly call the default behavior function following by break for each case preceding UO_Extension (UO_Plus,UO_Deref and UO_AddrOf). Or i missed something?

[kromanenkov]

lib/StaticAnalyzer/Core/ExprEngineC.cpp

899

Or maybe just move UO_AddrOf to the beginning of the case cascade and proceed with the default behavior if it falls (no need to change UO_Plus and UO_Deref in that case)?

[kromanenkov]

Thanks for your comments, Devin! You were right about the list of path specifiers construction order, so i fix it. Now the base specifier list is being used for figuring out the correct subobject field. Also this diff changes fallthrough behavior in some cases and renames functions with 'cons' prefix.

[dcoughlin]

Looks good to me, other than some super tiny nits! Thanks for iterating on this -- it is awesome that the analyzer will be able to model this now!!

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

463

I think this deserves a comment about why it is a NonLoc and why it needs to keep the PointerToMemberData with the list of CXXBaseSpecifiers.

lib/StaticAnalyzer/Core/ExprEngineC.cpp

269

While we're here we might as well correct the misspelling --> "path sensitivity".

test/Analysis/pointer-to-member.cpp

74–172

Can you include the warning text in the expected-warning directive? This will ensure we keep emitting the correct warning in the future.

[kromanenkov]

Fix issues pointed by @dcoughlin and rebase patch on master.

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

public:
 
   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 DeclaratorDecl *D;
+  llvm::ImmutableList<const CXXBaseSpecifier *> L;
+
+public:
+  PointerToMemberData(const DeclaratorDecl *D,
+                      llvm::ImmutableList<const CXXBaseSpecifier *> L)
+    : D(D), L(L) {}
+
+  typedef llvm::ImmutableList<const CXXBaseSpecifier *>::iterator iterator;
+  iterator begin() const { return L.begin(); }
+  iterator end() const { return L.end(); }
+
+  static void Profile(llvm::FoldingSetNodeID& ID, const DeclaratorDecl *D,
+                      llvm::ImmutableList<const CXXBaseSpecifier *> L);
+
+  void Profile(llvm::FoldingSetNodeID& ID) { Profile(ID, D, L); }
+  const DeclaratorDecl *getDeclaratorDecl() const {return D;}
+  llvm::ImmutableList<const CXXBaseSpecifier *> getCXXBaseList() const {
+    return L;
+  }
+};
+
 class BasicValueFactory {
   typedef llvm::FoldingSet<llvm::FoldingSetNodeWrapper<llvm::APSInt> >
           APSIntSetTy;
 
   ASTContext &Ctx;
   llvm::BumpPtrAllocator& BPAlloc;
 
   APSIntSetTy   APSIntSet;
   void *        PersistentSVals;
   void *        PersistentSValPairs;
 
   llvm::ImmutableList<SVal>::Factory SValListFactory;
+  llvm::ImmutableList<const CXXBaseSpecifier*>::Factory CXXBaseListFactory;
   llvm::FoldingSet<CompoundValData>  CompoundValDataSet;
   llvm::FoldingSet<LazyCompoundValData> LazyCompoundValDataSet;
+  llvm::FoldingSet<PointerToMemberData> PointerToMemberDataSet;
 
   // This is private because external clients should use the factory
   // method that takes a QualType.
   const llvm::APSInt& getValue(uint64_t X, unsigned BitWidth, bool isUnsigned);
 
 public:
   BasicValueFactory(ASTContext &ctx, llvm::BumpPtrAllocator &Alloc)
     : Ctx(ctx), BPAlloc(Alloc), PersistentSVals(nullptr),
-      PersistentSValPairs(nullptr), SValListFactory(Alloc) {}
+      PersistentSValPairs(nullptr), SValListFactory(Alloc),
+      CXXBaseListFactory(Alloc) {}
 
   ~BasicValueFactory();
 
   ASTContext &getContext() const { return Ctx; }
 
   const llvm::APSInt& getValue(const llvm::APSInt& X);
   const llvm::APSInt& getValue(const llvm::APInt& X, bool isUnsigned);
   const llvm::APSInt& getValue(uint64_t X, QualType T);
public:
   }
 
   const CompoundValData *getCompoundValData(QualType T,
                                             llvm::ImmutableList<SVal> Vals);
 
   const LazyCompoundValData *getLazyCompoundValData(const StoreRef &store,
                                             const TypedValueRegion *region);
 
+  const PointerToMemberData *getPointerToMemberData(
+      const DeclaratorDecl *DD,
+      llvm::ImmutableList<const CXXBaseSpecifier *> L);
+
   llvm::ImmutableList<SVal> getEmptySValList() {
     return SValListFactory.getEmptyList();
   }
 
   llvm::ImmutableList<SVal> consVals(SVal X, llvm::ImmutableList<SVal> L) {
     return SValListFactory.add(X, L);
   }
 
+  llvm::ImmutableList<const CXXBaseSpecifier *> getEmptyCXXBaseList() {
+    return CXXBaseListFactory.getEmptyList();
+  }
+
+  llvm::ImmutableList<const CXXBaseSpecifier *> consCXXBase(
+      const CXXBaseSpecifier *CBS,
+      llvm::ImmutableList<const CXXBaseSpecifier *> L) {
+    return CXXBaseListFactory.add(CBS, L);
+  }
+
+  const clang::ento::PointerToMemberData *accumCXXBase(
+      llvm::iterator_range<CastExpr::path_const_iterator> PathRange,
+      const nonloc::PointerToMember &PTM);
+
   const llvm::APSInt* evalAPSInt(BinaryOperator::Opcode Op,
                                      const llvm::APSInt& V1,
                                      const llvm::APSInt& V2);
 
   const std::pair<SVal, uintptr_t>&
   getPersistentSValWithData(const SVal& V, uintptr_t Data);
 
   const std::pair<SVal, SVal>&

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 DeclaratorDecl *DD);
+
   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));
   }
 
   NonLoc makeArrayIndex(uint64_t idx) {
     return nonloc::ConcreteInt(BasicVals.getValue(idx, ArrayIndexTy));
   }
 

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

 //==------------------------------------------------------------------------==//
 
 namespace clang {
 
 namespace ento {
 
 class CompoundValData;
 class LazyCompoundValData;
+class PointerToMemberData;
 class ProgramState;
 class BasicValueFactory;
 class MemRegion;
 class TypedValueRegion;
 class MemRegionManager;
 class ProgramStateManager;
 class SValBuilder;
 
static bool isKind(const SVal& V) {
     return V.getBaseKind() == NonLocKind &&
            V.getSubKind() == LazyCompoundValKind;
   }
   static bool isKind(const NonLoc& V) {
     return V.getSubKind() == LazyCompoundValKind;
   }
 };
 
+class PointerToMember : public NonLoc {
+  friend class ento::SValBuilder;
+
+public:
+  typedef llvm::PointerUnion<const DeclaratorDecl *,
+                             const PointerToMemberData *> PTMDataType;
+  const PTMDataType getPTMData() const {
+    return PTMDataType::getFromOpaqueValue(const_cast<void *>(Data));
+  }
+  bool isNullMemberPointer() const {
+    return getPTMData().isNull();
+  }
+  const DeclaratorDecl *getDecl() const;
+  template<typename AdjustedDecl>
+  const AdjustedDecl* getDeclAs() const {
+    return dyn_cast_or_null<AdjustedDecl>(getDecl());
+  }
+  typedef llvm::ImmutableList<const CXXBaseSpecifier *>::iterator iterator;
+  iterator begin() const;
+  iterator end() const;
+
+private:
+  explicit PointerToMember(const PTMDataType D)
+    : NonLoc(PointerToMemberKind, D.getOpaqueValue()) {}
+  friend class SVal;
+  PointerToMember() {}
+  static bool isKind(const SVal& V) {
+    return V.getBaseKind() == NonLocKind &&
+           V.getSubKind() == PointerToMemberKind;
+  }
+
+  static bool isKind(const NonLoc& V) {
+    return V.getSubKind() == PointerToMemberKind;
+  }
+};
+
 } // end namespace ento::nonloc
 
 //==------------------------------------------------------------------------==//
 //  Subclasses of Loc.
 //==------------------------------------------------------------------------==//
 
 namespace loc {
 

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

ABSTRACT_SVAL_WITH_KIND(Loc, DefinedSVal)
       LOC_SVAL(GotoLabel, Loc)
       LOC_SVAL(MemRegionVal, Loc)
     ABSTRACT_SVAL_WITH_KIND(NonLoc, DefinedSVal)
       NONLOC_SVAL(CompoundVal, NonLoc)
       NONLOC_SVAL(ConcreteInt, NonLoc)
       NONLOC_SVAL(LazyCompoundVal, NonLoc)
       NONLOC_SVAL(LocAsInteger, NonLoc)
       NONLOC_SVAL(SymbolVal, NonLoc)
+      NONLOC_SVAL(PointerToMember, NonLoc)
 
 #undef NONLOC_SVAL
 #undef LOC_SVAL
 #undef ABSTRACT_SVAL_WITH_KIND
 #undef ABSTRACT_SVAL
 #undef BASIC_SVAL

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 DeclaratorDecl *D,
+    llvm::ImmutableList<const CXXBaseSpecifier *> L) {
+  ID.AddPointer(D);
+  ID.AddPointer(L.getInternalPointer());
+}
+
 typedef std::pair<SVal, uintptr_t> SValData;
 typedef std::pair<SVal, SVal> SValPair;
 
 namespace llvm {
 template<> struct FoldingSetTrait<SValData> {
   static inline void Profile(const SValData& X, llvm::FoldingSetNodeID& ID) {
     X.first.Profile(ID);
     ID.AddPointer( (void*) X.second);
if (!D) {
     D = (LazyCompoundValData*) BPAlloc.Allocate<LazyCompoundValData>();
     new (D) LazyCompoundValData(store, region);
     LazyCompoundValDataSet.InsertNode(D, InsertPos);
   }
 
   return D;
 }
 
+const PointerToMemberData *BasicValueFactory::getPointerToMemberData(
+    const DeclaratorDecl *DD, llvm::ImmutableList<const CXXBaseSpecifier*> L) {
+  llvm::FoldingSetNodeID ID;
+  PointerToMemberData::Profile(ID, DD, L);
+  void *InsertPos;
+
+  PointerToMemberData *D =
+      PointerToMemberDataSet.FindNodeOrInsertPos(ID, InsertPos);
+
+  if (!D) {
+    D = (PointerToMemberData*) BPAlloc.Allocate<PointerToMemberData>();
+    new (D) PointerToMemberData(DD, L);
+    PointerToMemberDataSet.InsertNode(D, InsertPos);
+  }
+
+  return D;
+}
+
+const clang::ento::PointerToMemberData *BasicValueFactory::accumCXXBase(
+    llvm::iterator_range<CastExpr::path_const_iterator> PathRange,
+    const nonloc::PointerToMember &PTM) {
+  nonloc::PointerToMember::PTMDataType PTMDT = PTM.getPTMData();
+  const DeclaratorDecl *DD = nullptr;
+  llvm::ImmutableList<const CXXBaseSpecifier *> PathList;
+
+  if (PTMDT.isNull() || PTMDT.is<const DeclaratorDecl *>()) {
+    if (PTMDT.is<const DeclaratorDecl *>())
+      DD = PTMDT.get<const DeclaratorDecl *>();
+
+    PathList = CXXBaseListFactory.getEmptyList();
+  } else { // const PointerToMemberData *
+    const PointerToMemberData *PTMD =
+        PTMDT.get<const PointerToMemberData *>();
+    DD = PTMD->getDeclaratorDecl();
+
+    PathList = PTMD->getCXXBaseList();
+  }
+
+  for (const auto &I : PathRange)
+    PathList = consCXXBase(I, PathList);
+  return getPointerToMemberData(DD, PathList);
+}
+
 const llvm::APSInt*
 BasicValueFactory::evalAPSInt(BinaryOperator::Opcode Op,
                              const llvm::APSInt& V1, const llvm::APSInt& V2) {
 
   switch (Op) {
     default:
       assert (false && "Invalid Opcode.");
 

lib/StaticAnalyzer/Core/ExprEngineC.cpp

 //=-- ExprEngineC.cpp - ExprEngine support for C expressions ----*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file defines ExprEngine's support for C expressions.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 
 using namespace clang;
 using namespace ento;
 using llvm::APSInt;
 
 void ExprEngine::VisitBinaryOperator(const BinaryOperator* B,
for (ExplodedNodeSet::iterator I = dstPreStmt.begin(), E = dstPreStmt.end();
       const LocationContext *LCtx = subExprNode->getLocationContext();
       evalLoad(Dst, CastE, CastE, subExprNode, state, state->getSVal(Ex, LCtx));
     }
     return;
   }
 
   // All other casts.
   QualType T = CastE->getType();
   QualType ExTy = Ex->getType();
 
   if (const ExplicitCastExpr *ExCast=dyn_cast_or_null<ExplicitCastExpr>(CastE))
     T = ExCast->getTypeAsWritten();
 
   StmtNodeBuilder Bldr(dstPreStmt, Dst, *currBldrCtx);
   for (ExplodedNodeSet::iterator I = dstPreStmt.begin(), E = dstPreStmt.end();
        I != E; ++I) {
 
switch (CastE->getCastKind()) {
         ProgramStateRef state = Pred->getState();
         const LocationContext *LCtx = Pred->getLocationContext();
         SVal V = state->getSVal(Ex, LCtx);
         state = state->BindExpr(CastE, LCtx, V);
         Bldr.generateNode(CastE, Pred, state);
         continue;
       }
       case CK_MemberPointerToBoolean:
-        // FIXME: For now, member pointers are represented by void *.
-        // FALLTHROUGH
+      case CK_PointerToBoolean: {
+        SVal V = state->getSVal(Ex, LCtx);
+        auto PTMSV = V.getAs<nonloc::PointerToMember>();
+        if (PTMSV)
+          V = svalBuilder.makeTruthVal(!PTMSV->isNullMemberPointer(), ExTy);
+        if (V.isUndef() || PTMSV) {
+          state = state->BindExpr(CastE, LCtx, V);
+          Bldr.generateNode(CastE, Pred, state);
+          continue;
+        }
+      }
       case CK_Dependent:
       case CK_ArrayToPointerDecay:
       case CK_BitCast:
       case CK_AddressSpaceConversion:
       case CK_BooleanToSignedIntegral:
       case CK_NullToPointer:
       case CK_IntegralToPointer:
-      case CK_PointerToIntegral:
-      case CK_PointerToBoolean:
+      case CK_PointerToIntegral: {
+        SVal V = state->getSVal(Ex, LCtx);
+        if (V.getAs<nonloc::PointerToMember>()) {
+          state = state->BindExpr(CastE, LCtx, UnknownVal());
+          Bldr.generateNode(CastE, Pred, state);
+          continue;
+        }
+      }
       case CK_IntegralToBoolean:
       case CK_IntegralToFloating:
       case CK_FloatingToIntegral:
       case CK_FloatingToBoolean:
       case CK_FloatingCast:
       case CK_FloatingRealToComplex:
       case CK_FloatingComplexToReal:
       case CK_FloatingComplexToBoolean:
switch (CastE->getCastKind()) {
             svalBuilder.conjureSymbolVal(nullptr, CastE, LCtx, resultType,
                                          currBldrCtx->blockCount());
         }
         state = state->BindExpr(CastE, LCtx, val);
         Bldr.generateNode(CastE, Pred, state);
         continue;
       }
       case CK_NullToMemberPointer: {
-        // FIXME: For now, member pointers are represented by void *.
-        SVal V = svalBuilder.makeNull();
+        SVal V = svalBuilder.getMemberPointer(nullptr);
         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(CastE->getSubExpr(), 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));
+          state = state->BindExpr(CastE, LCtx, CastedPTMSV);
+          Bldr.generateNode(CastE, Pred, state);
+          continue;
+        }
+        // If getAs failed just fall through to default behaviour.
+      }
       // Various C++ casts that are not handled yet.
       case CK_ToUnion:
-      case CK_BaseToDerivedMemberPointer:
-      case CK_DerivedToBaseMemberPointer:
-      case CK_ReinterpretMemberPointer:
       case CK_VectorSplat: {
         // Recover some path-sensitivty by conjuring a new value.
         QualType resultType = CastE->getType();
         if (CastE->isGLValue())
           resultType = getContext().getPointerType(resultType);
         SVal result = svalBuilder.conjureSymbolVal(nullptr, CastE, LCtx,
                                                    resultType,
                                                    currBldrCtx->blockCount());
case UO_Imag: {
       Bldr.generateNode(U, *I, state->BindExpr(U, LCtx, X));
       break;
     }
 
     case UO_Plus:
       assert(!U->isGLValue());
       // FALL-THROUGH.
     case UO_Deref:
-    case UO_AddrOf:
+    case UO_AddrOf: {
+      // Process pointer-to-member address operation
+      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)) {
+          ProgramStateRef State = (*I)->getState();
+          const LocationContext *LCtx = (*I)->getLocationContext();
+          SVal SV = svalBuilder.getMemberPointer(cast<DeclaratorDecl>(VD));
+          Bldr.generateNode(U, *I, State->BindExpr(U, LCtx, SV));
+          break;
+        }
+      }
+      //Fall through in case of not pointer-to-member address operation
+    }
     case UO_Extension: {
       // FIXME: We can probably just have some magic in Environment::getSVal()
       // that propagates values, instead of creating a new node here.
       //
       // Unary "+" is a no-op, similar to a parentheses.  We still have places
       // where it may be a block-level expression, so we need to
       // generate an extra node that just propagates the value of the
       // subexpression.

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 DeclaratorDecl* DD) {
+  return nonloc::PointerToMember(DD);
+}
+
 DefinedSVal SValBuilder::getFunctionPointer(const FunctionDecl *func) {
   return loc::MemRegionVal(MemMgr.getFunctionCodeRegion(func));
 }
 
 DefinedSVal SValBuilder::getBlockPointer(const BlockDecl *block,
                                          CanQualType locTy,
                                          const LocationContext *locContext,
                                          unsigned blockCount) {
case Stmt::IntegerLiteralClass:
     return makeIntVal(cast<IntegerLiteral>(E));
 
   case Stmt::ObjCBoolLiteralExprClass:
     return makeBoolVal(cast<ObjCBoolLiteralExpr>(E));
 
   case Stmt::CXXNullPtrLiteralExprClass:
     return makeNull();
 
+  case Stmt::UnaryOperatorClass: {
+    const UnaryOperator *UO = dyn_cast<UnaryOperator>(E);
+    if (const DeclRefExpr *DRE =
+            dyn_cast<DeclRefExpr>(UO->getSubExpr()->IgnoreParenCasts())) {
+      if (const DeclaratorDecl *DD =
+              dyn_cast_or_null<DeclaratorDecl>(DRE->getDecl()))
+        if (isa<CXXMethodDecl>(DD) || isa<FieldDecl>(DD))
+          return getMemberPointer(DD);
+    }
+    return None;
+  }
+
   case Stmt::ImplicitCastExprClass: {
     const CastExpr *CE = cast<CastExpr>(E);
     switch (CE->getCastKind()) {
     default:
       break;
     case CK_ArrayToPointerDecay:
     case CK_BitCast: {
       const Expr *SE = CE->getSubExpr();

lib/StaticAnalyzer/Core/SVals.cpp

 //  abstract r-values for use with path-sensitive value tracking.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "llvm/Support/raw_ostream.h"
+#include "clang/AST/DeclCXX.h"
 using namespace clang;
 using namespace ento;
 using llvm::APSInt;
 
 //===----------------------------------------------------------------------===//
 // Symbol iteration within an SVal.
 //===----------------------------------------------------------------------===//
 

 const FunctionDecl *SVal::getAsFunctionDecl() const {
   if (Optional<loc::MemRegionVal> X = getAs<loc::MemRegionVal>()) {
     const MemRegion* R = X->getRegion();
     if (const FunctionCodeRegion *CTR = R->getAs<FunctionCodeRegion>())
       if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(CTR->getDecl()))
         return FD;
   }
 
+  if (auto X = getAs<nonloc::PointerToMember>()) {
+    if (const CXXMethodDecl *MD = dyn_cast_or_null<CXXMethodDecl>(X->getDecl()))
+      return MD;
+  }
   return nullptr;
 }
 
 /// \brief If this SVal is a location (subclasses Loc) and wraps a symbol,
 /// return that SymbolRef.  Otherwise return 0.
 ///
 /// Implicit casts (ex: void* -> char*) can turn Symbolic region into Element
 /// region. If that is the case, gets the underlining region.

 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();
 }
 
+const DeclaratorDecl *nonloc::PointerToMember::getDecl() const {
+  const auto PTMD = this->getPTMData();
+  if (PTMD.isNull())
+    return nullptr;
+
+  const DeclaratorDecl *DD = nullptr;
+  if (PTMD.is<const DeclaratorDecl *>())
+    DD = PTMD.get<const DeclaratorDecl *>();
+  else
+    DD = PTMD.get<const PointerToMemberData *>()->getDeclaratorDecl();
+
+  return DD;
+}
+
 //===----------------------------------------------------------------------===//
 // 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 DeclaratorDecl *>())
+    return nonloc::PointerToMember::iterator();
+  return PTMD.get<const PointerToMemberData *>()->begin();
+}
+
+nonloc::PointerToMember::iterator nonloc::PointerToMember::end() const {
+  const PTMDataType PTMD = getPTMData();
+  if (PTMD.is<const DeclaratorDecl *>())
+    return nonloc::PointerToMember::iterator();
+  return PTMD.get<const PointerToMemberData *>()->end();
+}
+
 //===----------------------------------------------------------------------===//
 // Useful predicates.
 //===----------------------------------------------------------------------===//
 
 bool SVal::isConstant() const {
   return getAs<nonloc::ConcreteInt>() || getAs<loc::ConcreteInt>();
 }
 
switch (getSubKind()) {
     }
     case nonloc::LazyCompoundValKind: {
       const nonloc::LazyCompoundVal &C = castAs<nonloc::LazyCompoundVal>();
       os << "lazyCompoundVal{" << const_cast<void *>(C.getStore())
          << ',' << C.getRegion()
          << '}';
       break;
     }
+    case nonloc::PointerToMemberKind: {
+      os << "pointerToMember{";
+      const nonloc::PointerToMember &CastRes =
+          castAs<nonloc::PointerToMember>();
+      if (CastRes.getDecl())
+        os << "|" << CastRes.getDecl()->getQualifiedNameAsString() << "|";
+      bool first = true;
+      for (const auto &I : CastRes) {
+        if (first) {
+          os << ' '; first = false;
+        }
+        else
+          os << ", ";
+
+        os << (*I).getType().getAsString();
+      }
+
+      os << '}';
+      break;
+    }
     default:
       assert (false && "Pretty-printed not implemented for this NonLoc.");
       break;
   }
 }
 
 void Loc::dumpToStream(raw_ostream &os) const {
   switch (getSubKind()) {

lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp

ProgramStateRef SimpleConstraintManager::assumeAux(ProgramStateRef state,
   }
 
   case nonloc::ConcreteIntKind: {
     bool b = Cond.castAs<nonloc::ConcreteInt>().getValue() != 0;
     bool isFeasible = b ? Assumption : !Assumption;
     return isFeasible ? state : nullptr;
   }
 
+  case nonloc::PointerToMemberKind: {
+    bool IsNull = !Cond.castAs<nonloc::PointerToMember>().isNullMemberPointer();
+    bool IsFeasible = IsNull ? Assumption : !Assumption;
+    return IsFeasible ? state : nullptr;
+  }
+
   case nonloc::LocAsIntegerKind:
     return assume(state, Cond.castAs<nonloc::LocAsInteger>().getLoc(),
                   Assumption);
   } // end switch
 }
 
 ProgramStateRef SimpleConstraintManager::assumeWithinInclusiveRange(
     ProgramStateRef State, NonLoc Value, const llvm::APSInt &From,

lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

SVal SimpleSValBuilder::dispatchCast(SVal Val, QualType CastTy) {
   return Val.getAs<Loc>() ? evalCastFromLoc(Val.castAs<Loc>(), CastTy)
                            : evalCastFromNonLoc(Val.castAs<NonLoc>(), CastTy);
 }
 
 SVal SimpleSValBuilder::evalCastFromNonLoc(NonLoc val, QualType castTy) {
 
   bool isLocType = Loc::isLocType(castTy);
 
+  if (val.getAs<nonloc::PointerToMember>())
+    return val;
+
   if (Optional<nonloc::LocAsInteger> LI = val.getAs<nonloc::LocAsInteger>()) {
     if (isLocType)
       return LI->getLoc();
 
     // FIXME: Correctly support promotions/truncations.
     unsigned castSize = Context.getTypeSize(castTy);
     if (castSize == LI->getNumBits())
       return val;
switch (op) {
       case BO_And:
         return evalCastFromNonLoc(lhs, resultTy);
     }
 
   while (1) {
     switch (lhs.getSubKind()) {
     default:
       return makeSymExprValNN(state, op, lhs, rhs, resultTy);
+    case nonloc::PointerToMemberKind: {
+      assert(rhs.getSubKind() == nonloc::PointerToMemberKind &&
+             "Both SVals should have pointer-to-member-type");
+      auto LPTM = lhs.castAs<nonloc::PointerToMember>(),
+           RPTM = rhs.castAs<nonloc::PointerToMember>();
+      auto LPTMD = LPTM.getPTMData(), RPTMD = RPTM.getPTMData();
+      switch (op) {
+        case BO_EQ:
+          return makeTruthVal(LPTMD == RPTMD, resultTy);
+        case BO_NE:
+          return makeTruthVal(LPTMD != RPTMD, resultTy);
+        default:
+          return UnknownVal();
+      }
+    }
     case nonloc::LocAsIntegerKind: {
       Loc lhsL = lhs.castAs<nonloc::LocAsInteger>().getLoc();
       switch (rhs.getSubKind()) {
         case nonloc::LocAsIntegerKind:
           return evalBinOpLL(state, op, lhsL,
                              rhs.castAs<nonloc::LocAsInteger>().getLoc(),
                              resultTy);
         case nonloc::ConcreteIntKind: {
case loc::MemRegionValKind: {
     return UnknownVal();
   }
   }
 }
 
 SVal SimpleSValBuilder::evalBinOpLN(ProgramStateRef state,
                                   BinaryOperator::Opcode op,
                                   Loc lhs, NonLoc rhs, QualType resultTy) {
+  if (op >= BO_PtrMemD && op <= BO_PtrMemI) {
+    if (auto PTMSV = rhs.getAs<nonloc::PointerToMember>()) {
+      if (PTMSV->isNullMemberPointer())
+        return UndefinedVal();
+      if (const FieldDecl *FD = PTMSV->getDeclAs<FieldDecl>())
+        return state->getLValue(FD, lhs);
+    }
+
+    return rhs;
+  }
+
   assert(!BinaryOperator::isComparisonOp(op) &&
          "arguments to comparison ops must be of the same type");
 
   // Special case: rhs is a zero constant.
   if (rhs.isZeroConstant())
     return lhs;
 
   // We are dealing with pointer arithmetic.

test/Analysis/pointer-to-member.cpp

void testConditionalUse() {
   clang_analyzer_eval(A::MemberFnPointer(0)); // expected-warning{{FALSE}}
 }
 
 
 void testComparison() {
   clang_analyzer_eval(&A::getPtr == &A::getPtr); // expected-warning{{TRUE}}
   clang_analyzer_eval(&A::getPtr == 0); // expected-warning{{FALSE}}
 
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(&A::m_ptr == &A::m_ptr); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(&A::m_ptr == &A::m_ptr); // expected-warning{{TRUE}}
 }
 
 namespace PR15742 {
   template <class _T1, class _T2> struct A {
     A (const _T1 &, const _T2 &);
   };
   
   typedef void *NPIdentifier;
public:
     bool Find(const NPIdentifier *, unsigned, NPIdentifier *);
   };
 
   void InitStaticData () {
     C::MethodMapMember(0, &C::Find); // don't crash
   }
 }
 
-// ---------------
-// FALSE NEGATIVES
-// ---------------
-
 bool testDereferencing() {
   A obj;
   obj.m_ptr = 0;
 
   A::MemberPointer member = &A::m_ptr;
 
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(obj.*member == 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(obj.*member == 0); // expected-warning{{TRUE}}
 
   member = 0;
 
-  // FIXME: Should emit a null dereference.
-  return obj.*member; // no-warning
+  return obj.*member; // expected-warning{{}}
+}
+
+namespace testPointerToMemberFunction {
+  struct A {
+    virtual int foo() { return 1; }
+    int bar() { return 2;  }
+  };
+
+  struct B : public A {
+    virtual int foo() { return 3; }
+  };
+
+  typedef int (A::*AFnPointer)();
+  typedef int (B::*BFnPointer)();
+
+  void testPointerToMemberCasts() {
+    AFnPointer AFP = &A::bar;
+    BFnPointer StaticCastedBase2Derived = static_cast<BFnPointer>(&A::bar),
+               CCastedBase2Derived = (BFnPointer) (&A::bar);
+    A a;
+    B b;
+
+    clang_analyzer_eval((a.*AFP)() == 2); // expected-warning{{TRUE}}
+    clang_analyzer_eval((b.*StaticCastedBase2Derived)() == 2); // expected-warning{{TRUE}}
+    clang_analyzer_eval(((b.*CCastedBase2Derived)() == 2)); // expected-warning{{TRUE}}
+  }
+
+  void testPointerToMemberVirtualCall() {
+    A a;
+    B b;
+    A *APtr = &a;
+    AFnPointer AFP = &A::foo;
+
+    clang_analyzer_eval((APtr->*AFP)() == 1); // expected-warning{{TRUE}}
+
+    APtr = &b;
+
+    clang_analyzer_eval((APtr->*AFP)() == 3); // expected-warning{{TRUE}}
+  }
+} // end of testPointerToMemberFunction namespace
+
+namespace testPointerToMemberData {
+  struct A {
+    int i;
+  };
+
+  void testPointerToMemberData() {
+    int A::*AMdPointer = &A::i;
+    A a;
+
+    a.i = 42;
+    a.*AMdPointer += 1;
+
+    clang_analyzer_eval(a.i == 43); // expected-warning{{TRUE}}
+  }
+} // end of testPointerToMemberData namespace
+
+namespace testPointerToMemberMiscCasts {
+struct B {
+  int f;
+};
+
+struct D : public B {
+  int g;
+};
+
+void foo() {
+  D d;
+  d.f = 7;
+
+  int B::* pfb = &B::f;
+  int D::* pfd = pfb;
+  int v = d.*pfd;
+
+  clang_analyzer_eval(v == 7); // expected-warning{{TRUE}}
+}
+} // end of testPointerToMember namespace
+
+namespace testPointerToMemberMiscCasts2 {
+struct B {
+  int f;
+};
+struct L : public B { };
+struct R : public B { };
+struct D : public L, R { };
+
+void foo() {
+  D d;
+
+  int B::* pb = &B::f;
+  int L::* pl = pb;
+  int R::* pr = pb;
+
+  int D::* pdl = pl;
+  int D::* pdr = pr;
+
+  clang_analyzer_eval(pdl == pdr); // expected-warning{{FALSE}}
+  clang_analyzer_eval(pb == pl); // expected-warning{{TRUE}}
 }
+} // end of testPointerToMemberMiscCasts2 namespace