This is an archive of the discontinued LLVM Phabricator instance.

[cfi-verify] Add DOT graph printing for GraphResult objects.
ClosedPublic

Authored by hctim on Nov 8 2017, 3:06 PM.

Details

Summary

Allows users to view GraphResult objects in a DOT directed-graph format. This feature can be turned on through the --print-graphs flag.

Also enabled pretty-printing of instructions in output. Together these features make analysis of unprotected CF instructions much easier by providing a visual control flow graph.

Event Timeline

hctim updated this revision to Diff 122164.Nov 8 2017, 3:08 PM

Minor formatting changes.

pcc edited edge metadata.Nov 10 2017, 11:04 AM

Test case?

hctim updated this revision to Diff 122492.Nov 10 2017, 11:34 AM

Updated with a graph printing test.

pcc added inline comments.Nov 10 2017, 4:25 PM
test/tools/llvm-cfi-verify/X86/dot-printing.s
7 ↗(On Diff #122492)

It is hard to get a clear picture of what this test is testing for. Can you show me what your actual output is for this test?

12 ↗(On Diff #122492)

I wouldn't test for this, presumably you have other tests for this functionality.

hctim updated this revision to Diff 122553.Nov 10 2017, 4:35 PM
hctim marked 2 inline comments as done.

Addressed Peter's comments: Updated dot printing integration test to be more descriptive.

pcc added inline comments.Nov 10 2017, 4:42 PM
test/tools/llvm-cfi-verify/X86/dot-printing.s
12 ↗(On Diff #122553)

Can you make the code deterministic by sorting the keys of IntermediateNodes before enumerating them? Then you can be more explicit in this test.

hctim added inline comments.Nov 13 2017, 12:31 PM
test/tools/llvm-cfi-verify/X86/dot-printing.s
12 ↗(On Diff #122553)

I'd be very worried about the perf implications of this - with --print_graphs every single CFG would be sorted, O(n . log n) over 100,000 elements with ~10 elements each would be extremely expensive.

pcc added inline comments.Nov 13 2017, 1:01 PM
test/tools/llvm-cfi-verify/X86/dot-printing.s
12 ↗(On Diff #122553)

This is a debugging feature, so the perf isn't too important. I wouldn't realistically expect people to render such large graphs, anyway.

hctim updated this revision to Diff 122713.Nov 13 2017, 1:15 PM
hctim marked an inline comment as done.

Sorted intermediate nodes before printing graphs to DOT format - making the output deterministic.

hctim updated this revision to Diff 122715.Nov 13 2017, 1:17 PM

... and merged in master.

pcc accepted this revision.Nov 14 2017, 2:31 PM

LGTM

This revision is now accepted and ready to land.Nov 14 2017, 2:31 PM
hctim updated this revision to Diff 122916.Nov 14 2017, 2:40 PM

Merged in master in preparation for submission.

This revision was automatically updated to reflect the committed changes.