[RFC] PowerPC prologue unwinder / epilogue detection improvements

[Daniel Jacobowitz <drow at false dot org>]

A MontaVista customer was trying software watchpoints.  They set one
on a local variable and then called printf in a loop; this takes us,
of course, all the way through the dynamic resolver and then the body
of printf itself.  If the unwinder becomes confused at any point
in this process, and the epilogue check doesn't bail us out, then GDB
will delete the out-of-scope watchpoint.

This patch fixes up a number of things I found in the PowerPC prologue
unwinder in the course of making that work.  Most of them are
straightforward:

  - A function can end with a bctr, if it performs a tail call.
  Recognize this as an epilogue return instruction if it leaves the
  function.

  - Just because r30 has been saved doesn't mean r31 has been, even
  though we know where it will be once the prologue reaches that.  GCC
  often stores the lower numbered registers first.

  - During the prologue, LR may be saved into r0 and then clobbered
  by a PIC base operation (bl .+4 or bl GOT-4 for a blrl) before it is
  eventually saved to the stack.  Look in r0 for it during that
  interval.

The patch also contains two things that are not straightforward and I
am not going to commit as they are:

  - Both the oldish MontaVista toolchain their customer was using, and
  the Debian Sarge toolchain on the test machine I used for the port
  to HEAD GDB, are built with GCC 3.4.  There's an annoying bug in
  this version of GCC.  Given a sequence like this:

       stwu    r1,-48(r1)
       mflr    r0
       bl      0x26384
       stw     r0,52(r1)
       ...

  GCC will not report the move of the return address from LR to r0.
  It leaves the return address column pointing at LR until the end of
  the prologue, and then updates it to point directly at 52(r1) on the
  stack.

  I forcibly detect this case in the patch below and use the prologue
  unwinder instead of the DWARF-2 unwinder if the prologue contains a
  bl instruction (-fpic or -fPIC).  This is much too heavy-handed.  Is
  it worth working around this bug in GDB?

  If so, we should find a way to limit it to some likely selection of
  GCC-generated objects.  If not - my inclination is not - I'll simply
  drop this part of the patch.  Anyone who needs it can find it in the
  list archives.

  - Also, I wrote a testcase for this situation.  Like the original
  report, it steps into printf.  The only way to handle this with
  software watchpoints is to have symbols or unwind tables for your
  ld.so / libc.so.  If you don't, you won't have symbol addresses;
  there is no practical way to backtrace from the prologue of a called
  function without that.

  I'd sort of like to commit the test.  But it's going to fail in lots
  of targets that are not as careful as I was below, and in cases
  where the necessary symbol data is not available.  I'm thinking
  of XFAILing it if the top function in the backtrace at a failure is
  "??".

  How does that sound?

Any comments?  Tested on powerpc-linux with no regressions.

-- 
Daniel Jacobowitz
CodeSourcery

2007-12-06  Daniel Jacobowitz  <dan@codesourcery.com>

	* rs6000-tdep.c (struct rs6000_framedata): Add gpr_mask, used_bl,
	lr_register.
	(rs6000_in_function_epilogue_p): Check for bctr.
	(skip_prologue): Initialize lr_register.  Set lr_reg to a register
	number.  Set gpr_mask and used_bl.  Continue scanning while some
	expected registers are not saved.  Set lr_register if LR is not
	stored.
	(rs6000_frame_cache): Handle gpr_mask and lr_register.
	(rs6000_force_frame_sniffer): New.
	(rs6000_gdbarch_init): Register rs6000_force_frame_sniffer.

	* gdb.base/watch-lib.c, gdb.base/watch-lib.exp: New files.
	* gdb.arch/powerpc-prologue.exp: Correct saved registers.

Index: rs6000-tdep.c
===================================================================
RCS file: /scratch/gcc/repos/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.300
diff -u -p -r1.300 rs6000-tdep.c
--- rs6000-tdep.c	19 Nov 2007 05:06:24 -0000	1.300
+++ rs6000-tdep.c	6 Dec 2007 20:09:33 -0000
@@ -118,17 +118,20 @@ struct rs6000_framedata
 				   by which we decrement sp to allocate
 				   the frame */
     int saved_gpr;		/* smallest # of saved gpr */
