Patchwork [Debug] Emit pubnames for reorder & partition case.

login
register
mail settings
Submitter IainS
Date Dec. 20, 2010, 11:18 a.m.
Message ID <56186808-25DF-4EB9-AED6-3EDF309FEAA6@sandoe-acoustics.co.uk>
Download mbox | patch
Permalink /patch/76185/
State New
Headers show

Comments

IainS - Dec. 20, 2010, 11:18 a.m.
At present, pubnames are not emitted when -freorder-blocks-and- 
partition is active.

This causes debug linking to fail on Darwin.

The following patch outputs code ranges for Dwarf >=3 and produces a  
work-around for Dwarf-2.

With this patch [and the one in http://gcc.gnu.org/ml/gcc-patches/2010-12/msg01553.html 
] we can re-enable -freorder-blocks-and-partition on Darwin and debug  
works (at least for the partition cases in the test-suite).

OK for trunk?
Iain

gcc:

	* dwarf2out.c (gen_subprogram_die):  Add pubnames with code ranges for
	DWARF >= 3.  Add pubnames for the primary section and a reduced DIE for
	the secondary code fragment when outputting for DWARF == 2.
Richard Henderson - Dec. 20, 2010, 9:15 p.m.
On 12/20/2010 03:18 AM, IainS wrote:
> +		  /* There is no real support in DW2 for this .. so we make
> +		     a work-around.  First, emit the pub name for the segment
> +		     containing the function label.  Then make and emit a
> +		     simplified subprogram DIE for the second segment with the
> +		     name post-fixed with _$hot$ or _$cold$.  We use the same
> +		     linkage name for the second die so that gdb will find both
> +		     sections when given "b foo".  */

This doesn't work when $ is not allowed, aka NO_DOLLAR_IN_LABEL.
Presumably we need this workaround since Darwin is strict dwarf2?


r~
IainS - Dec. 20, 2010, 9:20 p.m.
On 20 Dec 2010, at 21:15, Richard Henderson wrote:

> On 12/20/2010 03:18 AM, IainS wrote:
>> +		  /* There is no real support in DW2 for this .. so we make
>> +		     a work-around.  First, emit the pub name for the segment
>> +		     containing the function label.  Then make and emit a
>> +		     simplified subprogram DIE for the second segment with the
>> +		     name post-fixed with _$hot$ or _$cold$.  We use the same
>> +		     linkage name for the second die so that gdb will find both
>> +		     sections when given "b foo".  */
>
> This doesn't work when $ is not allowed, aka NO_DOLLAR_IN_LABEL.
> Presumably we need this workaround since Darwin is strict dwarf2?

Yes, we need the work-around ..
  but I'm not massively attached to $ (or even £ ;-))

is it OK apart from that?

cheers
Iain
Richard Henderson - Dec. 20, 2010, 9:38 p.m.
On 12/20/2010 01:20 PM, IainS wrote:
> 
> On 20 Dec 2010, at 21:15, Richard Henderson wrote:
> 
>> On 12/20/2010 03:18 AM, IainS wrote:
>>> +          /* There is no real support in DW2 for this .. so we make
>>> +             a work-around.  First, emit the pub name for the segment
>>> +             containing the function label.  Then make and emit a
>>> +             simplified subprogram DIE for the second segment with the
>>> +             name post-fixed with _$hot$ or _$cold$.  We use the same
>>> +             linkage name for the second die so that gdb will find both
>>> +             sections when given "b foo".  */
>>
>> This doesn't work when $ is not allowed, aka NO_DOLLAR_IN_LABEL.
>> Presumably we need this workaround since Darwin is strict dwarf2?
> 
> Yes, we need the work-around ..
>  but I'm not massively attached to $ (or even £ ;-))
> 
> is it OK apart from that?

Yeah, and I suppose you're after matching the symbols here with 
the symbols from that previous patch.

Ok with an extra bit akin to the use in clone_function_name:

#ifndef NO_DOT_IN_LABEL
  prefix[len] = '.';
#elif !defined NO_DOLLAR_IN_LABEL
  prefix[len] = '$';
#else
  prefix[len] = '_';
#endif

though of course you may swap the preferencing of [$.] to match
what you already have in darwin.c.


r~
IainS - Dec. 20, 2010, 9:49 p.m.
On 20 Dec 2010, at 21:38, Richard Henderson wrote:

