Re: [PATCH] [SH] Prologue skipping if there is none

[Thomas Schwinge <thomas@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 6168 bytes --]

Hi!

On Wed, 29 Feb 2012 12:05:01 +0100, I wrote:
> And, as already reported, there is this progression for sh-linux-gnu:
> 
>          -FAIL: gdb.base/gdb1250.exp: setting breakpoint at abort
>          +PASS: gdb.base/gdb1250.exp: backtrace from abort
> 
> The PLT stub for abort happens to be the last one in the .plt section,
> and (I suppose) your more advanced limit_pc/func_end mechanism (instead
> of hard-coding 28 instructions) helps to avoid hitting the
> end-of-.plt-section border.  (The question is whether it really makes
> sense to go looking for a prologue in a PLT stub, but that's what GDB is
> currently doing, and it should be without harm.)

Here is a patch to fix this ``break abort'' FAIL; no regressions for
sh-linux-gnu and sh-elf.

	* sh-tdep.c (sh_skip_prologue): Provide an upper limit on the function
	prologue to sh_analyze_prologue.
	(sh_analyze_prologue): Make better use of such an upper limit, and
	generally be more cautious about accessing memory.

Index: gdb/sh-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sh-tdep.c,v
retrieving revision 1.239
diff -u -p -r1.239 sh-tdep.c
--- gdb/sh-tdep.c	27 Feb 2012 16:40:48 -0000	1.239
+++ gdb/sh-tdep.c	1 Mar 2012 08:48:23 -0000
@@ -534,22 +534,18 @@ sh_breakpoint_from_pc (struct gdbarch *g
 
 static CORE_ADDR
 sh_analyze_prologue (struct gdbarch *gdbarch,
-		     CORE_ADDR pc, CORE_ADDR current_pc,
+		     CORE_ADDR pc, CORE_ADDR limit_pc,
 		     struct sh_frame_cache *cache, ULONGEST fpscr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   ULONGEST inst;
-  CORE_ADDR opc;
   int offset;
   int sav_offset = 0;
   int r3_val = 0;
   int reg, sav_reg = -1;
 
-  if (pc >= current_pc)
-    return current_pc;
-
   cache->uses_fp = 0;
-  for (opc = pc + (2 * 28); pc < opc; pc += 2)
+  for (; pc < limit_pc; pc += 2)
     {
       inst = read_memory_unsigned_integer (pc, 2, byte_order);
       /* See where the registers will be saved to.  */
@@ -594,6 +590,7 @@ sh_analyze_prologue (struct gdbarch *gdb
 		{
 		  sav_reg = reg;
 		  offset = (inst & 0xff) << 1;
+		  /* TODO: check that this is a valid address.	*/
 		  sav_offset =
 		    read_memory_integer ((pc + 4) + offset, 2, byte_order);
 		}
@@ -608,13 +605,15 @@ sh_analyze_prologue (struct gdbarch *gdb
 		{
 		  sav_reg = reg;
 		  offset = (inst & 0xff) << 2;
+		  /* TODO: check that this is a valid address.	*/
 		  sav_offset =
 		    read_memory_integer (((pc & 0xfffffffc) + 4) + offset,
 					 4, byte_order);
 		}
 	    }
 	}
-      else if (IS_MOVI20 (inst))
+      else if (IS_MOVI20 (inst)
+	       && (pc + 2 < limit_pc))
         {
 	  if (sav_reg < 0)
 	    {
@@ -623,14 +622,15 @@ sh_analyze_prologue (struct gdbarch *gdb
 	        {
 		  sav_reg = reg;
 		  sav_offset = GET_SOURCE_REG (inst) << 16;
-		  /* MOVI20 is a 32 bit instruction!  */
-		  pc += 2;
 		  sav_offset
-		    |= read_memory_unsigned_integer (pc, 2, byte_order);
+		    |= read_memory_unsigned_integer (pc + 2, 2, byte_order);
 		  /* Now sav_offset contains an unsigned 20 bit value.
 		     It must still get sign extended.  */
 		  if (sav_offset & 0x00080000)
 		    sav_offset |= 0xfff00000;
+
+		  /* MOVI20 is a 32-bit instruction.  */
+		  pc += 2;
 		}
 	    }
 	}
@@ -656,14 +656,16 @@ sh_analyze_prologue (struct gdbarch *gdb
 	}
       else if (IS_MOV_SP_FP (inst))
 	{
+	  pc += 2;
+	  limit_pc = min (limit_pc, pc + (2 * 6));	/* NUMERO MYSTERIOSO */
+
 	  cache->uses_fp = 1;
 	  /* At this point, only allow argument register moves to other
 	     registers or argument register moves to @(X,fp) which are
 	     moving the register arguments onto the stack area allocated
 	     by a former add somenumber to SP call.  Don't allow moving
 	     to an fp indirect address above fp + cache->sp_offset.  */
-	  pc += 2;
-	  for (opc = pc + 12; pc < opc; pc += 2)
+	  for (; pc < limit_pc; pc += 2)
 	    {
 	      inst = read_memory_integer (pc, 2, byte_order);
 	      if (IS_MOV_ARG_TO_IND_R14 (inst))
@@ -686,7 +688,8 @@ sh_analyze_prologue (struct gdbarch *gdb
 	    }
 	  break;
 	}
-      else if (IS_JSR (inst))
+      else if (IS_JSR (inst)
+	       && (pc + 2 < limit_pc))
 	{
 	  /* We have found a jsr that has been scheduled into the prologue.
 	     If we continue the scan and return a pc someplace after this,
@@ -716,13 +719,13 @@ sh_analyze_prologue (struct gdbarch *gdb
 static CORE_ADDR
 sh_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
 {
-  CORE_ADDR post_prologue_pc, func_addr;
+  CORE_ADDR post_prologue_pc, func_addr, func_end_addr, limit_pc;
   struct sh_frame_cache cache;
 
   /* 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))
+  if (find_pc_partial_function (pc, NULL, &func_addr, &func_end_addr))
     {
       post_prologue_pc = skip_prologue_using_sal (gdbarch, func_addr);
       if (post_prologue_pc != 0)
@@ -732,8 +735,20 @@ sh_skip_prologue (struct gdbarch *gdbarc
   /* 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 (gdbarch, pc);
+  if (limit_pc == 0)
+    limit_pc = pc + (2 * 28);				/* NUMERO MYSTERIOSO */
+
+  /* Do not allow limit_pc to be past the function end, if we know
+     where that end is...  */
+  if (func_end_addr != 0)
+    limit_pc = min (limit_pc, func_end_addr);
+
   cache.sp_offset = -4;
-  post_prologue_pc = sh_analyze_prologue (gdbarch, pc, (CORE_ADDR) -1, &cache, 0);
+  post_prologue_pc = sh_analyze_prologue (gdbarch, pc, limit_pc, &cache, 0);
   if (cache.uses_fp)
     pc = post_prologue_pc;
 


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]