diff mbox

[Debug] Emit pubnames for reorder & partition case.

Message ID 56186808-25DF-4EB9-AED6-3EDF309FEAA6@sandoe-acoustics.co.uk
State New
Headers show

Commit Message

Iain Sandoe Dec. 20, 2010, 11:18 a.m. UTC
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.

Comments

Richard Henderson Dec. 20, 2010, 9:15 p.m. UTC | #1
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~
Iain Sandoe Dec. 20, 2010, 9:20 p.m. UTC | #2
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. UTC | #3
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~
Iain Sandoe Dec. 20, 2010, 9:49 p.m. UTC | #4
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. UTC | #5
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~
Iain Sandoe Dec. 20, 2010, 10:43 p.m. UTC | #6
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
Iain Sandoe Dec. 21, 2010, 12:45 p.m. UTC | #7
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. UTC | #8
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. UTC | #9
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.
>
diff mbox

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