[PATCH/RFC] PR other/22313: Hot/cold sections vs. dwarf2 (take 2)

[Roger Sayle <roger at eyesopen dot com>]

This is a follow-up and improvement to my previous patch for this PR:
http://gcc.gnu.org/ml/gcc-patches/2006-07/msg01015.html

PR other/22313 is the oldest P1 regression affecting mainline.

As explained above hot/cold function sections interact badly with dwarf2
debugging information, as unwind tables are currently encoded using
DW_CFA_advance_loc4 which results in us emitting assembly language that
takes the difference of labels in different sections.  This breaks
profiledbootstrap on i486 on the 4.1 branch, but is latent everywhere
and on mainline.

My earlier posted patch solved this problem using a fairly blunt hammer
of using DW_CFA_set_loc everywhere if we've switched text sections.

A more elegant solution is to use "set_loc" whenever we we advance the
label over a section boundary, but continue use advance_loc4 within a
single section.  As mentioned in my previous e-mail, I tried implementing
this but without luck, as it appeared "the switch section calls are not
interleaved with add_fde_cfi".

Following Honza's comment about NOTE_INSN_SWITCH_SECTIONS in the bugzilla
PR, I decided to figure out how switch section calls are handled, and why
my previous attempt had failed.  Turns out that final.c's final_scan_insn
is checking for NOTE_INSN_SWITCH_TEXT_SECTIONS notes and then calling
debug_hooks->switch_text_section(), so we should be seeing the section
changes interleaved with add_fde_cfi.

After a little head scratching I tracked the problem down to a missing
line in dwarf2out.c's output_cfi.  The implementations of all of the
DW_CFA_advance_loc* cases, emit the delta from the current label, and
then update the current label.  But by a curious ommission, the case
handling the "set_loc" code doesn't update the current label.  This
means in a CFI, GCC is currently limited to always using advance_loc*
or always using set_loc, and never mixing the two.

The obvious functionality fix/correction is to update the current label
upon emitting a set_loc.  This allows use to interleave set_loc and
advance_loc*, which in turn provides us a nice solution to the hot/cold
section spanning issue.  i.e. this one-line change resolves the problem
with my initial solution to PR other/22313.


The following patch has been tested on i686-pc-linux-gnu, with a full
"make bootstrap", all default languages including Ada, on both mainline
and (with a minor tweak) the gcc-4_1-branch, with no new failures from
a top-level "make -k check".  It's also been tested with a full "make
profiledbootstrap" targetting i486-pc-linux-gnu on the 4.1 branch
where it resolves the profiledbootstrap failure.

I propose to apply this patch to mainline [and the 4.1 branch] in a
few days, unless one of our debugging guru's can point out a flaw in
my interpretation of dwarf-2 specification that seems to imply that
advance_loc* can advance from either the previous advance_loc* or
set_loc.  [I'm also curious whether binutils/gcc can/should perform
advance_loc relaxation.  Currently we always emit advance_loc4, but
for small deltas we could reduce the size of debugging information
by using advance_loc1 and/or advance_loc2].

Thoughts?  Ok for mainline and the 4.1 branch?



2006-08-31  Roger Sayle  <roger@eyesopen.com>

	PR other/22313
	* dwarf2out.c (add_fde_cfi): Use a set_loc if the current label is
	NULL, otherwise use an advance_loc4 to adjust relative to the
	current label.
	(output_cfi) <DW_CFA_set_loc>: Update the current label.
	(dwarf2out_switch_text_section): Reset the current label to avoid
	using advance_loc4 over section boundaries.


Index: dwarf2out.c
===================================================================
*** dwarf2out.c	(revision 116501)
--- dwarf2out.c	(working copy)
*************** add_fde_cfi (const char *label, dw_cfi_r
*** 628,640 ****
  	{
  	  dw_cfi_ref xcfi;

! 	  fde->dw_fde_current_label = label = xstrdup (label);

  	  /* Set the location counter to the new label.  */
  	  xcfi = new_cfi ();
! 	  xcfi->dw_cfi_opc = DW_CFA_advance_loc4;
  	  xcfi->dw_cfi_oprnd1.dw_cfi_addr = label;
  	  add_cfi (&fde->dw_fde_cfi, xcfi);
  	}

        add_cfi (&fde->dw_fde_cfi, cfi);
--- 628,646 ----
  	{
  	  dw_cfi_ref xcfi;

! 	  label = xstrdup (label);

  	  /* Set the location counter to the new label.  */
  	  xcfi = new_cfi ();
! 	  /* If we have a current label, advance from there, otherwise
! 	     set the location directly using set_loc.  */
! 	  xcfi->dw_cfi_opc = fde->dw_fde_current_label
! 			     ? DW_CFA_advance_loc4
! 			     : DW_CFA_set_loc;
  	  xcfi->dw_cfi_oprnd1.dw_cfi_addr = label;
  	  add_cfi (&fde->dw_fde_cfi, xcfi);
+
+ 	  fde->dw_fde_current_label = label;
  	}

        add_cfi (&fde->dw_fde_cfi, cfi);
*************** output_cfi (dw_cfi_ref cfi, dw_fde_ref f
*** 2069,2074 ****
--- 2075,2081 ----
  	  else
  	    dw2_asm_output_addr (DWARF2_ADDR_SIZE,
  				 cfi->dw_cfi_oprnd1.dw_cfi_addr, NULL);
+ 	  fde->dw_fde_current_label = cfi->dw_cfi_oprnd1.dw_cfi_addr;
  	  break;

  	case DW_CFA_advance_loc1:
*************** dwarf2out_switch_text_section (void)
*** 6919,6924 ****
--- 6926,6935 ----
    fde->dw_fde_unlikely_section_label = cfun->cold_section_label;
    fde->dw_fde_unlikely_section_end_label = cfun->cold_section_end_label;
    have_multiple_function_sections = true;
+
+   /* Reset the current label on switching text sections, so that we
+      don't attempt to advance_loc4 between labels in different sections.  */
+   fde->dw_fde_current_label = NULL;
  }

  /* Output the location list given to us.  */


Roger
--