[analyzer] Don't merge different return nodes in ExplodedGraph

Returns when calling an inline function should not be merged in the ExplodedGraph unless they are same.

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

llvm-svn: 283554

clang/include/clang/Analysis/ProgramPoint.h

@@ -622,8 +622,8 @@ class CallEnter : public ProgramPoint {
 class CallExitBegin : public ProgramPoint {
 public:
   // CallExitBegin uses the callee's location context.
-  CallExitBegin(const StackFrameContext *L)
-    : ProgramPoint(nullptr, CallExitBeginKind, L, nullptr) {}
+  CallExitBegin(const StackFrameContext *L, const ReturnStmt *RS)
+    : ProgramPoint(RS, CallExitBeginKind, L, nullptr) { }
 
 private:
   friend class ProgramPoint;

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h

@@ -109,7 +109,8 @@ class CoreEngine {
   CoreEngine(const CoreEngine &) = delete;
   void operator=(const CoreEngine &) = delete;
 
-  ExplodedNode *generateCallExitBeginNode(ExplodedNode *N);
+  ExplodedNode *generateCallExitBeginNode(ExplodedNode *N,
+                                          const ReturnStmt *RS);
 
 public:
   /// Construct a CoreEngine object to analyze the provided CFG.
@@ -172,7 +173,7 @@ class CoreEngine {
 
   /// \brief enqueue the nodes corresponding to the end of function onto the
   /// end of path / work list.
-  void enqueueEndOfFunction(ExplodedNodeSet &Set);
+  void enqueueEndOfFunction(ExplodedNodeSet &Set, const ReturnStmt *RS);
 
   /// \brief Enqueue a single node created as a result of statement processing.
   void enqueueStmtNode(ExplodedNode *N, const CFGBlock *Block, unsigned Idx);

clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h

@@ -262,7 +262,8 @@ class ExprEngine : public SubEngine {
   /// Called by CoreEngine.  Used to notify checkers that processing a
   /// function has ended. Called for both inlined and and top-level functions.
   void processEndOfFunction(NodeBuilderContext& BC,
-                            ExplodedNode *Pred) override;
+                            ExplodedNode *Pred,
+                            const ReturnStmt *RS=nullptr) override;
 
   /// Remove dead bindings/symbols before exiting a function.
   void removeDeadOnEndOfFunction(NodeBuilderContext& BC,

clang/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h

@@ -109,7 +109,8 @@ class SubEngine {
   /// Called by CoreEngine.  Used to notify checkers that processing a
   /// function has ended. Called for both inlined and and top-level functions.
   virtual void processEndOfFunction(NodeBuilderContext& BC,
-                                    ExplodedNode *Pred) = 0;
+                                    ExplodedNode *Pred,
+                                    const ReturnStmt *RS = nullptr) = 0;
 
   // Generate the entry node of the callee.
   virtual void processCallEnter(NodeBuilderContext& BC, CallEnter CE,

clang/lib/StaticAnalyzer/Core/CoreEngine.cpp

@@ -309,8 +309,19 @@ void CoreEngine::HandleBlockEdge(const BlockEdge &L, ExplodedNode *Pred) {
     assert (L.getLocationContext()->getCFG()->getExit().size() == 0
             && "EXIT block cannot contain Stmts.");
 
+    // Get return statement..
+    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;
+        }
+      }
+    }
+
     // Process the final state transition.
-    SubEng.processEndOfFunction(BuilderCtx, Pred);
+    SubEng.processEndOfFunction(BuilderCtx, Pred, RS);
 
     // This path is done. Don't enqueue any more nodes.
     return;
@@ -589,13 +600,14 @@ void CoreEngine::enqueueStmtNode(ExplodedNode *N,
     WList->enqueue(Succ, Block, Idx+1);
 }
 
-ExplodedNode *CoreEngine::generateCallExitBeginNode(ExplodedNode *N) {
+ExplodedNode *CoreEngine::generateCallExitBeginNode(ExplodedNode *N,
+                                                    const ReturnStmt *RS) {
   // Create a CallExitBegin node and enqueue it.
   const StackFrameContext *LocCtx
                          = cast<StackFrameContext>(N->getLocationContext());
 
   // Use the callee location context.
-  CallExitBegin Loc(LocCtx);
+  CallExitBegin Loc(LocCtx, RS);
 
   bool isNew;
   ExplodedNode *Node = G.getNode(Loc, N->getState(), false, &isNew);
@@ -619,12 +631,12 @@ void CoreEngine::enqueue(ExplodedNodeSet &Set,
   }
 }
 
-void CoreEngine::enqueueEndOfFunction(ExplodedNodeSet &Set) {
+void CoreEngine::enqueueEndOfFunction(ExplodedNodeSet &Set, const ReturnStmt *RS) {
   for (ExplodedNodeSet::iterator I = Set.begin(), E = Set.end(); I != E; ++I) {
     ExplodedNode *N = *I;
     // If we are in an inlined call, generate CallExitBegin node.
     if (N->getLocationContext()->getParent()) {
-      N = generateCallExitBeginNode(N);
+      N = generateCallExitBeginNode(N, RS);
       if (N)
         WList->enqueue(N);
     } else {

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

@@ -1766,7 +1766,8 @@ void ExprEngine::processBeginOfFunction(NodeBuilderContext &BC,
 /// ProcessEndPath - Called by CoreEngine.  Used to generate end-of-path
 ///  nodes when the control reaches the end of a function.
 void ExprEngine::processEndOfFunction(NodeBuilderContext& BC,
-                                      ExplodedNode *Pred) {
+                                      ExplodedNode *Pred,
+                                      const ReturnStmt *RS) {
   // FIXME: Assert that stackFrameDoesNotContainInitializedTemporaries(*Pred)).
   // We currently cannot enable this assert, as lifetime extended temporaries
   // are not modelled correctly.
@@ -1788,7 +1789,7 @@ void ExprEngine::processEndOfFunction(NodeBuilderContext& BC,
     getCheckerManager().runCheckersForEndFunction(BC, Dst, Pred, *this);
   }
 
-  Engine.enqueueEndOfFunction(Dst);
+  Engine.enqueueEndOfFunction(Dst, RS);
 }
 
 /// ProcessSwitch - Called by CoreEngine.  Used to generate successor

clang/test/Analysis/inlining/InlineObjCClassMethod.m

@@ -174,12 +174,12 @@ @interface MyClassSelf : MyParentSelf
 @implementation MyClassSelf
 + (int)testClassMethodByKnownVarDecl {
   int y = [MyParentSelf testSelf];
-  return 5/y; // Should warn here.
+  return 5/y; // expected-warning{{Division by zero}}
 }
 @end
 int foo2() {
   int y = [MyParentSelf testSelf];
-  return 5/y; // Should warn here.
+  return 5/y; // expected-warning{{Division by zero}}
 }
 
 // TODO: We do not inline 'getNum' in the following case, where the value of 

clang/test/Analysis/unreachable-code-path.c

@@ -194,3 +194,15 @@ void test12(int x) {
     break;
   }
 }
+
+// Don't merge return nodes in ExplodedGraph unless they are same.
+extern int table[];
+static int inlineFunction(const int i) {
+  if (table[i] != 0)
+    return 1;
+  return 0;
+}
+void test13(int i) {
+  int x = inlineFunction(i);
+  x && x < 10; // no-warning
+}