[PATCH, ARM] Fix line number data for PIC register setup code

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

Richard,

This patch fixes line number data for the PIC register setup code for ARM,
resulting in 174 removed FAILs for the gdb testsuite with -fPIC.

Consider break.c (minimized from gdb/testsuite/gdb.base/break.c):
...
     1	void *v;
     2	void a (void *x) { }
     3	void b (void) { }
     4	
     5	int
     6	main (int argc)
     7	{
     8	  if (argc == 12345)
     9	    {
    10	      a (v);
    11	      return 1;
    12	    }
    13	  b ();
    14	
    15	  return 0;
    16	}
...

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	{
...

The breakpoint should be at line number 8, the first line with user code.

The assembly for the start of main looks like this:
...
main:
.LFB2:
	.loc 1 7 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 7 0
	ldr	r2, .L6
.LPIC0:
	add	r2, pc, r2
	.loc 1 8 0
	ldr	r1, [fp, #-8]
	ldr	r3, .L6+4
	cmp	r1, r3
	bne	.L4
	.loc 1 10 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.
...

So gdb breaks at line 7, because it considers the prologue the code between the
first 2 .locs, and the user code for main to start at the second .loc, which has
line number 7.

The code at the second .loc is the PIC register setup code, generated by
require_pic_register:
...
	.loc 1 7 0
	ldr	r2, .L6
.LPIC0:
	add	r2, pc, r2
...

The second .loc is emitted by gcc, because it's the first insn after
NOTE_INSNS_FUNCTION_BEG (which sets final.c:force_source_line in
final.c:final_scan_insn).

The PIC register setup code is emitted after NOTE_INSNS_FUNCTION_BEG, because it
uses the insert_insn_on_edge mechanism, and the corresponding insertion in
cfgexpand.c:gimple_expand_cfg takes care to insert the code after the
parm_birth_insn:
...
	      /* Avoid putting insns before parm_birth_insn.  */
	      if (e->src == ENTRY_BLOCK_PTR
		  && single_succ_p (ENTRY_BLOCK_PTR)
		  && parm_birth_insn)
		{
		  rtx insns = e->insns.r;
		  e->insns.r = NULL_RTX;
		  emit_insn_after_noloc (insns, parm_birth_insn, e->dest);
		}
...
And in the case for this test-case, parm_birth_insn is the NOTE_INSNS_FUNCTION_BEG.

In summary, the problem seems to be caused by a discrepancy between:
- the line number of the PIC register setup code (prologue_location), and
- the location of the PIC register setup code (after NOTE_INSN_FUNCTION_BEG).

The problem can be reproduced using Ulrich's example here (
http://gcc.gnu.org/ml/gcc-patches/2010-12/msg01570.html ), but though related
it's not the same problem as explained there. There the wrong line number of the
breakpoint was after the first user line. Here the wrong line number of the
breakpoint is before the first user line.

We can see from that email that this problem was not present after the fix. The
problem was introduced by Jakub's fix for PR47028 (Insert entry edge insertions
after parm_birth_insn instead of at the beginning of first bb), 12 days after
Ulrich's fix.

This patch makes sure we emit insertions scheduled for the first real BB before
NOTE_INSN_FUNCTION_BEG. As a consequence, it moves the PIC register setup code
to before the NOTE_INSN_FUNCTION_BEG. This removes the second .loc, and the
breakpoint of main ends up at line 8.

Bootstrapped and regtested on x86_64 (ada inclusive), no issues found.

Tested gdb with target arm-none-linux-gnueabi and CFLAGS_FOR_TARGET=-fPIC. The
patch removes 174 FAILs.

Re-testing gcc with target arm-none-linux-gnueabi atm.

OK for trunk?

Thanks,
- Tom

2013-10-13  Tom de Vries  <tom@codesourcery.com>

	* cfgexpand.c (gimple_expand_cfg): Don't commit insertions after
	NOTE_INSN_FUNCTION_BEG.

	* gcc.target/arm/require-pic-register-loc.c: New test.

Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c (revision 421892)
+++ gcc/cfgexpand.c (working copy)
@@ -4618,14 +4618,19 @@ gimple_expand_cfg (void)
 	  if (e->insns.r)
 	    {
 	      rebuild_jump_labels_chain (e->insns.r);
-	      /* Avoid putting insns before parm_birth_insn.  */
+	      /* Put insns after parm birth, but before
+		 NOTE_INSNS_FUNCTION_BEG.  */
 	      if (e->src == ENTRY_BLOCK_PTR
 		  && single_succ_p (ENTRY_BLOCK_PTR)
 		  && parm_birth_insn)
 		{
 		  rtx insns = e->insns.r;
 		  e->insns.r = NULL_RTX;
-		  emit_insn_after_noloc (insns, parm_birth_insn, e->dest);
+		  if (NOTE_P (parm_birth_insn)
+		      && NOTE_KIND (parm_birth_insn) == NOTE_INSN_FUNCTION_BEG)
+		    emit_insn_before_noloc (insns, parm_birth_insn, e->dest);
+		  else
+		    emit_insn_after_noloc (insns, parm_birth_insn, e->dest);
 		}
 	      else
 		commit_one_edge_insertion (e);
Index: gcc/testsuite/gcc.target/arm/require-pic-register-loc.c
===================================================================
--- /dev/null (new file)
+++ gcc/testsuite/gcc.target/arm/require-pic-register-loc.c (revision 0)
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-g -fPIC" } */
+
+void *v;
+void a (void *x) { }
+void b (void) { }
+                       /* line 7.  */
+int                    /* line 8.  */
+main (int argc)        /* line 9.  */
+{                      /* line 10.  */
+  if (argc == 12345)   /* line 11.  */
+    {
+      a (v);
+      return 1;
+    }
+  b ();
+
+  return 0;
+}
+
+/* { dg-final { scan-assembler-not "\.loc 1 7 0" } } */
+/* { dg-final { scan-assembler-not "\.loc 1 8 0" } } */
+/* { dg-final { scan-assembler-not "\.loc 1 9 0" } } */
+
+/* The loc at the start of the prologue.  */
+/* { dg-final { scan-assembler-times "\.loc 1 10 0" 1 } } */
+
+/* The loc at the end of the prologue, with the first user line.  */
+/* { dg-final { scan-assembler-times "\.loc 1 11 0" 1 } } */