diff mbox

[1/n] dwarf2out refactoring for early (LTO) debug

Message ID alpine.LSU.2.11.1508211542210.4884@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener Aug. 21, 2015, 1:44 p.m. UTC 
On Thu, 20 Aug 2015, Richard Biener wrote:

> On Thu, 20 Aug 2015, Richard Biener wrote:
> 
> > On Wed, 19 Aug 2015, Richard Biener wrote:
> > 
> > > On Tue, 18 Aug 2015, Aldy Hernandez wrote:
> > > 
> > > > On 08/18/2015 07:20 AM, Richard Biener wrote:
> > > > > 
> > > > > This starts a series of patches (still in development) to refactor
> > > > > dwarf2out.c to better cope with early debug (and LTO debug).
> > > > 
> > > > Awesome!  Thanks.
> > > > 
> > > > > Aldyh, what other testing did you usually do for changes?  Run
> > > > > the gdb testsuite against the new compiler?  Anything else?
> > > > 
> > > > gdb testsuite, and make sure you test GCC with --enable-languages=all,go,ada,
> > > > though the latter is mostly useful while you iron out bugs initially.  I found
> > > > that ultimately, the best test was C++.
> > > 
> > > I see.
> > > 
> > > > Pre merge I also bootstrapped the compiler and compared .debug* section sizes
> > > > in object files to make sure things were within reason.
> > > > 
> > > > > +
> > > > > +static void
> > > > > +vmsdbgout_early_finish (const char *filename ATTRIBUTE_UNUSED)
> > > > > +{
> > > > > +  if (write_symbols == VMS_AND_DWARF2_DEBUG)
> > > > > +    (*dwarf2_debug_hooks.early_finish) (filename);
> > > > > +}
> > > > 
> > > > You can get rid of ATTRIBUTE_UNUSED now.
> > > 
> > > Done.  I've also refrained from moving
> > > 
> > >   gen_scheduled_generic_parms_dies ();
> > >   gen_remaining_tmpl_value_param_die_attribute ();
> > > 
> > > for now as that causes regressions I have to investigate.
> > 
> > So I thought gen_scheduled_generic_parms_dies was fine but it exposes
> > a hole in
> > 
> >   /* Generate early debug for global variables.  Any local variables will
> >      be handled by either handling reachable functions from
> >      finalize_compilation_unit (and by consequence, locally scoped
> >      symbols), or by rest_of_type_compilation below.
> > ...
> >       && !decl_type_context (decl))
> >     (*debug_hooks->early_global_decl) (decl);
> > 
> > for __timepunct_cache::_S_timezones where through the
> > rest_of_type_compilation we quickly finish processing __timepunct_cache
> > as it is TYPE_DECL_SUPPRESS_DEBUG (so ->type_decl exits w/o doing
> > anything).  So we fail to generate a type DIE for the global which
> > causes us, at late_global_decl time (we do output this global var)
> > ends up doing
> > 
> > #12 0x0000000000b831f1 in gen_decl_die (decl=0x7ffff5650a20, origin=0x0, 
> >     context_die=0x7ffff62ab000)
> >     at /space/rguenther/src/svn/trunk/gcc/dwarf2out.c:21535
> > 21532         /* And its containing type.  */
> > 21533         class_origin = decl_class_context (decl_or_origin);
> > 21534         if (class_origin != NULL_TREE)
> > 21535           gen_type_die_for_member (class_origin, decl_or_origin, 
> > context_die);
> > 
> > and thus create the type DIE for __timepunct_cache late.
> > 
> > This is a hole in current early-debug.  IMHO we should force
> > early_global_decl even for globals in decl_type_context ()
> > or at least for a type DIE to be created for TYPE_DECL_SUPPRESS_DEBUG
> > type decls.
> > 
> > Jason, any advice on this?  I note
> > 
> > /* In a TYPE_DECL nonzero means the detail info about this type is not 
> > dumped
> >    into stabs.  Instead it will generate cross reference ('x') of names.
> >    This uses the same flag as DECL_EXTERNAL.  */
> > #define TYPE_DECL_SUPPRESS_DEBUG(NODE) \
> >   (TYPE_DECL_CHECK (NODE)->decl_common.decl_flag_1)
> > 
> > refering to STABS and "This uses the same flag as DECL_EXTERNAL" so
> > maybe this is just bad co-incidence?  DECL_EXTERNAL doesn't check
> > it operates on a non-TYPE_DECL.
> > 
> > So without knowing that the flag is supposed to be doing in DWARF
> > (also the desired effect on any of its static members) I can only
> > suggest we maybe do not want any debug info for 
> > __timepunct_cache::_S_timezones at all?
> 
> Sorry to followup myself all the time ;)  The following seems to
> "work" for me here:
> 
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c     (revision 226937)
> +++ gcc/dwarf2out.c     (working copy)
> @@ -21647,7 +21652,8 @@ dwarf2out_late_global_decl (tree decl)
>       Skip over functions because they were handled by the
>       debug_hooks->function_decl() call in rest_of_handle_final.  */
>    if ((TREE_CODE (decl) != FUNCTION_DECL || !DECL_INITIAL (decl))
> -      && !POINTER_BOUNDS_P (decl))
> +      && !POINTER_BOUNDS_P (decl)
> +      && lookup_decl_die (decl) != NULL)
>      dwarf2out_decl (decl);
>  }
>  
> which basically means only annotate gloabl decls late if we
> created a DIE for it early.
> 
> Kind-of makes sense.  Going to test that more extensively separately.

