This fixes PR83157 (well, not the guality fail...) and avoids creating
references to abstract instances that actually end up refering to
the concrete instance thereby eventually picking up invalid location
attributes (and whatever else there may be).
As said in the PR a simple testcase
int foo (int i)
{
volatile int boring = 1;
return boring + i;
}
int bar()
{
int tem = 1;
return tem + foo (0);
}
shows the inline instance of foo in bar refering to the concrete
instance of 'boring' rather than the abstract instance. Analysis
of the guality fail referenced in the PR shows such a case where
gdb picks up a location attribute from the wrong place (the inline
instance doesn't have one).
The issue here is that dwarf2out_function_decl (late annotation
of the function) creates new concrete DIEs refering back to
the abstract DIEs but also registers those concrete DIEs with
equate_decl_number_to_die. This causes late annotation of
bar to pick those as abstract instance when creating inline
instance DIEs.
Somehow this doesn't happen with GCC 5, not sure why.
The fix is to not put concrete instances into the decl -> DIE
mapping over the dwarf2out_function_decl call but unwind any
changes that replaced DIEs (just occured to me we could assert
that the replacement DIEs always have an abstract origin of the
DIE replaced -- if that works, possibly not for multiple instances
of recursive inlining which may have its own share of similar
issues...).
A different fix might be to do sth like getting the ultimate
origin when adding an abstract origin (following DW_AT_abstract_origin
links on the destination), but GCC 5 didn't do that and I'm not
sure if that's the desired behavior anyway (it also might have
interesting side-effects for early LTO debug, not sure).
Bootstrapped and tested on x86_64-unknown-linux-gnu.
Not really asking for an OK (well, maybe yes?) but for further
ideas.
PR83157 will still need further investigation as the guality test
still fails after the patch.
No testcase, I haven't been able to produce a simple one that
gets wrong location attributes inherited and thus might be
turned into a guality test. I also don't have any good idea
how to scan-assembler the dwarf for the correct vs. the invalid
DW_AT_abstract_origin reference... I suppose the python dwarf
parsing testsuite thing would make this feasible somehow.
Thanks,
Richard.
2018-01-12 Richard Biener <rguent...@suse.de>
PR debug/83157
* dwarf2out.c (decl_die_table_undo_stack): New static vector.
(equate_decl_number_to_die): If there's an undo stack record
previous DIE for the decl.
(dwarf2out_function_decl): Initialize decl_die_table_undo_stack
and unwind changes before returning.
Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c (revision 256535)
+++ gcc/dwarf2out.c (working copy)
@@ -3159,6 +3159,8 @@ struct decl_die_hasher : ggc_ptr_hash<di
/* A hash table of references to DIE's that describe declarations.
The key is a DECL_UID() which is a unique number identifying each decl. */
static GTY (()) hash_table<decl_die_hasher> *decl_die_table;
+/* A stack to unwind changes to decl_die_table. */
+static vec<std::pair<tree, dw_die_ref> > *decl_die_table_undo_stack;
struct GTY ((for_user)) variable_value_struct {
unsigned int decl_id;
@@ -5762,7 +5764,10 @@ equate_decl_number_to_die (tree decl, dw
{
unsigned int decl_id = DECL_UID (decl);
- *decl_die_table->find_slot_with_hash (decl, decl_id, INSERT) = decl_die;
+ dw_die_ref *slot = decl_die_table->find_slot_with_hash (decl, decl_id,
INSERT);
+ if (*slot && decl_die_table_undo_stack)
+ decl_die_table_undo_stack->safe_push (std::make_pair (decl, *slot));
+ *slot = decl_die;
decl_die->decl_id = decl_id;
}
@@ -26044,6 +26049,10 @@ dwarf2out_decl (tree decl)
static void
dwarf2out_function_decl (tree decl)
{
+ vec<std::pair<tree, dw_die_ref> > *undo_stack
+ = new vec<std::pair<tree, dw_die_ref> > ();
+ decl_die_table_undo_stack = undo_stack;
+
dwarf2out_decl (decl);
call_arg_locations = NULL;
call_arg_loc_last = NULL;
@@ -26051,6 +26060,17 @@ dwarf2out_function_decl (tree decl)
tail_call_site_count = -1;
decl_loc_table->empty ();
cached_dw_loc_list_table->empty ();
+
+ /* Unwind changes to the decl -> DIE mapping so we keep the abstract
+ instances associated with the decl. */
+ decl_die_table_undo_stack = NULL;
+ while (! undo_stack->is_empty ())
+ {
+ std::pair<tree, dw_die_ref> op = undo_stack->pop ();
+ equate_decl_number_to_die (op.first, op.second);
+ }
+
+ delete undo_stack;
}
/* Output a marker (i.e. a label) for the beginning of the generated code for
