RFA: Adjust PowerPC prologue analyzer for PIC

RFA: Adjust PowerPC prologue analyzer for PIC

[Daniel Jacobowitz]

This patch is based on Till's patch from GNATS.  One of the three changes
(b1 in the diff attached to 2029) was fixed independently by Michael Snyder
last year, but I think it needs a small improvement: there's a comment that
says "skip over additional [mflr instructions]", and checks lr_reg < 0.  Now
that should be lr_reg == -1, since -2 means we've seen one and stored it.

(a) seems right - we should recognize "bcl 20,31,.+4" as a skippable
prologue instruction.  GCC generates this for PIC code, and in fact
backtracing out of shared libraries on GNU/Linux works badly without
recognizing this instruction.  I left out recognition for the equivalent bc
instruction.  Is there any reason I'm missing why this should be generated?

I left out (b) because I'm not sure that it's correct, and I don't have a
testcase for it.  That changed

!             && (lr_reg == -1 || fdata->nosavedpc == 0))
to
!             && (fdata->nosavedpc == 0))

but if lr_reg is -1, meaning we haven't seen an mflr, maybe we won't. I
guess that's necessary only in the case where instructions get scheduled
before the mflr?  If you still believe that's correct, let's try it
separately.

I've tested this patch on powerpc-linux, where it showed no change in the
testsuite results.  It also fixes Debian bug #312059.

Is this patch OK?

-- 
Daniel Jacobowitz
CodeSourcery, LLC

2005-12-10  Daniel Jacobowitz  <dan@codesourcery.com>

	PR tdep/2029
	Suggested by Till Straumann <strauman@slac.stanford.edu>:
	* rs6000-tdep.c (skip_prologue): Update check for later mtlr
	instructions.  Handle PIC bcl.

Index: gdb-6.4/gdb/rs6000-tdep.c
===================================================================
--- gdb-6.4.orig/gdb/rs6000-tdep.c	2005-11-01 14:32:36.000000000 -0500
+++ gdb-6.4/gdb/rs6000-tdep.c	2005-12-10 00:22:15.000000000 -0500
@@ -911,7 +911,7 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
 
 	     remember just the first one, but skip over additional
 	     ones.  */
-	  if (lr_reg < 0)
+	  if (lr_reg == -1)
 	    lr_reg = (op & 0x03e00000);
           if (lr_reg == 0)
             r0_contains_arg = 0;
@@ -1024,6 +1024,13 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
 	  continue;
 
 	}
+      else if ((op & 0xfe80ffff) == 0x42800005 && lr_reg != -1)
+	{
+	  /* bcl 20,xx,.+4 is used to get the current PC, with or without
+	     prediction bits.  If the LR has already been saved, we can
+	     skip it.  */
+	  continue;
+	}
       else if (op == 0x48000005)
 	{			/* bl .+4 used in 
 				   -mrelocatable */

Re: RFA: Adjust PowerPC prologue analyzer for PIC

[Jim Blandy]

Do we have a prologue analysis test that catches this problem?

Re: RFA: Adjust PowerPC prologue analyzer for PIC

[Daniel Jacobowitz]

On Sun, Dec 11, 2005 at 09:26:27AM -0800, Jim Blandy wrote:
> Do we have a prologue analysis test that catches this problem?

I'll write one (not today; the PowerPC machine I'm using is dreadfully
slow, I need to set up an alternative).

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: RFA: Adjust PowerPC prologue analyzer for PIC

[Mark Kettenis]

> Date: Sun, 11 Dec 2005 12:15:41 -0500
> From: Daniel Jacobowitz <drow@false.org>
> 
> 2005-12-10  Daniel Jacobowitz  <dan@codesourcery.com>
> 
> 	PR tdep/2029
> 	Suggested by Till Straumann <strauman@slac.stanford.edu>:
> 	* rs6000-tdep.c (skip_prologue): Update check for later mtlr
> 	instructions.  Handle PIC bcl.

