Page MenuHomePhabricator

[analyzer] Fix hidden node traversal in exploded graph dumps.
ClosedPublic

Authored by NoQ on Oct 17 2019, 4:43 PM.

Diff Detail

Event Timeline

NoQ created this revision.Oct 17 2019, 4:43 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 17 2019, 4:43 PM
NoQ marked an inline comment as done.Oct 17 2019, 4:44 PM
NoQ added inline comments.
clang/test/Analysis/dump_egraph.c
32

The actual filename is gracefully omitted so that not to confuse buildbots because it's going to be an absolute path.

Charusso accepted this revision.Oct 17 2019, 10:11 PM

When you see

static bool traverseHiddenNodes(
    const ExplodedNode *N,
    llvm::function_ref<void(const ExplodedNode *)> PreCallback,
    llvm::function_ref<void(const ExplodedNode *)> PostCallback,
    llvm::function_ref<bool(const ExplodedNode *)> Stop) {

that is 100% will produce errors, as it wants to be so smart. I think it is a very bad design from the beginning, so it will be still bugprone.

Other than that now every node makes sense, thanks!

clang/test/Analysis/dump_egraph.c
46

\"file\": \"{{(.+)}}dump_egraph.c\" \} is a cool workaround, but otherwise if you do not want to end line 32, could you remove this line's ending please?

This revision is now accepted and ready to land.Oct 17 2019, 10:11 PM
NoQ marked an inline comment as done.Oct 18 2019, 1:03 PM
NoQ added inline comments.
clang/test/Analysis/dump_egraph.c
46

Interestingly, this doesn't work in my case but does work in this older case, due to greediness: the first {(.+)}}dump_egraph.c matches everything until the second dump_egraph.c and then it can't find the PreStmtPurgeDeadSymbols line after it because it was already skipped.

I tried to make it non-greedy: {{(.+?)}}dump_egraph.c but it says it's not allowed.

I'm pretty bad with regexps so i decided to screw it :) But let's keep at least this test in order to test that the filename is actually printed.

This revision was automatically updated to reflect the committed changes.