| Message ID | 20200125005328.29478-1-dmalcolm@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | analyzer: fix build with gcc 4.4 (PR 93276) | expand |
On Fri, 2020-01-24 at 19:53 -0500, David Malcolm wrote: > This patch fixes various build failures seen with gcc 4.4 > > gcc prior to 4.6 complains about: > > error: #pragma GCC diagnostic not allowed inside functions > > for various uses of PUSH_IGNORE_WFORMAT and POP_IGNORE_WFORMAT. > This patch makes them a no-op with such compilers. > > The patch also fixes various errors with template base initializers > and redundant uses of "typename" that older g++ implementations > can't cope with. > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu > with gcc gcc 9.2.1. > Appears to fix the build with gcc 4.4: I was able to successfully > build stage1 xgcc and cc1 and run the analyzer test suite (though > I'm seeing an apparently unrelated: > pure virtual method called > terminate called without an active exception > in the selftests for diagnostic-show-locus.c) > > OK for master? > > gcc/analyzer/ChangeLog: > PR analyzer/93276 > * analyzer.h (PUSH_IGNORE_WFORMAT, POP_IGNORE_WFORMAT): Guard these > macros with GCC_VERSION >= 4006, making them no-op otherwise. > * engine.cc (exploded_edge::exploded_edge): Specify template for > base class initializer. > (exploded_graph::add_edge): Specify template when chaining up to > base class add_edge implementation. > (viz_callgraph_node::dump_dot): Drop redundant "typename". > (viz_callgraph_edge::viz_callgraph_edge): Specify template for > base class initializer. > * program-state.cc (sm_state_map::clone_with_remapping): Drop > redundant "typename". > (sm_state_map::print): Likewise. > (sm_state_map::hash): Likewise. > (sm_state_map::operator==): Likewise. > (sm_state_map::remap_svalue_ids): Likewise. > (sm_state_map::on_svalue_purge): Likewise. > (sm_state_map::validate): Likewise. > * program-state.h (sm_state_map::iterator_t): Likewise. > * supergraph.h (superedge::superedge): Specify template for base > class initializer. > > gcc/ChangeLog: > PR analyzer/93276 > * digraph.cc (test_edge::test_edge): Specify template for base > class initializer. OK. And more generally, if you're hacking on facilities just used by the analyzer go ahead. While we haven't gone through the formal process of naming you maintainer of hte analyzer, I'm certain we will once I can come up for air and propose it. jeff >
On Fri, Jan 24, 2020 at 07:53:28PM -0500, David Malcolm wrote: > This patch fixes various build failures seen with gcc 4.4 > > gcc prior to 4.6 complains about: > > error: #pragma GCC diagnostic not allowed inside functions > > for various uses of PUSH_IGNORE_WFORMAT and POP_IGNORE_WFORMAT. > This patch makes them a no-op with such compilers. I think this is wrong. All that is really needed is make sure you #include "diagnostic-core.h" before including pretty-print.h. By including diagnostic-core.h first, you do: #ifndef GCC_DIAG_STYLE #define GCC_DIAG_STYLE __gcc_tdiag__ #endif and then pretty-print.h will do: #ifdef GCC_DIAG_STYLE #define GCC_PPDIAG_STYLE GCC_DIAG_STYLE #else #define GCC_PPDIAG_STYLE __gcc_diag__ #endif If instead pretty-print.h is included first, then it will use __gcc_diag__ instead of __gcc_tdiag__ and thus will assume %E/%D etc. can't be handled. I've so far just tested that in stage3 with this patch analyzer builds without any -Wformat/-Wformat-extra-args warnings. Ok for trunk if it passes bootstrap/regtest? 2020-01-28 Jakub Jelinek <jakub@redhat.com> * analyzer.h (PUSH_IGNORE_WFORMAT, POP_IGNORE_WFORMAT): Remove. * constraint-manager.cc: Include diagnostic-core.h before graphviz.h. (range::dump, equiv_class::print): Don't use PUSH_IGNORE_WFORMAT or POP_IGNORE_WFORMAT. * state-purge.cc: Include diagnostic-core.h before gimple-pretty-print.h. (state_purge_annotator::add_node_annotations, print_vec_of_names): Don't use PUSH_IGNORE_WFORMAT or POP_IGNORE_WFORMAT. * region-model.cc: Move diagnostic-core.h include before graphviz.h. (path_var::dump, svalue::print, constant_svalue::print_details, region::dump_to_pp, region::dump_child_label, region::print_fields, map_region::print_fields, map_region::dump_dot_to_pp, map_region::dump_child_label, array_region::print_fields, array_region::dump_dot_to_pp): Don't use PUSH_IGNORE_WFORMAT or POP_IGNORE_WFORMAT. --- gcc/analyzer/analyzer.h.jj 2020-01-27 22:40:57.012420793 +0100 +++ gcc/analyzer/analyzer.h 2020-01-28 10:57:28.078715244 +0100 @@ -100,22 +100,6 @@ public: ~auto_cfun () { pop_cfun (); } }; -/* Macros for temporarily suppressing -Wformat and -Wformat-extra-args, - for those versions of GCC that support pragmas within a function - (4.6 onwards). */ - -#if GCC_VERSION >= 4006 -# define PUSH_IGNORE_WFORMAT \ - _Pragma("GCC diagnostic push") \ - _Pragma("GCC diagnostic ignored \"-Wformat\"") \ - _Pragma("GCC diagnostic ignored \"-Wformat-extra-args\"") -# define POP_IGNORE_WFORMAT \ - _Pragma("GCC diagnostic pop") -#else -# define PUSH_IGNORE_WFORMAT -# define POP_IGNORE_WFORMAT -#endif - /* A template for creating hash traits for a POD type. */ template <typename Type> --- gcc/analyzer/constraint-manager.cc.jj 2020-01-22 22:37:04.478533500 +0100 +++ gcc/analyzer/constraint-manager.cc 2020-01-28 10:54:41.403226986 +0100 @@ -28,6 +28,7 @@ along with GCC; see the file COPYING3. #include "gimple-iterator.h" #include "fold-const.h" #include "selftest.h" +#include "diagnostic-core.h" #include "graphviz.h" #include "function.h" #include "analyzer/analyzer.h" @@ -120,13 +121,11 @@ bound::get_relation_as_str () const void range::dump (pretty_printer *pp) const { -PUSH_IGNORE_WFORMAT pp_printf (pp, "%qE %s x %s %qE", m_lower_bound.m_constant, m_lower_bound.get_relation_as_str (), m_upper_bound.get_relation_as_str (), m_upper_bound.m_constant); -POP_IGNORE_WFORMAT } /* Determine if there is only one possible value for this range. @@ -200,9 +199,7 @@ equiv_class::print (pretty_printer *pp) { if (i > 0) pp_string (pp, " == "); -PUSH_IGNORE_WFORMAT pp_printf (pp, "%qE", m_constant); -POP_IGNORE_WFORMAT } pp_character (pp, '}'); } --- gcc/analyzer/state-purge.cc.jj 2020-01-14 22:57:20.802253484 +0100 +++ gcc/analyzer/state-purge.cc 2020-01-28 10:57:17.642872508 +0100 @@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. #include "tree-phinodes.h" #include "options.h" #include "ssa-iterators.h" +#include "diagnostic-core.h" #include "gimple-pretty-print.h" #include "function.h" #include "analyzer/analyzer.h" @@ -444,12 +445,10 @@ state_purge_annotator::add_node_annotati state_purge_per_ssa_name *per_name_data = (*iter).second; if (per_name_data->get_function () == n.m_fun) { -PUSH_IGNORE_WFORMAT if (per_name_data->needed_at_point_p (before_supernode)) pp_printf (pp, "%qE needed here", name); else pp_printf (pp, "%qE not needed here", name); -POP_IGNORE_WFORMAT } pp_newline (pp); } @@ -476,9 +475,7 @@ print_vec_of_names (graphviz_out *gv, co { if (i > 0) pp_string (pp, ", "); -PUSH_IGNORE_WFORMAT pp_printf (pp, "%qE", name); -POP_IGNORE_WFORMAT } pp_printf (pp, "}"); pp_write_text_as_html_like_dot_to_stream (pp); --- gcc/analyzer/region-model.cc.jj 2020-01-27 22:40:57.014420763 +0100 +++ gcc/analyzer/region-model.cc 2020-01-28 10:54:41.402227001 +0100 @@ -26,6 +26,7 @@ along with GCC; see the file COPYING3. #include "basic-block.h" #include "gimple.h" #include "gimple-iterator.h" +#include "diagnostic-core.h" #include "graphviz.h" #include "options.h" #include "cgraph.h" @@ -37,7 +38,6 @@ along with GCC; see the file COPYING3. #include "tree-pretty-print.h" #include "diagnostic-color.h" #include "diagnostic-metadata.h" -#include "diagnostic-core.h" #include "tristate.h" #include "bitmap.h" #include "selftest.h" @@ -88,14 +88,12 @@ dump_tree (pretty_printer *pp, tree t) void path_var::dump (pretty_printer *pp) const { -PUSH_IGNORE_WFORMAT if (m_tree == NULL_TREE) pp_string (pp, "NULL"); if (CONSTANT_CLASS_P (m_tree)) pp_printf (pp, "%qE", m_tree); else pp_printf (pp, "(%qE @ %i)", m_tree, m_stack_depth); -POP_IGNORE_WFORMAT } /* For use in printing a comma-separated list. */ @@ -318,13 +316,11 @@ svalue::print (const region_model &model this_sid.print (pp); pp_string (pp, ": {"); -PUSH_IGNORE_WFORMAT if (m_type) { gcc_assert (TYPE_P (m_type)); pp_printf (pp, "type: %qT, ", m_type); } -POP_IGNORE_WFORMAT /* vfunc. */ print_details (model, this_sid, pp); @@ -686,9 +682,7 @@ constant_svalue::print_details (const re svalue_id this_sid ATTRIBUTE_UNUSED, pretty_printer *pp) const { -PUSH_IGNORE_WFORMAT pp_printf (pp, "%qE", m_cst_expr); -POP_IGNORE_WFORMAT } /* Implementation of svalue::get_child_sid vfunc for constant_svalue. */ @@ -1284,9 +1278,7 @@ region::dump_to_pp (const region_model & } if (m_type) { -PUSH_IGNORE_WFORMAT pp_printf (pp, "%s type: %qT", field_prefix, m_type); -POP_IGNORE_WFORMAT pp_newline (pp); } @@ -1336,9 +1328,7 @@ region::dump_child_label (const region_m pp_string (pp, "active "); else pp_string (pp, "inactive "); -PUSH_IGNORE_WFORMAT pp_printf (pp, "view as %qT: ", child->get_type ()); -POP_IGNORE_WFORMAT } } @@ -1468,10 +1458,8 @@ region::print_fields (const region_model pp_printf (pp, ", sval: "); m_sval_id.print (pp); -PUSH_IGNORE_WFORMAT if (m_type) pp_printf (pp, ", type: %qT", m_type); -POP_IGNORE_WFORMAT } /* Determine if a pointer to this region must be non-NULL. @@ -1574,9 +1562,7 @@ map_region::print_fields (const region_m pp_string (pp, ", "); tree expr = (*iter).first; region_id child_rid = (*iter).second; -PUSH_IGNORE_WFORMAT pp_printf (pp, "%qE: ", expr); -POP_IGNORE_WFORMAT child_rid.print (pp); } pp_string (pp, "}"); @@ -1601,9 +1587,7 @@ map_region::dump_dot_to_pp (const region pp_printf (pp, "rid_label_%i [label=\"", child_rid.as_int ()); pp_write_text_to_stream (pp); -PUSH_IGNORE_WFORMAT pp_printf (pp, "%qE", expr); -POP_IGNORE_WFORMAT pp_write_text_as_dot_label_to_stream (pp, /*for_record=*/false); pp_string (pp, "\"];"); pp_newline (pp); @@ -1633,12 +1617,10 @@ map_region::dump_child_label (const regi if (child_rid == (*iter).second) { tree key = (*iter).first; -PUSH_IGNORE_WFORMAT if (DECL_P (key)) pp_printf (pp, "%qD: ", key); else pp_printf (pp, "%qE: ", key); -POP_IGNORE_WFORMAT } } } @@ -2235,9 +2217,7 @@ array_region::print_fields (const region pp_string (pp, ", "); int key = (*iter).first; region_id child_rid = (*iter).second; -PUSH_IGNORE_WFORMAT pp_printf (pp, "[%i]: ", key); -POP_IGNORE_WFORMAT child_rid.print (pp); } pp_string (pp, "}"); @@ -2262,9 +2242,7 @@ array_region::dump_dot_to_pp (const regi pp_printf (pp, "rid_label_%i [label=\"", child_rid.as_int ()); pp_write_text_to_stream (pp); -PUSH_IGNORE_WFORMAT pp_printf (pp, "%qi", key); -POP_IGNORE_WFORMAT pp_write_text_as_dot_label_to_stream (pp, /*for_record=*/false); pp_string (pp, "\"];"); pp_newline (pp); Jakub
On Tue, 2020-01-28 at 11:13 +0100, Jakub Jelinek wrote: > On Fri, Jan 24, 2020 at 07:53:28PM -0500, David Malcolm wrote: > > This patch fixes various build failures seen with gcc 4.4 > > > > gcc prior to 4.6 complains about: > > > > error: #pragma GCC diagnostic not allowed inside functions > > > > for various uses of PUSH_IGNORE_WFORMAT and POP_IGNORE_WFORMAT. > > This patch makes them a no-op with such compilers. > > I think this is wrong. > All that is really needed is make sure you #include "diagnostic- > core.h" > before including pretty-print.h. By including > diagnostic-core.h first, you do: > #ifndef GCC_DIAG_STYLE > #define GCC_DIAG_STYLE __gcc_tdiag__ > #endif > and then pretty-print.h will do: > #ifdef GCC_DIAG_STYLE > #define GCC_PPDIAG_STYLE GCC_DIAG_STYLE > #else > #define GCC_PPDIAG_STYLE __gcc_diag__ > #endif > If instead pretty-print.h is included first, then it will use > __gcc_diag__ > instead of __gcc_tdiag__ and thus will assume %E/%D etc. can't be > handled. > > I've so far just tested that in stage3 with this patch analyzer > builds > without any -Wformat/-Wformat-extra-args warnings. Aha! Thanks - that's much better. > Ok for trunk if it passes bootstrap/regtest? LGTM. Dave
diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h index e84e6958cec..9746c9e0123 100644 --- a/gcc/analyzer/analyzer.h +++ b/gcc/analyzer/analyzer.h @@ -98,17 +98,21 @@ public: ~auto_cfun () { pop_cfun (); } }; -/* Begin suppressing -Wformat and -Wformat-extra-args. */ +/* Macros for temporarily suppressing -Wformat and -Wformat-extra-args, + for those versions of GCC that support pragmas within a function + (4.6 onwards). */ -#define PUSH_IGNORE_WFORMAT \ +#if GCC_VERSION >= 4006 +# define PUSH_IGNORE_WFORMAT \ _Pragma("GCC diagnostic push") \ _Pragma("GCC diagnostic ignored \"-Wformat\"") \ _Pragma("GCC diagnostic ignored \"-Wformat-extra-args\"") - -/* Finish suppressing -Wformat and -Wformat-extra-args. */ - -#define POP_IGNORE_WFORMAT \ +# define POP_IGNORE_WFORMAT \ _Pragma("GCC diagnostic pop") +#else +# define PUSH_IGNORE_WFORMAT +# define POP_IGNORE_WFORMAT +#endif /* A template for creating hash traits for a POD type. */ diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc index 737ea1dd6e4..a2587a33a66 100644 --- a/gcc/analyzer/engine.cc +++ b/gcc/analyzer/engine.cc @@ -1377,7 +1377,7 @@ rewind_info_t::add_events_to_path (checker_path *emission_path, dst_stack_depth, this)); } -/* class exploded_edge : public dedge. */ +/* class exploded_edge : public dedge<eg_traits>. */ /* exploded_edge's ctor. */ @@ -1385,7 +1385,7 @@ exploded_edge::exploded_edge (exploded_node *src, exploded_node *dest, const superedge *sedge, const state_change &change, custom_info_t *custom_info) -: dedge (src, dest), m_sedge (sedge), m_change (change), +: dedge<eg_traits> (src, dest), m_sedge (sedge), m_change (change), m_custom_info (custom_info) { change.validate (dest->get_state ()); @@ -1991,7 +1991,7 @@ exploded_graph::add_edge (exploded_node *src, exploded_node *dest, exploded_edge::custom_info_t *custom_info) { exploded_edge *e = new exploded_edge (src, dest, sedge, change, custom_info); - digraph::add_edge (e); + digraph<eg_traits>::add_edge (e); return e; } @@ -3332,7 +3332,7 @@ public: // TODO: also show the per-callstring breakdown const exploded_graph::call_string_data_map_t *per_cs_data = args.m_eg->get_per_call_string_data (); - for (typename exploded_graph::call_string_data_map_t::iterator iter + for (exploded_graph::call_string_data_map_t::iterator iter = per_cs_data->begin (); iter != per_cs_data->end (); ++iter) @@ -3391,7 +3391,7 @@ class viz_callgraph_edge : public dedge<viz_callgraph_traits> public: viz_callgraph_edge (viz_callgraph_node *src, viz_callgraph_node *dest, const call_superedge *call_sedge) - : dedge (src, dest), + : dedge<viz_callgraph_traits> (src, dest), m_call_sedge (call_sedge) {} diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc index ba19ad1490e..a9e300fba0f 100644 --- a/gcc/analyzer/program-state.cc +++ b/gcc/analyzer/program-state.cc @@ -87,7 +87,7 @@ sm_state_map::clone_with_remapping (const one_way_svalue_id_map &id_map) const { sm_state_map *result = new sm_state_map (); result->m_global_state = m_global_state; - for (typename map_t::iterator iter = m_map.begin (); + for (map_t::iterator iter = m_map.begin (); iter != m_map.end (); ++iter) { @@ -121,7 +121,7 @@ sm_state_map::print (const state_machine &sm, pretty_printer *pp) const pp_printf (pp, "global: %s", sm.get_state_name (m_global_state)); first = false; } - for (typename map_t::iterator iter = m_map.begin (); + for (map_t::iterator iter = m_map.begin (); iter != m_map.end (); ++iter) { @@ -172,7 +172,7 @@ sm_state_map::hash () const /* Accumulate the result by xoring a hash for each slot, so that the result doesn't depend on the ordering of the slots in the map. */ - for (typename map_t::iterator iter = m_map.begin (); + for (map_t::iterator iter = m_map.begin (); iter != m_map.end (); ++iter) { @@ -199,7 +199,7 @@ sm_state_map::operator== (const sm_state_map &other) const if (m_map.elements () != other.m_map.elements ()) return false; - for (typename map_t::iterator iter = m_map.begin (); + for (map_t::iterator iter = m_map.begin (); iter != m_map.end (); ++iter) { @@ -395,7 +395,7 @@ sm_state_map::remap_svalue_ids (const svalue_id_map &map) map_t tmp_map; /* Build an intermediate map, using the new sids. */ - for (typename map_t::iterator iter = m_map.begin (); + for (map_t::iterator iter = m_map.begin (); iter != m_map.end (); ++iter) { @@ -411,7 +411,7 @@ sm_state_map::remap_svalue_ids (const svalue_id_map &map) m_map.empty (); /* Copy over from intermediate map. */ - for (typename map_t::iterator iter = tmp_map.begin (); + for (map_t::iterator iter = tmp_map.begin (); iter != tmp_map.end (); ++iter) { @@ -437,7 +437,7 @@ sm_state_map::on_svalue_purge (const state_machine &sm, /* TODO: ideally remove the slot directly; for now do it in two stages. */ auto_vec<svalue_id> to_remove; - for (typename map_t::iterator iter = m_map.begin (); + for (map_t::iterator iter = m_map.begin (); iter != m_map.end (); ++iter) { @@ -507,7 +507,7 @@ sm_state_map::validate (const state_machine &sm, return; #endif - for (typename map_t::iterator iter = m_map.begin (); + for (map_t::iterator iter = m_map.begin (); iter != m_map.end (); ++iter) { diff --git a/gcc/analyzer/program-state.h b/gcc/analyzer/program-state.h index a2c5c9a1137..adc71a4eda2 100644 --- a/gcc/analyzer/program-state.h +++ b/gcc/analyzer/program-state.h @@ -132,7 +132,7 @@ public: svalue_id m_origin; }; typedef hash_map <svalue_id, entry_t> map_t; - typedef typename map_t::iterator iterator_t; + typedef map_t::iterator iterator_t; sm_state_map (); diff --git a/gcc/analyzer/supergraph.h b/gcc/analyzer/supergraph.h index 3580ae60c08..0eac0b8bfc9 100644 --- a/gcc/analyzer/supergraph.h +++ b/gcc/analyzer/supergraph.h @@ -304,7 +304,7 @@ class superedge : public dedge<supergraph_traits> protected: superedge (supernode *src, supernode *dest, enum edge_kind kind) - : dedge (src, dest), + : dedge<supergraph_traits> (src, dest), m_kind (kind) {} diff --git a/gcc/digraph.cc b/gcc/digraph.cc index 02ff93dac13..31b3e19851f 100644 --- a/gcc/digraph.cc +++ b/gcc/digraph.cc @@ -62,7 +62,7 @@ struct test_node : public dnode<test_graph_traits> struct test_edge : public dedge<test_graph_traits> { test_edge (node_t *src, node_t *dest) - : dedge (src, dest) + : dedge<test_graph_traits> (src, dest) {} void dump_dot (graphviz_out *gv, const dump_args_t &) const OVERRIDE