[debug-early] emit early dwarf for locally scoped functions

[debug-early] emit early dwarf for locally scoped functions

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 1366 bytes --]

Hi Jason.

I found that for locally scoped functions we were not emitting early 
dwarf.  I've removed the restriction that only emitted non 
function-context functions to handle the case below.  BTW, this 
shouldn't be a (bloat) problem, as we are going to clean up unused DIEs 
later (well, next week :)).

void foobar ()
{
   class Object {
   public:
     char Object_method ()
     {
       return 5;
     }
   };

   Object local;
   local.Object_method();
}

There was also a GDB regression with the above test (distilled from 
gdb.cp/local.cc) where Object_method's type was being generated as

	char Object_method (const Object *)

(or something like it).  The problem was that gen_formal_types_die was 
creating nameless DIEs for the formal parameters when generating an 
object's members, but mainline was removing these nameless DIEs and I 
had mistakenly removed that bit.  I'm putting the code back in, but 
guarding it with early_dwarf_dumping, since by the time we get to late 
debug, we should have the correct named parameters which should then be 
augmented with location information.

This patch fixes the gdb.cp/local.cc regressions, while generating early 
dwarf for Object_method and associates.

I'm committing to the branch.  Let me know if you have a problem with this.

Tested with the guality.exp suite as well as the GDB testsuite.

Aldy

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 1731 bytes --]

commit 8673cbf8204fcd7099507293a859b173343a0f9a
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Tue Mar 24 10:47:30 2015 -0700

    Emit early dwarf for locally scoped functions.
    
    Only remove cached DW_TAG_formal_parameter's when early dwarf dumping.

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index e60acd5..4a7b14d 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -2444,8 +2444,7 @@ symbol_table::finalize_compilation_unit (void)
      locally scoped symbols.  */
   struct cgraph_node *cnode;
   FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (cnode)
-    if (!decl_function_context (cnode->decl))
-      (*debug_hooks->early_global_decl) (cnode->decl);
+    (*debug_hooks->early_global_decl) (cnode->decl);
 
   /* Clean up anything that needs cleaning up after initial debug
      generation.  */
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 48e2eed..bcc1111 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -18792,6 +18792,18 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
 	     parameters so they can be augmented with location
 	     information later.  */
 	  remove_AT (subr_die, DW_AT_declaration);
+
+	  /* gen_formal_types_die could have created nameless DIEs for
+	     the formal parameters when generating an object's
+	     members.  Remove if early dumping; they will be shortly
+	     recreated correctly.  If we're not early dumping, we
+	     should've already removed them and should have actual
+	     named parameters.  */
+	  if (early_dwarf_dumping)
+	    {
+	      remove_AT (subr_die, DW_AT_object_pointer);
+	      remove_child_TAG (subr_die, DW_TAG_formal_parameter);
+	    }
 	}
       /* Make a specification pointing to the previously built
 	 declaration.  */

Re: [debug-early] emit early dwarf for locally scoped functions

[Jason Merrill]

On 03/24/2015 02:00 PM, Aldy Hernandez wrote:
> I found that for locally scoped functions we were not emitting early
> dwarf.

Why weren't they being emitted as part of their enclosing function? 
They should be.

Jason

Re: [debug-early] emit early dwarf for locally scoped functions

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 1525 bytes --]

On 03/25/2015 12:37 PM, Jason Merrill wrote:
> On 03/24/2015 02:00 PM, Aldy Hernandez wrote:
>> I found that for locally scoped functions we were not emitting early
>> dwarf.
>
> Why weren't they being emitted as part of their enclosing function? They
> should be.
>
> Jason
>

Hmm, you're right.  Sorry for being so sloppy.

What is actually happening is that when the declaration is seen, 
nameless DIEs for the types are generated, which are then used when the 
cached subprogram DIE is seen the second time.  The nameless DIEs end up 
looking like this because we don't have the "this" name:

     char Object_method(Object * const);

whereas the function type should be:

     char Object_method(void);

