]> granicus.if.org Git - clang/commitdiff
[analyzer] Expose return statement from CallExit program point
authorGeorge Karpenkov <ekarpenkov@apple.com>
Fri, 2 Feb 2018 02:19:43 +0000 (02:19 +0000)
committerGeorge Karpenkov <ekarpenkov@apple.com>
Fri, 2 Feb 2018 02:19:43 +0000 (02:19 +0000)
If the return statement is stored, we might as well allow querying
against it.
Also fix the bug where the return statement is not stored
if there is no return value.
This change un-merges two ExplodedNodes during call exit when the state
is otherwise identical - the CallExitBegin node itself and the "Bind
Return Value"-tagged node.
And expose the return statement through
getStatement helper function.

Differential Revision: https://reviews.llvm.org/D42130

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@324052 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Analysis/ProgramPoint.h
lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
lib/StaticAnalyzer/Core/CoreEngine.cpp
lib/StaticAnalyzer/Core/PathDiagnostic.cpp
test/Analysis/return-stmt-merge.cpp [new file with mode: 0644]

index 8a72399069e8bdbcaca150f83e11ccdb89a60488..f7bd1be0da8703425c8ef4cbc54d67b6c642dc81 100644 (file)
@@ -641,6 +641,10 @@ public:
   CallExitBegin(const StackFrameContext *L, const ReturnStmt *RS)
     : ProgramPoint(RS, CallExitBeginKind, L, nullptr) { }
 
+  const ReturnStmt *getReturnStmt() const {
+    return static_cast<const ReturnStmt *>(getData1());
+  }
+
 private:
   friend class ProgramPoint;
   CallExitBegin() = default;
index e2a35c04266324ae4c184035dae03f616f836785..dc9dd5bd06d46fc88077608902c5769f8f2c11b4 100644 (file)
@@ -37,7 +37,9 @@ class AnalysisOrderChecker
                      check::PostCall,
                      check::NewAllocator,
                      check::Bind,
-                     check::RegionChanges> {
+                     check::RegionChanges,
+                     check::LiveSymbols> {
+
   bool isCallbackEnabled(AnalyzerOptions &Opts, StringRef CallbackName) const {
     return Opts.getBooleanOption("*", false, this) ||
         Opts.getBooleanOption(CallbackName, false, this);
@@ -118,6 +120,11 @@ public:
       llvm::errs() << "Bind\n";
   }
 
+  void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SymReaper) const {
+    if (isCallbackEnabled(State, "LiveSymbols"))
+      llvm::errs() << "LiveSymbols\n";
+  }
+
   ProgramStateRef
   checkRegionChanges(ProgramStateRef State,
                      const InvalidatedSymbols *Invalidated,
index a06c311590a8fea3ba5943fcc4f1c2d68e9bf055..115b5a1c29ba37840381193a7b470f70419e3e7b 100644 (file)
@@ -307,10 +307,7 @@ void CoreEngine::HandleBlockEdge(const BlockEdge &L, ExplodedNode *Pred) {
     const ReturnStmt *RS = nullptr;
     if (!L.getSrc()->empty()) {
       if (Optional<CFGStmt> LastStmt = L.getSrc()->back().getAs<CFGStmt>()) {
-        if ((RS = dyn_cast<ReturnStmt>(LastStmt->getStmt()))) {
-          if (!RS->getRetValue())
-            RS = nullptr;
-        }
+        RS = dyn_cast<ReturnStmt>(LastStmt->getStmt());
       }
     }
 
index 2b041241623e45c7a6ec9fda0b1cb4fb71f9b124..3d4b377627f60d034fdc13b31b5dc7cea5528cee 100644 (file)
@@ -742,6 +742,8 @@ const Stmt *PathDiagnosticLocation::getStmt(const ExplodedNode *N) {
     return CEE->getCalleeContext()->getCallSite();
   if (Optional<PostInitializer> PIPP = P.getAs<PostInitializer>())
     return PIPP->getInitializer()->getInit();
+  if (Optional<CallExitBegin> CEB = P.getAs<CallExitBegin>())
+    return CEB->getReturnStmt();
 
   return nullptr;
 }
diff --git a/test/Analysis/return-stmt-merge.cpp b/test/Analysis/return-stmt-merge.cpp
new file mode 100644 (file)
index 0000000..a6bacc5
--- /dev/null
@@ -0,0 +1,37 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.AnalysisOrder,debug.ExprInspection -analyzer-config debug.AnalysisOrder:PreCall=true,debug.AnalysisOrder:PostCall=true,debug.AnalysisOrder:LiveSymbols=true %s 2>&1 | FileCheck %s
+
+// This test ensures that check::LiveSymbols is called as many times on the
+// path through the second "return" as it is through the first "return"
+// (three), and therefore the two paths were not merged prematurely before the
+// respective return statement is evaluated.
+// The paths would still be merged later, so we'd have only one post-call for
+// foo(), but it is incorrect to merge them in the middle of evaluating two
+// different statements.
+int coin();
+
+void foo() {
+  int x = coin();
+  if (x > 0)
+    return;
+  else
+    return;
+}
+
+void bar() {
+  foo();
+}
+
+// CHECK:      LiveSymbols
+// CHECK-NEXT: LiveSymbols
+// CHECK-NEXT: PreCall (foo)
+// CHECK-NEXT: LiveSymbols
+// CHECK-NEXT: LiveSymbols
+// CHECK-NEXT: PreCall (coin)
+// CHECK-NEXT: PostCall (coin)
+// CHECK-NEXT: LiveSymbols
+// CHECK-NEXT: LiveSymbols
+// CHECK-NEXT: LiveSymbols
+// CHECK-NEXT: PostCall (foo)
+// CHECK-NEXT: LiveSymbols
+// CHECK-NEXT: LiveSymbols
+// CHECK-NEXT: LiveSymbols