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

Authored by hctim on Wed, Nov 8, 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.

Diff Detail

Repository
rL LLVM
hctim updated this revision to Diff 122164.Wed, Nov 8, 3:08 PM

Minor formatting changes.

pcc added a comment.Fri, Nov 10, 11:04 AM

Test case?

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

Updated with a graph printing test.

pcc added inline comments.Fri, Nov 10, 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.Fri, Nov 10, 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.Fri, Nov 10, 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.Mon, Nov 13, 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.Mon, Nov 13, 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.Mon, Nov 13, 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.Mon, Nov 13, 1:17 PM

... and merged in master.

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

LGTM

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

Merged in master in preparation for submission.

This revision was automatically updated to reflect the committed changes.