> On 12/20/2010 01:20 PM, IainS wrote:
>>
>> On 20 Dec 2010, at 21:15, Richard Henderson wrote:
>>
>>> On 12/20/2010 03:18 AM, IainS wrote:
>>>> +          /* There is no real support in DW2 for this .. so we  
>>>> make
>>>> +             a work-around.  First, emit the pub name for the  
>>>> segment
>>>> +             containing the function label.  Then make and emit a
>>>> +             simplified subprogram DIE for the second segment  
>>>> with the
>>>> +             name post-fixed with _$hot$ or _$cold$.  We use the  
>>>> same
>>>> +             linkage name for the second die so that gdb will  
>>>> find both
>>>> +             sections when given "b foo".  */
>>>
>>> This doesn't work when $ is not allowed, aka NO_DOLLAR_IN_LABEL.
>>> Presumably we need this workaround since Darwin is strict dwarf2?
>>
>> Yes, we need the work-around ..

FWIW, the problem is that we support only a sub-set of DW3 - so if you  
try gcc -gdwarf-3 either the linker or dsymutil dies.
so, we're stuck with DW2/strict.

>> but I'm not massively attached to $ (or even £ ;-))
>>
>> is it OK apart from that?
>
> Yeah, and I suppose you're after matching the symbols here with
> the symbols from that previous patch.
>
> Ok with an extra bit akin to the use in clone_function_name:
>
> #ifndef NO_DOT_IN_LABEL
>  prefix[len] = '.';
> #elif !defined NO_DOLLAR_IN_LABEL
>  prefix[len] = '$';
> #else
>  prefix[len] = '_';
> #endif
>
> though of course you may swap the preferencing of [$.] to match
> what you already have in darwin.c.

what I'm after is
(a) avoiding a name clash.
(b) having a generated  tag which is clearly understandable to the  
person debugging ..

If I prefix (rather than the current post-fix) with  "__hot_" and  
"__cold_"  (or even __hot_partition_) that should be safe and not  
require the extra hoops?

(I"m happy to do what you've suggested above, of course, if that's the  
best solution).

thanks,
Iain
Richard Henderson - Dec. 20, 2010, 10:06 p.m.
On 12/20/2010 01:49 PM, IainS wrote:
> what I'm after is
> (a) avoiding a name clash.
> (b) having a generated  tag which is clearly understandable to the person debugging ..
> 
> If I prefix (rather than the current post-fix) with  "__hot_" and
> "__cold_"  (or even __hot_partition_) that should be safe and not
> require the extra hoops?

Yes, that should work.  I do question whether you want this symbol
to match up with the name generated in the "atom" patch though.


r~
IainS - Dec. 20, 2010, 10:43 p.m.
On 20 Dec 2010, at 22:06, Richard Henderson wrote:

> On 12/20/2010 01:49 PM, IainS wrote:
>> what I'm after is
>> (a) avoiding a name clash.
>> (b) having a generated  tag which is clearly understandable to the  
>> person debugging ..
>>
>> If I prefix (rather than the current post-fix) with  "__hot_" and
>> "__cold_"  (or even __hot_partition_) that should be safe and not
>> require the extra hoops?
>
> Yes, that should work.  I do question whether you want this symbol
> to match up with the name generated in the "atom" patch though.


Yes, I think that is a good idea - I will amend the other patch to  
match this one, assuming Mike also OKs it,
thanks, for your reviews,
Iain
IainS - Dec. 21, 2010, 12:45 p.m.
On 20 Dec 2010, at 22:43, IainS wrote:

>
> On 20 Dec 2010, at 22:06, Richard Henderson wrote:
>
>> On 12/20/2010 01:49 PM, IainS wrote:
>>> what I'm after is
>>> (a) avoiding a name clash.
>>> (b) having a generated  tag which is clearly understandable to the  
>>> person debugging ..
>>>
>>> If I prefix (rather than the current post-fix) with  "__hot_" and
>>> "__cold_"  (or even __hot_partition_) that should be safe and not
>>> require the extra hoops?
>>
>> Yes, that should work.  I do question whether you want this symbol
>> to match up with the name generated in the "atom" patch though.
>
>
> Yes, I think that is a good idea - I will amend the other patch to  
> match this one, assuming Mike also OKs it,
> thanks, for your reviews,

I've now amended the darwf2 patch thus:

		  /* There is no real support in DW2 for this .. so we make
		     a work-around.  First, emit the pub name for the segment
		     containing the function label.  Then make and emit a
		     simplified subprogram DIE for the second segment with the
		     name pre-fixed by __hot/cold_sect_of_.  We use the same
		     linkage name for the second die so that gdb will find both
		     sections when given "b foo".  */

