[cfe-commits] r162791 - in /cfe/trunk: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp test/Analysis/dtor.cpp

[Jordan Rose]

Author: jrose
Date: Tue Aug 28 15:52:13 2012
New Revision: 162791

URL: http://llvm.org/viewvc/llvm-project?rev=162791&view=rev
Log:
[analyzer] When we look for the last stmt in a function, skip implicit dtors.

When exiting a function, the analyzer looks for the last statement in the
function to see if it's a return statement (and thus bind the return value).
However, the search for "the last statement" was accepting statements that
were in implicitly-generated inlined functions (i.e. destructors). So we'd
go and get the statement from the destructor, and then say "oh look, this
function had no explicit return...guess there's no return value". And /that/
led to the value being returned being declared dead, and all our leak
checkers complaining.

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
    cfe/trunk/test/Analysis/dtor.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=162791&r1=162790&r2=162791&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Tue Aug 28 15:52:13 2012
@@ -66,22 +66,31 @@
   const StackFrameContext *SF =
           Node->getLocation().getLocationContext()->getCurrentStackFrame();
 
-  // Back up through the ExplodedGraph until we reach a statement node.
+  // Back up through the ExplodedGraph until we reach a statement node in this
+  // stack frame.
   while (Node) {
     const ProgramPoint &PP = Node->getLocation();
 
-    if (const StmtPoint *SP = dyn_cast<StmtPoint>(&PP)) {
-      S = SP->getStmt();
-      break;
-    } else if (const CallExitEnd *CEE = dyn_cast<CallExitEnd>(&PP)) {
-      S = CEE->getCalleeContext()->getCallSite();
-      if (S)
+    if (PP.getLocationContext()->getCurrentStackFrame() == SF) {
+      if (const StmtPoint *SP = dyn_cast<StmtPoint>(&PP)) {
+        S = SP->getStmt();
         break;
-      // If we have an implicit call, we'll probably end up with a
-      // StmtPoint inside the callee, which is acceptable.
-      // (It's possible a function ONLY contains implicit calls -- such as an
-      // implicitly-generated destructor -- so we shouldn't just skip back to
-      // the CallEnter node and keep going.)
+      } else if (const CallExitEnd *CEE = dyn_cast<CallExitEnd>(&PP)) {
+        S = CEE->getCalleeContext()->getCallSite();
+        if (S)
+          break;
+
+        // If there is no statement, this is an implicitly-generated call.
+        // We'll walk backwards over it and then continue the loop to find
+        // an actual statement.
+        const CallEnter *CE;
+        do {
+          Node = Node->getFirstPred();
+          CE = Node->getLocationAs<CallEnter>();
+        } while (!CE || CE->getCalleeContext() != CEE->getCalleeContext());
+
+        // Continue searching the graph.
+      }
     } else if (const CallEnter *CE = dyn_cast<CallEnter>(&PP)) {
       // If we reached the CallEnter for this function, it has no statements.
       if (CE->getCalleeContext() == SF)

Modified: cfe/trunk/test/Analysis/dtor.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dtor.cpp?rev=162791&r1=162790&r2=162791&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/dtor.cpp (original)
+++ cfe/trunk/test/Analysis/dtor.cpp Tue Aug 28 15:52:13 2012
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store region -analyzer-ipa=inlining -cfg-add-implicit-dtors -Wno-null-dereference -verify %s
 
 void clang_analyzer_eval(bool);
+void clang_analyzer_checkInlined(bool);
 
 class A {
 public:
@@ -102,7 +103,7 @@
     // Remove dead bindings...
     doSomething();
     // destructor called here
-    // expected-warning at 27 {{Attempt to free released memory}}
+    // expected-warning at 28 {{Attempt to free released memory}}
   }
 }
 
@@ -227,3 +228,26 @@
     clang_analyzer_eval(c == 3); // expected-warning{{TRUE}}
   }
 }
+
+
+namespace DestructorsShouldNotAffectReturnValues {
+  class Dtor {
+  public:
+    ~Dtor() {
+      clang_analyzer_checkInlined(true); // expected-warning{{TRUE}}
+    }
+  };
+
+  void *allocate() {
+    Dtor d;
+    return malloc(4); // no-warning
+  }
+
+  void test() {
+    // At one point we had an issue where the statements inside an
+    // inlined destructor kept us from finding the return statement,
+    // leading the analyzer to believe that the malloc'd memory had leaked.
+    void *p = allocate();
+    free(p); // no-warning
+  }
+}