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

[Kevin Buettner <kevinb at redhat dot com>]

On Wed, 15 Feb 2012 14:51:31 +0100
Thomas Schwinge <thomas@codesourcery.com> wrote:

> The prologue skipping issue is that GDB fails to place breakpoints
> correctly at the beginning of a function -- such as for ``break main'' --
> for the case that there is no prologue in that function.

Hi Thomas,

I've been sitting on a patch which is similar to what you just posted,
though I think it might provide more accurate results in some instances.
Would you mind checking to see if it solves your problem?

	* sh-tdep.c (sh_analyze_prologue): Change loop to run to
	the limit PC.  Keep track of the PC value after frame
	related instructions; return this value.
	(after_prologue): Delete.
	(sh_skip_prologue):  Find the function limit and pass that
	as the limit address to sh_analyze_prologue().  Also use
	skip_prologue_using_sal() and return the lower result.

	
Index: gdb/sh-tdep.c
===================================================================
RCS file: /cvs/cvsfiles/gnupro/gdb/sh-tdep.c,v
retrieving revision 1.21
diff -u -p -r1.21 sh-tdep.c
--- gdb/sh-tdep.c	2 Jul 2011 02:04:18 -0000	1.21
+++ gdb/sh-tdep.c	28 Oct 2011 00:27:21 -0000
@@ -520,39 +520,43 @@ 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;
+  CORE_ADDR after_last_frame_setup_insn = pc;
+  CORE_ADDR next_pc;
   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 = next_pc)
     {
       inst = read_memory_unsigned_integer (pc, 2, byte_order);
+      next_pc = pc + 2;
+
       /* See where the registers will be saved to.  */
       if (IS_PUSH (inst))
 	{
 	  cache->saved_regs[GET_SOURCE_REG (inst)] = cache->sp_offset;
 	  cache->sp_offset += 4;
+	  after_last_frame_setup_insn = next_pc;
 	}
       else if (IS_STS (inst))
 	{
 	  cache->saved_regs[PR_REGNUM] = cache->sp_offset;
 	  cache->sp_offset += 4;
+	  after_last_frame_setup_insn = next_pc;
 	}
       else if (IS_MACL_STS (inst))
 	{
 	  cache->saved_regs[MACL_REGNUM] = cache->sp_offset;
 	  cache->sp_offset += 4;
+	  after_last_frame_setup_insn = next_pc;
 	}
       else if (IS_MOV_R3 (inst))
 	{
@@ -565,11 +569,14 @@ sh_analyze_prologue (struct gdbarch *gdb
       else if (IS_ADD_R3SP (inst))
 	{
 	  cache->sp_offset += -r3_val;
+	  after_last_frame_setup_insn = next_pc;
 	}
       else if (IS_ADD_IMM_SP (inst))
 	{
 	  offset = ((inst & 0xff) ^ 0x80) - 0x80;
 	  cache->sp_offset -= offset;
+	  if (offset < 0)
+	    after_last_frame_setup_insn = next_pc;
 	}
       else if (IS_MOVW_PCREL_TO_REG (inst))
 	{
@@ -628,6 +635,7 @@ sh_analyze_prologue (struct gdbarch *gdb
 	      sav_reg = -1;
 	    }
 	  cache->sp_offset += sav_offset;
+	  after_last_frame_setup_insn = next_pc;
 	}
       else if (IS_FPUSH (inst))
 	{
@@ -639,17 +647,20 @@ sh_analyze_prologue (struct gdbarch *gdb
 	    {
 	      cache->sp_offset += 4;
 	    }
+	  after_last_frame_setup_insn = next_pc;
 	}
       else if (IS_MOV_SP_FP (inst))
 	{
+	  CORE_ADDR opc;
 	  cache->uses_fp = 1;
+	  after_last_frame_setup_insn = next_pc;
 	  /* 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 (opc = pc + 12; pc < opc && pc < limit_pc; pc += 2)
 	    {
 	      inst = read_memory_integer (pc, 2, byte_order);
 	      if (IS_MOV_ARG_TO_IND_R14 (inst))
@@ -683,7 +694,10 @@ sh_analyze_prologue (struct gdbarch *gdb
 	     so, note that before returning the current pc.  */
 	  inst = read_memory_integer (pc + 2, 2, byte_order);
 	  if (IS_MOV_SP_FP (inst))
-	    cache->uses_fp = 1;
+	    {
+	      cache->uses_fp = 1;
+	      after_last_frame_setup_insn = pc;
+	    }
 	  break;
 	}
 #if 0		/* This used to just stop when it found an instruction
@@ -695,61 +709,30 @@ sh_analyze_prologue (struct gdbarch *gdb
 #endif
     }
 
-  return pc;
+  return after_last_frame_setup_insn;
 }

 /* Skip any prologue before the guts of a function.  */
 
-/* Skip the prologue using the debug information.  If this fails we'll
-   fall back on the 'guess' method below.  */
-static CORE_ADDR
-after_prologue (CORE_ADDR pc)
-{
-  struct symtab_and_line sal;
-  CORE_ADDR func_addr, func_end;
-
-  /* If we can not find the symbol in the partial symbol table, then
-     there is no hope we can determine the function's start address
-     with this code.  */
-  if (!find_pc_partial_function (pc, NULL, &func_addr, &func_end))
-    return 0;
-
-  /* Get the line associated with FUNC_ADDR.  */
-  sal = find_pc_line (func_addr, 0);
-
-  /* There are only two cases to consider.  First, the end of the source line
-     is within the function bounds.  In that case we return the end of the
-     source line.  Second is the end of the source line extends beyond the
-     bounds of the current function.  We need to use the slow code to
-     examine instructions in that case.  */
-  if (sal.end < func_end)
-    return sal.end;
-  else
-    return 0;
-}
-
 static CORE_ADDR
 sh_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR start_pc)
 {
-  CORE_ADDR pc;
+  CORE_ADDR pc, sal_end, func_addr, func_end;
   struct sh_frame_cache cache;
+  char *name;
 
-  /* 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.  */
-  pc = after_prologue (start_pc);
-
-  /* If after_prologue returned a useful address, then use it.  Else
-     fall back on the instruction skipping code.  */
-  if (pc)
-    return max (pc, start_pc);
+  /* Try to find the extent of the function that contains PC.  */
+  if (!find_pc_partial_function (start_pc, &name, &func_addr, &func_end))
+    return start_pc;
 
   cache.sp_offset = -4;
-  pc = sh_analyze_prologue (gdbarch, start_pc, (CORE_ADDR) -1, &cache, 0);
-  if (!cache.uses_fp)
-    return start_pc;
+  pc = sh_analyze_prologue (gdbarch, func_addr, func_end, &cache, 0);
 
-  return pc;
+  sal_end = skip_prologue_using_sal (gdbarch, start_pc);
+  if (sal_end != 0 && sal_end != start_pc && sal_end < pc)
+    return sal_end;
+  else
+    return pc;
 }
 
 /* The ABI says: