diff mbox

[PR,debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites

Message ID 55783717.8060505@adacore.com
State New
Headers show

Commit Message

Pierre-Marie de Rodat June 10, 2015, 1:09 p.m. UTC 
Thank you for your answer, Richard!

On 06/10/2015 08:58 AM, Richard Biener wrote:
> Hmm, so the underlying issue is that we don't associate comp_unit_die ()
> with any TRANSLATION_UNIT_DECL.

Indeed.

> For the LTO early debug work I did the following at the very
> beginning of dwarf2out_early_finish:
>
>    /* Pick the first TRANSLATION_UNIT_DECL we didn't create a DIE for
>       and equate it with our default CU DIE.  LTO output needs to be
>       able to lookup DIEs for translation unit decls.  */
>    unsigned i;
>    tree decl;
>    FOR_EACH_VEC_SAFE_ELT (all_translation_units, i, decl)
>      if (!lookup_decl_die (decl))
>        equate_decl_number_to_die (decl, comp_unit_die ());

If I understand correctly, this does not only "pick the first" (as the 
comment says) but do equate for all DIE-less units, right? Why isn't 
this hunk in mainline yet by the way?

> We create some DIEs for builtin types too early before frontends
> got a chance to build their TRANSLATION_UNIT_DECL, so comp_unit_die
> gets created before there is any TRANSLATION_UNIT_DECL.  Another
> approach would of course be that the Frontends register their main
> TRANSLATION_UNIT_DECL with dwarf2out via a debug hook.
>
> But - does the above work for you and fix the regression?  If so
> adding a debug hook would probably be the cleaner solution still.

Yes it does, thanks! However we need to fix this on the 4.9 branch as 
well and this patch would need reworking to be applied there (no debug 
early). So here's a patch that introduce a 
register_main_translation_unit debug hook: is this what you had in mind? 
This works for me on the 4.9 branch and on mainline as well, regtested 
on x86_64-linux.

Comments

Richard Biener June 10, 2015, 1:36 p.m. UTC | #1 
On Wed, 10 Jun 2015, Pierre-Marie de Rodat wrote:

> Thank you for your answer, Richard!
> 
> On 06/10/2015 08:58 AM, Richard Biener wrote:
> > Hmm, so the underlying issue is that we don't associate comp_unit_die ()
> > with any TRANSLATION_UNIT_DECL.
> 
> Indeed.
> 
> > For the LTO early debug work I did the following at the very
> > beginning of dwarf2out_early_finish:
> > 
> >    /* Pick the first TRANSLATION_UNIT_DECL we didn't create a DIE for
> >       and equate it with our default CU DIE.  LTO output needs to be
> >       able to lookup DIEs for translation unit decls.  */
> >    unsigned i;
> >    tree decl;
> >    FOR_EACH_VEC_SAFE_ELT (all_translation_units, i, decl)
> >      if (!lookup_decl_die (decl))
> >        equate_decl_number_to_die (decl, comp_unit_die ());
> 
> If I understand correctly, this does not only "pick the first" (as the comment
> says) but do equate for all DIE-less units, right? Why isn't this hunk in
> mainline yet by the way?

Hmm, yes.  It meant to break after the first ;)  (without LTO
there usually is only one TU decl, apart from Java I think).
The hunk isn't in mainline because it was part of an experimental patch I 
did on the early-debug branch.

> 
> > We create some DIEs for builtin types too early before frontends
> > got a chance to build their TRANSLATION_UNIT_DECL, so comp_unit_die
> > gets created before there is any TRANSLATION_UNIT_DECL.  Another
> > approach would of course be that the Frontends register their main
> > TRANSLATION_UNIT_DECL with dwarf2out via a debug hook.
> > 
> > But - does the above work for you and fix the regression?  If so
> > adding a debug hook would probably be the cleaner solution still.
> 
> Yes it does, thanks! However we need to fix this on the 4.9 branch as well and
> this patch would need reworking to be applied there (no debug early). So
> here's a patch that introduce a register_main_translation_unit debug hook: is
> this what you had in mind? This works for me on the 4.9 branch and on mainline
> as well, regtested on x86_64-linux.

Yeah, that looks great!

Of course I wonder about Java (builds multiple ones, one for each
input file) and Go (no idea).  I suppose Java would need to build
another one where all the "defaults" go (or it doesn't have any
such entities).