I now understand what this was doing in mainline:

	  /* Clear out the declaration attribute and the formal parameters.
	     Do not remove all children, because it is possible that this
	     declaration die was forced using force_decl_die(). In such
	     cases die that forced declaration die (e.g. TAG_imported_module)
	     is one of the children that we do not want to remove.  */
	  remove_AT (subr_die, DW_AT_declaration);
	  remove_AT (subr_die, DW_AT_object_pointer);
	  remove_child_TAG (subr_die, DW_TAG_formal_parameter);

I suppose we could re-use the DW_AT_object_pointer and 
DW_TAG_formal_parameter, and tack on the DW_AT_name now that we know it? 
  Or we could cheat and just remove them as mainline does, but only when 
reusing a declaration (as in the attached patch).

What do you think?

Aldy

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 2133 bytes --]

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 48e2eed..4bc945f 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -3113,7 +3113,7 @@ static inline dw_die_ref get_AT_ref (dw_die_ref, enum dwarf_attribute);
 static bool is_cxx (void);
 static bool is_fortran (void);
 static bool is_ada (void);
-static void remove_AT (dw_die_ref, enum dwarf_attribute);
+static bool remove_AT (dw_die_ref, enum dwarf_attribute);
 static void remove_child_TAG (dw_die_ref, enum dwarf_tag);
 static void add_child_die (dw_die_ref, dw_die_ref);
 static dw_die_ref new_die (enum dwarf_tag, dw_die_ref, tree);
@@ -4752,16 +4752,17 @@ is_ada (void)
   return lang == DW_LANG_Ada95 || lang == DW_LANG_Ada83;
 }
 
-/* Remove the specified attribute if present.  */
+/* Remove the specified attribute if present.  Return TRUE if removal
+   was successful.  */
 
-static void
+static bool
 remove_AT (dw_die_ref die, enum dwarf_attribute attr_kind)
 {
   dw_attr_ref a;
   unsigned ix;
 
   if (! die)
-    return;
+    return false;
 
   FOR_EACH_VEC_SAFE_ELT (die->die_attr, ix, a)
     if (a->dw_attr == attr_kind)
@@ -4773,8 +4774,9 @@ remove_AT (dw_die_ref die, enum dwarf_attribute attr_kind)
 	/* vec::ordered_remove should help reduce the number of abbrevs
 	   that are needed.  */
 	die->die_attr->ordered_remove (ix);
-	return;
+	return true;
       }
+  return false;
 }
 
 /* Remove CHILD from its parent.  PREV must have the property that
@@ -18790,8 +18792,15 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
 
 	  /* Clear out the declaration attribute, but leave the
 	     parameters so they can be augmented with location
-	     information later.  */
-	  remove_AT (subr_die, DW_AT_declaration);
+	     information later.  Unless this was a declaration, in
+	     which case, wipe out the nameless parameters and recreate
+	     them further down.  */
+	  if (remove_AT (subr_die, DW_AT_declaration))
+	    {
+
+	      remove_AT (subr_die, DW_AT_object_pointer);
+	      remove_child_TAG (subr_die, DW_TAG_formal_parameter);
+	    }
 	}
       /* Make a specification pointing to the previously built
 	 declaration.  */

Re: [debug-early] emit early dwarf for locally scoped functions

[Jason Merrill]

On 03/25/2015 05:05 PM, Aldy Hernandez wrote:
>   Or we could cheat and just remove them as mainline does, but only when
> reusing a declaration (as in the attached patch).

This seems right to me.

Jason

Re: [debug-early] emit early dwarf for locally scoped functions

[Aldy Hernandez]

On 03/25/2015 07:07 PM, Jason Merrill wrote:
> On 03/25/2015 05:05 PM, Aldy Hernandez wrote:
>>   Or we could cheat and just remove them as mainline does, but only when
>> reusing a declaration (as in the attached patch).
>
> This seems right to me.
>
> Jason
>
>

Ok thanks, committed.

Aldy