Hi,
this is a series of fixes for the exception handling code, with the same goal
of preventing instructions from inheriting random source location information
in the debug info generated by the compiler.
Tested on x86_64-suse-linux, both GCC and GDB, OK for mainline?
2019-07-03 Eric Botcazou <ebotca...@adacore.com>
* except.c (emit_to_new_bb_before): Make sure to put a location on SEQ.
* tree-eh.c (replace_goto_queue_1) <GIMPLE_GOTO>: Propagate location.
(emit_eh_dispatch): Delete.
(lower_catch): Emit the eh_dispatch manually and set the location of
the first catch statement onto it.
(lower_eh_filter): Emit the eh_dispatch manually and set location.
(lower_eh_dispatch): Propagate location.
* tree-outof-ssa.c (set_location_for_edge): Handle EH edges specially.
(eliminate_build): Likewise.
--
Eric BotcazouIndex: except.c
===================================================================
--- except.c (revision 272930)
+++ except.c (working copy)
@@ -921,7 +921,7 @@ assign_filter_values (void)
static basic_block
emit_to_new_bb_before (rtx_insn *seq, rtx_insn *insn)
{
- rtx_insn *last;
+ rtx_insn *next, *last;
basic_block bb;
edge e;
edge_iterator ei;
@@ -934,7 +934,16 @@ emit_to_new_bb_before (rtx_insn *seq, rt
force_nonfallthru (e);
else
ei_next (&ei);
- last = emit_insn_before (seq, insn);
+
+ /* Make sure to put the location of INSN or a subsequent instruction on SEQ
+ to avoid inheriting the location of the previous instruction. */
+ next = insn;
+ while (next && !NONDEBUG_INSN_P (next))
+ next = NEXT_INSN (next);
+ if (next)
+ last = emit_insn_before_setloc (seq, insn, INSN_LOCATION (next));
+ else
+ last = emit_insn_before (seq, insn);
if (BARRIER_P (last))
last = PREV_INSN (last);
bb = create_basic_block (seq, last, BLOCK_FOR_INSN (insn)->prev_bb);
Index: tree-eh.c
===================================================================
--- tree-eh.c (revision 272930)
+++ tree-eh.c (working copy)
@@ -503,7 +503,11 @@ replace_goto_queue_1 (gimple *stmt, stru
seq = find_goto_replacement (tf, temp);
if (seq)
{
- gsi_insert_seq_before (gsi, gimple_seq_copy (seq), GSI_SAME_STMT);
+ gimple_stmt_iterator i;
+ seq = gimple_seq_copy (seq);
+ for (i = gsi_start (seq); !gsi_end_p (i); gsi_next (&i))
+ gimple_set_location (gsi_stmt (i), gimple_location (stmt));
+ gsi_insert_seq_before (gsi, seq, GSI_SAME_STMT);
gsi_remove (gsi, false);
return;
}
@@ -811,15 +815,6 @@ emit_resx (gimple_seq *seq, eh_region re
record_stmt_eh_region (region->outer, x);
}
-/* Emit an EH_DISPATCH statement into SEQ for REGION. */
-
-static void
-emit_eh_dispatch (gimple_seq *seq, eh_region region)
-{
- geh_dispatch *x = gimple_build_eh_dispatch (region->index);
- gimple_seq_add_stmt (seq, x);
-}
-
/* Note that the current EH region may contain a throw, or a
call to a function which itself may contain a throw. */
@@ -1762,7 +1757,9 @@ lower_catch (struct leh_state *state, gt
tree out_label;
gimple_seq new_seq, cleanup;
gimple *x;
+ geh_dispatch *eh_dispatch;
location_t try_catch_loc = gimple_location (tp);
+ location_t catch_loc = UNKNOWN_LOCATION;
if (flag_exceptions)
{
@@ -1776,7 +1773,8 @@ lower_catch (struct leh_state *state, gt
return gimple_try_eval (tp);
new_seq = NULL;
- emit_eh_dispatch (&new_seq, try_region);
+ eh_dispatch = gimple_build_eh_dispatch (try_region->index);
+ gimple_seq_add_stmt (&new_seq, eh_dispatch);
emit_resx (&new_seq, try_region);
this_state.cur_region = state->cur_region;
@@ -1799,6 +1797,8 @@ lower_catch (struct leh_state *state, gt
gimple_seq handler;
catch_stmt = as_a <gcatch *> (gsi_stmt (gsi));
+ if (catch_loc == UNKNOWN_LOCATION)
+ catch_loc = gimple_location (catch_stmt);
c = gen_eh_region_catch (try_region, gimple_catch_types (catch_stmt));
handler = gimple_catch_handler (catch_stmt);
@@ -1822,6 +1822,10 @@ lower_catch (struct leh_state *state, gt
break;
}
+ /* Try to set a location on the dispatching construct to avoid inheriting
+ the location of the previous statement. */
+ gimple_set_location (eh_dispatch, catch_loc);
+
gimple_try_set_cleanup (tp, new_seq);
gimple_seq new_eh_seq = eh_seq;
@@ -1857,11 +1861,13 @@ lower_eh_filter (struct leh_state *state
if (!eh_region_may_contain_throw (this_region))
return gimple_try_eval (tp);
- new_seq = NULL;
this_state.cur_region = state->cur_region;
this_state.ehp_region = this_region;
- emit_eh_dispatch (&new_seq, this_region);
+ new_seq = NULL;
+ x = gimple_build_eh_dispatch (this_region->index);
+ gimple_set_location (x, gimple_location (tp));
+ gimple_seq_add_stmt (&new_seq, x);
emit_resx (&new_seq, this_region);
this_region->u.allowed.label = create_artificial_label (UNKNOWN_LOCATION);
@@ -3752,6 +3758,7 @@ lower_eh_dispatch (basic_block src, geh_
filter = create_tmp_var (TREE_TYPE (TREE_TYPE (fn)));
filter = make_ssa_name (filter, x);
gimple_call_set_lhs (x, filter);
+ gimple_set_location (x, gimple_location (stmt));
gsi_insert_before (&gsi, x, GSI_SAME_STMT);
/* Turn the default label into a default case. */
@@ -3759,6 +3766,7 @@ lower_eh_dispatch (basic_block src, geh_
sort_case_labels (labels);
x = gimple_build_switch (filter, default_label, labels);
+ gimple_set_location (x, gimple_location (stmt));
gsi_insert_before (&gsi, x, GSI_SAME_STMT);
}
}
@@ -3775,6 +3783,7 @@ lower_eh_dispatch (basic_block src, geh_
filter = create_tmp_var (TREE_TYPE (TREE_TYPE (fn)));
filter = make_ssa_name (filter, x);
gimple_call_set_lhs (x, filter);
+ gimple_set_location (x, gimple_location (stmt));
gsi_insert_before (&gsi, x, GSI_SAME_STMT);
r->u.allowed.label = NULL;
Index: tree-outof-ssa.c
===================================================================
--- tree-outof-ssa.c (revision 272930)
+++ tree-outof-ssa.c (working copy)
@@ -171,14 +171,43 @@ struct elim_graph
use its location. Otherwise search instructions in predecessors
of E for a location, and use that one. That makes sense because
we insert on edges for PHI nodes, and effects of PHIs happen on
- the end of the predecessor conceptually. */
+ the end of the predecessor conceptually. An exception is made
+ for EH edges because we don't want to drag the source location
+ of unrelated statements at the beginning of handlers; they would
+ be further reused for various EH constructs, which would damage
+ the coverage information. */
static void
set_location_for_edge (edge e)
{
if (e->goto_locus)
+ set_curr_insn_location (e->goto_locus);
+ else if (e->flags & EDGE_EH)
{
- set_curr_insn_location (e->goto_locus);
+ basic_block bb = e->dest;
+ gimple_stmt_iterator gsi;
+
+ do
+ {
+ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+ {
+ gimple *stmt = gsi_stmt (gsi);
+ if (is_gimple_debug (stmt))
+ continue;
+ if (gimple_has_location (stmt) || gimple_block (stmt))
+ {
+ set_curr_insn_location (gimple_location (stmt));
+ return;
+ }
+ }
+ /* Nothing found in this basic block. Make a half-assed attempt
+ to continue with another block. */
+ if (single_succ_p (bb))
+ bb = single_succ (bb);
+ else
+ bb = e->dest;
+ }
+ while (bb != e->dest);
}
else
{
@@ -564,7 +593,11 @@ eliminate_build (elim_graph *g)
continue;
Ti = PHI_ARG_DEF (phi, g->e->dest_idx);
- locus = gimple_phi_arg_location_from_edge (phi, g->e);
+ /* See set_location_for_edge for the rationale. */
+ if (g->e->flags & EDGE_EH)
+ locus = UNKNOWN_LOCATION;
+ else
+ locus = gimple_phi_arg_location_from_edge (phi, g->e);
/* If this argument is a constant, or a SSA_NAME which is being
left in SSA form, just queue a copy to be emitted on this
