[PATCH] Fix crash in CFGReachabilityAnalysis triggered by IdempotentOperationChecker.

[Alexander Kornienko]

  I've spent some more time on this and came up with a way to extend current
  tests to model the condition, that leads to this crash. I've also included a
  fix, which relies on the assumption, that it is fine that there are blocks from
  different CFGs in `CoreEngine::blocksExhausted`.

  Without the fix the newly added test crashes:

    CFGReachabilityAnalysis.cpp:28: bool clang::CFGReverseBlockReachabilityAnalysis::isReachable(const clang::CFGBlock *, const clang::CFGBlock *): Assertion `Src->getParent() == Dst->getParent()' failed.
    0  clang           0x0000000001d6472e llvm::sys::PrintStackTrace(_IO_FILE*) + 46
    1  clang           0x0000000001d649eb
    2  clang           0x0000000001d64c5e
    3  libpthread.so.0 0x00002b23f2206cb0
    4  libc.so.6       0x00002b23f2e75425 gsignal + 53
    5  libc.so.6       0x00002b23f2e78b8b abort + 379
    6  libc.so.6       0x00002b23f2e6e0ee
    7  libc.so.6       0x00002b23f2e6e192
    8  clang           0x0000000003322896 clang::CFGReverseBlockReachabilityAnalysis::isReachable(clang::CFGBlock const*, clang::CFGBlock const*) + 102
    9  clang           0x0000000002022e81
    10 clang           0x0000000002022705
    11 clang           0x0000000002022438
    12 clang           0x00000000021f415a
    13 clang           0x00000000021f0f97 clang::ento::CheckerManager::runCheckersForEndAnalysis(clang::ento::ExplodedGraph&, clang::ento::BugReporter&, clang::ento::ExprEngine&) + 119
    14 clang           0x0000000002223d54 clang::ento::ExprEngine::processEndWorklist(bool) + 68
    15 clang           0x000000000220ee18 clang::ento::CoreEngine::ExecuteWorkList(clang::LocationContext const*, unsigned int, llvm::IntrusiveRefCntPtr<clang::ento::ProgramState const>) + 1112
    16 clang           0x0000000001f9b7ad
    17 clang           0x0000000001f65246
    18 clang           0x0000000001f65014
    19 clang           0x0000000001f64925
    20 clang           0x0000000001f6458a
    21 clang           0x0000000001f634e9
    22 clang           0x0000000002b5b37d clang::ParseAST(clang::Sema&, bool, bool) + 813
    23 clang           0x0000000001e529d9 clang::ASTFrontendAction::ExecuteAction() + 345
    24 clang           0x0000000001e524ff clang::FrontendAction::Execute() + 191
    25 clang           0x0000000001e1c5a0 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) + 800
    26 clang           0x0000000001f5dcf8 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) + 1048
    27 clang           0x00000000008ca29a cc1_main(char const**, char const**, char const*, void*) + 698
    28 clang           0x00000000008c21a2 main + 802
    29 libc.so.6       0x00002b23f2e6076d __libc_start_main + 237
    30 clang           0x00000000008c1d45


  Is it fine to commit it like this?

Hi krememek,

http://llvm-reviews.chandlerc.com/D2427

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D2427?vs=6162&id=6183#toc

Files:
  lib/Analysis/CFGReachabilityAnalysis.cpp
  lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp
  test/Analysis/idempotent-operations.c

Index: lib/Analysis/CFGReachabilityAnalysis.cpp
===================================================================
--- lib/Analysis/CFGReachabilityAnalysis.cpp
+++ lib/Analysis/CFGReachabilityAnalysis.cpp
@@ -23,7 +23,9 @@
   : analyzed(cfg.getNumBlockIDs(), false) {}
 
 bool CFGReverseBlockReachabilityAnalysis::isReachable(const CFGBlock *Src,
-                                          const CFGBlock *Dst) {
+                                                      const CFGBlock *Dst) {
+  // Blocks must be from the same CFG.
+  assert(Src->getParent() == Dst->getParent());
 
   const unsigned DstBlockID = Dst->getBlockID();
   
Index: lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp
@@ -541,7 +541,7 @@
                                                       const CoreEngine &CE) {
 
   CFGReverseBlockReachabilityAnalysis *CRA = AC->getCFGReachablityAnalysis();
-  
+
   // Test for reachability from any aborted blocks to this block
   typedef CoreEngine::BlocksExhausted::const_iterator ExhaustedIterator;
   for (ExhaustedIterator I = CE.blocks_exhausted_begin(),
@@ -556,16 +556,18 @@
     // While technically reachable, it means we aborted the analysis on
     // a path that included that block.
     const CFGBlock *destBlock = BE.getDst();
-    if (destBlock == CB || CRA->isReachable(destBlock, CB))
+    if (destBlock == CB || (destBlock->getParent() == CB->getParent() &&
+                            CRA->isReachable(destBlock, CB)))
       return false;
   }
 
   // Test for reachability from blocks we just gave up on.
   typedef CoreEngine::BlocksAborted::const_iterator AbortedIterator;
   for (AbortedIterator I = CE.blocks_aborted_begin(),
        E = CE.blocks_aborted_end(); I != E; ++I) {
     const CFGBlock *destBlock = I->first;
-    if (destBlock == CB || CRA->isReachable(destBlock, CB))
+    if (destBlock == CB || (destBlock->getParent() == CB->getParent() &&
+                            CRA->isReachable(destBlock, CB)))
       return false;
   }
   
Index: test/Analysis/idempotent-operations.c
===================================================================
--- test/Analysis/idempotent-operations.c
+++ test/Analysis/idempotent-operations.c
@@ -78,6 +78,21 @@
   }
 }
 
+// Regression test for a crash while calling isReachable on blocks from
+// different CFGs.
+void bailout2() {
+  int result = 4;
+  result = result; // expected-warning {{Assigned value is always the same as the existing value}}
+
+  for (unsigned bg = 0; bg < 1024; bg ++) {
+    result = bg * result; // no-warning
+  }
+}
+void call_bailout2() {
+  for (int i = 0; i < 10; ++i)
+    bailout2();
+}
+
 // Relaxed liveness - check that we don't kill liveness at assignments
 typedef unsigned uintptr_t;
 void kill_at_assign() {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2427.3.patch
Type: text/x-patch
Size: 2955 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131219/6018add8/attachment.bin>