In theory we could have changed dwarf2out_init to get a
translation-unit-decl argument as well.  But your patch looks like
we don't have such at the point of dwarf2out_init in all frontends.

Your patch is ok (and ok to backport) IMHO, though please give
others the chance to chime in.

Thanks,
Richard.
Pierre-Marie de Rodat June 10, 2015, 2:30 p.m. UTC | #2 
On 06/10/2015 03:36 PM, Richard Biener wrote:
> Hmm, yes.  It meant to break after the first ;)  (without LTO
> there usually is only one TU decl, apart from Java I think).
> The hunk isn't in mainline because it was part of an experimental patch I
> did on the early-debug branch.

Understood, thanks!

> Yeah, that looks great!
>
> Of course I wonder about Java (builds multiple ones, one for each
> input file) and Go (no idea).  I suppose Java would need to build
> another one where all the "defaults" go (or it doesn't have any
> such entities).

Yeah, I'm not familiar with them neither...

> In theory we could have changed dwarf2out_init to get a
> translation-unit-decl argument as well.  But your patch looks like
> we don't have such at the point of dwarf2out_init in all frontends.
>
> Your patch is ok (and ok to backport) IMHO, though please give
> others the chance to chime in.

Thank you very much. :-) I will soon become unavailable until July, so I 
think I'll wait until then to commit anyway.
Richard Biener June 11, 2015, 7:47 a.m. UTC | #3 
On Wed, 10 Jun 2015, Pierre-Marie de Rodat wrote:

> On 06/10/2015 03:36 PM, Richard Biener wrote:
> > Hmm, yes.  It meant to break after the first ;)  (without LTO
> > there usually is only one TU decl, apart from Java I think).
> > The hunk isn't in mainline because it was part of an experimental patch I
> > did on the early-debug branch.
> 
> Understood, thanks!
> 
> > Yeah, that looks great!
> > 
> > Of course I wonder about Java (builds multiple ones, one for each
> > input file) and Go (no idea).  I suppose Java would need to build
> > another one where all the "defaults" go (or it doesn't have any
> > such entities).
> 
> Yeah, I'm not familiar with them neither...
> 
> > In theory we could have changed dwarf2out_init to get a
> > translation-unit-decl argument as well.  But your patch looks like
> > we don't have such at the point of dwarf2out_init in all frontends.
> > 
> > Your patch is ok (and ok to backport) IMHO, though please give
> > others the chance to chime in.
> 
> Thank you very much. :-) I will soon become unavailable until July, so I think
> I'll wait until then to commit anyway.

Ok - can you please open a bugreport for the regression on the 4.9
branch so we don't miss the fix for the next 4.9 release which should
be in about 3 weeks from now (in fact you might miss the RC deadline
for that release).

So I suggest you commit the patch to trunk this week so others can
take care of backporting.

Thanks,
Richard.
diff mbox

Patch

From 285ea98dfdf3d45f32fc0141aa182f07d172612d Mon Sep 17 00:00:00 2001
From: Pierre-Marie de Rodat <derodat@adacore.com>
Date: Wed, 10 Jun 2015 10:59:40 +0200
Subject: [PATCH] Restore DW_AT_abstract_origin for cross-unit call sites

gcc/ChangeLog:
	* debug.h (struct gcc_debug_hooks): Add a
	register_main_translation_unit hook.
	* debug.c (do_nothing_debug_hooks): Provide a function for this
	new hook.
	* dbxout.c (dbx_debug_hooks): Likewise.
	* sdbout.c (sdb_debug_hooks): Likewise.
	* vmsdbgout.c (vmsdbg_debug_hooks): Likewise.
	* dwarf2out.c (main_translation_unit): New global variable.
	(dwarf2out_register_main_translation_unit): New function
	implementing the new hook.
	(dwarf2_debug_hooks): Assign
	dwarf2out_register_main_translation_unit to this new hook.
	(dwarf2out_init): Associate any main translation unit to
	comp_unit_die ().
	* c/c-decl.c (pop_scope): Register the main translation unit
	through the new debug hook.
	* cp/decl.c (cxx_init_decl_processing): Likewise.

