Recent versions of graphviz reject the .dot files emitted by the analyzer, with messages such as:
Warning: flat edge between adjacent nodes one of which has a record shape - replace records with HTML-like labels Edge node_9 -> node_8 The above warning seems to have been added to graphviz on 2014-07-25 in: https://gitlab.com/graphviz/graphviz/commit/763f02ef3cb7fbb62336f372b42279bd63266727 This patch reworks the .dot output from the analyzer so that it works with newer graphviz (tested with graphviz-2.40.1-46.fc30.x86_64), and adds DejaGnu test coverage that the .dot files can be parsed by "dot", if installed on the host. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to branch "dmalcolm/analyzer" on the GCC git mirror. gcc/ChangeLog: * analyzer/engine.cc (exploded_node::dump_dot): Replace 'shape=record' with 'shape=none,margin=0' and eliminate outermost braces. (viz_callgraph_node::dump_dot): Replace 'shape=record' with 'shape=none,margin=0' and use an HTML-like TABLE label. * analyzer/graphviz.cc (graphviz_out::begin_tr): New function. (graphviz_out::end_tr): New function. * analyzer/graphviz.h (graphviz_out::begin_tr): New decl. (graphviz_out::end_tr): New decl. * analyzer/region-model.cc (region::dump_dot_to_pp): Replace 'shape=record' with 'shape=none,margin=0', and switch to non-record escaping. * analyzer/state-purge.cc (state_purge_annotator::add_node_annotations): Pass in a graphviz_out * rather than a pretty_printer *. Replace 'shape=record' with 'shape=none,margin=0' and eliminate outermost braces. (print_vec_of_names): Pass in a graphviz_out * rather than a pretty_printer *. Wrap the output in a <TR><TD>. (state_purge_annotator::add_stmt_annotations): Pass in a graphviz_out * rather than a pretty_printer *. * analyzer/state-purge.h (state_purge_annotator::add_node_annotations): Pass in a graphviz_out * rather than a pretty_printer *. (state_purge_annotator::add_stmt_annotations): Likewise. * analyzer/supergraph.cc (supernode::dump_dot): Update for above annotator changes. Replace 'shape=record' with 'shape=none,margin=0' and convert label to an HTML-like TABLE. * analyzer/supergraph.h (dot_annotator::add_node_annotations): Pass in a graphviz_out * rather than a pretty_printer *. (dot_annotator::add_stmt_annotations): Likewise. * pretty-print.c (pp_write_text_as_html_like_dot_to_stream): New function. * pretty-print.h (pp_write_text_as_html_like_dot_to_stream): New decl. gcc/testsuite/ChangeLog: * gcc.dg/analyzer/dot-output.c: New test. * lib/gcc-defs.exp (dg-check-dot): New procedure. * lib/target-supports-dg.exp (dg-require-dot): New procedure. * lib/target-supports.exp (check_dot_available): New procedure. --- gcc/analyzer/engine.cc | 30 +++++++++----- gcc/analyzer/graphviz.cc | 20 +++++++++ gcc/analyzer/graphviz.h | 3 ++ gcc/analyzer/region-model.cc | 6 +-- gcc/analyzer/state-purge.cc | 29 +++++++------ gcc/analyzer/state-purge.h | 4 +- gcc/analyzer/supergraph.cc | 44 +++++++++++++------- gcc/analyzer/supergraph.h | 4 +- gcc/pretty-print.c | 48 ++++++++++++++++++++++ gcc/pretty-print.h | 3 ++ gcc/testsuite/gcc.dg/analyzer/dot-output.c | 33 +++++++++++++++ gcc/testsuite/lib/gcc-defs.exp | 21 ++++++++++ gcc/testsuite/lib/target-supports-dg.exp | 10 +++++ gcc/testsuite/lib/target-supports.exp | 13 ++++++ 14 files changed, 224 insertions(+), 44 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/dot-output.c diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc index aa4a3590574d..e02ac7de6577 100644 --- a/gcc/analyzer/engine.cc +++ b/gcc/analyzer/engine.cc @@ -702,10 +702,8 @@ exploded_node::dump_dot (graphviz_out *gv, const dump_args_t &args) const pretty_printer *pp = gv->get_pp (); dump_dot_id (pp); - pp_printf (pp, " [shape=%s,style=filled,fillcolor=%s,label=\"", - "record", + pp_printf (pp, " [shape=none,margin=0,style=filled,fillcolor=%s,label=\"", get_dot_fillcolor ()); - pp_left_brace (pp); pp_write_text_to_stream (pp); pp_printf (pp, "EN: %i", m_index); @@ -735,7 +733,6 @@ exploded_node::dump_dot (graphviz_out *gv, const dump_args_t &args) const pp_write_text_as_dot_label_to_stream (pp, /*for_record=*/true); - pp_right_brace (pp); pp_string (pp, "\"];\n\n"); pp_flush (pp); } @@ -3125,17 +3122,24 @@ public: pretty_printer *pp = gv->get_pp (); dump_dot_id (pp); - pp_printf (pp, " [shape=%s,style=filled,fillcolor=%s,label=\"", - "record", "lightgrey"); - pp_left_brace (pp); + pp_printf (pp, " [shape=none,margin=0,style=filled,fillcolor=%s,label=<", + "lightgrey"); + pp_string (pp, "<TABLE BORDER=\"0\">"); pp_write_text_to_stream (pp); + gv->begin_tr (); pp_printf (pp, "VCG: %i: %s", m_index, function_name (m_fun)); + gv->end_tr (); pp_newline (pp); + gv->begin_tr (); pp_printf (pp, "supernodes: %i\n", m_num_supernodes); + gv->end_tr (); pp_newline (pp); + + gv->begin_tr (); pp_printf (pp, "superedges: %i\n", m_num_superedges); + gv->end_tr (); pp_newline (pp); if (args.m_eg) @@ -3148,7 +3152,9 @@ public: if (enode->get_point ().get_function () == m_fun) num_enodes++; } + gv->begin_tr (); pp_printf (pp, "enodes: %i\n", num_enodes); + gv->end_tr (); pp_newline (pp); // TODO: also show the per-callstring breakdown @@ -3170,8 +3176,11 @@ public: } if (num_enodes > 0) { + gv->begin_tr (); cs->print (pp); pp_printf (pp, ": %i\n", num_enodes); + pp_write_text_as_html_like_dot_to_stream (pp); + gv->end_tr (); } } @@ -3180,13 +3189,14 @@ public: if (data) { pp_newline (pp); + gv->begin_tr (); pp_printf (pp, "summaries: %i\n", data->m_summaries.length ()); + pp_write_text_as_html_like_dot_to_stream (pp); + gv->end_tr (); } } - pp_write_text_as_dot_label_to_stream (pp, /*for_record=*/true); - pp_right_brace (pp); - pp_string (pp, "\"];\n\n"); + pp_string (pp, "</TABLE>>];\n\n"); pp_flush (pp); } diff --git a/gcc/analyzer/graphviz.cc b/gcc/analyzer/graphviz.cc index 46e719447d67..4ba9330cd8c8 100644 --- a/gcc/analyzer/graphviz.cc +++ b/gcc/analyzer/graphviz.cc @@ -79,3 +79,23 @@ graphviz_out::write_indent () for (int i = 0; i < m_indent * 2; ++i) pp_space (m_pp); } + +/* Write the start of an HTML-like row via <TR><TD>, writing to the stream + so that followup text can be escaped. */ + +void +graphviz_out::begin_tr () +{ + pp_string (m_pp, "<TR><TD ALIGN=\"LEFT\">"); + pp_write_text_to_stream (m_pp); +} + +/* Write the end of an HTML-like row via </TD></TR>, writing to the stream + so that followup text can be escaped. */ + +void +graphviz_out::end_tr () +{ + pp_string (m_pp, "</TD></TR>"); + pp_write_text_to_stream (m_pp); +} diff --git a/gcc/analyzer/graphviz.h b/gcc/analyzer/graphviz.h index d097834502ad..7ddd4ce36c15 100644 --- a/gcc/analyzer/graphviz.h +++ b/gcc/analyzer/graphviz.h @@ -40,6 +40,9 @@ class graphviz_out { void write_indent (); + void begin_tr (); + void end_tr (); + pretty_printer *get_pp () const { return m_pp; } private: diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index 197f1be23746..56aa44dae142 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -1210,11 +1210,11 @@ region::dump_dot_to_pp (const region_model &model, pretty_printer *pp) const { this_rid.dump_node_name_to_pp (pp); - pp_printf (pp, " [shape=%s,style=filled,fillcolor=%s,label=\"", - "record", "lightgrey"); + pp_printf (pp, " [shape=none,margin=0,style=filled,fillcolor=%s,label=\"", + "lightgrey"); pp_write_text_to_stream (pp); print (model, this_rid, pp); - pp_write_text_as_dot_label_to_stream (pp, /*for_record=*/true); + pp_write_text_as_dot_label_to_stream (pp, /*for_record=*/false); pp_string (pp, "\"];"); pp_newline (pp); diff --git a/gcc/analyzer/state-purge.cc b/gcc/analyzer/state-purge.cc index caaa94650e99..79eb5549197c 100644 --- a/gcc/analyzer/state-purge.cc +++ b/gcc/analyzer/state-purge.cc @@ -408,16 +408,17 @@ state_purge_per_ssa_name::process_point (const function_point &point, to the supernode N. */ void -state_purge_annotator::add_node_annotations (pretty_printer *pp, +state_purge_annotator::add_node_annotations (graphviz_out *gv, const supernode &n) const { if (m_map == NULL) return; + pretty_printer *pp = gv->get_pp (); + pp_printf (pp, "annotation_for_node_%i", n.m_index); - pp_printf (pp, " [shape=%s,style=filled,fillcolor=%s,label=\"", - "record", "lightblue"); - pp_left_brace (pp); + pp_printf (pp, " [shape=none,margin=0,style=filled,fillcolor=%s,label=\"", + "lightblue"); pp_write_text_to_stream (pp); // FIXME: passing in a NULL in-edge means we get no hits @@ -442,23 +443,23 @@ POP_IGNORE_WFORMAT pp_newline (pp); } - pp_right_brace (pp); - pp_string (pp, "\"];\n\n"); pp_flush (pp); } -/* Print V to PP as a comma-separated list in braces, titling it - with TITLE. +/* Print V to GV as a comma-separated list in braces within a <TR>, + titling it with TITLE. Subroutine of state_purge_annotator::add_stmt_annotations. */ static void -print_vec_of_names (pretty_printer *pp, const char *title, +print_vec_of_names (graphviz_out *gv, const char *title, const auto_vec<tree> &v) { + pretty_printer *pp = gv->get_pp (); tree name; unsigned i; + gv->begin_tr (); pp_printf (pp, "%s: {", title); FOR_EACH_VEC_ELT (v, i, name) { @@ -469,6 +470,8 @@ PUSH_IGNORE_WFORMAT POP_IGNORE_WFORMAT } pp_printf (pp, "}"); + pp_write_text_as_html_like_dot_to_stream (pp); + gv->end_tr (); pp_newline (pp); } @@ -478,7 +481,7 @@ POP_IGNORE_WFORMAT Add text showing which names are purged at STMT. */ void -state_purge_annotator::add_stmt_annotations (pretty_printer *pp, +state_purge_annotator::add_stmt_annotations (graphviz_out *gv, const gimple *stmt) const { if (m_map == NULL) @@ -487,6 +490,8 @@ state_purge_annotator::add_stmt_annotations (pretty_printer *pp, if (stmt->code == GIMPLE_PHI) return; + pretty_printer *pp = gv->get_pp (); + pp_newline (pp); const supernode *supernode = m_map->get_sg ().get_supernode_for_stmt (stmt); @@ -511,6 +516,6 @@ state_purge_annotator::add_stmt_annotations (pretty_printer *pp, } } - print_vec_of_names (pp, "needed here", needed); - print_vec_of_names (pp, "not needed here", not_needed); + print_vec_of_names (gv, "needed here", needed); + print_vec_of_names (gv, "not needed here", not_needed); } diff --git a/gcc/analyzer/state-purge.h b/gcc/analyzer/state-purge.h index a86afc28ccb9..f50c239503ea 100644 --- a/gcc/analyzer/state-purge.h +++ b/gcc/analyzer/state-purge.h @@ -157,10 +157,10 @@ class state_purge_annotator : public dot_annotator public: state_purge_annotator (const state_purge_map *map) : m_map (map) {} - void add_node_annotations (pretty_printer *pp, const supernode &n) + void add_node_annotations (graphviz_out *gv, const supernode &n) const FINAL OVERRIDE; - void add_stmt_annotations (pretty_printer *pp,const gimple *stmt) + void add_stmt_annotations (graphviz_out *gv, const gimple *stmt) const FINAL OVERRIDE; private: diff --git a/gcc/analyzer/supergraph.cc b/gcc/analyzer/supergraph.cc index da5b13a316ef..17cfe5d5f2d7 100644 --- a/gcc/analyzer/supergraph.cc +++ b/gcc/analyzer/supergraph.cc @@ -412,7 +412,7 @@ supergraph::add_return_superedge (supernode *src, supernode *dest, /* Implementation of dnode::dump_dot vfunc for supernodes. - Write a cluster for the node, and within it a record showing + Write a cluster for the node, and within it a .dot node showing the phi nodes and stmts. Call into any node annotator from ARGS to potentially add other records to the cluster. */ @@ -431,34 +431,41 @@ supernode::dump_dot (graphviz_out *gv, const dump_args_t &args) const pretty_printer *pp = gv->get_pp (); if (args.m_node_annotator) - args.m_node_annotator->add_node_annotations (pp, *this); + args.m_node_annotator->add_node_annotations (gv, *this); gv->write_indent (); dump_dot_id (pp); - pp_printf (pp, " [shape=%s,style=filled,fillcolor=%s,label=\"", - "record", "lightgrey"); - pp_left_brace (pp); + pp_printf (pp, + " [shape=none,margin=0,style=filled,fillcolor=%s,label=<", + "lightgrey"); + pp_string (pp, "<TABLE BORDER=\"0\">"); pp_write_text_to_stream (pp); if (m_returning_call) { + gv->begin_tr (); pp_string (pp, "returning call: "); + gv->end_tr (); + + gv->begin_tr (); pp_gimple_stmt_1 (pp, m_returning_call, 0, (dump_flags_t)0); + pp_write_text_as_html_like_dot_to_stream (pp); + gv->end_tr (); + if (args.m_node_annotator) - args.m_node_annotator->add_stmt_annotations (pp, m_returning_call); - pp_write_text_as_dot_label_to_stream (pp, /*for_record=*/true); + args.m_node_annotator->add_stmt_annotations (gv, m_returning_call); pp_newline (pp); } if (entry_p ()) { - pp_string (pp, "ENTRY"); + pp_string (pp, "<TR><TD>ENTRY</TD></TR>"); pp_newline (pp); } if (return_p ()) { - pp_string (pp, "EXIT"); + pp_string (pp, "<TR><TD>EXIT</TD></TR>"); pp_newline (pp); } @@ -467,10 +474,14 @@ supernode::dump_dot (graphviz_out *gv, const dump_args_t &args) const !gsi_end_p (gpi); gsi_next (&gpi)) { const gimple *stmt = gsi_stmt (gpi); + gv->begin_tr (); pp_gimple_stmt_1 (pp, stmt, 0, (dump_flags_t)0); + pp_write_text_as_html_like_dot_to_stream (pp); + gv->end_tr (); + if (args.m_node_annotator) - args.m_node_annotator->add_stmt_annotations (pp, stmt); - pp_write_text_as_dot_label_to_stream (pp, /*for_record=*/true); + args.m_node_annotator->add_stmt_annotations (gv, stmt); + pp_newline (pp); } @@ -479,15 +490,18 @@ supernode::dump_dot (graphviz_out *gv, const dump_args_t &args) const gimple *stmt; FOR_EACH_VEC_ELT (m_stmts, i, stmt) { + gv->begin_tr (); pp_gimple_stmt_1 (pp, stmt, 0, (dump_flags_t)0); + pp_write_text_as_html_like_dot_to_stream (pp); + gv->end_tr (); + if (args.m_node_annotator) - args.m_node_annotator->add_stmt_annotations (pp, stmt); - pp_write_text_as_dot_label_to_stream (pp, /*for_record=*/true); + args.m_node_annotator->add_stmt_annotations (gv, stmt); + pp_newline (pp); } - pp_right_brace (pp); - pp_string (pp, "\"];\n\n"); + pp_string (pp, "</TABLE>>];\n\n"); pp_flush (pp); /* Terminate "subgraph" */ diff --git a/gcc/analyzer/supergraph.h b/gcc/analyzer/supergraph.h index 78ae93aa95d1..a13d6730b3ca 100644 --- a/gcc/analyzer/supergraph.h +++ b/gcc/analyzer/supergraph.h @@ -547,10 +547,10 @@ class dot_annotator { public: virtual ~dot_annotator () {} - virtual void add_node_annotations (pretty_printer *pp ATTRIBUTE_UNUSED, + virtual void add_node_annotations (graphviz_out *gv ATTRIBUTE_UNUSED, const supernode &n ATTRIBUTE_UNUSED) const {} - virtual void add_stmt_annotations (pretty_printer *pp ATTRIBUTE_UNUSED, + virtual void add_stmt_annotations (graphviz_out *gv ATTRIBUTE_UNUSED, const gimple *stmt ATTRIBUTE_UNUSED) const {} }; diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c index d5aa451c1453..4ab70c9470e0 100644 --- a/gcc/pretty-print.c +++ b/gcc/pretty-print.c @@ -906,6 +906,54 @@ pp_write_text_as_dot_label_to_stream (pretty_printer *pp, bool for_record) pp_clear_output_area (pp); } +/* As pp_write_text_to_stream, but for GraphViz HTML-like strings. + + Flush the formatted text of pretty-printer PP onto the attached stream, + escaping these characters + " & < > + using XML escape sequences. + + http://www.graphviz.org/doc/info/lang.html#html states: + special XML escape sequences for ", &, <, and > may be necessary in + order to embed these characters in attribute values or raw text + This doesn't list "'" (which would normally be escaped in XML + as "'" or in HTML as "'");. + + Experiments show that escaping "'" doesn't seem to be necessary. */ + +void +pp_write_text_as_html_like_dot_to_stream (pretty_printer *pp) +{ + const char *text = pp_formatted_text (pp); + const char *p = text; + FILE *fp = pp_buffer (pp)->stream; + + for (;*p; p++) + { + switch (*p) + { + case '"': + fputs (""", fp); + break; + case '&': + fputs ("&", fp); + break; + case '<': + fputs ("<", fp); + break; + case '>': + fputs (">",fp); + break; + + default: + fputc (*p, fp); + break; + } + } + + pp_clear_output_area (pp); +} + /* Wrap a text delimited by START and END into PRETTY-PRINTER. */ static void pp_wrap_text (pretty_printer *pp, const char *start, const char *end) diff --git a/gcc/pretty-print.h b/gcc/pretty-print.h index 493507d41419..86b9e869eeb7 100644 --- a/gcc/pretty-print.h +++ b/gcc/pretty-print.h @@ -393,8 +393,11 @@ extern void pp_indent (pretty_printer *); extern void pp_newline (pretty_printer *); extern void pp_character (pretty_printer *, int); extern void pp_string (pretty_printer *, const char *); + extern void pp_write_text_to_stream (pretty_printer *); extern void pp_write_text_as_dot_label_to_stream (pretty_printer *, bool); +extern void pp_write_text_as_html_like_dot_to_stream (pretty_printer *pp); + extern void pp_maybe_space (pretty_printer *); extern void pp_begin_quote (pretty_printer *, bool); diff --git a/gcc/testsuite/gcc.dg/analyzer/dot-output.c b/gcc/testsuite/gcc.dg/analyzer/dot-output.c new file mode 100644 index 000000000000..586e14421e0e --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/dot-output.c @@ -0,0 +1,33 @@ +/* Verify that the various .dot output files from the analyzer are readable + by .dot. */ + +/* { dg-require-dot "" } */ +/* { dg-additional-options "-fdump-analyzer-callgraph -fdump-analyzer-exploded-graph -fdump-analyzer-state-purge -fdump-analyzer-supergraph" } */ + +#include <stdlib.h> + +int some_call (int i, char ch) +{ + return i * i; +} + +int *test (int *buf, int n, int *out) +{ + int i; + int *result = malloc (sizeof (int) * n); + + /* A loop, to ensure we have phi nodes. */ + for (i = 0; i < n; i++) + result[i] = buf[i] + i; /* { dg-warning "possibly-NULL" "" { xfail *-*-* } } */ + /* TODO(xfail): why isn't the warning appearing? */ + + /* Example of a "'" (to test quoting). */ + *out = some_call (i, 'a'); + + return result; +} + +/* { dg-final { dg-check-dot "dot-output.c.callgraph.dot" } } */ +/* { dg-final { dg-check-dot "dot-output.c.eg.dot" } } */ +/* { dg-final { dg-check-dot "dot-output.c.state-purge.dot" } } */ +/* { dg-final { dg-check-dot "dot-output.c.supergraph.dot" } } */ diff --git a/gcc/testsuite/lib/gcc-defs.exp b/gcc/testsuite/lib/gcc-defs.exp index 945b48afa398..44e583aba064 100644 --- a/gcc/testsuite/lib/gcc-defs.exp +++ b/gcc/testsuite/lib/gcc-defs.exp @@ -403,3 +403,24 @@ proc handle-dg-regexps { text } { return $text } + +# Verify that the initial arg is a valid .dot file +# (by running dot -Tpng on it, and verifying the exit code is 0). + +proc dg-check-dot { args } { + verbose "dg-check-dot: args: $args" 2 + + set testcase [testname-for-summary] + + set dotfile [lindex $args 0] + verbose " dotfile: $dotfile" 2 + + set status [remote_exec host "dot" "-O -Tpng $dotfile"] + verbose " status: $status" 2 + if { [lindex $status 0] != 0 } { + fail "$testcase dg-check-dot $dotfile" + return 0 + } + + pass "$testcase dg-check-dot $dotfile" +} diff --git a/gcc/testsuite/lib/target-supports-dg.exp b/gcc/testsuite/lib/target-supports-dg.exp index e1da57abcf7b..88c6bd9d7db8 100644 --- a/gcc/testsuite/lib/target-supports-dg.exp +++ b/gcc/testsuite/lib/target-supports-dg.exp @@ -180,6 +180,16 @@ proc dg-require-iconv { args } { } } +# If this host does not have "dot", skip this test. + +proc dg-require-dot { args } { + verbose "dg-require-dot" 2 + if { ![ check_dot_available ] } { + upvar dg-do-what dg-do-what + set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"] + } +} + # If this target does not have sufficient stack size, skip this test. proc dg-require-stack-size { args } { diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 2ac2f08cb968..59e0540ee237 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -514,6 +514,19 @@ proc check_gc_sections_available { } { }] } +# Returns 1 if "dot" is supported on the host. + +proc check_dot_available { } { + verbose "check_dot_available" 2 + + set status [remote_exec host "dot" "-V"] + verbose " status: $status" 2 + if { [lindex $status 0] != 0 } { + return 0 + } + return 1 +} + # Return 1 if according to target_info struct and explicit target list # target is supposed to support trampolines. -- 2.21.0