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

Thomas Schwinge
2012-02-29T11:05:52Z

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

Hi Kevin!

On Fri, 24 Feb 2012 14:46:57 -0700, Kevin Buettner <kevinb@redhat.com> wrote:
> On Fri, 24 Feb 2012 12:06:14 +0100
> Thomas Schwinge <thomas@codesourcery.com> wrote:
>
> > Working on your patch is the next step.
>
> Are you going to do that? Or did you want me to?
>
> (I'm okay with it either way...)

I now made your patch apply on top of the current sources (see below),
and tested it on both sh-linux-gnu hardware and sh-elf simulator.

As we already had figured out, it introduces the following nine
regressions (PASS -> FAIL):

FAIL: gdb.base/store.exp: continue to add_charest
FAIL: gdb.base/store.exp: continue to add_short
FAIL: gdb.base/store.exp: continue to add_int
FAIL: gdb.base/store.exp: continue to add_long
FAIL: gdb.base/store.exp: continue to add_longest
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-td-tf
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-td-tf
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tf-td
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tf-td

As already reported, an additional regression for sh-linux-gnu:

Running /scratch/tschwing/FM_sh-linux-gnu/src/gdb-mainline/gdb/testsuite/gdb.threads/staticthreads.exp ...
PASS: gdb.threads/staticthreads.exp: successfully compiled posix threads test case
PASS: gdb.threads/staticthreads.exp: set print sevenbit-strings
PASS: gdb.threads/staticthreads.exp: break sem_post
-PASS: gdb.threads/staticthreads.exp: Continue to main's call of sem_post
+FAIL: gdb.threads/staticthreads.exp: Continue to main's call of sem_post (the program exited)
PASS: gdb.threads/staticthreads.exp: rerun to main
PASS: gdb.threads/staticthreads.exp: handle SIG32 nostop noprint pass
-PASS: gdb.threads/staticthreads.exp: handle SIG32 helps
-PASS: gdb.threads/staticthreads.exp: info threads
+FAIL: gdb.threads/staticthreads.exp: handle SIG32 helps (the program exited)
+KFAIL: gdb.threads/staticthreads.exp: info threads (PRMS: gdb/1328)
PASS: gdb.threads/staticthreads.exp: GDB exits with static thread program

The breakpoint for sem_post is mis-calculated to be somewhere near the
end of the function (after the epilogue, even).

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.)

As for the other advancements that you reported, do you have any
additional patches in your tree; see the questions in my 2012-02-21
email, Message-ID: <8762f09stp.fsf@schwinge.name>.

I'll now try to carry over your ``more advanced limit_pc/func_end
mechanism'' separately.

Your patch on top of the current sources:

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 29 Feb 2012 09:34:32 -0000
@@ -534,39 +534,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))
{
@@ -579,11 +583,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))
{
@@ -642,6 +649,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))
{
@@ -653,17 +661,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))
@@ -697,7 +708,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
@@ -709,35 +723,29 @@ sh_analyze_prologue (struct gdbarch *gdb
#endif
}

- return pc;
+ return after_last_frame_setup_insn;
}

/* Skip any prologue before the guts of a function. */
static CORE_ADDR
sh_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
{
- CORE_ADDR post_prologue_pc, func_addr;
+ CORE_ADDR post_prologue_pc, sal_end, func_addr, func_end;
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))
- {
- post_prologue_pc = skip_prologue_using_sal (gdbarch, func_addr);
- if (post_prologue_pc != 0)
- return max (pc, post_prologue_pc);
- }
-
- /* Can't determine prologue from the symbol table, need to examine
- instructions. */
+ /* Try to find the extent of the function that contains PC. */
+ if (!find_pc_partial_function (pc, NULL, &func_addr, &func_end))
+ return pc;

cache.sp_offset = -4;
- post_prologue_pc = sh_analyze_prologue (gdbarch, pc, (CORE_ADDR) -1, &cache, 0);
- if (cache.uses_fp)
- pc = post_prologue_pc;
+ post_prologue_pc
+ = sh_analyze_prologue (gdbarch, func_addr, func_end, &cache, 0);

- return pc;
+ sal_end = skip_prologue_using_sal (gdbarch, pc);
+ if (sal_end != 0 && sal_end != pc && sal_end < post_prologue_pc)
+ return sal_end;
+ else
+ return post_prologue_pc;
}

/* The ABI says: