Assert in performTrivialCopy - Bug report and a possible solution

[szepet]

Summary

I ran into a bug, when I was implementing a checker and tested it on LLVM with CodeChecker. And I got the next error message:

performTrivialCopy_assert.txt17 KBDownload

I got the same results even when I tried to use MallocChecker or other (probably well tested) checkers. From that I draw the next conclusion: The error was probably triggered by the last changes of SymbolRecord.h (line:260, commit: https://reviews.llvm.org/rL270511#ffc63071 line:112 in the commit) and during the analysis of that line the getArgSVal() return an UndefinedVal. (It can happen, see: http://clang.llvm.org/doxygen/Environment_8cpp_source.html#l00069) And it seems the perfromTrivialCopy function does not handle this case. (If it should not happen than the call itself is the problem, but at least I could draw attention on this assert.)

I changed the code to accept UndefinedVal too and it seems working (okay,it is more precise to say it`s not asserting...), but I have only limited knowledge in this topic.

[dcoughlin]

I'm not sure that weakening the assert is the right thing to do here. It seems like if V is undef that the analyzer ideally ought to have issued a diagnostic (and a sink) somewhere before. Do you have a test case that reproduces? (It would be good to add that to the tests in any event!).

Also: I think r270511 is unlikely to be the change that caused this -- that is a change in LLVM's treatment of DebugInfo, which shouldn't affect the analyzer.

[xazax.hun]

Also: I think r270511 is unlikely to be the change that caused this -- that is a change in LLVM's treatment of DebugInfo, which shouldn't affect the analyzer.

I think Peter means that, that revision introduced the code that the analyzer fails to analyze (and not the bug in the analyzer).

[zaks.anna]

In D23853#524945, @xazax.hun wrote:

Also: I think r270511 is unlikely to be the change that caused this -- that is a change in LLVM's treatment of DebugInfo, which shouldn't affect the analyzer.

I think Peter means that, that revision introduced the code that the analyzer fails to analyze (and not the bug in the analyzer).

Can you add a reduced test case?

[szepet]

At first I was not able to reproduce it. Then I realized it was my foul because I used the analyzer without the core checkers in this case. Sorry for the false positive.

[zaks.anna]

Should this revision be closed?

lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

   if (Optional<Loc> L = V.getAs<Loc>())
     V = Pred->getState()->getSVal(*L);
   else
-    assert(V.isUnknown());
+    assert(V.isUnknownOrUndef());
 
   const Expr *CallExpr = Call.getOriginExpr();
   evalBind(Dst, CallExpr, Pred, ThisVal, V, true);