From: Anna Zaks Date: Wed, 13 Mar 2013 20:20:14 +0000 (+0000) Subject: [analyzer] BugReporter - more precise tracking of C++ references X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6022c4e17c0d2ad9c43ef6bc830d394b670a4705;p=clang [analyzer] BugReporter - more precise tracking of C++ references When BugReporter tracks C++ references involved in a null pointer violation, we want to differentiate between a null reference and a reference to a null pointer. In the first case, we want to track the region for the reference location; in the second, we want to track the null pointer. In addition, the core creates CXXTempObjectRegion to represent the location of the C++ reference, so teach FindLastStoreBRVisitor about it. This helps null pointer suppression to kick in. (Patch by Anna and Jordan.) git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176969 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index 517c1a10f8..56d6d26133 100644 --- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -470,6 +470,11 @@ PathDiagnosticPiece *FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ, IsParam = true; } } + + // If this is a CXXTempObjectRegion, the Expr responsible for its creation + // is wrapped inside of it. + if (const CXXTempObjectRegion *TmpR = dyn_cast(R)) + InitE = TmpR->getExpr(); } if (!StoreSite) @@ -756,6 +761,27 @@ SuppressInlineDefensiveChecksVisitor::VisitNode(const ExplodedNode *Succ, return 0; } +static const MemRegion *getLocationRegionIfReference(const Expr *E, + const ExplodedNode *N) { + if (const DeclRefExpr *DR = dyn_cast(E)) { + if (const VarDecl *VD = dyn_cast(DR->getDecl())) { + if (!VD->getType()->isReferenceType()) + return 0; + ProgramStateManager &StateMgr = N->getState()->getStateManager(); + MemRegionManager &MRMgr = StateMgr.getRegionManager(); + return MRMgr.getVarRegion(VD, N->getLocationContext()); + } + } + + // FIXME: This does not handle other kinds of null references, + // for example, references from FieldRegions: + // struct Wrapper { int &ref; }; + // Wrapper w = { *(int *)0 }; + // w.ref = 1; + + return 0; +} + bool bugreporter::trackNullOrUndefValue(const ExplodedNode *ErrorNode, const Stmt *S, BugReport &report, bool IsArg) { @@ -804,33 +830,37 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *ErrorNode, if (Inner && ExplodedGraph::isInterestingLValueExpr(Inner)) { const MemRegion *R = 0; - // First check if this is a DeclRefExpr for a C++ reference type. - // For those, we want the location of the reference. - if (const DeclRefExpr *DR = dyn_cast(Inner)) { - if (const VarDecl *VD = dyn_cast(DR->getDecl())) { - if (VD->getType()->isReferenceType()) { - ProgramStateManager &StateMgr = state->getStateManager(); - MemRegionManager &MRMgr = StateMgr.getRegionManager(); - R = MRMgr.getVarRegion(VD, N->getLocationContext()); - } + // Find the ExplodedNode where the lvalue (the value of 'Ex') + // was computed. We need this for getting the location value. + const ExplodedNode *LVNode = N; + while (LVNode) { + if (Optional P = LVNode->getLocation().getAs()) { + if (P->getStmt() == Inner) + break; } + LVNode = LVNode->getFirstPred(); } + assert(LVNode && "Unable to find the lvalue node."); + ProgramStateRef LVState = LVNode->getState(); + SVal LVal = LVState->getSVal(Inner, LVNode->getLocationContext()); + + if (LVState->isNull(LVal).isConstrainedTrue()) { + // In case of C++ references, we want to differentiate between a null + // reference and reference to null pointer. + // If the LVal is null, check if we are dealing with null reference. + // For those, we want to track the location of the reference. + if (const MemRegion *RR = getLocationRegionIfReference(Inner, N)) + R = RR; + } else { + R = LVState->getSVal(Inner, LVNode->getLocationContext()).getAsRegion(); - // For all other cases, find the location by scouring the ExplodedGraph. - if (!R) { - // Find the ExplodedNode where the lvalue (the value of 'Ex') - // was computed. We need this for getting the location value. - const ExplodedNode *LVNode = N; - while (LVNode) { - if (Optional P = LVNode->getLocation().getAs()) { - if (P->getStmt() == Inner) - break; - } - LVNode = LVNode->getFirstPred(); + // If this is a C++ reference to a null pointer, we are tracking the + // pointer. In additon, we should find the store at which the reference + // got initialized. + if (const MemRegion *RR = getLocationRegionIfReference(Inner, N)) { + if (Optional KV = LVal.getAs()) + report.addVisitor(new FindLastStoreBRVisitor(*KV, RR)); } - assert(LVNode && "Unable to find the lvalue node."); - ProgramStateRef LVState = LVNode->getState(); - R = LVState->getSVal(Inner, LVNode->getLocationContext()).getAsRegion(); } if (R) { diff --git a/test/Analysis/diagnostics/deref-track-symbolic-region.cpp b/test/Analysis/diagnostics/deref-track-symbolic-region.cpp index e166109ef8..6d348415aa 100644 --- a/test/Analysis/diagnostics/deref-track-symbolic-region.cpp +++ b/test/Analysis/diagnostics/deref-track-symbolic-region.cpp @@ -25,4 +25,19 @@ void testRefParam(int *ptr) { extern void use(int &ref); use(ref); // expected-warning{{Forming reference to null pointer}} // expected-note@-1{{Forming reference to null pointer}} +} + +int testRefToNullPtr() { + int *p = 0; // expected-note {{'p' initialized to a null pointer value}} + int *const &p2 = p; // expected-note{{'p2' initialized here}} + int *p3 = p2; // expected-note {{'p3' initialized to a null pointer value}} + return *p3; // expected-warning {{Dereference of null pointer}} + // expected-note@-1{{Dereference of null pointer}} +} + +int testRefToNullPtr2() { + int *p = 0; // expected-note {{'p' initialized to a null pointer value}} + int *const &p2 = p;// expected-note{{'p2' initialized here}} + return *p2; //expected-warning {{Dereference of null pointer}} + // expected-note@-1{{Dereference of null pointer}} } \ No newline at end of file diff --git a/test/Analysis/inlining/false-positive-suppression.cpp b/test/Analysis/inlining/false-positive-suppression.cpp index f27c7cf364..613f421110 100644 --- a/test/Analysis/inlining/false-positive-suppression.cpp +++ b/test/Analysis/inlining/false-positive-suppression.cpp @@ -125,3 +125,52 @@ namespace References { #endif } } + +class X{ +public: + void get(); +}; + +X *getNull() { + return 0; +} + +void deref1(X *const &p) { + return p->get(); + #ifndef SUPPRESSED + // expected-warning@-2 {{Called C++ object pointer is null}} + #endif +} + +void test1() { + return deref1(getNull()); +} + +void deref2(X *p3) { + p3->get(); + #ifndef SUPPRESSED + // expected-warning@-2 {{Called C++ object pointer is null}} + #endif +} + +void pass2(X *const &p2) { + deref2(p2); +} + +void test2() { + pass2(getNull()); +} + +void deref3(X *const &p2) { + X *p3 = p2; + p3->get(); + #ifndef SUPPRESSED + // expected-warning@-2 {{Called C++ object pointer is null}} + #endif +} + +void test3() { + deref3(getNull()); +} + +