Re: Improve end check on rs6000 prologue analyzer

[Daniel Jacobowitz <drow at false dot org>]

Mark told me that OpenBSD is using gcc 3.3.5.  I tried to get qemu to
run an OpenBSD image, but it ended poorly.  I did find a powerpc-linux
system using GCC 3.3.3, though.

Short version: I reproduced one failure and fixed it, and I expect
that to fix the second failure too, but I'm not sure about the
selftest.exp problems.

Mark, could you try the patch at the bottom of this message?  If it
still causes selftest.exp "odd location" failures, I would appreciate
a copy of the GDB binary.  I can probably figure out what's happened
from that.  I don't think I'm going to get OpenBSD to run any time
soon, though I am going to keep trying.

On Wed, Oct 18, 2006 at 09:58:08PM +0200, Mark Kettenis wrote:
> @@ -5011,7 +5011,7 @@
>  PASS: gdb.base/sepdebug.exp: breakpoint function, optimized file
>  PASS: gdb.base/sepdebug.exp: breakpoint small function, optimized file
>  PASS: gdb.base/sepdebug.exp: run until function breakpoint, optimized file
> -PASS: gdb.base/sepdebug.exp: run until breakpoint set at small function, optimized file
> +FAIL: gdb.base/sepdebug.exp: run until breakpoint set at small function, optimized file
>  Running ../../../../src/gdb/gdb/testsuite/gdb.base/sepsymtab.exp ...
>  PASS: gdb.base/sepsymtab.exp: info sym main
>  Running ../../../../src/gdb/gdb/testsuite/gdb.base/setshow.exp ...

I can reproduce this one.  Here's the difference:

-Breakpoint 2, 0x10000500 in marker4 (d=177601976) at
 ../../../src/gdb/testsuite/gdb.base/sepdebug.c:71
 71     void marker4 (d) long d; {}  /* set breakpoint 13 here */
-PASS: gdb.base/sepdebug.exp: run until breakpoint set at small
 function, optimized file

+Breakpoint 2, marker4 (d=1) at
 ../../../src/gdb/testsuite/gdb.base/sepdebug.c:71
 71     void marker4 (d) long d; {}  /* set breakpoint 13 here */
+(gdb) FAIL: gdb.base/sepdebug.exp: run until breakpoint set at small
 function, optimized file

The argument value is wrong.  The old version stopped at 0x10000500,
which the debug info reports as being in the middle of a line.  The
new version stopped at 0x100004f0, which is the very start of the
function.

0x100004f0 <marker4+0>: stwu    r1,-32(r1)
0x100004f4 <marker4+4>: stw     r31,28(r1)
0x100004f8 <marker4+8>: mr      r31,r1
0x100004fc <marker4+12>:        stw     r3,8(r31)
0x10000500 <marker4+16>:        lwz     r11,0(r1)
0x10000504 <marker4+20>:        lwz     r31,-4(r11)
0x10000508 <marker4+24>:        mr      r1,r11
0x1000050c <marker4+28>:        blr

GCC doesn't generate a second line note for this function.  It's just
got the one for the start, and GDB trusted it.  That's easy to fix -
skip_prologue_using_sal says "returns 0 if the skip fails", but in
this case it returns the start of the function instead of 0.  Another
fix is needed to make the prologue analyzer not analyze right through
into the epilogue, treating it as optimized code moved into the
prologue.

With those changes, I get no regressions.  My GCC 3.3.3 didn't
show the m-data.exp or selftest.exp regressions to begin with, so I
can't be sure I fixed them - there were some changes in selftest.exp
but no failures.  But I'm pretty sure m-data.exp will be fixed now,
since that failure is also at an empty one line marker function.

-- 
Daniel Jacobowitz
CodeSourcery

2007-03-09  Daniel Jacobowitz  <dan@codesourcery.com>

	* rs6000-tdep.c (rs6000_skip_prologue): Use skip_prologue_using_sal.
	(rs6000_in_function_epilogue_p): Use extract_unsigned_integer.
	(refine_prologue_limit): Delete.
	(skip_prologue): Don't call it.  Use extract_unsigned_integer.
	Assume lim_pc is set.  Correct check for incomplete prologues.
	Do not skip clobbers of the frame pointer.
	* symtab.c (skip_prologue_using_sal): Fail if there is only one
	sal.

2007-03-09  Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb.arch/powerpc-prologue.c (optimized_1_marker, gdb2029_marker)
	(optimized_1): New.
	(main): Call optimized_1.
	(gdb2029): Correct typos.  Call gdb2029_marker.
	* gdb.arch/powerpc-prologue.exp: Run new test.  Use a breakpoint
	for gdb2029.

