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

[Devin Coughlin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL289873: [analyzer] Add a new SVal to support 
pointer-to-member operations. (authored by dcoughlin).

Changed prior to commit:
  https://reviews.llvm.org/D25475?vs=81598=81655#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D25475

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/SVals.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  cfe/trunk/test/Analysis/pointer-to-member.cpp

Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -479,6 +479,22 @@
 return X.isValid() ? svalBuilder.evalComplement(X.castAs()) : X;
   }
 
+  ProgramStateRef handleLValueBitCast(ProgramStateRef state, const Expr *Ex,
+  const LocationContext *LCtx, QualType T,
+  QualType ExTy, const CastExpr *CastE,
+  StmtNodeBuilder ,
+  ExplodedNode *Pred);
+
+  ProgramStateRef handleLVectorSplat(ProgramStateRef state,
+ const LocationContext *LCtx,
+ const CastExpr *CastE,
+ StmtNodeBuilder ,
+ ExplodedNode *Pred);
+
+  void handleUOExtension(ExplodedNodeSet::iterator I,
+ const UnaryOperator* U,
+ StmtNodeBuilder );
+
 public:
 
   SVal evalBinOp(ProgramStateRef state, BinaryOperator::Opcode op,
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
@@ -66,6 +66,7 @@
   NONLOC_SVAL(LazyCompoundVal, NonLoc)
   NONLOC_SVAL(LocAsInteger, NonLoc)
   NONLOC_SVAL(SymbolVal, NonLoc)
+  NONLOC_SVAL(PointerToMember, NonLoc)
 
 #undef NONLOC_SVAL
 #undef LOC_SVAL
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -204,6 +204,8 @@
const LocationContext *LCtx,
unsigned count);
 
+  DefinedSVal getMemberPointer(const DeclaratorDecl *DD);
+
   DefinedSVal getFunctionPointer(const FunctionDecl *func);
   
   DefinedSVal getBlockPointer(const BlockDecl *block, CanQualType locTy,
@@ -226,6 +228,14 @@
 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));
   }
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -59,6 +59,29 @@
   void Profile(llvm::FoldingSetNodeID& ID) { Profile(ID, store, region); }
 };
 
+class PointerToMemberData: public llvm::FoldingSetNode {
+  const DeclaratorDecl *D;
+  llvm::ImmutableList L;
+
+public:
+  PointerToMemberData(const DeclaratorDecl *D,
+  llvm::ImmutableList L)
+: D(D), L(L) {}
+
+  typedef llvm::ImmutableList::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 L);
+
+  void Profile(llvm::FoldingSetNodeID& ID) { Profile(ID, D, L); }
+  const DeclaratorDecl *getDeclaratorDecl() const {return D;}
+  

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

[Kirill Romanenkov via Phabricator via cfe-commits]

kromanenkov updated this revision to Diff 81598.
kromanenkov added a comment.

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


https://reviews.llvm.org/D25475

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  lib/StaticAnalyzer/Core/BasicValueFactory.cpp
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SVals.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/pointer-to-member.cpp

Index: test/Analysis/pointer-to-member.cpp
===
--- test/Analysis/pointer-to-member.cpp
+++ test/Analysis/pointer-to-member.cpp
@@ -35,8 +35,7 @@
   clang_analyzer_eval(::getPtr == ::getPtr); // expected-warning{{TRUE}}
   clang_analyzer_eval(::getPtr == 0); // expected-warning{{FALSE}}
 
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(::m_ptr == ::m_ptr); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(::m_ptr == ::m_ptr); // expected-warning{{TRUE}}
 }
 
 namespace PR15742 {
@@ -62,21 +61,156 @@
   }
 }
 