Without this patch debugging shared library code on OpenBSD/macppc
seems to be impossible, so I'd defenitely like to see this go in.

Mark

Re: RFA: Adjust PowerPC prologue analyzer for PIC

[Kevin Buettner]

On Sun, 11 Dec 2005 12:15:41 -0500
Daniel Jacobowitz <drow@false.org> wrote:

> 2005-12-10  Daniel Jacobowitz  <dan@codesourcery.com>
> 
> 	PR tdep/2029
> 	Suggested by Till Straumann <strauman@slac.stanford.edu>:
> 	* rs6000-tdep.c (skip_prologue): Update check for later mtlr
> 	instructions.  Handle PIC bcl.

Okay.

Kevin

Re: RFA: Adjust PowerPC prologue analyzer for PIC

[Daniel Jacobowitz]

On Thu, Dec 15, 2005 at 01:06:33PM -0700, Kevin Buettner wrote:
> On Sun, 11 Dec 2005 12:15:41 -0500
> Daniel Jacobowitz <drow@false.org> wrote:
> 
> > 2005-12-10  Daniel Jacobowitz  <dan@codesourcery.com>
> > 
> > 	PR tdep/2029
> > 	Suggested by Till Straumann <strauman@slac.stanford.edu>:
> > 	* rs6000-tdep.c (skip_prologue): Update check for later mtlr
> > 	instructions.  Handle PIC bcl.
> 
> Okay.

Thanks.  I've belatedly checked this in, along with the attached
testcase.

-- 
Daniel Jacobowitz
CodeSourcery

2006-01-22  Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb.arch/powerpc-prologue.c, gdb.arch/powerpc-prologue.exp: New
	files.

Index: gdb.arch/powerpc-prologue.c
===================================================================
RCS file: gdb.arch/powerpc-prologue.c
diff -N gdb.arch/powerpc-prologue.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb.arch/powerpc-prologue.c	23 Jan 2006 01:46:50 -0000
@@ -0,0 +1,53 @@
+/* Unwinder test program.
+
+   Copyright 2006 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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 2 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, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.  */
+
+#ifdef SYMBOL_PREFIX
+#define SYMBOL(str)	SYMBOL_PREFIX #str
+#else
+#define SYMBOL(str)	#str
+#endif
+
+void gdb2029 (void);
+
+int
+main (void)
+{
+  gdb2029 ();
+  return 0;
+}
+
+/* A typical PIC prologue from GCC.  */
+
+asm(".text\n"
+    "    .align 8\n"
+    SYMBOL (gdb2029) ":\n"
+    "	stw	%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"
+    "	lwz	%r0, 36(%r1)\n"
+    "	lwz	%r30, 24(%r1)\n"
+    "	mtlr	%r0\n"
+    "	addi	%r0, %r0, 32\n"
+    "	blr");
Index: gdb.arch/powerpc-prologue.exp
===================================================================
RCS file: gdb.arch/powerpc-prologue.exp
diff -N gdb.arch/powerpc-prologue.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb.arch/powerpc-prologue.exp	23 Jan 2006 01:46:50 -0000
@@ -0,0 +1,61 @@
+# Copyright 2006 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 2 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, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+
+# Test PowerPC prologue analyzer.
+
+# Do not run on AIX (where we won't be able to build the tests without
+# some surgery) or on PowerPC64 (ditto, dot symbols).
+if {[istarget *-*-aix*] || ![istarget "powerpc-*-*"]} then {
+    verbose "Skipping PowerPC prologue tests."
+    return
+}
+
+set testfile "powerpc-prologue"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+# Don't use "debug", so that we don't have line information for the assembly
+# fragments.
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {}] != "" } {
+    untested "PowerPC prologue tests"
+    return -1
+}
+
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+#
+# Run to `main' where we begin our tests.
+#
+
+if ![runto_main] then {
+    gdb_suppress_tests
+}
+
+# Testcase for PIC prologue.
+
+gdb_test "continue" "Program received signal SIGTRAP.*" "continue to 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"