Index: rs6000-tdep.c
===================================================================
RCS file: /home/gcc/repos/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.265
diff -u -p -r1.265 rs6000-tdep.c
--- rs6000-tdep.c	27 Feb 2007 23:04:28 -0000	1.265
+++ rs6000-tdep.c	9 Mar 2007 14:01:26 -0000
@@ -482,7 +482,29 @@ static CORE_ADDR
 rs6000_skip_prologue (CORE_ADDR pc)
 {
   struct rs6000_framedata frame;
-  pc = skip_prologue (pc, 0, &frame);
+  CORE_ADDR limit_pc, func_addr;
+
+  /* See if we can determine the end of the prologue via the symbol table.
+     If so, then return either PC, or the PC after the prologue, whichever
+     is greater.  */
+  if (find_pc_partial_function (pc, NULL, &func_addr, NULL))
+    {
+      CORE_ADDR post_prologue_pc = skip_prologue_using_sal (func_addr);
+      if (post_prologue_pc != 0)
+	return max (pc, post_prologue_pc);
+    }
+
+  /* Can't determine prologue from the symbol table, need to examine
+     instructions.  */
+
+  /* Find an upper limit on the function prologue using the debug
+     information.  If the debug information could not be used to provide
+     that bound, then use an arbitrary large number as the upper bound.  */
+  limit_pc = skip_prologue_using_sal (pc);
+  if (limit_pc == 0)
+    limit_pc = pc + 100;          /* Magic.  */
+
+  pc = skip_prologue (pc, limit_pc, &frame);
   return pc;
 }
 
