Static Analyzer RunPathSensitiveChecks overwrites files with subsequent calls

[Quuxplusone]

Bugzilla Link	PR42601
Status	CONFIRMED
Importance	P normal
Reported by	Erich Keane (erich.keane@intel.com)
Reported on	2019-07-12 07:52:43 -0700
Last modified on	2019-07-12 09:10:45 -0700
Version	trunk
Hardware	PC Linux
CC	dcoughlin@apple.com, denis.bakhvalov@intel.com, ekarpenkov@apple.com, ganna@apple.com, llvm-bugs@lists.llvm.org, noqnoqneo@gmail.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

I got here in a little different of a way, but I was able to reproduce this
pretty trivially.

To reproduce, run test/Analysis/exploded-graph-
rewriter/objects_under_construction.cpp (only the clang-cc1 invocation is
necessary to see the problem, so this isn't a exploded-graph-rewriter bug),
except add a move or copy constructor to struct "S".

The .dot file ends up containing way less data, because AnalysisConsumer's
HandleDeclsCallGraph ends up calling HandleCode 2x, which then calls
RunPathSensitiveChecks.  THAT calls DumpGraph 2x, which creates a new stream
and overwrites the previous one.

While the move/copy ctor are required to get this to reproduce (thanks to logic
in shouldSkipFunction), I believe the root problem (multiple calls to DumpGraph
destroy/rewrite the file) is the true bug here.

My Diff:
diff --git a/clang/test/Analysis/exploded-graph-
rewriter/objects_under_construction.cpp b/clang/test/Analysis/exploded-graph-
rewriter/objects_under_construction.cpp
index b3d4aef..f7aa892 100644
--- a/clang/test/Analysis/exploded-graph-rewriter/objects_under_construction.cpp
+++ b/clang/test/Analysis/exploded-graph-rewriter/objects_under_construction.cpp
@@ -10,6 +10,7 @@

 struct S {
   S() {}
+  S(S&&){}
 };

 void test() {

My invocation: ./bin/clang -cc1 -analyze -triple x86_64-unknown-linux-gnu -
analyzer-checker=core -analyzer-dump-egraph=test.dot
../clang/test/Analysis/exploded-graph-rewriter/objects_under_construction.cpp -
mllvm -stats

[Quuxplusone]

Yeah, that's kinda a bug, but nobody cares most of the time.

Because the Static Analyzer conducts multiple path-sensitive analyses per invocation (usually one per top-level function in the translation unit's call graph), it is natural that it produces multiple exploded graph dumps - exactly one graph per analysis.

Historically the exploded graph dumps were emitted via -analyzer-viz-egraph-graphviz and/or via -analyzer-checker debug.ViewExplodedGraph. In both variants every analysis graph goes into a new temporary file with a random name somewhere in /tmp.

However at some point we decided to add tests for exploded graph dumps, so we added a new flag, -analyzer-dump-egraph=file.dot, so that to have a stable filename that you can later pass FileCheck.

Now, this new flag obviously doesn't work for multiple analyses, so any test that would require multiple analyses would have to be split into multiple translation units. This is a problem but not a real problem, as you can always split the test up.

Additionally, the new option is still useful for everyday debugging (after all, exploded graph dumper is a self-debugging facility of the Static Analyzer). It's easier to remember and you don't need to copy-paste the file name from the standard output in order to process it. Of course, most real-world translation units contain multiple analysis entry points. However, when debugging, you usually don't care about all analyses at once; you usually care about only one of them. And once you make up your mind on what top-level function you want to debug, you can use -analyze-function to only analyze that function, and therefore display analyze one exploded graph, so no overwriting is going on.

See also https://clang-analyzer.llvm.org/checker_dev_manual.html#commands