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

Thomas Schwinge
2012-03-01T09:00:35Z

[-- 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;