<snip>
		  if (fde->in_std_section)
		    {
		      name = concat ("__cold_sect_of_", name, NULL);

etc.

and the config/darwin.c patch to match this.

FAOD, are the two patches now OK to apply, assuming Mike is happy with  
the Darwin aspects?

thanks
Iain
Richard Henderson - Dec. 21, 2010, 4:06 p.m.
On 12/21/2010 04:45 AM, IainS wrote:
>               name = concat ("__cold_sect_of_", name, NULL);

Yes, this is fine.


r~
Mike Stump - Dec. 23, 2010, 7:27 p.m.
On Dec 20, 2010, at 3:18 AM, IainS <developer@sandoe-acoustics.co.uk> wrote:
> At present, pubnames are not emitted when -freorder-blocks-and-partition is active.

Ok with the change to not use . or $ in the label.
>

Patch

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 168084)
+++ gcc/dwarf2out.c	(working copy)
@@ -18897,7 +18897,6 @@ 
 static void
 gen_subprogram_die (tree decl, dw_die_ref context_die)
 {
-  char label_id[MAX_ARTIFICIAL_LABEL_BYTES];
   tree origin = decl_ultimate_origin (decl);
   dw_die_ref subr_die;
   tree fn_arg_types;
@@ -19067,12 +19066,24 @@ 
 
       if (!flag_reorder_blocks_and_partition)
 	{
-	  ASM_GENERATE_INTERNAL_LABEL (label_id, FUNC_BEGIN_LABEL,
-				       current_function_funcdef_no);
-	  add_AT_lbl_id (subr_die, DW_AT_low_pc, label_id);
-	  ASM_GENERATE_INTERNAL_LABEL (label_id, FUNC_END_LABEL,
-				       current_function_funcdef_no);
-	  add_AT_lbl_id (subr_die, DW_AT_high_pc, label_id);
+	  dw_fde_ref fde = &fde_table[current_funcdef_fde];
+	  if (fde->dw_fde_begin)
+	    {
+	      /* We have already generated the labels.  */
+	      add_AT_lbl_id (subr_die, DW_AT_low_pc, fde->dw_fde_begin);
+	      add_AT_lbl_id (subr_die, DW_AT_high_pc, fde->dw_fde_end);
+	    }
+	  else
+	    {
+	      /* Create start/end labels and add the range.  */
+	      char label_id[MAX_ARTIFICIAL_LABEL_BYTES];
+	      ASM_GENERATE_INTERNAL_LABEL (label_id, FUNC_BEGIN_LABEL,
+					   current_function_funcdef_no);
+	      add_AT_lbl_id (subr_die, DW_AT_low_pc, label_id);
+	      ASM_GENERATE_INTERNAL_LABEL (label_id, FUNC_END_LABEL,
+					   current_function_funcdef_no);
+	      add_AT_lbl_id (subr_die, DW_AT_high_pc, label_id);
+	    }
 
 #if VMS_DEBUGGING_INFO
       /* HP OpenVMS Industry Standard 64: DWARF Extensions
@@ -19088,8 +19099,6 @@ 
 	 attributes allow a compiler to communicate the location(s) to use.  */
 
       {
-        dw_fde_ref fde = &fde_table[current_funcdef_fde];
-
         if (fde->dw_fde_vms_end_prologue)
           add_AT_vms_delta (subr_die, DW_AT_HP_prologue,
 	    fde->dw_fde_begin, fde->dw_fde_vms_end_prologue);
@@ -19104,19 +19113,116 @@ 
 	  add_arange (decl, subr_die);
 	}
       else
-	{  /* Do nothing for now; maybe need to duplicate die, one for
-	      hot section and one for cold section, then use the hot/cold
-	      section begin/end labels to generate the aranges...  */
-	  /*
-	    add_AT_lbl_id (subr_die, DW_AT_low_pc, hot_section_label);
-	    add_AT_lbl_id (subr_die, DW_AT_high_pc, hot_section_end_label);
-	    add_AT_lbl_id (subr_die, DW_AT_lo_user, unlikely_section_label);
-	    add_AT_lbl_id (subr_die, DW_AT_hi_user, cold_section_end_label);
+	{  /* Generate pubnames entries for the split function code
+	      ranges.  */
+	  dw_fde_ref fde = &fde_table[current_funcdef_fde];
 
-	    add_pubname (decl, subr_die);
-	    add_arange (decl, subr_die);
-	    add_arange (decl, subr_die);
-	   */
+	  if (fde->dw_fde_switched_sections)
+	    {
+	      if (dwarf_version >= 3 || !dwarf_strict)
+		{
+		  /* We should use ranges for non-contiguous code section 
+		     addresses.  Use the actual code range for the initial
+		     section, since the HOT/COLD labels might precede an 
+		     alignment offset.  */
+		  bool range_list_added = false;
+		  if (fde->in_std_section)
+		    {
+		      add_ranges_by_labels (subr_die,
+					    fde->dw_fde_begin,
+					    fde->dw_fde_end,
+					    &range_list_added);
+		      add_ranges_by_labels (subr_die,
+					    fde->dw_fde_unlikely_section_label,
+					    fde->dw_fde_unlikely_section_end_label,
+					    &range_list_added);
+		    }
+		  else
+		    {
+		      add_ranges_by_labels (subr_die,
+					    fde->dw_fde_begin,
+					    fde->dw_fde_end,
+					    &range_list_added);
+		      add_ranges_by_labels (subr_die,
+					    fde->dw_fde_hot_section_label,
+					    fde->dw_fde_hot_section_end_label,
+					    &range_list_added);
+		    }
+		  add_pubname (decl, subr_die);
+		  if (range_list_added)
+		    add_ranges (NULL);
+		}
+	      else
+		{
+		  /* There is no real support in DW2 for this .. so we make
+		     a work-around.  First, emit the pub name for the segment
+		     containing the function label.  Then make and emit a
+		     simplified subprogram DIE for the second segment with the
+		     name post-fixed with _$hot$ or _$cold$.  We use the same
+		     linkage name for the second die so that gdb will find both
+		     sections when given "b foo".  */
+		  const char *name = NULL;
+		  tree decl_name = DECL_NAME (decl);
+		  dw_die_ref seg_die;
+
+		  /* Do the 'primary' section.   */
+		  add_AT_lbl_id (subr_die, DW_AT_low_pc,
+				 fde->dw_fde_begin);
+		  add_AT_lbl_id (subr_die, DW_AT_high_pc,
+				 fde->dw_fde_end);
+		  /* Add it.   */
+		  add_pubname (decl, subr_die);
+		  add_arange (decl, subr_die);
+
+		  /* Build a minimal DIE for the secondary section.  */
+		  seg_die = new_die (DW_TAG_subprogram,
+				     subr_die->die_parent, decl);
+
+		  if (TREE_PUBLIC (decl))
+		    add_AT_flag (seg_die, DW_AT_external, 1);
+
+		  if (decl_name != NULL 
+		      && IDENTIFIER_POINTER (decl_name) != NULL)
+		    {
+		      name = dwarf2_name (decl, 1);
+		      if (! DECL_ARTIFICIAL (decl))
+			add_src_coords_attributes (seg_die, decl);
+
+		      add_linkage_name (seg_die, decl);
+		    }
+		  gcc_assert (name!=NULL);
+		  add_pure_or_virtual_attribute (seg_die, decl);
+		  if (DECL_ARTIFICIAL (decl))
+		    add_AT_flag (seg_die, DW_AT_artificial, 1);
+
+		  if (fde->in_std_section)
+		    {
+		      name = concat (name, "_$cold$", NULL); 
+		      add_AT_lbl_id (seg_die, DW_AT_low_pc,
+				     fde->dw_fde_unlikely_section_label);
+		      add_AT_lbl_id (seg_die, DW_AT_high_pc,
+				     fde->dw_fde_unlikely_section_end_label); 
+		    }
+		  else 
+		    {
+		      name = concat (name, "_$hot$", NULL); 
+		      add_AT_lbl_id (seg_die, DW_AT_low_pc,
+				     fde->dw_fde_hot_section_label);
+		      add_AT_lbl_id (seg_die, DW_AT_high_pc,
+				     fde->dw_fde_hot_section_end_label); 
+		    }
+		  add_name_attribute (seg_die, name);
+		  add_pubname_string (name, seg_die);
+		  add_arange (decl, seg_die);
+		}
+	    }
+	  else
+	    {
+	      add_AT_lbl_id (subr_die, DW_AT_low_pc, fde->dw_fde_begin);
+	      add_AT_lbl_id (subr_die, DW_AT_high_pc, fde->dw_fde_end);
+	      add_pubname (decl, subr_die);
+	      add_arange (decl, subr_die);
+	    }
 	}
 
 #ifdef MIPS_DEBUGGING_INFO