+    unsigned int gpr_mask;	/* Each bit is an individual saved GPR.  */
     int saved_fpr;		/* smallest # of saved fpr */
     int saved_vr;               /* smallest # of saved vr */
     int saved_ev;               /* smallest # of saved ev */
     int alloca_reg;		/* alloca register number (frame ptr) */
     char frameless;		/* true if frameless functions. */
     char nosavedpc;		/* true if pc not saved. */
+    char used_bl;		/* true if link register clobbered */
     int gpr_offset;		/* offset of saved gprs from prev sp */
     int fpr_offset;		/* offset of saved fprs from prev sp */
     int vr_offset;              /* offset of saved vrs from prev sp */
     int ev_offset;              /* offset of saved evs from prev sp */
     int lr_offset;		/* offset of saved lr */
+    int lr_register;		/* register of saved lr, if trustworthy */
     int cr_offset;		/* offset of saved cr */
     int vrsave_offset;          /* offset of saved vrsave register */
   };
@@ -838,6 +841,7 @@ insn_changes_sp_or_jumps (unsigned long 
 static int
 rs6000_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   bfd_byte insn_buf[PPC_INSN_SIZE];
   CORE_ADDR scan_pc, func_start, func_end, epilogue_start, epilogue_end;
   unsigned long insn;
@@ -865,6 +869,17 @@ rs6000_in_function_epilogue_p (struct gd
       insn = extract_unsigned_integer (insn_buf, PPC_INSN_SIZE);
       if (insn == 0x4e800020)
         break;
+      /* Assume a bctr is a tail call unless it points strictly within
+	 this function.  */
+      if (insn == 0x4e800420)
+	{
+	  CORE_ADDR ctr = get_frame_register_unsigned (curfrm,
+						       tdep->ppc_ctr_regnum);
+	  if (ctr > func_start && ctr < func_end)
+	    return 0;
+	  else
+	    break;
+	}
       if (insn_changes_sp_or_jumps (insn))
         return 0;
     }
@@ -1283,6 +1298,7 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
   fdata->alloca_reg = -1;
   fdata->frameless = 1;
   fdata->nosavedpc = 1;
+  fdata->lr_register = -1;
 
   for (;; pc += 4)
     {
@@ -1324,7 +1340,7 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
 	     remember just the first one, but skip over additional
 	     ones.  */
 	  if (lr_reg == -1)
-	    lr_reg = (op & 0x03e00000);
+	    lr_reg = (op & 0x03e00000) >> 21;
           if (lr_reg == 0)
             r0_contains_arg = 0;
 	  continue;
@@ -1355,6 +1371,10 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
 	{
 
 	  reg = GET_SRC_REG (op);
+	  if ((op & 0xfc1f0000) == 0xbc010000)
+	    fdata->gpr_mask |= ~((1U << reg) - 1);
+	  else
+	    fdata->gpr_mask |= 1U << reg;
 	  if (fdata->saved_gpr == -1 || fdata->saved_gpr > reg)
 	    {
 	      fdata->saved_gpr = reg;
@@ -1446,6 +1466,7 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
       else if (op == 0x48000005)
 	{			/* bl .+4 used in 
 				   -mrelocatable */
+	  fdata->used_bl = 1;
 	  continue;
 
 	}
@@ -1470,7 +1491,10 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
 	  /* If the return address has already been saved, we can skip
 	     calls to blrl (for PIC).  */
           if (lr_reg != -1 && bl_to_blrl_insn_p (pc, op))
-	    continue;
+	    {
+	      fdata->used_bl = 1;
+	      continue;
+	    }
 
 	  /* Don't skip over the subroutine call if it is not within
 	     the first three instructions of the prologue and either
@@ -1496,8 +1520,9 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
 	  if (op == 0x4def7b82 || op == 0)	/* crorc 15, 15, 15 */
 	    break;		/* don't skip over 
 				   this branch */
-	  continue;
 
+	  fdata->used_bl = 1;
+	  continue;
 	}
       /* update stack pointer */
       else if ((op & 0xfc1f0000) == 0x94010000)
@@ -1758,11 +1783,15 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
 
       else
 	{
+	  unsigned int all_mask = ~((1U << fdata->saved_gpr) - 1);
+
 	  /* Not a recognized prologue instruction.
 	     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 && fdata->nosavedpc == 0)
+	     address is not yet saved in the frame.  Also skip instructions
+	     if some of the GPRs expected to be saved are not yet saved.  */
+	  if (fdata->frameless == 0 && fdata->nosavedpc == 0
+	      && (fdata->gpr_mask & all_mask) == all_mask)
 	    break;
 
 	  if (op == 0x4e800020		/* blr */
@@ -1815,6 +1844,9 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
     }
 #endif /* 0 */
 
+  if (pc == lim_pc && lr_reg >= 0)
+    fdata->lr_register = lr_reg;
+
   fdata->offset = -fdata->offset;
   return last_prologue_pc;
 }
@@ -2954,7 +2986,8 @@ rs6000_frame_cache (struct frame_info *n
     }
 
   /* if != -1, fdata.saved_gpr is the smallest number of saved_gpr.
-     All gpr's from saved_gpr to gpr31 are saved.  */
+     All gpr's from saved_gpr to gpr31 are saved (except during the
+     prologue).  */
 
   if (fdata.saved_gpr >= 0)
     {
@@ -2962,7 +2995,8 @@ rs6000_frame_cache (struct frame_info *n
       CORE_ADDR gpr_addr = cache->base + fdata.gpr_offset;
       for (i = fdata.saved_gpr; i < ppc_num_gprs; i++)
 	{
-	  cache->saved_regs[tdep->ppc_gp0_regnum + i].addr = gpr_addr;
+	  if (fdata.gpr_mask & (1U << i))
+	    cache->saved_regs[tdep->ppc_gp0_regnum + i].addr = gpr_addr;
 	  gpr_addr += wordsize;
 	}
     }
@@ -3009,6 +3043,8 @@ rs6000_frame_cache (struct frame_info *n
      holds the LR.  */
   if (fdata.lr_offset != 0)
     cache->saved_regs[tdep->ppc_lr_regnum].addr = cache->base + fdata.lr_offset;
+  else if (fdata.lr_register != -1)
+    cache->saved_regs[tdep->ppc_lr_regnum].realreg = fdata.lr_register;
   /* The PC is found in the link register.  */
   cache->saved_regs[gdbarch_pc_regnum (gdbarch)] =
     cache->saved_regs[tdep->ppc_lr_regnum];
@@ -3066,6 +3102,33 @@ rs6000_frame_sniffer (struct frame_info 
   return &rs6000_frame_unwind;
 }
 
+static const struct frame_unwind *
+rs6000_force_frame_sniffer (struct frame_info *next_frame)
+{
+  /* HACK: The unwind tables generated by GCC do not account for the
+     copy of LR to r0 before a PIC bl instruction pointing at a blrl.
+     We must avoid the FDE in this case if we are between the blrl
+     and the stack save.  */
+  CORE_ADDR pc = frame_pc_unwind (next_frame);
+  struct rs6000_framedata fdata;
+  CORE_ADDR prologue_end, func_addr;
+
+  func_addr = frame_func_unwind (next_frame, NORMAL_FRAME);
+  if (func_addr != 0)
+    {
+      /* 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.  */
+      prologue_end = skip_prologue (func_addr, pc, &fdata);
+      if (fdata.used_bl)
+	/* (fdata.lr_register != -1 || fdata.lr_offset != 0)
+	   && prologue_end == pc) */
+	return &rs6000_frame_unwind;
+    }
+
+  return NULL;
+}
+
 
 
 static CORE_ADDR
@@ -3690,6 +3753,10 @@ rs6000_gdbarch_init (struct gdbarch_info
     (gdbarch, rs6000_in_solib_return_trampoline);
   set_gdbarch_skip_trampoline_code (gdbarch, rs6000_skip_trampoline_code);
 
+  /* HACK: Override the DWARF-2 frame sniffer to correct for invalid data
+     during prologues.  */
+  frame_unwind_append_sniffer (gdbarch, rs6000_force_frame_sniffer);
+
   /* Hook in the DWARF CFI frame unwinder.  */
   frame_unwind_append_sniffer (gdbarch, dwarf2_frame_sniffer);
   dwarf2_frame_set_adjust_regnum (gdbarch, rs6000_adjust_frame_regnum);
Index: testsuite/gdb.base/watch-lib.c
===================================================================
RCS file: testsuite/gdb.base/watch-lib.c
diff -N testsuite/gdb.base/watch-lib.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/watch-lib.c	6 Dec 2007 19:39:37 -0000
@@ -0,0 +1,30 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2007
+   Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main ()
+{
+  char buf[32];
+  int a = 0;
+
+  buf[0] = 0; /* set breakpoint here */
+  for (a = 0; a < 5; a++)
+    sprintf (buf, "Message %d", a);
+
+  return 0;
+}
Index: testsuite/gdb.base/watch-lib.exp
===================================================================
RCS file: testsuite/gdb.base/watch-lib.exp
diff -N testsuite/gdb.base/watch-lib.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/watch-lib.exp	6 Dec 2007 19:53:37 -0000
@@ -0,0 +1,68 @@
+# Copyright 2007
+# Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+set testfile "watch-lib"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+     untested watch-lib.exp
+     return -1
+}
+
+proc test_lib_watchpoint { type } {
+    global pf_prefix decimal
+
+    set old_pf_prefix $pf_prefix
+    set pf_prefix "$pf_prefix $type:"
+
+    runto [gdb_get_line_number "set breakpoint here"]
+    
+    gdb_test "print a" "\\\$$decimal = 0"
+    gdb_test "watch a" ".*atchpoint $decimal: a"
+    gdb_test "continue" "Old value = 0\r\nNew value = 1.*" "first continue"
+    gdb_test "backtrace" "" ""
+    gdb_test "continue" "Old value = 1\r\nNew value = 2.*" "second continue"
+    gdb_test "backtrace" "" ""
+
+    set pf_prefix $old_pf_prefix
+}
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load $binfile
+set prev_timeout $timeout
+set timeout 600	
+verbose "Timeout now 600 sec.\n"
+
+gdb_test "set can-use-hw-watchpoints 0" "" ""
+test_lib_watchpoint sw
+
+if {! [target_info exists gdb,no_hardware_watchpoints]} {
+    gdb_exit
+    gdb_start
+    gdb_reinitialize_dir $srcdir/$subdir
+    gdb_load $binfile
+    gdb_test "set can-use-hw-watchpoints 1" "" ""
+    test_lib_watchpoint hw
+}
+
+# Restore old timeout
+set timeout $prev_timeout
+verbose "Timeout now $timeout sec.\n"
Index: testsuite/gdb.arch/powerpc-prologue.exp
===================================================================
RCS file: /scratch/gcc/repos/src/src/gdb/testsuite/gdb.arch/powerpc-prologue.exp,v
retrieving revision 1.4
diff -u -p -r1.4 powerpc-prologue.exp
--- testsuite/gdb.arch/powerpc-prologue.exp	23 Aug 2007 18:14:16 -0000	1.4
+++ testsuite/gdb.arch/powerpc-prologue.exp	6 Dec 2007 21:39:53 -0000
@@ -84,5 +84,5 @@ gdb_test "backtrace 10" \
 	"backtrace in optimized"
 
 gdb_test "info frame" \
-	".*Saved registers:.*r30 at.*r31 at.*pc at.*lr at.*" \
+	".*Saved registers:.*r30 at.*pc at.*lr at.*" \
 	"saved registers in optimized"