[cfe-dev] [StaticAnalyzer] False positive unreachable code

[Devin Coughlin via cfe-dev]

> On Sep 19, 2016, at 4:36 AM, Daniel Marjamäki via cfe-dev <cfe-dev at lists.llvm.org> wrote:
> 
> 
> Hello!
> 
> I am looking at a false positive.
> 
> Here is a reduced example code:
> 
>    void f(int e) {
>      switch (e) {
>        case 1:
>          break;  // <- FP
>        case 2:
>          do { } while (0);
>          break;
>      }
>    }
> 
> Clang says that the first "break;" is unreachable.
> 
> 
> In UnreachableCodeChecker::checkEndAnalysis() the check will first iterate over the ExplodedGraph and then the CFG. For the example code above the ExplodedGraph and CFG both have nodes for "case E1" but the block numbers are different.
> 
> I have made one fix that will work if the block numbers are different. I added a ReachableStmts variable. Does that sounds ok?

I’m not quite sure what you’re proposing — it might be best to create a Phabricator patch.

> Otherwise I'll try to see why the block numbers are different. Are the AnalysisManager::getCFG() and AnalysisDeclContext::getUnoptimizedCFG() supposed to be guaranteed to generate the same block numbers?

I don’t think any truly strong guarantees — the only client of the unoptimized CFG is this one (alpha) checker. But based on the name of the flag used in getUnoptimizedCFG() the difference between the optimized and unoptimized CFGs is that the optimized CFG removes edges that are known to be false.

It looks to me like the difference between the two block numbers is due to the creation of the empty block for the “loop-back” transition in CFGBuilder::VisitDoStmt() — which is skipped in the optimized CFG when the transition is known to be false (as in do {}  while (0);)

There is a FIXME there about about not creating that block, which might be a good place to start. Alternatively, you could hoist the block creation above the check for whether the edge is trivial false.

Devin