Re: Improve end check on rs6000 prologue analyzer

[Daniel Jacobowitz <drow at false dot org>]

On Sat, Sep 30, 2006 at 09:32:00PM +0200, Mark Kettenis wrote:
> > Date: Fri, 29 Sep 2006 17:37:26 -0400
> > From: Daniel Jacobowitz <drow@false.org>
> > 
> > Any comments on this patch?  Otherwise, I'll plan to commit it in a few
> > days.
> 
> I get quite a few new regressions on OpenBSD/powerpc.  Please don't
> commit this.

Hi Mark,

Do you think you could try out this one, instead?  I worked out what
was going wrong for me: sign extension.  I was testing a 64-bit GDB
binary on 32-bit programs, which caused (op >> 22) == 0x20f to fail;
when it didn't fail, on a 32-bit host, that check allows any load into
r31 to be considered part of the prologue.  But it was advancing so far
that for small functions, the breakpoint was placed after the restore
of r31 in the epilogue - much too far.

This patch fixes the original bug, fixes the sign extension problem,
and generally overhauls the prologue skipping (as opposed to scanning)
to work more like MIPS's.  It tests with no regressions on
powerpc-linux, using a 32-bit binary this time.

-- 
Daniel Jacobowitz
CodeSourcery

2006-10-17  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.

2006-10-17  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.258
diff -u -p -r1.258 rs6000-tdep.c
--- rs6000-tdep.c	23 Apr 2006 14:15:01 -0000	1.258
+++ rs6000-tdep.c	17 Oct 2006 21:15:47 -0000
@@ -487,7 +487,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;
 }
 
@@ -570,7 +592,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))
@@ -585,7 +607,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;
     }
@@ -780,57 +802,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
@@ -926,21 +897,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;
@@ -961,7 +917,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;
@@ -969,7 +925,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 */
@@ -1414,8 +1370,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: testsuite/gdb.arch/powerpc-prologue.c
===================================================================
RCS file: /home/gcc/repos/src/src/gdb/testsuite/gdb.arch/powerpc-prologue.c,v
retrieving revision 1.1
diff -u -p -r1.1 powerpc-prologue.c
--- testsuite/gdb.arch/powerpc-prologue.c	23 Jan 2006 01:47:32 -0000	1.1
+++ testsuite/gdb.arch/powerpc-prologue.c	17 Oct 2006 18:45:44 -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.1
diff -u -p -r1.1 powerpc-prologue.exp
--- testsuite/gdb.arch/powerpc-prologue.exp	23 Jan 2006 01:47:32 -0000	1.1
+++ testsuite/gdb.arch/powerpc-prologue.exp	17 Oct 2006 18:45:44 -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"