[PATCH] Shared library fixes for GNU/Linux/PPC port

[Kevin Buettner <kevinb at cygnus dot com>]

I've just committed several changes which fix two problems for the
GNU/Linux/PPC port.

1)  Calling an inferior function which returns a structure didn't
    work right.  The structure address was not getting stored in r3 as
    mandated by the SysV ABI.  Also, the first argument to the
    function was winding up in r4 instead of r3 in this case.

2)  It was not possible to start gdb and then set a breakpoint on
    a shared library function.  Before I describe the exact nature
    of the problem, I need to first describe some of the details
    related to dynamic linking on this platform.

    A call to a shared library function is accomplished via a bl
    (branch-and-link) instruction whose branch target is an entry
    in the procedure linkage table (PLT).  The PLT (in the object)
    file is uninitialized.  To gdb, prior to running the program, the
    entries in the PLT are all zeros.

    Once the program starts running, the shared libraries are loaded
    and the procedure linkage table is initialized, but the entries
    in the table are not (necessarily) resolved.  Once a function
    is actually called, the code in the PLT is hit and the function
    is resolved.  In order to better illustrate this, I think an
    example is in order; the following example is shmain from the gdb
    testsuite.  (Note that I ran this example *after* fixing the bug.)
	    
	We start the program shmain.

	    [kev@arroyo testsuite]$ ../gdb gdb.base/shmain
	    GNU gdb 20000204
	    [...]

	We place two breakpoints, one on shr1 and the other on main.

	    (gdb) b shr1
	    Breakpoint 1 at 0x100409d4
	    (gdb) b main
	    Breakpoint 2 at 0x100006a0: file gdb.base/shmain.c, line 44.

	Examine the instruction (and the immediatly following instruction)
	upon which the breakpoint was placed.  Note that the PLT entry
	for shr1 contains zeros.

	    (gdb) x/2i 0x100409d4
	    0x100409d4 <shr1>:      .long 0x0
	    0x100409d8 <shr1+4>:    .long 0x0

	Now run 'til main.

	    (gdb) r
	    Starting program: gdb.base/shmain 
	    Breakpoint 1 at 0xffaf790: file gdb.base/shr1.c, line 19.

	    Breakpoint 2, main ()
		at gdb.base/shmain.c:44
	    44        g = 1;

	Examine the PLT again.  Note that the loading of the shared
	library has initialized the PLT to code which loads a constant
	(which I think is an index into the GOT) into r11 and then
	branchs a short distance to the code which actually does the
	resolving.

	    (gdb) x/2i 0x100409d4
	    0x100409d4 <shr1>:      li      r11,4
	    0x100409d8 <shr1+4>:    b       0x10040984 <sg+4>
	    (gdb) c
	    Continuing.

	    Breakpoint 1, shr1 (x=1)
		at gdb.base/shr1.c:19
	    19        l = 1;

	Now we've hit the breakpoint at shr1.  (The breakpoint was
	reset from the PLT entry to the actual shr1 function after the
	shared library was loaded.) Note that the PLT entry has been
	resolved to contain a branch that takes us directly to shr1. 
	(The real one, not the PLT entry.)

	    (gdb) x/2i 0x100409d4
	    0x100409d4 <shr1>:      b       0xffaf76c <shr1>
	    0x100409d8 <shr1+4>:    b       0x10040984 <sg+4>

    The thing to note here is that the PLT entry for shr1 has been
    changed twice.

    There were two problems.

    The first was that the prologue matching code would (incorrectly)
    skip over the zeros because it was considering these to be one
    of the following instructions:

	st cr, NUM(r1)
    or
	st lr, NUM(r1)

    It was a simple matter to fix the prologue scanner to only match
    the above instructions.  But this was a serious problem because
    gdb would skip over *all of the zeros* in the PLT and would wind
    up trying to set a breakpoint in some bit of unaccessible memory.

    With that fix in place, however, there was still a problem.  GDB
    would place a breakpoint (a trap instruction) on the zero value 
    of the PLT entry for shr1.  Later on, after the shared library
    had been loaded and the PLT initialized, gdb would get a signal
    indicating this fact and would (as it always does when it stops)
    remove all the breakpoints.

    This removal of the breakpoints was causing the former contents (a
    zero word) to be written back to the now initialized PLT entry
    thus destroying a portion of the initialization that had occurred
    only a short time ago.  When execution continued, the zero word
    would be executed as an instruction an an illegal instruction trap
    was generated instead.  (0 is not a legal instruction.)

    The fix for this was a bit more complicated.  What I did was to
    make a copy of memory_remove_breakpoint () from mem-break.c and
    rename it to ppc_linux_memory_remove_breakpoint ().  (I also made
    a few minor changes which I'll describe shortly.) I made this new
    function live in ppc-linux-tdep.c.  In tm-linux.h, I then defined
    MEMORY_REMOVE_BREAKPOINT to call this new function.

    The differences between ppc_linux_memory_remove_breakpoint () and
    memory_remove_breakpoint () are minor.  All that the former does
    that the latter does not is check to make sure that the breakpoint
    location actually contains a breakpoint (trap instruction) prior
    to attempting to write back the old contents.  If it does contain
    a trap instruction, we allow the old contents to be written back. 
    Otherwise, we silently do nothing.

    It seems to me that we ought to be using this version in
    mem-break.c for all other targets (using the mem-break.c
    facilities) too.  The only downside that more traffic is generated
    for remote targets since we'll have an extra fetch of a memory
    word each time a breakpoint is removed.

    For the time being, I'll leave this self-modifying-code-friendly
    version in ppc-linux-tdep.c, but I think it ought to moved to
    mem-break.c and be made the generic version.

    Comments?

Below are the patches.  I'm now seeing between 17 and 20 unexpected
failures on the GNU/Linux/PPC port.  The exact number varies depending
upon whether gdb is compiled with optimization or not.  (Some of the
tests in selftest.exp will fail when gdb is compiled with optimization
turned on.)

	* ppc-linux-tdep.c (ppc_sysv_abi_push_arguments): Put address
	of return structure in r3 if necessary.
	(ppc_linux_memory_remove_breakpoints): New function.
	* rs6000-tdep.c (skip_prologue): Make sure that the cases
	for storing either cr or lr to the stack only handle those
	cases.  (I.e, don't let these cases match 0x00000000 which is
	found found in the shared library trampoline prior to the
	loading of the shared library.)
	* config/powerpc/tm-linux.h (ppc_linux_memory_remove_breakpoint):
	Declare.
	(MEMORY_REMOVE_BREAKPOINT): Define.

Index: ppc-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-linux-tdep.c,v
retrieving revision 1.2
diff -u -p -r1.2 ppc-linux-tdep.c
--- ppc-linux-tdep.c	2000/02/22 18:47:41	1.2
+++ ppc-linux-tdep.c	2000/02/24 22:46:53
@@ -522,6 +522,14 @@ ppc_sysv_abi_push_arguments (nargs, args
   structoffset = argoffset + argstkspace;
   freg = 1;
   greg = 3;
+  /* Fill in r3 with the return structure, if any */
+  if (struct_return)
+    {
+      char val_buf[4];
+      store_address (val_buf, 4, struct_addr);
+      memcpy (&registers[REGISTER_BYTE (greg)], val_buf, 4);
+      greg++;
+    }
   /* Now fill in the registers and stack... */
   for (argno = 0; argno < nargs; argno++)
     {
@@ -605,4 +613,30 @@ ppc_sysv_abi_push_arguments (nargs, args
 
   target_store_registers (-1);
   return sp;
+}
+
+/* This version of ppc_linux_memory_remove_breakpoints handles the
+   case of self modifying code */
+int
+ppc_linux_memory_remove_breakpoint (CORE_ADDR addr, char *contents_cache)
+{
+  unsigned char *bp;
+  int val;
+  int bplen;
+  char old_contents[BREAKPOINT_MAX];
+
+  /* Determine appropriate breakpoint contents and size for this address.  */
+  bp = BREAKPOINT_FROM_PC (&addr, &bplen);
+  if (bp == NULL)
+    error ("Software breakpoints not implemented for this target.");
+
+  val = target_read_memory (addr, old_contents, bplen);
+
+  /* If our breakpoint is no longer at the address, this means that the
+     program modified the code on us, so it is wrong to put back the
+     old value */
+  if (val == 0 && memcmp (bp, old_contents, bplen) == 0)
+    val = target_write_memory (addr, contents_cache, bplen);
+
+  return val;
 }
Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.3
diff -u -p -r1.3 rs6000-tdep.c
--- rs6000-tdep.c	2000/02/22 01:20:32	1.3
+++ rs6000-tdep.c	2000/02/24 22:46:55
@@ -313,8 +313,8 @@ skip_prologue (pc, fdata)
   char buf[4];
   unsigned long op;
   long offset = 0;
-  int lr_reg = 0;
-  int cr_reg = 0;
+  int lr_reg = -1;
+  int cr_reg = -1;
   int reg;
   int framep = 0;
   int minimal_toc_loaded = 0;
@@ -391,7 +391,7 @@ skip_prologue (pc, fdata)
 	  continue;
 
 	}
-      else if ((op & 0xffff0000) == lr_reg)
+      else if (lr_reg != -1 && (op & 0xffff0000) == lr_reg)
 	{			/* st Rx,NUM(r1) 
 				   where Rx == lr */
 	  fdata->lr_offset = SIGNED_SHORT (op) + offset;
@@ -400,7 +400,7 @@ skip_prologue (pc, fdata)
 	  continue;
 
 	}
-      else if ((op & 0xffff0000) == cr_reg)
+      else if (cr_reg != -1 && (op & 0xffff0000) == cr_reg)
 	{			/* st Rx,NUM(r1) 
 				   where Rx == cr */
 	  fdata->cr_offset = SIGNED_SHORT (op) + offset;
Index: config/powerpc/tm-linux.h
===================================================================
RCS file: /cvs/src/src/gdb/config/powerpc/tm-linux.h,v
retrieving revision 1.2
diff -u -p -r1.2 tm-linux.h
--- tm-linux.h	2000/02/22 18:47:41	1.2
+++ tm-linux.h	2000/02/24 22:46:56
@@ -93,6 +93,13 @@ CORE_ADDR ppc_sysv_abi_push_arguments PA
 #define PROLOGUE_FIRSTLINE_OVERLAP
 #endif
 
+/* Needed to handled the self-modifying code situation due to the dynamic
+   linker. */
+int ppc_linux_memory_remove_breakpoint (CORE_ADDR addr, char *contents_cache);
+#undef MEMORY_REMOVE_BREAKPOINT
+#define MEMORY_REMOVE_BREAKPOINT(addr, contents_cache) \
+  ppc_linux_memory_remove_breakpoint(addr, contents_cache)
+
 /* N_FUN symbols in shared libaries have 0 for their values and need
    to be relocated. */
 #define SOFUN_ADDRESS_MAYBE_MISSING