Fix line number data for PIC register setup code

[Tom de Vries <Tom_deVries at mentor dot com>]

Richard,

( see also related discussion
http://gcc.gnu.org/ml/gcc-patches/2010-12/msg01570.html )

Consider break.c (minimized from gdb/testsuite/gdb.base/break.c):
...
void *v;
void a (void *x) { }
void b (void) { }

int
main (int argc)
{
  if (argc == 12345)
    {
      a (v);
      return 1;
    }
  b ();

  return 0;
}
...

We compile like this with -fPIC:
...
$ arm-none-linux-gnueabi-gcc break.c -g -fPIC
...

and run to a breakpoint in main:
...
(gdb) b main
Breakpoint 1 at 0x8410: file break.c, line 7.
(gdb) c
Continuing.

Breakpoint 1, main (argc=1) at break.c:7
7	{
...

When we compile with -fno-PIC, we break at another line:
...
(gdb) b main
Breakpoint 1 at 0x83f8: file break.c, line 8.
(gdb) continue
Continuing.

Breakpoint 1, main (argc=1) at break.c:8
8	  if (argc == 12345)
...

AFAIU, the correct line number is 8, so the case with -fPIC needs to be fixed.


The assembly looks like this:
...
main:
.LFB0:
	.file 1 "break.c"
	.loc 1 88 0
	.cfi_startproc
	@ args = 0, pretend = 0, frame = 8
	@ frame_needed = 1, uses_anonymous_args = 0
	stmfd	sp!, {fp, lr}
	.cfi_def_cfa_offset 8
	.cfi_offset 11, -8
	.cfi_offset 14, -4
	add	fp, sp, #4
	.cfi_def_cfa 11, 4
	sub	sp, sp, #8
	str	r0, [fp, #-8]
	.loc 1 88 0
	ldr	r2, .L4
.LPIC0:
	add	r2, pc, r2
	.loc 1 89 0
	ldr	r1, [fp, #-8]
	ldr	r3, .L4+4
	cmp	r1, r3
	bne	.L2
	.loc 1 91 0
...

>From the point of view of the debugger, in the presence of .loc info, the
prologue is the code in between the 2 first .loc markers. See this comment in
gdb/arm-tdep.c:arm_skip_prologue:
...
      /* GCC always emits a line note before the prologue and another
	 one after, even if the two are at the same address or on the
	 same line.  Take advantage of this so that we do not need to
	 know every instruction that might appear in the prologue.
...

Manually removing the second '.loc 1 88 0' (in the pic register setup generated
by require_pic_register) from the assembly gives us the required behaviour.

An easy way to achieve this in the compiler is this patch:
...
Index: config/arm/arm.c
===================================================================
--- config/arm/arm.c	(revision 202646)
+++ config/arm/arm.c	(working copy)
@@ -5542,9 +5542,12 @@ require_pic_register (void)
 	      seq = get_insns ();
 	      end_sequence ();

 	      for (insn = seq; insn; insn = NEXT_INSN (insn))
 		if (INSN_P (insn))
-		  INSN_LOCATION (insn) = prologue_location;
+		  INSN_LOCATION (insn) = UNKNOWN_LOCATION;

 	      /* We can be called during expansion of PHI nodes, where
 	         we can't yet emit instructions directly in the final
...

However, it looks to me somewhat contradictory that when we mark this code as
UNKNOWN_LOCATION gdb recognized it as part of the prologue, and when we mark it
with prologue_location, gdb doesn't recognize it as part of the prologue.


Looking in more detail why we're emitting the .loc, it's because the pic
register setup follows the FUNCTION_BEG insn-note, which forces a .loc on the
next insn using the final.c:force_source_line variable.

According to this comment in dwarf2out_source_line:
...
     ... NOTE_INSN_FUNCTION_BEG, i.e. the first
     insn that corresponds to something the user wrote.
...
NOTE_INSN_FUNCTION_BEG marks the beginning of user code. To me it seems that we
have to make a choice here: Either the pic register setup code generated by
require_pic_register is user code, or it is prologue code.

If it's prologue code, we need to emit it before the FUNCTION_BEG insn-note
(rough proof-of-concept patch attached), such that no .loc will be generated for
it. Gdb will recognize it as part of the prologue, and the breakpoint will have
the correct line number.

It it's user code, we need to mark it with the first user line, such that a .loc
with the first user line will be generated. Gdb then won't count it as part of
the prologue, and the breakpoint will have the correct line number.


My preference would be to mark it as prologue code, since that's the case for
other uses of arm_load_pic_register.

What is the proper way to fix this?

Thanks,
- Tom

2013-09-15  Tom de Vries  <tom@codesourcery.com>

	gcc/
	* cfgexpand.c (gimple_expand_cfg): Emit insns_before_parm_birth_insn.
	* function.h (struct rtl_data): Add x_insns_before_parm_birth_insn
	field.
	(insns_before_parm_birth_insn): Define new macro.
	* config/arm/arm.c (require_pic_register): Use
	insns_before_parm_birth_insn.

Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c (revision 418548)
+++ gcc/cfgexpand.c (working copy)
@@ -4609,6 +4609,10 @@ gimple_expand_cfg (void)
      split edges which edge insertions might do.  */
   rebuild_jump_labels (get_insns ());
 
+  if (insns_before_parm_birth_insn && single_succ_p (ENTRY_BLOCK_PTR))
+    emit_insn_before_noloc (insns_before_parm_birth_insn, parm_birth_insn,
+			    single_succ (ENTRY_BLOCK_PTR));
+
   FOR_BB_BETWEEN (bb, ENTRY_BLOCK_PTR, EXIT_BLOCK_PTR, next_bb)
     {
       edge e;
Index: gcc/function.h
===================================================================
--- gcc/function.h (revision 418548)
+++ gcc/function.h (working copy)
@@ -313,6 +313,9 @@ struct GTY(()) rtl_data {
   /* Insn after which register parms and SAVE_EXPRs are born, if nonopt.  */
   rtx x_parm_birth_insn;
 
+  /* Insns inserted before parm_birth_insn.  */
+  rtx x_insns_before_parm_birth_insn;
+
   /* List of all used temporaries allocated, by level.  */
   VEC(temp_slot_p,gc) *x_used_temp_slots;
 
@@ -452,6 +455,7 @@ struct GTY(()) rtl_data {
 #define naked_return_label (crtl->x_naked_return_label)
 #define stack_slot_list (crtl->x_stack_slot_list)
 #define parm_birth_insn (crtl->x_parm_birth_insn)
+#define insns_before_parm_birth_insn (crtl->x_insns_before_parm_birth_insn)
 #define frame_offset (crtl->x_frame_offset)
 #define stack_check_probe_note (crtl->x_stack_check_probe_note)
 #define arg_pointer_save_area (crtl->x_arg_pointer_save_area)
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c (revision 418548)
+++ gcc/config/arm/arm.c (working copy)
@@ -5427,7 +5427,7 @@ require_pic_register (void)
 	         we can't yet emit instructions directly in the final
 		 insn stream.  Queue the insns on the entry edge, they will
 		 be committed after everything else is expanded.  */
-	      insert_insn_on_edge (seq, single_succ_edge (ENTRY_BLOCK_PTR));
+	      insns_before_parm_birth_insn = seq;
 	    }
 	}
     }