So the following passes bootstrap & test & gdb test on 
x86_64-unknown-linux-gnu.  But it breaks old-style LTO (obviously).

I'm at the moment trying to refactor dwarf2out_function_decl in a
similar way to avoid adding another different entry-kind to
gen_subprogram_die and friends (late LTO).

Richard.
diff mbox

Patch

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c     (revision 226966)
+++ gcc/dwarf2out.c     (working copy)
@@ -21641,14 +21641,16 @@  dwarf2out_early_global_decl (tree decl)
 static void
 dwarf2out_late_global_decl (tree decl)
 {
-  /* Output any global decls we missed or fill-in any location
-     information we were unable to determine on the first pass.
-
-     Skip over functions because they were handled by the
-     debug_hooks->function_decl() call in rest_of_handle_final.  */
-  if ((TREE_CODE (decl) != FUNCTION_DECL || !DECL_INITIAL (decl))
+  /* Fill-in any location information we were unable to determine
+     on the first pass.  */
+  if (TREE_CODE (decl) == VAR_DECL
       && !POINTER_BOUNDS_P (decl))
-    dwarf2out_decl (decl);
+    {
+      dw_die_ref die = lookup_decl_die (decl);
+      if (die)
+       add_location_or_const_value_attribute (die, decl, false,
+                                              DW_AT_location);
+    }
 }
 
 /* Output debug information for type decl DECL.  Called from toplev.c
Index: gcc/passes.c
===================================================================
--- gcc/passes.c        (revision 226966)
+++ gcc/passes.c        (working copy)
@@ -318,7 +318,15 @@  rest_of_decl_compilation (tree decl,
       && !decl_function_context (decl)
       && !current_function_decl
       && DECL_SOURCE_LOCATION (decl) != BUILTINS_LOCATION
-      && !decl_type_context (decl))
+      && (!decl_type_context (decl)
+         /* If we created a varpool node for the decl make sure to
+            call early_global_decl.  Otherwise we miss changes
+            introduced by member definitions like
+              struct A { static int staticdatamember; };
+              int A::staticdatamember;
+            and thus have incomplete early debug.  */
+         || (TREE_CODE (decl) == VAR_DECL
+             && TREE_STATIC (decl) && !DECL_EXTERNAL (decl))))
     (*debug_hooks->early_global_decl) (decl);
 }