-// ---
-// FALSE NEGATIVES
-// ---
-
 bool testDereferencing() {
   A obj;
   obj.m_ptr = 0;
 
   A::MemberPointer member = ::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{{The result of the '.*' expression is undefined}}
+}
+
+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 = ::bar;
+BFnPointer StaticCastedBase2Derived = static_cast(::bar),
+   CCastedBase2Derived = (BFnPointer) (::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 = 
+AFnPointer AFP = ::foo;
+
+clang_analyzer_eval((APtr->*AFP)() == 1); // expected-warning{{TRUE}}
+
+APtr = 
+
+clang_analyzer_eval((APtr->*AFP)() == 3); // expected-warning{{TRUE}}
+  }
+} // end of testPointerToMemberFunction namespace
+
+namespace testPointerToMemberData {
+  struct A {
+int i;
+  };
+
+  void testPointerToMemberData() {
+int A::*AMdPointer = ::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 = ::f;
+  int D::* pfd = pfb;
+  int v = d.*pfd;
+
+  clang_analyzer_eval(v == 7); // expected-warning{{TRUE}}
+}
+} // end of testPointerToMemberMiscCasts 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 = ::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
+
+namespace testPointerToMemberDiamond {
+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()->f = 7;
+  static_cast()->f = 16;
+
+  int L1::* pl1 = ::f;
+  int M::* pm_via_l1 = pl1;
+
+  int R1::* pr1 = ::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}}
+}
+
+void double_diamond() {
+  D2 d2;
+
+  static_cast(static_cast())->f = 1;
+  static_cast(static_cast())->f = 2;
+  static_cast(static_cast())->f = 3;
+  static_cast(static_cast())->f = 4;
+
+  clang_analyzer_eval(d2.*(static_cast(static_cast(static_cast(::f == 1); // expected-warning {{TRUE}}
+  

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

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

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!!




Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h:463
 
+class PointerToMember : public NonLoc {
+  friend class ento::SValBuilder;

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.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:269
+StmtNodeBuilder , ExplodedNode* Pred) {
+  // Recover some path-sensitivty by conjuring a new value.
+  QualType resultType = CastE->getType();

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



Comment at: test/Analysis/pointer-to-member.cpp:74
 
-  // FIXME: Should emit a null dereference.
-  return obj.*member; // no-warning
+  return obj.*member; // expected-warning{{}}
+}

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


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Kirill Romanenkov via Phabricator via cfe-commits]

kromanenkov updated this revision to Diff 81352.
kromanenkov added a comment.

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.


https://reviews.llvm.org/D25475

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  lib/StaticAnalyzer/Core/BasicValueFactory.cpp
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SVals.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/pointer-to-member.cpp

Index: test/Analysis/pointer-to-member.cpp
===
--- test/Analysis/pointer-to-member.cpp
+++ test/Analysis/pointer-to-member.cpp
@@ -35,8 +35,7 @@
   clang_analyzer_eval(::getPtr == ::getPtr); // expected-warning{{TRUE}}
   clang_analyzer_eval(::getPtr == 0); // expected-warning{{FALSE}}
 
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(::m_ptr == ::m_ptr); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(::m_ptr == ::m_ptr); // expected-warning{{TRUE}}
 }
 
 namespace PR15742 {
@@ -62,21 +61,156 @@
   }
 }
 
-// ---
-// FALSE NEGATIVES
-// ---
-
 bool testDereferencing() {
   A obj;
   obj.m_ptr = 0;
 
   A::MemberPointer member = ::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 = ::bar;
+BFnPointer StaticCastedBase2Derived = static_cast(::bar),
+   CCastedBase2Derived = (BFnPointer) (::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 = 
+AFnPointer AFP = ::foo;
+
+clang_analyzer_eval((APtr->*AFP)() == 1); // expected-warning{{TRUE}}
+
+APtr = 
+
+clang_analyzer_eval((APtr->*AFP)() == 3); // expected-warning{{TRUE}}
+  }
+} // end of testPointerToMemberFunction namespace
+
+namespace testPointerToMemberData {
+  struct A {
+int i;
+  };
+
+  void testPointerToMemberData() {
+int A::*AMdPointer = ::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 = ::f;
+  int D::* pfd = pfb;
+  int v = d.*pfd;
+
+  clang_analyzer_eval(v == 7); // expected-warning{{TRUE}}
+}
+} // end of testPointerToMemberMiscCasts 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 = ::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
+
+namespace testPointerToMemberDiamond {
+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()->f = 7;
+  static_cast()->f = 16;
+
+  int L1::* pl1 = ::f;
+  int M::* pm_via_l1 = pl1;
+
+  int R1::* pr1 = ::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}}
+}
+
+void double_diamond() {
+  D2 d2;
+
+  static_cast(static_cast())->f = 1;
+  

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

[Kirill Romanenkov via Phabricator via cfe-commits]

kromanenkov added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:899
+case UO_AddrOf: {
+  // Process pointer-to-member address operation
+  const Expr *Ex = U->getSubExpr()->IgnoreParens();

kromanenkov wrote:
> dcoughlin wrote:
> > 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.
> 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?
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)?


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Kirill Romanenkov via Phabricator via cfe-commits]

