[PATCH] D25475: [analyzer] Add a new SVal to support pointer-to-member operations.
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.
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.
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.
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.
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.
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
===
---
