This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Add more info to exploded graph dumps
ClosedPublic

Authored by NoQ on Jul 21 2016, 3:43 AM.

Details

Summary

debug.ViewExplodedGraph, aka -analyzer-viz-egraph-graphviz, is often the only way to understand the otherwise confusing analyzer report. Exploded graphs are often heavy (see also -trim-egraph) and hard to navigate (especially when using a specialized .dot-file viewer like xdot - i found it much more comfortable to convert the file to .svg and view in a web browser), and it often takes many hours to figure out what peculiar path has the analyzer found or what particular thing about the program he doesn't understand.

While it is clear that everybody's desire is to provide the information about reports to the user in a more friendly manner, i've made a quick patch that adds an explanation of location contexts to the dumps. Not only such info would help the developer understand what location context is (it's far from obvious), but also i find it extremely convenient to quickly understand "what are we analyzing here" by looking at any node of the graph, which simplifies navigation dramatically for me.

Perhaps we could add more useful info? I'm also thinking of simplifying store and environment dumps for easier reading and less size, eg. mention every cluster or location context only once rather than on every line.

This patch removes some of the surrounding dead code, because the FIXMEs have already been addressed by introducing checker name tags and visitors.

Diff Detail

Repository
rL LLVM

Event Timeline

NoQ updated this revision to Diff 64852.Jul 21 2016, 3:43 AM
NoQ retitled this revision from to [analyzer] Add more info to exploded graph dumps.
NoQ updated this object.
NoQ added reviewers: zaks.anna, dcoughlin.
NoQ added subscribers: xazax.hun, a.sidorin, cfe-commits.
NoQ added a comment.Jul 21 2016, 3:45 AM

Whoops forgot the screenshots:

Also not sure how to write tests for these, because we can't choose the directory path for the dump, can we?

zaks.anna edited edge metadata.Jul 22 2016, 8:33 AM

Great!

How does this work when a path spans multiple files, specifically, when the definitions from a header file are inlined? We could add file names but only in cases the file name does not match the main file.

and hard to navigate (especially when using a specialized .dot-file viewer like xdot - i found it
much more comfortable to convert the file to .svg and view in a web browser

This is not something that the patch addresses. Feel free to add a patch to the checker developer manual with instructions for this.

NoQ updated this revision to Diff 65223.Jul 23 2016, 4:11 AM
NoQ edited edge metadata.
NoQ added a comment.Jul 23 2016, 4:14 AM

We could add file names but only in cases the file name does not match the main file.

Whoops, right, i guess i'd just fall back to the default source location printing routine for this case (or for macros).

This is not something that the patch addresses.

Yeah, just since we're all here anyway... By the way, -analyzer-config prune-paths=false is also very handy.

zaks.anna accepted this revision.Jul 23 2016, 2:13 PM
zaks.anna edited edge metadata.

Yeah, just since we're all here anyway... By the way, -analyzer-config prune-paths=false is also very handy.

I think you should add these to the checker writer manual in the debugging section.

This revision is now accepted and ready to land.Jul 23 2016, 2:13 PM
This revision was automatically updated to reflect the committed changes.