kromanenkov added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:899
+case UO_AddrOf: {
+  // Process pointer-to-member address operation
+  const Expr *Ex = U->getSubExpr()->IgnoreParens();

dcoughlin wrote:
> 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.
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?


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin added a comment.

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()->f = 7;
static_cast()->f = 16;
  
int L1::* pl1 = ::f;
int M::* pm_via_l1 = pl1;
  
int R1::* pr1 = ::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(static_cast())->f = 1;
static_cast(static_cast())->f = 2;
static_cast(static_cast())->f = 3;
static_cast(static_cast())->f = 4;
  
clang_analyzer_eval(d2.*(static_cast(static_cast(static_cast(::f == 1); // expected-warning {{TRUE}}
clang_analyzer_eval(d2.*(static_cast(static_cast(static_cast(::f == 2); // expected-warning {{TRUE}}
clang_analyzer_eval(d2.*(static_cast(static_cast(static_cast(::f == 3); // expected-warning {{TRUE}}
clang_analyzer_eval(d2.*(static_cast(static_cast(static_cast(::f == 4); // expected-warning {{TRUE}}
  }




Comment at: 
include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h:217
+
+  llvm::ImmutableList consCXXBase(
+  const CXXBaseSpecifier *CBS,

NoQ wrote:
> 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".
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. 



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:322
+  Bldr.generateNode(CastE, Pred, state);
+  continue;
+}

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.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:472
+}
+// If getAs failed just fall through to default behaviour.
+  }

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



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:899
+case UO_AddrOf: {
+  // Process pointer-to-member address operation
+  const Expr *Ex = U->getSubExpr()->IgnoreParens();

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.



Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:298
 
+  case Stmt::UnaryOperatorClass: {
+const UnaryOperator *UO = dyn_cast(E);

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


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Kirill Romanenkov via Phabricator via cfe-commits]

kromanenkov marked an inline comment as done.
kromanenkov added inline comments.



Comment at: 
include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h:217
+
+  llvm::ImmutableList consCXXBase(
+  const CXXBaseSpecifier *CBS,

NoQ wrote:
> 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".
In fact I thought that this entails "consume" :)



Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:882
+return UndefinedVal();
+  if (const FieldDecl *FD = PTMSV->getDeclAs())
+return state->getLValue(FD, lhs);

NoQ wrote:
> 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.
AFAIU CXXMethodDecl is processed in SVal::getAsFunctionDecl() so i'm for no 
action.


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Kirill Romanenkov via Phabricator via cfe-commits]

kromanenkov updated this revision to Diff 79560.
kromanenkov added a comment.

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


https://reviews.llvm.org/D25475

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  lib/StaticAnalyzer/Core/BasicValueFactory.cpp
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SVals.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/pointer-to-member.cpp

Index: test/Analysis/pointer-to-member.cpp
===
--- test/Analysis/pointer-to-member.cpp
+++ test/Analysis/pointer-to-member.cpp
@@ -35,8 +35,7 @@
   clang_analyzer_eval(::getPtr == ::getPtr); // expected-warning{{TRUE}}
   clang_analyzer_eval(::getPtr == 0); // expected-warning{{FALSE}}
 
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(::m_ptr == ::m_ptr); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(::m_ptr == ::m_ptr); // expected-warning{{TRUE}}
 }
 
 namespace PR15742 {
@@ -62,21 +61,114 @@
   }
 }
 
-// ---
-// FALSE NEGATIVES
-// ---
-
 bool testDereferencing() {
   A obj;
   obj.m_ptr = 0;
 
   A::MemberPointer member = ::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 = ::bar;
+BFnPointer StaticCastedBase2Derived = static_cast(::bar),
+   CCastedBase2Derived = (BFnPointer) (::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 = 
+AFnPointer AFP = ::foo;
+
+clang_analyzer_eval((APtr->*AFP)() == 1); // expected-warning{{TRUE}}
+
+APtr = 
+
+clang_analyzer_eval((APtr->*AFP)() == 3); // expected-warning{{TRUE}}
+  }
+} // end of testPointerToMemberFunction namespace
+
+namespace testPointerToMemberData {
+  struct A {
+int i;
+  };
+
+  void testPointerToMemberData() {
+int A::*AMdPointer = ::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 = ::f;
+  int D::* pfd = pfb;
+  int v = d.*pfd;
+
+  clang_analyzer_eval(v == 7); // expected-warning{{TRUE}}
+}
+} // end of testPointerToMemberMiscCasts 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 = ::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
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -69,6 +69,9 @@
 
   bool isLocType = Loc::isLocType(castTy);
 
+  if (val.getAs())
+return val;
+
   if (Optional LI = val.getAs()) {
 if (isLocType)
   return LI->getLoc();
@@ -335,6 +338,21 @@
 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(),
+   RPTM = rhs.castAs();
+  auto LPTMD = LPTM.getPTMData(), RPTMD = RPTM.getPTMData();
+  switch (op) {
+case BO_EQ:
+  return makeTruthVal(LPTMD == RPTMD, 

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

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




Comment at: 
include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h:217
+
+  llvm::ImmutableList consCXXBase(
+  const CXXBaseSpecifier *CBS,

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".



Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:882
+return UndefinedVal();
+  if (const FieldDecl *FD = PTMSV->getDeclAs())
+return state->getLValue(FD, lhs);

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.


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Kirill Romanenkov via Phabricator via cfe-commits]

kromanenkov added a comment.

ping


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Kirill Romanenkov via cfe-commits]

kromanenkov added a comment.

ping


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Kirill Romanenkov via cfe-commits]

kromanenkov updated this revision to Diff 77041.
kromanenkov added a comment.

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.


https://reviews.llvm.org/D25475

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  lib/StaticAnalyzer/Core/BasicValueFactory.cpp
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SVals.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/pointer-to-member.cpp

Index: test/Analysis/pointer-to-member.cpp
===
--- test/Analysis/pointer-to-member.cpp
+++ test/Analysis/pointer-to-member.cpp
@@ -35,8 +35,7 @@
   clang_analyzer_eval(::getPtr == ::getPtr); // expected-warning{{TRUE}}
   clang_analyzer_eval(::getPtr == 0); // expected-warning{{FALSE}}
 
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(::m_ptr == ::m_ptr); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(::m_ptr == ::m_ptr); // expected-warning{{TRUE}}
 }
 
 namespace PR15742 {
@@ -62,21 +61,114 @@
   }
 }
 
-// ---
-// FALSE NEGATIVES
-// ---
-
 bool testDereferencing() {
   A obj;
   obj.m_ptr = 0;
 
   A::MemberPointer member = ::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 = ::bar;
+BFnPointer StaticCastedBase2Derived = static_cast(::bar),
+   CCastedBase2Derived = (BFnPointer) (::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 = 
+AFnPointer AFP = ::foo;
+
+clang_analyzer_eval((APtr->*AFP)() == 1); // expected-warning{{TRUE}}
+
+APtr = 
+
+clang_analyzer_eval((APtr->*AFP)() == 3); // expected-warning{{TRUE}}
+  }
+} // end of testPointerToMemberFunction namespace
+
+namespace testPointerToMemberData {
+  struct A {
+int i;
+  };
+
+  void testPointerToMemberData() {
+int A::*AMdPointer = ::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 = ::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 = ::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
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -69,6 +69,9 @@
 
   bool isLocType = Loc::isLocType(castTy);
 
+  if (val.getAs())
+return val;
+
   if (Optional LI = val.getAs()) {
 if (isLocType)
   return LI->getLoc();
@@ -335,6 +338,21 @@
 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(),
+   RPTM = rhs.castAs();
+  auto LPTMD = LPTM.getPTMData(), RPTMD = RPTM.getPTMData();
+  

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

[Devin Coughlin via cfe-commits]

dcoughlin added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:462
+  case CK_ReinterpretMemberPointer: {
+const Expr *UOExpr = CastE->getSubExpr()->IgnoreParenCasts();
+assert(isa(UOExpr) &&

kromanenkov wrote:
> dcoughlin wrote:
> > dcoughlin wrote:
> > > dcoughlin wrote:
> > > > 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 = ::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 = ::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. 
> > > > 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.
> > (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.
> 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?
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.


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Kirill Romanenkov via cfe-commits]

kromanenkov added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:462
+  case CK_ReinterpretMemberPointer: {
+const Expr *UOExpr = CastE->getSubExpr()->IgnoreParenCasts();
+assert(isa(UOExpr) &&

dcoughlin wrote:
> dcoughlin wrote:
> > dcoughlin wrote:
> > > 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 = ::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 = ::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. 
> > > 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.
> (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.
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?


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Devin Coughlin via cfe-commits]

dcoughlin added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:462
+  case CK_ReinterpretMemberPointer: {
+const Expr *UOExpr = CastE->getSubExpr()->IgnoreParenCasts();
+assert(isa(UOExpr) &&

dcoughlin wrote:
> dcoughlin wrote:
> > 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 = ::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 = ::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. 
> > 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.
(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.


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Devin Coughlin via cfe-commits]

dcoughlin added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:462
+  case CK_ReinterpretMemberPointer: {
+const Expr *UOExpr = CastE->getSubExpr()->IgnoreParenCasts();
+assert(isa(UOExpr) &&

dcoughlin wrote:
> 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 = ::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 = ::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. 
> 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.


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Devin Coughlin via cfe-commits]

dcoughlin added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:462
+  case CK_ReinterpretMemberPointer: {
+const Expr *UOExpr = CastE->getSubExpr()->IgnoreParenCasts();
+assert(isa(UOExpr) &&

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 = ::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 = ::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. 


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Kirill Romanenkov via cfe-commits]

kromanenkov updated this revision to Diff 75020.
kromanenkov added a comment.

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.


https://reviews.llvm.org/D25475

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SVals.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/pointer-to-member.cpp

Index: test/Analysis/pointer-to-member.cpp
===
--- test/Analysis/pointer-to-member.cpp
+++ test/Analysis/pointer-to-member.cpp
@@ -35,8 +35,7 @@
   clang_analyzer_eval(::getPtr == ::getPtr); // expected-warning{{TRUE}}
   clang_analyzer_eval(::getPtr == 0); // expected-warning{{FALSE}}
 
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(::m_ptr == ::m_ptr); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(::m_ptr == ::m_ptr); // expected-warning{{TRUE}}
 }
 
 namespace PR15742 {
@@ -62,21 +61,70 @@
   }
 }
 
-// ---
-// FALSE NEGATIVES
-// ---
-
 bool testDereferencing() {
   A obj;
   obj.m_ptr = 0;
 
   A::MemberPointer member = ::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 = ::bar;
+BFnPointer StaticCastedBase2Derived = static_cast(::bar),
+   CCastedBase2Derived = (BFnPointer) (::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 = 
+AFnPointer AFP = ::foo;
+
+clang_analyzer_eval((APtr->*AFP)() == 1); // expected-warning{{TRUE}}
+
+APtr = 
+
+clang_analyzer_eval((APtr->*AFP)() == 3); // expected-warning{{TRUE}}
+  }
+} // end of testPointerToMemberFunction namespace
+
+namespace testPointerToMemberData {
+  struct A {
+int i;
+  };
+
+  void testPointerToMemberData() {
+int A::*AMdPointer = ::i;
+A a;
+
+a.i = 42;
+a.*AMdPointer += 1;
+
+clang_analyzer_eval(a.i == 43); // expected-warning{{TRUE}}
+  }
+} // end of testPointerToMemberData namespace
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -69,6 +69,9 @@
 
   bool isLocType = Loc::isLocType(castTy);
 
+  if (val.getAs())
+return val;
+
   if (Optional LI = val.getAs()) {
 if (isLocType)
   return LI->getLoc();
@@ -335,6 +338,21 @@
 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");
+  const DeclaratorDecl *LDD =
+  lhs.castAs().getDecl(),
+  *RDD = rhs.castAs().getDecl();
+  switch (op) {
+case BO_EQ:
+  return makeTruthVal(LDD == RDD, resultTy);
+case BO_NE:
+  return makeTruthVal(LDD != RDD, resultTy);
+default:
+  return UnknownVal();
+  }
+}
 case nonloc::LocAsIntegerKind: {
   Loc lhsL = lhs.castAs().getLoc();
   switch (rhs.getSubKind()) {
@@ -857,6 +875,17 @@
 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()) {
+  if (PTMSV->isNullMemberPointer())
+return UndefinedVal();
+  if (const FieldDecl *FD = PTMSV->getDeclAs())
+return state->getLValue(FD, lhs);
+}
+
+return rhs;
+  }
+
   assert(!BinaryOperator::isComparisonOp(op) &&
  

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

[Artem Dergachev via cfe-commits]

NoQ added inline comments.



Comment at: test/Analysis/pointer-to-member.cpp:79
   // FIXME: Should emit a null dereference.
   return obj.*member; // no-warning
 }

kromanenkov wrote:
> NoQ wrote:
> > 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.
> 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?
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.


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Kirill Romanenkov via cfe-commits]

kromanenkov added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:2314
 {
+  // Return to fulfil assert condition
+  if (location.getAs())

NoQ wrote:
> 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 `SVal`s is 
> incorrect), because otherwise i think that making `PointerToMember` a NonLoc 
> is correct - we cannot store things in it or load things from it.
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.



Comment at: test/Analysis/pointer-to-member.cpp:79
   // FIXME: Should emit a null dereference.
   return obj.*member; // no-warning
 }

NoQ wrote:
> 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.
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?


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via cfe-commits]

NoQ added a comment.

Yay, thanks for posting this! :)

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




Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:2314
 {
+  // Return to fulfil assert condition
+  if (location.getAs())

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 `SVal`s is 
incorrect), because otherwise i think that making `PointerToMember` a NonLoc is 
correct - we cannot store things in it or load things from it.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:465
+   "UnaryOperator as Cast's child was expected");
+if (const UnaryOperator *UO = dyn_cast(UOExpr)) {
+  const Expr *DREExpr = UO->getSubExpr()->IgnoreParenCasts();

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



Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:69
+// Add check to fulfil assert condition
+if (!V.getAs())
+  assert(V.isUnknown());

Same concern: Why are we copying a `NonLoc`?



Comment at: test/Analysis/pointer-to-member.cpp:79
   // FIXME: Should emit a null dereference.
   return obj.*member; // no-warning
 }

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.


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Kirill Romanenkov via cfe-commits]

kromanenkov created this revision.
kromanenkov added reviewers: zaks.anna, NoQ, dcoughlin.
kromanenkov added subscribers: a.sidorin, cfe-commits.

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.


https://reviews.llvm.org/D25475

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SVals.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/pointer-to-member.cpp

Index: test/Analysis/pointer-to-member.cpp
===
--- test/Analysis/pointer-to-member.cpp
+++ test/Analysis/pointer-to-member.cpp
@@ -35,8 +35,7 @@
   clang_analyzer_eval(::getPtr == ::getPtr); // expected-warning{{TRUE}}
   clang_analyzer_eval(::getPtr == 0); // expected-warning{{FALSE}}
 
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(::m_ptr == ::m_ptr); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(::m_ptr == ::m_ptr); // expected-warning{{TRUE}}
 }
 
 namespace PR15742 {
@@ -72,11 +71,65 @@
 
   A::MemberPointer member = ::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
 }
+
+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 = ::bar;
+BFnPointer StaticCastedBase2Derived = static_cast(::bar),
+   CCastedBase2Derived = (BFnPointer) (::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 = 
+AFnPointer AFP = ::foo;
+
+clang_analyzer_eval((APtr->*AFP)() == 1); // expected-warning{{TRUE}}
+
+APtr = 
+
+clang_analyzer_eval((APtr->*AFP)() == 3); // expected-warning{{TRUE}}
+  }
+} // end of testPointerToMemberFunction namespace
+
+namespace testPointerToMemberData {
+  struct A {
+int i;
+  };
+
+  void testPointerToMemberData() {
+int A::*AMdPointer = ::i;
+A a;
+
+a.i = 42;
+a.*AMdPointer += 1;
+
+clang_analyzer_eval(a.i == 43); // expected-warning{{TRUE}}
+  }
+} // end of testPointerToMemberData namespace
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -69,6 +69,9 @@
 
   bool isLocType = Loc::isLocType(castTy);
 
+  if (val.getAs())
+return val;
+
   if (Optional LI = val.getAs()) {
 if (isLocType)
   return LI->getLoc();
@@ -335,6 +338,21 @@
 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");
+  const DeclaratorDecl *LDD =
+  lhs.castAs().getDecl(),
+  *RDD = rhs.castAs().getDecl();
+  switch (op) {
+case BO_EQ:
+  return makeTruthVal(LDD == RDD, resultTy);
+case BO_NE:
+  return makeTruthVal(LDD != RDD, resultTy);
+default:
+  return UnknownVal();
+  }
+}
 case nonloc::LocAsIntegerKind: {
   Loc lhsL = lhs.castAs().getLoc();
   switch (rhs.getSubKind()) {
@@ -857,6 +875,14 @@
 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())
+  if (const FieldDecl *FD = PTMSV->getDeclAs())
+return state->getLValue(FD, lhs);
+
+return rhs;
+  }
+
   assert(!BinaryOperator::isComparisonOp(op) &&
  "arguments to comparison ops must be of the same type");
 
Index: lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
===
---