[PATCH] Fix crash in CFGReachabilityAnalysis triggered by IdempotentOperationChecker.

Alexander Kornienko alexfh at google.com
Wed Dec 18 13:30:06 PST 2013


  > With this patch I don’t see any test failures. Did you see the crash on our tests, or some other code?
  > With this patch, does the assertion fire on your original example that triggered the crash?

  The analyzer-bug.cpp attached to the phab item triggers the assertion fail (without this patch it triggers the assertion a bit deeper - in the BitVector. I reliable reproduce the failure on x86-64 Linux with this command:
    clang --analyze -Xanalyzer -analyzer-checker=alpha.deadcode.IdempotentOperations -std=c++11 analyzer-bug.cpp

  I've not converted this input to a test, as it is still quite large and also depends on the standard headers, so it may may not fail on other systems. There's probably a way to reduce this patch, but I'm far from being familiar with the static analyzer internals, so I only vaguely imagine the conditions required for reproducing this crash.

  The crash happens after `IdempotentOperationChecker::pathWasCompletelyAnalyzed` tries to find out if the CFG block in question is reachable from any of the blocks, that the static analyzer gave up analyzing (`CoreEngine::blocksExhausted`). Is it fine, that there are blocks from different CFGs in `CoreEngine::blocksExhausted`? If yes, then the fix should be as easy, as adding the appropriate check before calling `CFGReverseBlockReachabilityAnalysis::isReachable`:

    Index: tools/clang/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp
    ===================================================================
    --- tools/clang/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp	(revision 197556)
    +++ tools/clang/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp	(working copy)
    @@ -556,7 +556,8 @@
         // 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;
       }

  If `CoreEngine::blocksExhausted` should only contain blocks from a single CFG, then the fix requires a bit deeper understanding of how `IdempotentOperationChecker` works.

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



More information about the cfe-commits mailing list