[PATCH] skip_prologue() revisions in rs6000-tdep.c

Kevin Buettner kevinb@cygnus.com
Sat Feb 26 02:05:00 GMT 2000


I've just committed the patch below.

It consists of two parts.  The first is just a big comment explaining
the issues that lead to my recent addition of the function
ppc_linux_memory_remove_breakpoint() to ppc-linux-tdep.c.  (Andrew's
suggestion.)

The second consists of a number of revisions to skip_prologue().  The
most important revision (and the reason I was in this code this time
around) is the introduction of the notion of instructions which may
appear in the prologue, but which must not be the last instruction in
the prologue.  A good example of this is the nop instruction.  Consider
the disassembly of loop_count() from testsuite/gdb.base/call-ar-st.c:

    (gdb) x/20i loop_count
    0x100009b0 <loop_count>:        stwu    r1,-32(r1)
    0x100009b4 <loop_count+4>:      stw     r31,28(r1)
    0x100009b8 <loop_count+8>:      mr      r31,r1
    0x100009bc <loop_count+12>:     nop
    0x100009c0 <loop_count+16>:     li      r0,0
    0x100009c4 <loop_count+20>:     stw     r0,8(r31)
    0x100009c8 <loop_count+24>:     lwz     r0,8(r31)
    0x100009cc <loop_count+28>:     cmpwi   r0,3
    0x100009d0 <loop_count+32>:     ble     0x100009d8 <loop_count+40>
    0x100009d4 <loop_count+36>:     b       0x100009e8 <loop_count+56>
    0x100009d8 <loop_count+40>:     lwz     r9,8(r31)
    0x100009dc <loop_count+44>:     addi    r0,r9,1
    0x100009e0 <loop_count+48>:     stw     r0,8(r31)
    0x100009e4 <loop_count+52>:     b       0x100009c8 <loop_count+24>
    0x100009e8 <loop_count+56>:     lwz     r11,0(r1)
    0x100009ec <loop_count+60>:     lwz     r31,-4(r11)
    0x100009f0 <loop_count+64>:     mr      r1,r11
    0x100009f4 <loop_count+68>:     blr

Here's the source:

    void loop_count () {

	 int index;

	 for (index=0; index<4; index++);
    }

Prior to my change, the nop at loop_count+12 was considered part of the
prologue.  Yet, according to the line number info, this instruction
is the first instruction in the body.  On the powerpc, it is sometimes
necessary to use two instructions to load a constant into a register,
so the nop is actually just appart of the "index=0" assignment.

Anyway... since gdb was erroneously concluding that the nop was part
of the prologue, skip_prologue would end up returning the address of
the "li r0,0" which is part way through the first statement of the
function.  When you attempt to place a breakpoint on loop_count,
gdb will attempt to find the first executable statement after the
prologue.  If the prologue scanning code returns an address that
is part way through a statement, the first address of the next
statement is used.

As a result, the breakpoint was being placed on loop_count+56 which
is the epilogue of the function.  This is definitely wrong.

The solution is to allow certain instructions like nop to appear in
the prologue, but not permit them to form the final instruction in the
prologue.  (I think there are certain situations where a nop can
validly appear in the prologue.)  I expect that over time we'll
find other instructions that should be classified the same way.

Here are the ChangeLog entries and the diffs...

	* ppc-linux-tdep.c (ppc_linux_memory_remove_breakpoint): Add
	comment explaining motivation behind this function and why
	the generic facilities won't work for this platform.
	* rs6000-tdep.c (skip_prologue): Always test to make sure
	that an instruction is read successfully from the target's
	memory.  Introduce notion of instructions which may appear in
	the prologue, but must not terminate the prologue.  Added
	explicit check for nop instruction.  Use memset() to zero the
	frame data instead of assignment from a statically allocated,
	uninitialized structure.

Index: ppc-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-linux-tdep.c,v
retrieving revision 1.3
diff -u -p -r1.3 ppc-linux-tdep.c
--- ppc-linux-tdep.c	2000/02/24 23:06:48	1.3
+++ ppc-linux-tdep.c	2000/02/26 09:10:28
@@ -615,8 +615,130 @@ ppc_sysv_abi_push_arguments (nargs, args
   return sp;
 }
 