@@ -565,7 +587,7 @@ rs6000_in_function_epilogue_p (struct gd
     {
       if (!safe_frame_unwind_memory (curfrm, scan_pc, insn_buf, PPC_INSN_SIZE))
         return 0;
-      insn = extract_signed_integer (insn_buf, PPC_INSN_SIZE);
+      insn = extract_unsigned_integer (insn_buf, PPC_INSN_SIZE);
       if (insn == 0x4e800020)
         break;
       if (insn_changes_sp_or_jumps (insn))
@@ -580,7 +602,7 @@ rs6000_in_function_epilogue_p (struct gd
     {
       if (!safe_frame_unwind_memory (curfrm, scan_pc, insn_buf, PPC_INSN_SIZE))
         return 0;
-      insn = extract_signed_integer (insn_buf, PPC_INSN_SIZE);
+      insn = extract_unsigned_integer (insn_buf, PPC_INSN_SIZE);
       if (insn_changes_sp_or_jumps (insn))
         return 1;
     }
@@ -775,57 +797,6 @@ rs6000_software_single_step (enum target
    of the prologue is expensive.  */
 static int max_skip_non_prologue_insns = 10;
 
-/* Given PC representing the starting address of a function, and
-   LIM_PC which is the (sloppy) limit to which to scan when looking
-   for a prologue, attempt to further refine this limit by using
-   the line data in the symbol table.  If successful, a better guess
-   on where the prologue ends is returned, otherwise the previous
-   value of lim_pc is returned.  */
-
-/* FIXME: cagney/2004-02-14: This function and logic have largely been
-   superseded by skip_prologue_using_sal.  */
-
-static CORE_ADDR
-refine_prologue_limit (CORE_ADDR pc, CORE_ADDR lim_pc)
-{
-  struct symtab_and_line prologue_sal;
-
-  prologue_sal = find_pc_line (pc, 0);
-  if (prologue_sal.line != 0)
-    {
-      int i;
-      CORE_ADDR addr = prologue_sal.end;
-
-      /* Handle the case in which compiler's optimizer/scheduler
-         has moved instructions into the prologue.  We scan ahead
-	 in the function looking for address ranges whose corresponding
-	 line number is less than or equal to the first one that we
-	 found for the function.  (It can be less than when the
-	 scheduler puts a body instruction before the first prologue
-	 instruction.)  */
-      for (i = 2 * max_skip_non_prologue_insns; 
-           i > 0 && (lim_pc == 0 || addr < lim_pc);
-	   i--)
-        {
-	  struct symtab_and_line sal;
-
-	  sal = find_pc_line (addr, 0);
-	  if (sal.line == 0)
-	    break;
-	  if (sal.line <= prologue_sal.line 
-	      && sal.symtab == prologue_sal.symtab)
-	    {
-	      prologue_sal = sal;
-	    }
-	  addr = sal.end;
-	}
-
-      if (lim_pc == 0 || prologue_sal.end < lim_pc)
-	lim_pc = prologue_sal.end;
-    }
-  return lim_pc;
-}
-
 /* Return nonzero if the given instruction OP can be part of the prologue
    of a function and saves a parameter on the stack.  FRAMEP should be
    set if one of the previous instructions in the function has set the
@@ -945,21 +916,6 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
   int r0_contains_arg = 0;
   const struct bfd_arch_info *arch_info = gdbarch_bfd_arch_info (current_gdbarch);
   struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
-  
-  /* Attempt to find the end of the prologue when no limit is specified.
-     Note that refine_prologue_limit() has been written so that it may
-     be used to "refine" the limits of non-zero PC values too, but this
-     is only safe if we 1) trust the line information provided by the
-     compiler and 2) iterate enough to actually find the end of the
-     prologue.  
-     
-     It may become a good idea at some point (for both performance and
-     accuracy) to unconditionally call refine_prologue_limit().  But,
-     until we can make a clear determination that this is beneficial,
-     we'll play it safe and only use it to obtain a limit when none
-     has been specified.  */
-  if (lim_pc == 0)
-    lim_pc = refine_prologue_limit (pc, lim_pc);
 
   memset (fdata, 0, sizeof (struct rs6000_framedata));
   fdata->saved_gpr = -1;
@@ -980,7 +936,7 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
 	last_prologue_pc = pc;
 
       /* Stop scanning if we've hit the limit.  */
-      if (lim_pc != 0 && pc >= lim_pc)
+      if (pc >= lim_pc)
 	break;
 
       prev_insn_was_prologue_insn = 1;
@@ -988,7 +944,7 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
       /* Fetch the instruction and convert it to an integer.  */
       if (target_read_memory (pc, buf, 4))
 	break;
-      op = extract_signed_integer (buf, 4);
+      op = extract_unsigned_integer (buf, 4);
 
       if ((op & 0xfc1fffff) == 0x7c0802a6)
 	{			/* mflr Rx */
@@ -1221,9 +1177,11 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
  	  offset = fdata->offset;
  	  continue;
  	}
-      /* Load up minimal toc pointer */
+      /* Load up minimal toc pointer.  Do not treat an epilogue restore
+	 of r31 as a minimal TOC load.  */
       else if (((op >> 22) == 0x20f	||	/* l r31,... or l r30,... */
 	       (op >> 22) == 0x3af)		/* ld r31,... or ld r30,... */
+	       && !framep
 	       && !minimal_toc_loaded)
 	{
 	  minimal_toc_loaded = 1;
@@ -1446,8 +1404,7 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
 	     Handle optimizer code motions into the prologue by continuing
 	     the search if we have no valid frame yet or if the return
 	     address is not yet saved in the frame.  */
-	  if (fdata->frameless == 0
-	      && (lr_reg == -1 || fdata->nosavedpc == 0))
+	  if (fdata->frameless == 0 && fdata->nosavedpc == 0)
 	    break;
 
 	  if (op == 0x4e800020		/* blr */
Index: symtab.c
===================================================================
RCS file: /home/gcc/repos/src/src/gdb/symtab.c,v
retrieving revision 1.154
diff -u -p -r1.154 symtab.c
--- symtab.c	21 Jan 2007 16:55:49 -0000	1.154
+++ symtab.c	9 Mar 2007 13:48:32 -0000
@@ -4092,7 +4092,7 @@ skip_prologue_using_sal (CORE_ADDR func_
 	 then it is probably a single line function, like
 	 "foo(){}". */
       if (prologue_sal.end == end_pc)
-	return start_pc;
+	return 0;
       while (prologue_sal.end < end_pc)
 	{
 	  struct symtab_and_line sal;
Index: testsuite/gdb.arch/powerpc-prologue.c
===================================================================
RCS file: /home/gcc/repos/src/src/gdb/testsuite/gdb.arch/powerpc-prologue.c,v
retrieving revision 1.2
diff -u -p -r1.2 powerpc-prologue.c
--- testsuite/gdb.arch/powerpc-prologue.c	9 Jan 2007 17:59:09 -0000	1.2
+++ testsuite/gdb.arch/powerpc-prologue.c	9 Mar 2007 12:46:28 -0000
@@ -31,23 +31,54 @@ int
 main (void)
 {
   gdb2029 ();
+  optimized_1 ();
   return 0;
 }
 
+void
+optimized_1_marker (void)
+{
+}
+
+void
+gdb2029_marker (void)
+{
+}
+
 /* A typical PIC prologue from GCC.  */
 
 asm(".text\n"
-    "    .align 8\n"
+    "    .p2align 3\n"
     SYMBOL (gdb2029) ":\n"
-    "	stw	%r1, -32(%r1)\n"
+    "	stwu	%r1, -32(%r1)\n"
     "	mflr	%r0\n"
     "	bcl-	20,31,.+4\n"
     "	stw	%r30, 24(%r1)\n"
     "	mflr	%r30\n"
     "	stw	%r0, 36(%r1)\n"
-    "	twge	%r2, %r2\n"
+    "	bl	gdb2029_marker\n"
     "	lwz	%r0, 36(%r1)\n"
     "	lwz	%r30, 24(%r1)\n"
     "	mtlr	%r0\n"
-    "	addi	%r0, %r0, 32\n"
+    "	addi	%r1, %r1, 32\n"
+    "	blr");
+
+/* A heavily scheduled prologue.  */
+asm(".text\n"
+    "	.p2align 3\n"
+    SYMBOL (optimized_1) ":\n"
+    "	stwu	%r1,-32(%r1)\n"
+    "	lis	%r9,-16342\n"
+    "	lis	%r11,-16342\n"
+    "	mflr	%r0\n"
+    "	addi	%r11,%r11,3776\n"
+    "	stmw	%r27,12(%r1)\n"
+    "	addi	%r31,%r9,3152\n"
+    "	cmplw	%cr7,%r31,%r11\n"
+    "	stw	%r0,36(%r1)\n"
+    "	mr	%r30,%r3\n"
+    "	bl	optimized_1_marker\n"
+    "	lwz	%r0,36(%r1)\n"
+    "	lmw	%r27,12(%r1)\n"
+    "	addi	%r1,%r1,32\n"
     "	blr");
Index: testsuite/gdb.arch/powerpc-prologue.exp
===================================================================
RCS file: /home/gcc/repos/src/src/gdb/testsuite/gdb.arch/powerpc-prologue.exp,v
retrieving revision 1.2
diff -u -p -r1.2 powerpc-prologue.exp
--- testsuite/gdb.arch/powerpc-prologue.exp	9 Jan 2007 17:59:09 -0000	1.2
+++ testsuite/gdb.arch/powerpc-prologue.exp	9 Mar 2007 12:46:28 -0000
@@ -50,12 +50,40 @@ if ![runto_main] then {
 
 # Testcase for PIC prologue.
 
-gdb_test "continue" "Program received signal SIGTRAP.*" "continue to PIC"
+gdb_breakpoint "gdb2029_marker"
+gdb_test "continue" "Breakpoint $decimal, $hex in gdb2029_marker \\(\\)" \
+	"continue to PIC"
 
 gdb_test "backtrace 10" \
-	"#0\[ \t\]*$hex in gdb2029.*\r\n#1\[ \t\]*$hex in main.*" \
+	"#0\[ \t\]*$hex in gdb2029_marker.*\r\n#1\[ \t\]*$hex in gdb2029.*\r\n#2\[ \t\]*$hex in main.*" \
+	"backtrace in PIC marker"
+
+gdb_test "finish" ".*$hex in gdb2029 .*" "finish from PIC"
+
+gdb_test "backtrace 10" \
+	"#0\[ \t\]*$hex in gdb2029 .*\r\n#1\[ \t\]*$hex in main.*" \
 	"backtrace in PIC"
 
 gdb_test "info frame" \
 	".*Saved registers:.*r30 at.*r31 at.*pc at.*lr at.*" \
 	"saved registers in PIC"
+
+# Testcase for scheduled prologue.
+
+gdb_breakpoint "optimized_1_marker"
+gdb_test "continue" "Breakpoint $decimal, $hex in optimized_1_marker \\(\\)" \
+	"continue to optimized"
+
+gdb_test "backtrace 10" \
+	"#0\[ \t\]*$hex in optimized_1_marker.*\r\n#1\[ \t\]*$hex in optimized_1.*\r\n#2\[ \t\]*$hex in main.*" \
+	"backtrace in optimized marker"
+
+gdb_test "finish" ".*$hex in optimized_1 .*" "finish from optimized"
+
+gdb_test "backtrace 10" \
+	"#0\[ \t\]*$hex in optimized_1 .*\r\n#1\[ \t\]*$hex in main.*" \
+	"backtrace in optimized"
+
+gdb_test "info frame" \
+	".*Saved registers:.*r30 at.*r31 at.*pc at.*lr at.*" \
+	"saved registers in optimized"