gcc/ada/ChangeLog:
	* gcc-interface/utils.c (get_global_context): Register the main
	translation unit through the new debug hook.

gcc/fortran/ChangeLog:
	* f95-lang.c (gfc_create_decls): Register the main translation
	unit through the new debug hook.
---
 gcc/ada/gcc-interface/utils.c |  5 ++++-
 gcc/c/c-decl.c                |  1 +
 gcc/cp/decl.c                 |  2 ++
 gcc/dbxout.c                  |  1 +
 gcc/debug.c                   |  1 +
 gcc/debug.h                   |  4 ++++
 gcc/dwarf2out.c               | 27 +++++++++++++++++++++++++++
 gcc/fortran/f95-lang.c        |  1 +
 gcc/sdbout.c                  |  1 +
 gcc/vmsdbgout.c               |  1 +
 10 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/gcc/ada/gcc-interface/utils.c b/gcc/ada/gcc-interface/utils.c
index 9076529..655bfa1 100644
--- a/gcc/ada/gcc-interface/utils.c
+++ b/gcc/ada/gcc-interface/utils.c
@@ -667,7 +667,10 @@  static tree
 get_global_context (void)
 {
   if (!global_context)
-    global_context = build_translation_unit_decl (NULL_TREE);
+    {
+      global_context = build_translation_unit_decl (NULL_TREE);
+      debug_hooks->register_main_translation_unit (global_context);
+    }
   return global_context;
 }
 
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 7fd662d..3fde22f 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -1196,6 +1196,7 @@  pop_scope (void)
     {
       tree file_decl = build_translation_unit_decl (NULL_TREE);
       context = file_decl;
+      debug_hooks->register_main_translation_unit (file_decl);
     }
   else
     context = block;
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 3bed538..ffd068a 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -3831,6 +3831,8 @@  cxx_init_decl_processing (void)
   global_namespace = build_lang_decl (NAMESPACE_DECL, global_scope_name,
 				      void_type_node);
   DECL_CONTEXT (global_namespace) = build_translation_unit_decl (NULL_TREE);
+  debug_hooks->register_main_translation_unit
+    (DECL_CONTEXT (global_namespace));
   TREE_PUBLIC (global_namespace) = 1;
   begin_scope (sk_namespace, global_namespace);
 
diff --git a/gcc/dbxout.c b/gcc/dbxout.c
index 48b5065..94fac42 100644
--- a/gcc/dbxout.c
+++ b/gcc/dbxout.c
@@ -380,6 +380,7 @@  const struct gcc_debug_hooks dbx_debug_hooks =
   debug_nothing_tree,		         /* begin_function */
 #endif
   debug_nothing_int,		         /* end_function */
+  debug_nothing_tree,			 /* register_main_translation_unit */
   dbxout_function_decl,
   dbxout_early_global_decl,		 /* early_global_decl */
   dbxout_late_global_decl,		 /* late_global_decl */
diff --git a/gcc/debug.c b/gcc/debug.c
index 9c621f8..ab92cc8 100644
--- a/gcc/debug.c
+++ b/gcc/debug.c
@@ -46,6 +46,7 @@  const struct gcc_debug_hooks do_nothing_debug_hooks =
   debug_nothing_int_charstar,	         /* end_epilogue */
   debug_nothing_tree,		         /* begin_function */
   debug_nothing_int,		         /* end_function */
+  debug_nothing_tree,		         /* register_main_translation_unit */
   debug_nothing_tree,		         /* function_decl */
   debug_nothing_tree,	         	 /* early_global_decl */
   debug_nothing_tree,	         	 /* late_global_decl */
diff --git a/gcc/debug.h b/gcc/debug.h
index e7e1334..269c4d8 100644
--- a/gcc/debug.h
+++ b/gcc/debug.h
@@ -89,6 +89,10 @@  struct gcc_debug_hooks
   /* Record end of function.  LINE is highest line number in function.  */
   void (* end_function) (unsigned int line);
 
+  /* Register UNIT as the main translation unit.  Called from front-ends when
+     they create their main translation unit.  */
+  void (* register_main_translation_unit) (tree);
+
   /* Debug information for a function DECL.  This might include the
      function name (a symbol), its parameters, and the block that
      makes up the function's body, and the local variables of the
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index ee2bcb1..8a36fe8 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -2446,6 +2446,7 @@  static void dwarf2out_abstract_function (tree);
 static void dwarf2out_var_location (rtx_insn *);
 static void dwarf2out_begin_function (tree);
 static void dwarf2out_end_function (unsigned int);
+static void dwarf2out_register_main_translation_unit (tree unit);
 static void dwarf2out_set_name (tree, tree);
 
 /* The debug hooks structure.  */