-/* This version of ppc_linux_memory_remove_breakpoints handles the
-   case of self modifying code */
+/* ppc_linux_memory_remove_breakpoints attempts to remove a breakpoint
+   in much the same fashion as memory_remove_breakpoint in mem-break.c,
+   but is careful not to write back the previous contents if the code
+   in question has changed in between inserting the breakpoint and
+   removing it.
+
+   Here is the problem that we're trying to solve...
+
+   Once upon a time, before introducing this function to remove
+   breakpoints from the inferior, setting a breakpoint on a shared
+   library function prior to running the program would not work
+   properly.  In order to understand the problem, it is first
+   necessary to understand a little bit about 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, an example is in
+   order; the following example is from the gdb testsuite.
+	    
+	We start the program shmain.
+
+	    [kev@arroyo testsuite]$ ../gdb gdb.base/shmain
+	    [...]
+
+	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.
+
+   Now the problem should be obvious.  GDB places 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 gets a signal indicating this fact and attempts
+   (as it always does when it stops) to remove all the breakpoints.
+
+   The breakpoint removal 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 problem was fairly straightforward.  The function
+   memory_remove_breakpoint from mem-break.c was copied to this file,
+   modified slightly, and renamed to ppc_linux_memory_remove_breakpoint.
+   In tm-linux.h, MEMORY_REMOVE_BREAKPOINT is defined 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.
+
+   The big question is whether memory_remove_breakpoint () should be
+   changed to have the same functionality.  The downside is 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, we'll leave this self-modifying-code-friendly
+   version in ppc-linux-tdep.c, but it ought to be migrated somewhere
+   else in the event that some other platform has similar needs with
+   regard to removing breakpoints in some potentially self modifying
+   code.  */
 int
 ppc_linux_memory_remove_breakpoint (CORE_ADDR addr, char *contents_cache)
 {
Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.4
diff -u -p -r1.4 rs6000-tdep.c
--- rs6000-tdep.c	2000/02/24 23:06:48	1.4
+++ rs6000-tdep.c	2000/02/26 09:10:31
@@ -305,11 +305,10 @@ rs6000_software_single_step (signal, ins
 #define GET_SRC_REG(x) (((x) >> 21) & 0x1f)
 
 CORE_ADDR
-skip_prologue (pc, fdata)
-     CORE_ADDR pc;
-     struct rs6000_framedata *fdata;
+skip_prologue (CORE_ADDR pc, struct rs6000_framedata *fdata)
 {
   CORE_ADDR orig_pc = pc;
+  CORE_ADDR last_prologue_pc;
   char buf[4];
   unsigned long op;
   long offset = 0;
@@ -318,24 +317,31 @@ skip_prologue (pc, fdata)
   int reg;
   int framep = 0;
   int minimal_toc_loaded = 0;
-  static struct rs6000_framedata zero_frame;
+  int prev_insn_was_prologue_insn = 1;
 
-  *fdata = zero_frame;
+  memset (fdata, 0, sizeof (struct rs6000_framedata));
   fdata->saved_gpr = -1;
   fdata->saved_fpr = -1;
   fdata->alloca_reg = -1;
   fdata->frameless = 1;
   fdata->nosavedpc = 1;
 
-  if (target_read_memory (pc, buf, 4))
-    return pc;			/* Can't access it -- assume no prologue. */
-
-  /* Assume that subsequent fetches can fail with low probability.  */
   pc -= 4;
   for (;;)
     {
       pc += 4;
-      op = read_memory_integer (pc, 4);
+
+      /* Sometimes it isn't clear if an instruction is a prologue
+         instruction or not.  When we encounter one of these ambiguous
+	 cases, we'll set prev_insn_was_prologue_insn to 0 (false).
+	 Otherwise, we'll assume that it really is a prologue instruction. */
+      if (prev_insn_was_prologue_insn)
+	last_prologue_pc = pc;
+      prev_insn_was_prologue_insn = 1;
+
+      if (target_read_memory (pc, buf, 4))
+	break;
+      op = extract_signed_integer (buf, 4);
 
       if ((op & 0xfc1fffff) == 0x7c0802a6)
 	{			/* mflr Rx */
@@ -375,6 +381,16 @@ skip_prologue (pc, fdata)
 	  continue;
 
 	}
+      else if ((op & 0xffff0000) == 0x60000000)
+        {
+	  			/* nop */
+	  /* Allow nops in the prologue, but do not consider them to
+	     be part of the prologue unless followed by other prologue
+	     instructions. */
+	  prev_insn_was_prologue_insn = 0;
+	  continue;
+
+	}
       else if ((op & 0xffff0000) == 0x3c000000)
 	{			/* addis 0,0,NUM, used
 				   for >= 32k frames */
@@ -564,7 +580,7 @@ skip_prologue (pc, fdata)
 #endif /* 0 */
 
   fdata->offset = -fdata->offset;
-  return pc;
+  return last_prologue_pc;
 }
 
 



More information about the Gdb-patches mailing list