@@ -2475,6 +2476,7 @@  const struct gcc_debug_hooks dwarf2_debug_hooks =
   dwarf2out_end_epilogue,
   dwarf2out_begin_function,
   dwarf2out_end_function,	/* end_function */
+  dwarf2out_register_main_translation_unit,
   dwarf2out_function_decl,	/* function_decl */
   dwarf2out_early_global_decl,
   dwarf2out_late_global_decl,
@@ -22505,6 +22507,26 @@  dwarf2out_end_function (unsigned int)
   maybe_at_text_label_p = false;
 }
 
+/* Temporary holder for dwarf2out_register_main_translation_unit.  Used to let
+   front-ends register a translation unit even before dwarf2out_init is
+   called.  */
+static tree main_translation_unit = NULL_TREE;
+
+/* Hook called by front-ends after they built their main translation unit.
+   Associate comp_unit_die to UNIT.  */
+
+static void
+dwarf2out_register_main_translation_unit (tree unit)
+{
+  gcc_assert (TREE_CODE (unit) == TRANSLATION_UNIT_DECL
+	      && main_translation_unit == NULL_TREE);
+  main_translation_unit = unit;
+  /* If dwarf2out_init has not been called yet, it will perform the association
+     itself looking at main_translation_unit.  */
+  if (decl_die_table != NULL)
+    equate_decl_number_to_die (unit, comp_unit_die ());
+}
+
 /* Add OPCODE+VAL as an entry at the end of the opcode array in TABLE.  */
 
 static void
@@ -23242,6 +23264,11 @@  dwarf2out_init (const char *filename ATTRIBUTE_UNUSED)
   /* Make sure the line number table for .text always exists.  */
   text_section_line_info = new_line_info_table ();
   text_section_line_info->end_label = text_end_label;
+
+  /* If front-ends already registered a main translation unit but we were not
+     ready to perform the association, do this now.  */
+  if (main_translation_unit != NULL_TREE)
+    equate_decl_number_to_die (main_translation_unit, comp_unit_die ());
 }
 
 /* Called before compile () starts outputtting functions, variables
diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c
index 6daac83..2c055f5 100644
--- a/gcc/fortran/f95-lang.c
+++ b/gcc/fortran/f95-lang.c
@@ -204,6 +204,7 @@  gfc_create_decls (void)
 
   /* Build our translation-unit decl.  */
   current_translation_unit = build_translation_unit_decl (NULL_TREE);
+  debug_hooks->register_main_translation_unit (current_translation_unit);
 }
 
 
diff --git a/gcc/sdbout.c b/gcc/sdbout.c
index f5671c6..033886a 100644
--- a/gcc/sdbout.c
+++ b/gcc/sdbout.c
@@ -296,6 +296,7 @@  const struct gcc_debug_hooks sdb_debug_hooks =
   sdbout_end_epilogue,		         /* end_epilogue */
   sdbout_begin_function,	         /* begin_function */
   sdbout_end_function,		         /* end_function */
+  debug_nothing_tree,		         /* register_main_translation_unit */
   debug_nothing_tree,		         /* function_decl */
   sdbout_early_global_decl,		 /* early_global_decl */
   sdbout_late_global_decl,		 /* late_global_decl */
diff --git a/gcc/vmsdbgout.c b/gcc/vmsdbgout.c
index 8297e02..8c917e0 100644
--- a/gcc/vmsdbgout.c
+++ b/gcc/vmsdbgout.c
@@ -194,6 +194,7 @@  const struct gcc_debug_hooks vmsdbg_debug_hooks
    vmsdbgout_end_epilogue,
    vmsdbgout_begin_function,
    vmsdbgout_end_function,
+   debug_nothing_tree,		  /* register_main_translation_unit */
    vmsdbgout_function_decl,
    vmsdbgout_early_global_decl,
    vmsdbgout_late_global_decl,
-- 
2.3.3.199.g52cae64