* [PATCH] [SH] Prologue skipping if there is none
@ 2012-02-15 14:00 Thomas Schwinge
2012-02-15 14:54 ` Pedro Alves
2012-02-15 16:09 ` [PATCH] [SH] Prologue skipping if there is none Kevin Buettner
0 siblings, 2 replies; 34+ messages in thread
From: Thomas Schwinge @ 2012-02-15 14:00 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 15261 bytes --]
Hi!
(No specific SH maintainer CCed as none is listed in MAINTAINERS.)
First, for SH GNU/Linux, when running the testsuite via gdbserver on
pristine sources, I'm getting a large number of ERRORs, in the 350s. Is
that normal or at least known? They're basically all of the kind
``ERROR: Process no longer exists'', and then follow-up errors until the
end of the specific testcase. From a very quick glance, they all seem to
be happening after the testcase has instructed GDB to invoke a function,
such as:
(gdb) PASS: gdb.cp/classes.exp: print g_D.p
call class_param.Aptr_a (&g_A)
ERROR: Process no longer exists
UNRESOLVED: gdb.cp/classes.exp: call class_param.Aptr_a (&g_A)
ERROR: Couldn't send call class_param.Aptr_x (&g_A) to GDB.
This probably suggests where to begin looking unless that's know already.
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. In such a case,
it will currently place the breakpoint *after* the first source code line
instead of at its beginning, for example ``break main'' for
gdb.base/arrayidx.c will put a breakpoint on line 25 instead of 23:
20 int
21 main (void)
22 {
23 array[0] = 5;
24
25 return 0;
26 }
Dump of assembler code for function main:
0x00400600 <+0>: mov.l 0x400610 <main+16>,r1 ! 0x41083c <array>
0x00400602 <+2>: mov #5,r2
0x00400604 <+4>: mov.l r2,@r1
0x00400606 <+6>: mov #0,r1
0x00400608 <+8>: mov r1,r0
0x0040060a <+10>: rts
0x0040060c <+12>: nop
0x0040060e <+14>: nop
0x00400610 <+16>: mov.b @(r0,r3),r8
0x00400612 <+18>: .word 0x0041
End of assembler dump.
Crude log of debugging the SH GNU/Linux GDB when advising it to ``break
main'':
Breakpoint 3, sh_skip_prologue (gdbarch=0x90043c8, start_pc=4195840)
OK, this is start_pc == 0x400600.
715 if (!find_pc_partial_function (pc, NULL, &func_addr, &func_end))
Value returned is $3 = 1
(gdb) print/x func_addr
$4 = 0x400600
(gdb) print/x func_end
$5 = 0x400614
OK.
2137 return find_pc_sect_line (pc, section, notcurrent);
Value returned is $8 = {pspace = 0x8fed340, symtab = 0x901a21c, section = 0x0, line = 23, pc = 4195840, end = 4195846, explicit_pc = 0, explicit_line = 0}
OK, still $8->pc == 0x400600, and $8->end == 0x400606, and these are
attached to line 23, ``array[0] = 5;''.
after_prologue (pc=4195840) at /scratch/tschwing/ST_sh-linux-gnu-lite/obj/gdb-src-2012.03-999999-sh-linux-gnu-i686-pc-linux-gnu/gdb/sh-tdep.c:726
726 if (sal.end < func_end)
(gdb) n
727 return sal.end;
(gdb) print/x sal.end
$9 = 0x400606
Wrong, because we now have not skipped the prologue, but the first real
instruction.
Anyway, apart from the 350 ERRORs (with and without my patch), there is a
clear tendency of improvement, and no regressions:
Progressions and removed failures:
FAIL -> PASS: default/gdb.sum:gdb.base/arrayidx.exp: Print array with array-indexes off
FAIL -> PASS: default/gdb.sum:gdb.base/arrayidx.exp: Print array with array-indexes on
FAIL -> PASS: default/gdb.sum:gdb.base/break-always.exp: continue to breakpoint: bar
FAIL -> PASS: default/gdb.sum:gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tc
FAIL -> PASS: default/gdb.sum:gdb.base/call-sc.exp: advance to fun for finish; return call-sc-td
FAIL -> PASS: default/gdb.sum:gdb.base/call-sc.exp: advance to fun for finish; return call-sc-te
FAIL -> PASS: default/gdb.sum:gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tf
FAIL -> PASS: default/gdb.sum:gdb.base/call-sc.exp: advance to fun for finish; return call-sc-ti
FAIL -> PASS: default/gdb.sum:gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tl
FAIL -> PASS: default/gdb.sum:gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tld
FAIL -> PASS: default/gdb.sum:gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tll
FAIL -> PASS: default/gdb.sum:gdb.base/call-sc.exp: advance to fun for finish; return call-sc-ts
FAIL -> PASS: default/gdb.sum:gdb.base/call-sc.exp: advance to fun for return; return call-sc-tc
FAIL -> PASS: default/gdb.sum:gdb.base/call-sc.exp: advance to fun for return; return call-sc-td
FAIL -> PASS: default/gdb.sum:gdb.base/call-sc.exp: advance to fun for return; return call-sc-te
FAIL -> PASS: default/gdb.sum:gdb.base/call-sc.exp: advance to fun for return; return call-sc-tf
FAIL -> PASS: default/gdb.sum:gdb.base/call-sc.exp: advance to fun for return; return call-sc-ti
FAIL -> PASS: default/gdb.sum:gdb.base/call-sc.exp: advance to fun for return; return call-sc-tl
FAIL -> PASS: default/gdb.sum:gdb.base/call-sc.exp: advance to fun for return; return call-sc-tld
FAIL -> PASS: default/gdb.sum:gdb.base/call-sc.exp: advance to fun for return; return call-sc-tll
FAIL -> PASS: default/gdb.sum:gdb.base/call-sc.exp: advance to fun for return; return call-sc-ts
FAIL -> PASS: default/gdb.sum:gdb.base/gdb11531.exp: watchpoint variable triggers at next
FAIL -> PASS: default/gdb.sum:gdb.base/reread.exp: breakpoint foo in first file
FAIL -> PASS: default/gdb.sum:gdb.base/reread.exp: run to foo()
FAIL -> PASS: default/gdb.sum:gdb.base/return.exp: continue to return of -5
FAIL -> PASS: default/gdb.sum:gdb.base/return.exp: continue to return of -5.0
FAIL -> PASS: default/gdb.sum:gdb.base/return2.exp: void function returned successfully
FAIL -> PASS: default/gdb.sum:gdb.base/scope.exp: print funclocal at bar
FAIL -> PASS: default/gdb.sum:gdb.base/scope.exp: print funclocal_bss at bar
FAIL -> PASS: default/gdb.sum:gdb.base/skip.exp: step after deleting 1 (3)
FAIL -> PASS: default/gdb.sum:gdb.base/skip.exp: step after disabling 3 (5)
FAIL -> PASS: default/gdb.sum:gdb.base/skip.exp: step after enable 3 (3)
FAIL -> PASS: default/gdb.sum:gdb.base/step-resume-infcall.exp: step
FAIL -> PASS: default/gdb.sum:gdb.base/step-test.exp: step into
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tc
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-td
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tf
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-ti
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tl
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tld
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tll
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-ts
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tc-ti
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tc-tl
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tf
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tf-tc
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-ti
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-ti-tc
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tl
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tl-tc
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tc
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-td
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tf
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-ti
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tl
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tld
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tll
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-ts
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tc-ti
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tc-tl
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tf
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tf-tc
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-ti
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-ti-tc
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tl
FAIL -> PASS: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tl-tc
FAIL -> PASS: default/gdb.sum:gdb.base/value-double-free.exp: continue
FAIL -> PASS: default/gdb.sum:gdb.base/watch-cond-infcall.exp: sw: continue
FAIL -> PASS: default/gdb.sum:gdb.base/watchpoints.exp: Watchpoint hit count is 2
FAIL -> PASS: default/gdb.sum:gdb.base/watchpoints.exp: Watchpoint hit count is 3
FAIL -> PASS: default/gdb.sum:gdb.base/watchpoints.exp: Watchpoint hit count is 4
FAIL -> PASS: default/gdb.sum:gdb.base/watchpoints.exp: Watchpoint hit count is 5
FAIL -> PASS: default/gdb.sum:gdb.base/watchpoints.exp: Watchpoint ival1 hit count is 2
FAIL -> PASS: default/gdb.sum:gdb.base/watchpoints.exp: Watchpoint ival1 hit count is 3
FAIL -> PASS: default/gdb.sum:gdb.base/watchpoints.exp: watchpoint hit, fifth time
FAIL -> PASS: default/gdb.sum:gdb.base/watchpoints.exp: watchpoint hit, fourth time
FAIL -> PASS: default/gdb.sum:gdb.base/watchpoints.exp: watchpoint hit, second time
FAIL -> PASS: default/gdb.sum:gdb.base/watchpoints.exp: watchpoint hit, third time
FAIL -> PASS: default/gdb.sum:gdb.base/watchpoints.exp: watchpoint ival1 hit, second time
FAIL -> PASS: default/gdb.sum:gdb.base/watchpoints.exp: watchpoint ival1 hit, third time
FAIL -> PASS: default/gdb.sum:gdb.cp/expand-sals.exp: continue to breakpoint: func
FAIL -> PASS: default/gdb.sum:gdb.cp/expand-sals.exp: continue to breakpoint: next caller func
FAIL -> PASS: default/gdb.sum:gdb.cp/extern-c.exp: continue to breakpoint: c_func
FAIL -> PASS: default/gdb.sum:gdb.cp/extern-c.exp: continue to breakpoint: c_funcs_1
FAIL -> PASS: default/gdb.sum:gdb.cp/extern-c.exp: continue to breakpoint: c_funcs_2
FAIL -> PASS: default/gdb.sum:gdb.cp/namespace-nested-import.exp: print C::x
FAIL -> PASS: default/gdb.sum:gdb.linespec/linespec.exp: set breakpoint at lspec.cc instance of NameSpace::overload
FAIL -> PASS: default/gdb.sum:gdb.linespec/linespec.exp: set breakpoint at specific instance of NameSpace::overload
Removed FAIL: default/gdb.sum:gdb.base/chng-syms.exp: continue until exit at breakpoint first time through (the program is no longer running)
Removed FAIL: default/gdb.sum:gdb.base/chng-syms.exp: running to stop_here first time
The patch is modeled after what most of all other targets are doing for
getting past the prologue (if there is one), and have been changed
accordingly before, such as in Git commits
93afe5e97284a3d460c19f72f096c4f561673e48,
42294ab40a733dbca9258929d0a6ec72e3059fbc,
5c176a3a457b50fb047477d5e32bdde5ca2dd267, for example.
Probably the very same patch should be applied to sh64-tdep.c, but I
don't have any possibility for testing that. Shall I to do that change
``blindly''?
2012-02-15 Thomas Schwinge <thomas@codesourcery.com>
* sh-tdep.c (sh_skip_prologue): Use skip_prologue_using_sal.
(after_prologue): Remove.
Index: gdb/sh-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sh-tdep.c,v
retrieving revision 1.236
diff -u -p -r1.236 sh-tdep.c
--- gdb/sh-tdep.c 28 Jan 2012 18:08:20 -0000 1.236
+++ gdb/sh-tdep.c 15 Feb 2012 13:08:39 -0000
@@ -697,50 +697,25 @@ sh_analyze_prologue (struct gdbarch *gdb
}
/* 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, func_addr;
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. */
- pc = after_prologue (start_pc);
+ if (find_pc_partial_function (start_pc, NULL, &func_addr, NULL))
+ {
+ CORE_ADDR post_prologue_pc
+ = skip_prologue_using_sal (gdbarch, func_addr);
+ if (post_prologue_pc != 0)
+ return max (start_pc, post_prologue_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);
+ /* Can't determine prologue from the symbol table, need to examine
+ instructions. */
cache.sp_offset = -4;
pc = sh_analyze_prologue (gdbarch, start_pc, (CORE_ADDR) -1, &cache, 0);
Grüße,
Thomas
[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-02-15 14:00 [PATCH] [SH] Prologue skipping if there is none Thomas Schwinge
@ 2012-02-15 14:54 ` Pedro Alves
2012-02-16 15:27 ` [PATCH] [SH] GDB crash in sh_is_renesas_calling_convention, TYPE_CALLING_CONVENTION (was: Prologue skipping if there is none) Thomas Schwinge
2012-02-15 16:09 ` [PATCH] [SH] Prologue skipping if there is none Kevin Buettner
1 sibling, 1 reply; 34+ messages in thread
From: Pedro Alves @ 2012-02-15 14:54 UTC (permalink / raw)
To: Thomas Schwinge; +Cc: gdb-patches
Hey Thomas,
On 02/15/2012 01:51 PM, Thomas Schwinge wrote:
> First, for SH GNU/Linux, when running the testsuite via gdbserver on
> pristine sources, I'm getting a large number of ERRORs, in the 350s. Is
> that normal or at least known? They're basically all of the kind
> ``ERROR: Process no longer exists'', and then follow-up errors until the
> end of the specific testcase. From a very quick glance, they all seem to
> be happening after the testcase has instructed GDB to invoke a function,
> such as:
>
> (gdb) PASS: gdb.cp/classes.exp: print g_D.p
> call class_param.Aptr_a (&g_A)
> ERROR: Process no longer exists
> UNRESOLVED: gdb.cp/classes.exp: call class_param.Aptr_a (&g_A)
> ERROR: Couldn't send call class_param.Aptr_x (&g_A) to GDB.
>
> This probably suggests where to begin looking unless that's know already.
This means that GDB crashed. Just run the test with
$ ulimit -c unlimited
$ make check RUNTESTFLAGS="classes.exp"
so that the crash ends up creating a core, and you'll probably find that
all the ERRORs are caused by a single bug.
--
Pedro Alves
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-02-15 14:00 [PATCH] [SH] Prologue skipping if there is none Thomas Schwinge
2012-02-15 14:54 ` Pedro Alves
@ 2012-02-15 16:09 ` Kevin Buettner
2012-02-16 0:13 ` Kevin Buettner
1 sibling, 1 reply; 34+ messages in thread
From: Kevin Buettner @ 2012-02-15 16:09 UTC (permalink / raw)
To: gdb-patches
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:
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-02-15 16:09 ` [PATCH] [SH] Prologue skipping if there is none Kevin Buettner
@ 2012-02-16 0:13 ` Kevin Buettner
2012-02-16 16:59 ` Thomas Schwinge
0 siblings, 1 reply; 34+ messages in thread
From: Kevin Buettner @ 2012-02-16 0:13 UTC (permalink / raw)
To: gdb-patches
On Wed, 15 Feb 2012 07:54:13 -0700
Kevin Buettner <kevinb@redhat.com> wrote:
> 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?
Below is an updated patch which builds cleanly against current
sources. I've verified that it produces better test results than not
having the patch. I have not compared the test results to Thomas'
patch.
I ran my tests against the simulator. I found that a sim patch is
necessary to do so. I'll post that in a moment.
Kevin
* 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: sh-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sh-tdep.c,v
retrieving revision 1.236
diff -u -p -r1.236 sh-tdep.c
--- sh-tdep.c 28 Jan 2012 18:08:20 -0000 1.236
+++ sh-tdep.c 15 Feb 2012 23:55:14 -0000
@@ -518,39 +518,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))
{
@@ -563,11 +567,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))
{
@@ -626,6 +633,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))
{
@@ -637,17 +645,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))
@@ -681,7 +692,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
@@ -693,61 +707,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;
+ const 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:
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH] [SH] GDB crash in sh_is_renesas_calling_convention, TYPE_CALLING_CONVENTION (was: Prologue skipping if there is none)
2012-02-15 14:54 ` Pedro Alves
@ 2012-02-16 15:27 ` Thomas Schwinge
2012-02-16 19:38 ` [PATCH] [SH] GDB crash in sh_is_renesas_calling_convention, TYPE_CALLING_CONVENTION Tom Tromey
0 siblings, 1 reply; 34+ messages in thread
From: Thomas Schwinge @ 2012-02-16 15:27 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 4520 bytes --]
Hi!
On Wed, 15 Feb 2012 14:00:15 +0000, Pedro Alves <palves@redhat.com> wrote:
> On 02/15/2012 01:51 PM, Thomas Schwinge wrote:
>
> > First, for SH GNU/Linux, when running the testsuite via gdbserver on
> > pristine sources, I'm getting a large number of ERRORs, in the 350s. Is
> > that normal or at least known? They're basically all of the kind
> > ``ERROR: Process no longer exists'', and then follow-up errors until the
> > end of the specific testcase. From a very quick glance, they all seem to
> > be happening after the testcase has instructed GDB to invoke a function,
> > such as:
> >
> > (gdb) PASS: gdb.cp/classes.exp: print g_D.p
> > call class_param.Aptr_a (&g_A)
> > ERROR: Process no longer exists
> > UNRESOLVED: gdb.cp/classes.exp: call class_param.Aptr_a (&g_A)
> > ERROR: Couldn't send call class_param.Aptr_x (&g_A) to GDB.
> >
> > This probably suggests where to begin looking unless that's know already.
>
> This means that GDB crashed. Just run the test with
>
> $ ulimit -c unlimited
> $ make check RUNTESTFLAGS="classes.exp"
>
> so that the crash ends up creating a core, and you'll probably find that
> all the ERRORs are caused by a single bug.
Indeed:
Core was generated by `sh-linux-gnu-gdb -nw -nx -data-directory /scratch/tschwing/FM_sh-linux-gnu/obj/'.
Program terminated with signal 11, Segmentation fault.
#0 0x0804c239 in sh_is_renesas_calling_convention (func_type=0xa07b154) at /scratch/tschwing/FM_sh-linux-gnu/obj/gdb-src-mainline-0-sh-linux-gnu-i686-pc-linux-gnu/gdb/sh-tdep.c:92
92 return ((func_type
(gdb) print *func_type
$1 = {pointer_type = 0x0, reference_type = 0x0, chain = 0xa07b154, instance_flags = 0, length = 1, main_type = 0xa07b16c}
(gdb) list
87 };
88
89 static int
90 sh_is_renesas_calling_convention (struct type *func_type)
91 {
92 return ((func_type
93 && TYPE_CALLING_CONVENTION (func_type) == DW_CC_GNU_renesas_sh)
94 || sh_active_calling_convention == sh_cc_renesas);
95 }
96
gdb/gdbtypes.h:
#define TYPE_CALLING_CONVENTION(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->calling_convention
(gdb) print *func_type->main_type
$2 = {code = TYPE_CODE_METHOD, flag_unsigned = 0, flag_nosign = 0, flag_stub = 0, flag_target_stub = 0, flag_static = 0, flag_prototyped = 0, flag_incomplete = 0, flag_varargs = 0, flag_vector = 0, flag_stub_supported = 0,
flag_gnu_ifunc = 0, flag_fixed_instance = 0, flag_objfile_owned = 1, flag_declared_class = 0, flag_flag_enum = 0, type_specific_field = TYPE_SPECIFIC_NONE, nfields = 1, vptr_fieldno = 0, name = 0x0, tag_name = 0x0, owner = {
objfile = 0xa0673b8, gdbarch = 0xa0673b8}, target_type = 0xa07a87c, flds_bnds = {fields = 0xa07b1dc, bounds = 0xa07b1dc}, vptr_basetype = 0xa06eea8, type_specific = {cplus_stuff = 0x0, gnat_stuff = 0x0, floatformat = 0x0,
func_stuff = 0x0}}
(gdb) print func_type->main_type->type_specific.func_stuff
$3 = (struct func_type *) 0x0
The cure is the same as has been applied before,
<http://sourceware.org/ml/gdb-patches/2011-11/msg00439.html>.
gdb/
2012-02-16 Thomas Schwinge <thomas@codesourcery.com>
* sh-tdep.c (sh_is_renesas_calling_convention): Fix handling of
TYPE_CALLING_CONVENTION annotation.
Index: gdb/sh-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sh-tdep.c,v
retrieving revision 1.236
diff -u -p -r1.236 sh-tdep.c
--- gdb/sh-tdep.c 28 Jan 2012 18:08:20 -0000 1.236
+++ gdb/sh-tdep.c 16 Feb 2012 15:13:53 -0000
@@ -89,9 +89,24 @@ struct sh_frame_cache
static int
sh_is_renesas_calling_convention (struct type *func_type)
{
- return ((func_type
- && TYPE_CALLING_CONVENTION (func_type) == DW_CC_GNU_renesas_sh)
- || sh_active_calling_convention == sh_cc_renesas);
+ int val = 0;
+
+ if (func_type)
+ {
+ func_type = check_typedef (func_type);
+
+ if (TYPE_CODE (func_type) == TYPE_CODE_PTR)
+ func_type = check_typedef (TYPE_TARGET_TYPE (func_type));
+
+ if (TYPE_CODE (func_type) == TYPE_CODE_FUNC
+ && TYPE_CALLING_CONVENTION (func_type) == DW_CC_GNU_renesas_sh)
+ val = 1;
+ }
+
+ if (sh_active_calling_convention == sh_cc_renesas)
+ val = 1;
+
+ return val;
}
static const char *
Grüße,
Thomas
[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-02-16 0:13 ` Kevin Buettner
@ 2012-02-16 16:59 ` Thomas Schwinge
2012-02-17 2:30 ` Kevin Buettner
0 siblings, 1 reply; 34+ messages in thread
From: Thomas Schwinge @ 2012-02-16 16:59 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 10770 bytes --]
Hi!
On Wed, 15 Feb 2012 16:59:07 -0700, Kevin Buettner <kevinb@redhat.com> wrote:
> On Wed, 15 Feb 2012 07:54:13 -0700
> Kevin Buettner <kevinb@redhat.com> wrote:
>
> > 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.
> >
> > 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?
>
> Below is an updated patch which builds cleanly against current
> sources. I've verified that it produces better test results than not
> having the patch. I have not compared the test results to Thomas'
> patch.
I now have (on a SH7785-based board). My patch fixes a few more failures
than yours. ;-P
Here's the difference from mine to yours:
Regressions and new failures:
PASS -> FAIL: default/gdb.sum:gdb.base/store.exp: continue to add_charest
PASS -> FAIL: default/gdb.sum:gdb.base/store.exp: continue to add_double
PASS -> FAIL: default/gdb.sum:gdb.base/store.exp: continue to add_doublest
PASS -> FAIL: default/gdb.sum:gdb.base/store.exp: continue to add_float
PASS -> FAIL: default/gdb.sum:gdb.base/store.exp: continue to add_int
PASS -> FAIL: default/gdb.sum:gdb.base/store.exp: continue to add_long
PASS -> FAIL: default/gdb.sum:gdb.base/store.exp: continue to add_longest
PASS -> FAIL: default/gdb.sum:gdb.base/store.exp: continue to add_short
PASS -> FAIL: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-td-tf
PASS -> FAIL: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tf-td
PASS -> FAIL: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-td-tf
PASS -> FAIL: default/gdb.sum:gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tf-td
New FAIL: default/gdb.sum:gdb.threads/staticthreads.exp: Continue to main's call of sem_post (the program exited)
New FAIL: default/gdb.sum:gdb.threads/staticthreads.exp: handle SIG32 helps (the program exited)
Looking at the first one:
(gdb) PASS: gdb.base/store.exp: var doublest l; print incremented l, expecting 2
tbreak add_charest
Temporary breakpoint 10 at 0x400720: file /scratch/tschwing/FM_sh-linux-gnu/src/gdb-mainline/gdb/testsuite/gdb.base/store.c, line 14.
(gdb) PASS: gdb.base/store.exp: tbreak add_charest
continue
Continuing.
Temporary breakpoint 10, add_charest (u=-1 '\377', v=32 ' ') at /scratch/tschwing/FM_sh-linux-gnu/src/gdb-mainline/gdb/testsuite/gdb.base/store.c:14
14 {
(gdb) FAIL: gdb.base/store.exp: continue to add_charest
gdb.base/store.c:
10 typedef signed char charest;
11
12 charest
13 add_charest (register charest u, register charest v)
14 {
15 return u + v;
16 }
So the ``tbreak add_charest'' chose line 14 instead of 15.
Just some quick review comments without too much detail; basically me
thinking aloud a bit...
> * 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: sh-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/sh-tdep.c,v
> retrieving revision 1.236
> diff -u -p -r1.236 sh-tdep.c
> --- sh-tdep.c 28 Jan 2012 18:08:20 -0000 1.236
> +++ sh-tdep.c 15 Feb 2012 23:55:14 -0000
> @@ -518,39 +518,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;
Save the original pc instead of having the caller do that.
> + 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;
That sets cache->uses_fp to false even in case that the limit_pc (was:
current_pc) was already reached.
> - for (opc = pc + (2 * 28); pc < opc; pc += 2)
> +
> + for (;pc < limit_pc; pc = next_pc)
The limit of 28 instructions is no longer checked here, but is now the
caller's responsibility. (Are all callers doing the right thing?)
> {
> 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))
> {
> @@ -563,11 +567,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))
> {
> @@ -626,6 +633,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))
> {
> @@ -637,17 +645,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))
> @@ -681,7 +692,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;
> + }
Shouldn't this perhaps be pc + 2?
> break;
> }
> #if 0 /* This used to just stop when it found an instruction
> @@ -693,61 +707,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;
> + const 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;
Now start_pc is directly returned if find_pc_partial_function fails,
without invoking sh_analyze_prologue. Is that right?
>
> 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);
I had func_addr here, instead of start_pc.
> + if (sal_end != 0 && sal_end != start_pc && sal_end < pc)
> + return sal_end;
> + else
> + return pc;
> }
>
> /* The ABI says:
Grüße,
Thomas
[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] GDB crash in sh_is_renesas_calling_convention, TYPE_CALLING_CONVENTION
2012-02-16 15:27 ` [PATCH] [SH] GDB crash in sh_is_renesas_calling_convention, TYPE_CALLING_CONVENTION (was: Prologue skipping if there is none) Thomas Schwinge
@ 2012-02-16 19:38 ` Tom Tromey
0 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2012-02-16 19:38 UTC (permalink / raw)
To: Thomas Schwinge; +Cc: Pedro Alves, gdb-patches
>>>>> "Thomas" == Thomas Schwinge <thomas@codesourcery.com> writes:
Thomas> 2012-02-16 Thomas Schwinge <thomas@codesourcery.com>
Thomas> * sh-tdep.c (sh_is_renesas_calling_convention): Fix handling of
Thomas> TYPE_CALLING_CONVENTION annotation.
Looks good to me. This is ok.
Tom
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-02-16 16:59 ` Thomas Schwinge
@ 2012-02-17 2:30 ` Kevin Buettner
2012-02-20 16:19 ` Thomas Schwinge
` (2 more replies)
0 siblings, 3 replies; 34+ messages in thread
From: Kevin Buettner @ 2012-02-17 2:30 UTC (permalink / raw)
To: gdb-patches
Hi Thomas,
On Thu, 16 Feb 2012 17:32:18 +0100
Thomas Schwinge <thomas@codesourcery.com> wrote:
> I now have (on a SH7785-based board). My patch fixes a few more failures
> than yours. ;-P
This will require more study and discussion then. I tested against
the simulator using the default multilib. I compared results using
each of our patches to an unpatched sh-tdep.c.
Here are the FAILs that my patch fixed. There are 246 of them.
FAIL: gdb.arch/gdb1291.exp: set breakpoint
FAIL: gdb.arch/gdb1291.exp: get to sub1 (the program exited)
FAIL: gdb.arch/gdb1291.exp: backtrace with local variable less than or equal to 256 bytes
FAIL: gdb.arch/gdb1291.exp: set breakpoint
FAIL: gdb.arch/gdb1291.exp: get to sub2 (the program is no longer running)
FAIL: gdb.arch/gdb1291.exp: backtrace with local variable larger than 256 bytes
FAIL: gdb.arch/gdb1431.exp: get to sub1
FAIL: gdb.arch/gdb1431.exp: advance returns from sub1 frame
FAIL: gdb.arch/gdb1431.exp: get to sub2
FAIL: gdb.base/arrayidx.exp: Print array with array-indexes off
FAIL: gdb.base/arrayidx.exp: Print array with array-indexes on
FAIL: gdb.base/break-always.exp: continue to breakpoint: bar
FAIL: gdb.base/break-inline.exp: start
FAIL: gdb.base/break.exp: next over recursive call
FAIL: gdb.base/break.exp: backtrace from factorial(5.1)
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tc
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tc
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-ts
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-ts
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-ti
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-ti
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tl
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tl
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tll
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tll
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tf
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tf
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-td
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-td
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tld
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tld
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-te
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-te
FAIL: gdb.base/chng-syms.exp: running to stop_here first time
FAIL: gdb.base/chng-syms.exp: continue until exit at breakpoint first time through (the program is no longer running)
FAIL: gdb.base/gdb11531.exp: watchpoint variable triggers at next
FAIL: gdb.base/longjmp.exp: next over setjmp (1)
FAIL: gdb.base/longjmp.exp: next to longjmp (1)
FAIL: gdb.base/longjmp.exp: next over setjmp (2)
FAIL: gdb.base/nodebug.exp: backtrace from inner in nodebug.exp
FAIL: gdb.base/nodebug.exp: backtrace from middle in nodebug.exp
FAIL: gdb.base/pc-fp.exp: get hexadecimal valueof "$fp" (timeout)
FAIL: gdb.base/pc-fp.exp: display/i $pc
FAIL: gdb.base/reread.exp: breakpoint foo in first file
FAIL: gdb.base/reread.exp: run to foo()
FAIL: gdb.base/return-nodebug.exp: signed-char: return from function with no debug info with a cast
FAIL: gdb.base/return-nodebug.exp: signed-char: full width of the returned result
FAIL: gdb.base/return-nodebug.exp: short: return from function with no debug info with a cast
FAIL: gdb.base/return-nodebug.exp: short: full width of the returned result
FAIL: gdb.base/return-nodebug.exp: int: return from function with no debug info with a cast
FAIL: gdb.base/return-nodebug.exp: int: full width of the returned result
FAIL: gdb.base/return-nodebug.exp: long: return from function with no debug info with a cast
FAIL: gdb.base/return-nodebug.exp: long: full width of the returned result
FAIL: gdb.base/return-nodebug.exp: long-long: return from function with no debug info with a cast
FAIL: gdb.base/return-nodebug.exp: long-long: full width of the returned result
FAIL: gdb.base/return.exp: continue to return of -5
FAIL: gdb.base/return.exp: continue to return of -5.0
FAIL: gdb.base/return2.exp: void function returned successfully
FAIL: gdb.base/scope.exp: print funclocal at bar
FAIL: gdb.base/scope.exp: print funclocal_bss at bar
FAIL: gdb.base/sepdebug.exp: next over recursive call
FAIL: gdb.base/sepdebug.exp: backtrace from factorial(5.1)
FAIL: gdb.base/skip.exp: step after deleting 1 (3)
FAIL: gdb.base/skip.exp: step after disabling 3 (5)
FAIL: gdb.base/skip.exp: step after enable 3 (3)
FAIL: gdb.base/step-resume-infcall.exp: step
FAIL: gdb.base/step-test.exp: step into
FAIL: gdb.base/step-test.exp: large struct by value
FAIL: gdb.base/store.exp: var struct 2 u; next to add_struct_2 call
FAIL: gdb.base/store.exp: var struct 2 u; print old u, expecting {s = \{0, 0}}
FAIL: gdb.base/store.exp: var struct 2 u; set u to s_2
FAIL: gdb.base/store.exp: var struct 2 u; print new u, expecting {s = \{1, 2}}
FAIL: gdb.base/store.exp: var struct 3 u; next to add_struct_3 call
FAIL: gdb.base/store.exp: var struct 3 u; print old u, expecting {s = \{0, 0, 0}}
FAIL: gdb.base/store.exp: var struct 3 u; set u to s_3
FAIL: gdb.base/store.exp: var struct 3 u; print new u, expecting {s = \{1, 2, 3}}
FAIL: gdb.base/store.exp: var struct 4 u; next to add_struct_4 call
FAIL: gdb.base/store.exp: var struct 4 u; print old u, expecting {s = \{0, 0, 0, 0}}
FAIL: gdb.base/store.exp: var struct 4 u; set u to s_4
FAIL: gdb.base/store.exp: var struct 4 u; print new u, expecting {s = \{1, 2, 3, 4}}
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tc
FAIL: gdb.base/structs.exp: return foo<n>; return 2 structs-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tc
FAIL: gdb.base/structs.exp: zed L<n> for finish; return 2 structs-tc
FAIL: gdb.base/structs.exp: finish foo<n>; return 2 structs-tc
FAIL: gdb.base/structs.exp: value foo<n> finished; return 2 structs-tc
FAIL: gdb.base/structs.exp: return foo<n>; return 3 structs-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 3 structs-tc
FAIL: gdb.base/structs.exp: zed L<n> for finish; return 3 structs-tc
FAIL: gdb.base/structs.exp: finish foo<n>; return 3 structs-tc
FAIL: gdb.base/structs.exp: value foo<n> finished; return 3 structs-tc
FAIL: gdb.base/structs.exp: return foo<n>; return 4 structs-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 4 structs-tc
FAIL: gdb.base/structs.exp: zed L<n> for finish; return 4 structs-tc
FAIL: gdb.base/structs.exp: finish foo<n>; return 4 structs-tc
FAIL: gdb.base/structs.exp: value foo<n> finished; return 4 structs-tc
FAIL: gdb.base/structs.exp: return foo<n>; return 5 structs-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 5 structs-tc
FAIL: gdb.base/structs.exp: zed L<n> for finish; return 5 structs-tc
FAIL: gdb.base/structs.exp: finish foo<n>; return 5 structs-tc
FAIL: gdb.base/structs.exp: value foo<n> finished; return 5 structs-tc
FAIL: gdb.base/structs.exp: return foo<n>; return 6 structs-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 6 structs-tc
FAIL: gdb.base/structs.exp: zed L<n> for finish; return 6 structs-tc
FAIL: gdb.base/structs.exp: finish foo<n>; return 6 structs-tc
FAIL: gdb.base/structs.exp: value foo<n> finished; return 6 structs-tc
FAIL: gdb.base/structs.exp: return foo<n>; return 7 structs-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 7 structs-tc
FAIL: gdb.base/structs.exp: zed L<n> for finish; return 7 structs-tc
FAIL: gdb.base/structs.exp: finish foo<n>; return 7 structs-tc
FAIL: gdb.base/structs.exp: value foo<n> finished; return 7 structs-tc
FAIL: gdb.base/structs.exp: return foo<n>; return 8 structs-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 8 structs-tc
FAIL: gdb.base/structs.exp: zed L<n> for finish; return 8 structs-tc
FAIL: gdb.base/structs.exp: finish foo<n>; return 8 structs-tc
FAIL: gdb.base/structs.exp: value foo<n> finished; return 8 structs-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-ts
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-ts
FAIL: gdb.base/structs.exp: return foo<n>; return 2 structs-ts
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-ts
FAIL: gdb.base/structs.exp: zed L<n> for finish; return 2 structs-ts
FAIL: gdb.base/structs.exp: finish foo<n>; return 2 structs-ts
FAIL: gdb.base/structs.exp: value foo<n> finished; return 2 structs-ts
FAIL: gdb.base/structs.exp: return foo<n>; return 3 structs-ts
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 3 structs-ts
FAIL: gdb.base/structs.exp: zed L<n> for finish; return 3 structs-ts
FAIL: gdb.base/structs.exp: finish foo<n>; return 3 structs-ts
FAIL: gdb.base/structs.exp: value foo<n> finished; return 3 structs-ts
FAIL: gdb.base/structs.exp: return foo<n>; return 4 structs-ts
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 4 structs-ts
FAIL: gdb.base/structs.exp: zed L<n> for finish; return 4 structs-ts
FAIL: gdb.base/structs.exp: finish foo<n>; return 4 structs-ts
FAIL: gdb.base/structs.exp: value foo<n> finished; return 4 structs-ts
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-ti
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-ti
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-ti
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-ti
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tl
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tl
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tl
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tl
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tll
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tll
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tf
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tf
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tf
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tf
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-td
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-td
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tld
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tld
FAIL: gdb.base/structs.exp: return foo<n>; return 2 structs-ts-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-ts-tc
FAIL: gdb.base/structs.exp: zed L<n> for finish; return 2 structs-ts-tc
FAIL: gdb.base/structs.exp: finish foo<n>; return 2 structs-ts-tc
FAIL: gdb.base/structs.exp: value foo<n> finished; return 2 structs-ts-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-ti-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-ti-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tl-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tl-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tf-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tf-tc
FAIL: gdb.base/structs.exp: return foo<n>; return 2 structs-tc-ts
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tc-ts
FAIL: gdb.base/structs.exp: zed L<n> for finish; return 2 structs-tc-ts
FAIL: gdb.base/structs.exp: finish foo<n>; return 2 structs-tc-ts
FAIL: gdb.base/structs.exp: value foo<n> finished; return 2 structs-tc-ts
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tc-ti
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tc-ti
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tc-tl
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tc-tl
FAIL: gdb.base/value-double-free.exp: continue
FAIL: gdb.base/watch-cond-infcall.exp: hw: continue
FAIL: gdb.base/watch-cond-infcall.exp: sw: continue
FAIL: gdb.base/watchpoint-cond-gone.exp: Place the watchpoint
FAIL: gdb.base/watchpoint-cond-gone.exp: Catch the no longer valid watchpoint
FAIL: gdb.base/watchpoint.exp: global_ptr next
FAIL: gdb.base/watchpoint.exp: next over ptr init
FAIL: gdb.base/watchpoint.exp: next over buffer set
FAIL: gdb.base/watchpoint.exp: global_ptr_ptr next
FAIL: gdb.base/watchpoint.exp: next over global_ptr_ptr init
FAIL: gdb.base/watchpoint.exp: next over global_ptr_ptr buffer set
FAIL: gdb.base/watchpoint.exp: next over global_ptr_ptr pointer advance
FAIL: gdb.base/watchpoint.exp: no-hw: global_ptr next
FAIL: gdb.base/watchpoint.exp: no-hw: next over ptr init
FAIL: gdb.base/watchpoint.exp: no-hw: next over buffer set
FAIL: gdb.base/watchpoint.exp: no-hw: global_ptr_ptr next
FAIL: gdb.base/watchpoint.exp: no-hw: next over global_ptr_ptr init
FAIL: gdb.base/watchpoint.exp: no-hw: next over global_ptr_ptr buffer set
FAIL: gdb.base/watchpoint.exp: no-hw: next over global_ptr_ptr pointer advance
FAIL: gdb.base/watchpoints.exp: watchpoint ival1 hit, second time
FAIL: gdb.base/watchpoints.exp: Watchpoint ival1 hit count is 2
FAIL: gdb.base/watchpoints.exp: watchpoint hit, second time
FAIL: gdb.base/watchpoints.exp: Watchpoint hit count is 2
FAIL: gdb.base/watchpoints.exp: watchpoint ival1 hit, third time
FAIL: gdb.base/watchpoints.exp: Watchpoint ival1 hit count is 3
FAIL: gdb.base/watchpoints.exp: watchpoint hit, third time
FAIL: gdb.base/watchpoints.exp: Watchpoint hit count is 3
FAIL: gdb.base/watchpoints.exp: watchpoint hit, fourth time
FAIL: gdb.base/watchpoints.exp: Watchpoint hit count is 4
FAIL: gdb.base/watchpoints.exp: watchpoint hit, fifth time
FAIL: gdb.base/watchpoints.exp: Watchpoint hit count is 5
FAIL: gdb.cp/expand-sals.exp: continue to breakpoint: func
FAIL: gdb.cp/expand-sals.exp: continue to breakpoint: next caller func
FAIL: gdb.cp/extern-c.exp: continue to breakpoint: c_func
FAIL: gdb.cp/extern-c.exp: continue to breakpoint: c_funcs_1
FAIL: gdb.cp/extern-c.exp: continue to breakpoint: c_funcs_2
FAIL: gdb.cp/namespace-nested-import.exp: print C::x
FAIL: gdb.dwarf2/dw2-cp-infcall-ref-static.exp: p f()
FAIL: gdb.dwarf2/dw2-ref-missing-frame.exp: func_nofb backtrace
FAIL: gdb.dwarf2/dw2-ref-missing-frame.exp: func_loopfb backtrace
FAIL: gdb.linespec/linespec.exp: set breakpoint at lspec.cc instance of NameSpace::overload
FAIL: gdb.linespec/linespec.exp: set breakpoint at specific instance of NameSpace::overload
FAIL: gdb.mi/dw2-ref-missing-frame.exp: test func_nofb_marker
FAIL: gdb.mi/mi-var-display.exp: print FP register
FAIL: gdb.mi/mi2-var-display.exp: print FP register
FAIL: gdb.opt/inline-bt.exp: continue to bar (1)
FAIL: gdb.opt/inline-bt.exp: continue to bar (2)
FAIL: gdb.opt/inline-bt.exp: continue to bar (3)
FAIL: gdb.opt/inline-cmds.exp: continue to bar (1)
FAIL: gdb.opt/inline-cmds.exp: continue to bar (2)
FAIL: gdb.opt/inline-cmds.exp: continue to marker
FAIL: gdb.opt/inline-cmds.exp: next over inlined functions
FAIL: gdb.opt/inline-cmds.exp: next past inlined func1
FAIL: gdb.opt/inline-cmds.exp: step into finish marker
FAIL: gdb.opt/inline-cmds.exp: enter inlined_fn from noinline
FAIL: gdb.opt/inline-cmds.exp: backtrace at inlined_fn from noinline
FAIL: gdb.opt/inline-cmds.exp: inlined_fn from noinline inlined
FAIL: gdb.opt/inline-cmds.exp: up to noinline
FAIL: gdb.opt/inline-cmds.exp: noinline from outer_inline1 not inlined
FAIL: gdb.opt/inline-cmds.exp: up to outer_inline1
FAIL: gdb.opt/inline-cmds.exp: outer_inline1 inlined
FAIL: gdb.opt/inline-cmds.exp: up to outer_inline2
FAIL: gdb.opt/inline-cmds.exp: outer_inline2 inlined
FAIL: gdb.opt/inline-cmds.exp: up from outer_inline2
FAIL: gdb.opt/inline-cmds.exp: main not inlined
FAIL: gdb.opt/inline-locals.exp: continue to bar (1)
FAIL: gdb.opt/inline-locals.exp: continue to bar (2)
FAIL: gdb.opt/inline-locals.exp: continue to bar (3)
FAIL: gdb.stabs/gdb11479.exp: Stop at first breakpoint forced_stabs (the program exited)
FAIL: gdb.stabs/gdb11479.exp: Inspect t in test2 forced_stabs
FAIL: gdb.stabs/gdb11479.exp: sizeof (e) in test2 forced_stabs
FAIL: gdb.stabs/gdb11479.exp: Stop at first breakpoint forced_stabs (the program is no longer running)
FAIL: gdb.stabs/gdb11479.exp: Inspect t in test forced_stabs
FAIL: gdb.stabs/gdb11479.exp: sizeof (e) in test forced_stabs
Here is a list of FAILs that my patch introduced (regressions). There
are 9 of them:
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
Here are the FAILs that your patch fixed. There are 127 of them.
FAIL: gdb.base/arrayidx.exp: Print array with array-indexes off
FAIL: gdb.base/arrayidx.exp: Print array with array-indexes on
FAIL: gdb.base/break-always.exp: continue to breakpoint: bar
FAIL: gdb.base/break-inline.exp: start
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tc
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tc
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-ts
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-ts
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-ti
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-ti
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tl
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tl
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tll
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tll
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tf
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tf
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-td
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-td
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tld
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tld
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-te
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-te
FAIL: gdb.base/chng-syms.exp: running to stop_here first time
FAIL: gdb.base/chng-syms.exp: continue until exit at breakpoint first time through (the program is no longer running)
FAIL: gdb.base/gdb11531.exp: watchpoint variable triggers at next
FAIL: gdb.base/reread.exp: breakpoint foo in first file
FAIL: gdb.base/reread.exp: run to foo()
FAIL: gdb.base/return.exp: continue to return of -5
FAIL: gdb.base/return.exp: continue to return of -5.0
FAIL: gdb.base/return2.exp: void function returned successfully
FAIL: gdb.base/scope.exp: print funclocal at bar
FAIL: gdb.base/scope.exp: print funclocal_bss at bar
FAIL: gdb.base/skip.exp: step after deleting 1 (3)
FAIL: gdb.base/skip.exp: step after disabling 3 (5)
FAIL: gdb.base/skip.exp: step after enable 3 (3)
FAIL: gdb.base/step-resume-infcall.exp: step
FAIL: gdb.base/step-test.exp: step into
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-ts
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-ts
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-ti
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-ti
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-ti
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-ti
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tl
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tl
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tl
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tl
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tll
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tll
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tf
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tf
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tf
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tf
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-td
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-td
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tld
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tld
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-ti-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-ti-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tl-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tl-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tf-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tf-tc
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tc-ti
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tc-ti
FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tc-tl
FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tc-tl
FAIL: gdb.base/value-double-free.exp: continue
FAIL: gdb.base/watch-cond-infcall.exp: hw: continue
FAIL: gdb.base/watch-cond-infcall.exp: sw: continue
FAIL: gdb.base/watchpoint.exp: global_ptr next
FAIL: gdb.base/watchpoint.exp: next over ptr init
FAIL: gdb.base/watchpoint.exp: next over buffer set
FAIL: gdb.base/watchpoint.exp: global_ptr_ptr next
FAIL: gdb.base/watchpoint.exp: next over global_ptr_ptr init
FAIL: gdb.base/watchpoint.exp: next over global_ptr_ptr buffer set
FAIL: gdb.base/watchpoint.exp: next over global_ptr_ptr pointer advance
FAIL: gdb.base/watchpoint.exp: no-hw: global_ptr next
FAIL: gdb.base/watchpoint.exp: no-hw: next over ptr init
FAIL: gdb.base/watchpoint.exp: no-hw: next over buffer set
FAIL: gdb.base/watchpoint.exp: no-hw: global_ptr_ptr next
FAIL: gdb.base/watchpoint.exp: no-hw: next over global_ptr_ptr init
FAIL: gdb.base/watchpoint.exp: no-hw: next over global_ptr_ptr buffer set
FAIL: gdb.base/watchpoint.exp: no-hw: next over global_ptr_ptr pointer advance
FAIL: gdb.base/watchpoints.exp: watchpoint ival1 hit, second time
FAIL: gdb.base/watchpoints.exp: Watchpoint ival1 hit count is 2
FAIL: gdb.base/watchpoints.exp: watchpoint hit, second time
FAIL: gdb.base/watchpoints.exp: Watchpoint hit count is 2
FAIL: gdb.base/watchpoints.exp: watchpoint ival1 hit, third time
FAIL: gdb.base/watchpoints.exp: Watchpoint ival1 hit count is 3
FAIL: gdb.base/watchpoints.exp: watchpoint hit, third time
FAIL: gdb.base/watchpoints.exp: Watchpoint hit count is 3
FAIL: gdb.base/watchpoints.exp: watchpoint hit, fourth time
FAIL: gdb.base/watchpoints.exp: Watchpoint hit count is 4
FAIL: gdb.base/watchpoints.exp: watchpoint hit, fifth time
FAIL: gdb.base/watchpoints.exp: Watchpoint hit count is 5
FAIL: gdb.cp/expand-sals.exp: continue to breakpoint: func
FAIL: gdb.cp/expand-sals.exp: continue to breakpoint: next caller func
FAIL: gdb.cp/extern-c.exp: continue to breakpoint: c_func
FAIL: gdb.cp/extern-c.exp: continue to breakpoint: c_funcs_1
FAIL: gdb.cp/extern-c.exp: continue to breakpoint: c_funcs_2
FAIL: gdb.cp/namespace-nested-import.exp: print C::x
FAIL: gdb.linespec/linespec.exp: set breakpoint at lspec.cc instance of NameSpace::overload
FAIL: gdb.linespec/linespec.exp: set breakpoint at specific instance of NameSpace::overload
FAIL: gdb.opt/inline-bt.exp: continue to bar (1)
FAIL: gdb.opt/inline-bt.exp: continue to bar (2)
FAIL: gdb.opt/inline-bt.exp: continue to bar (3)
FAIL: gdb.opt/inline-cmds.exp: continue to bar (1)
FAIL: gdb.opt/inline-cmds.exp: continue to bar (2)
FAIL: gdb.opt/inline-cmds.exp: continue to marker
FAIL: gdb.opt/inline-cmds.exp: step into finish marker
FAIL: gdb.opt/inline-cmds.exp: enter inlined_fn from noinline
FAIL: gdb.opt/inline-cmds.exp: backtrace at inlined_fn from noinline
FAIL: gdb.opt/inline-cmds.exp: inlined_fn from noinline inlined
FAIL: gdb.opt/inline-cmds.exp: up to noinline
FAIL: gdb.opt/inline-cmds.exp: noinline from outer_inline1 not inlined
FAIL: gdb.opt/inline-cmds.exp: up to outer_inline1
FAIL: gdb.opt/inline-cmds.exp: outer_inline1 inlined
FAIL: gdb.opt/inline-cmds.exp: up to outer_inline2
FAIL: gdb.opt/inline-cmds.exp: outer_inline2 inlined
FAIL: gdb.opt/inline-cmds.exp: up from outer_inline2
FAIL: gdb.opt/inline-cmds.exp: main not inlined
FAIL: gdb.opt/inline-locals.exp: continue to bar (1)
FAIL: gdb.opt/inline-locals.exp: continue to bar (2)
FAIL: gdb.opt/inline-locals.exp: continue to bar (3)
Your patch did not introduce any regressions.
So my testing showed that my patch fixed more failures, but introduced
regressions. I find it conceivable, however, that my patch might not
fare as well on some other target. (That's what your testing
demonstrates, right?)
Is there a different simulator multilib that I should use for testing?
> Looking at the first one:
>
> (gdb) PASS: gdb.base/store.exp: var doublest l; print incremented l, expecting 2
> tbreak add_charest
> Temporary breakpoint 10 at 0x400720: file /scratch/tschwing/FM_sh-linux-gnu/src/gdb-mainline/gdb/testsuite/gdb.base/store.c, line 14.
> (gdb) PASS: gdb.base/store.exp: tbreak add_charest
> continue
> Continuing.
>
> Temporary breakpoint 10, add_charest (u=-1 '\377', v=32 ' ') at /scratch/tschwing/FM_sh-linux-gnu/src/gdb-mainline/gdb/testsuite/gdb.base/store.c:14
> 14 {
> (gdb) FAIL: gdb.base/store.exp: continue to add_charest
>
> gdb.base/store.c:
>
> 10 typedef signed char charest;
> 11
> 12 charest
> 13 add_charest (register charest u, register charest v)
> 14 {
> 15 return u + v;
> 16 }
>
> So the ``tbreak add_charest'' chose line 14 instead of 15.
I took a look at this regression. add_charest is a leaf function.
0x1154 <add_charest>: mov r4,r2
0x1156 <add_charest+2>: mov r5,r1
0x1158 <add_charest+4>: exts.b r2,r2
0x115a <add_charest+6>: exts.b r1,r1
0x115c <add_charest+8>: extu.b r2,r2
0x115e <add_charest+10>: extu.b r1,r1
0x1160 <add_charest+12>: add r2,r1
0x1162 <add_charest+14>: extu.b r1,r1
0x1164 <add_charest+16>: exts.b r1,r1
0x1166 <add_charest+18>: mov r1,r0
0x1168 <add_charest+20>: rts
0x116a <add_charest+22>: nop
I asked it about line 14 too...
(gdb) info line *0x1154
Line 14 of "/ironwood1/sourceware-sh/sh-elf/../src/gdb/testsuite/gdb.base/store.c" starts at address 0x1154 <add_charest> and ends at 0x115c <add_charest+8>.
So, according to that, the prologue consists of "mov r4,r2", "mov r5,r1",
"exts.b r2,r2", "exts.b r1,r1". These certainly have nothing to do with
setting up a frame. I think it will be hard for the prologue analyzer to
distinguish these from the body instructions. That's always a problem
with prologue analyzers.
I'll have to think about it some more to see if there's a way around
this. E.g. maybe we can tweak the test at the end of sh_skip_prologue
to give us better results.
I'll try to answer your questions about my patch, though it's been a
while since I wrote it. But perhaps this exercise will help to
refresh my memory...
> > Index: sh-tdep.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/sh-tdep.c,v
> > retrieving revision 1.236
> > diff -u -p -r1.236 sh-tdep.c
> > --- sh-tdep.c 28 Jan 2012 18:08:20 -0000 1.236
> > +++ sh-tdep.c 15 Feb 2012 23:55:14 -0000
> > @@ -518,39 +518,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;
>
> Save the original pc instead of having the caller do that.
The goal here is to wind up with after_last_frame_setup_insn set the
instruction immediately after the last discernable prologue
instruction. These will be things like manipulation of SP, FP, and
instructions which store callee saved registers to the stack.
We've used this method with some success in other architecture ports.
I had hoped that the variable name was self documenting. ;)
> > + 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;
>
> That sets cache->uses_fp to false even in case that the limit_pc (was:
> current_pc) was already reached.
True. But I don't believe that sh_analyze_prologue is ever called
in an instance where it will have a useful value ahead of time. It
needs to be initialized.
> > - for (opc = pc + (2 * 28); pc < opc; pc += 2)
> > +
> > + for (;pc < limit_pc; pc = next_pc)
>
> The limit of 28 instructions is no longer checked here, but is now the
> caller's responsibility. (Are all callers doing the right thing?)
For sh_skip_prologue(), we limit the scan to the end of the function
in question. For sh_frame_cache(), the scan is limited to the current
pc.
Note too that the scan will be terminated when a call (JSR)
instruction is found.
> > @@ -681,7 +692,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;
> > + }
>
> Shouldn't this perhaps be pc + 2?
This is the JSR case. If we set it to be pc+2, we'll be past the JSR
which definitely isn't part of the prologue. Yet the instruction in
the delay slot is part of the prologue.
I have a vague recollection of trying it with
after_last_frame_setup_insn = nextpc;
and getting bad results.
I should have put a comment on that one since it differs from the
norm.
[...]
> > 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;
> > + const 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;
>
> Now start_pc is directly returned if find_pc_partial_function fails,
> without invoking sh_analyze_prologue. Is that right?
I may be wrong, but I don't think we can do a useful prologue analysis
unless we start at the beginning of the function. That said, it seems
likely that start_pc is the start of the function in this instance.
> > 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);
>
> I had func_addr here, instead of start_pc.
I like your way better. I think that start_pc == func_addr, but I'm
not entirely certain that this is true.
Kevin
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-02-17 2:30 ` Kevin Buettner
@ 2012-02-20 16:19 ` Thomas Schwinge
2012-02-21 5:25 ` Kevin Buettner
2012-02-21 15:23 ` Thomas Schwinge
2012-02-22 14:54 ` Simulator testing for sh and sh64 (was: [PATCH] [SH] Prologue skipping if there is none) Thomas Schwinge
2 siblings, 1 reply; 34+ messages in thread
From: Thomas Schwinge @ 2012-02-20 16:19 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 2072 bytes --]
Hi!
On Thu, 16 Feb 2012 18:25:44 -0700, Kevin Buettner <kevinb@redhat.com> wrote:
> On Thu, 16 Feb 2012 17:32:18 +0100
> Thomas Schwinge <thomas@codesourcery.com> wrote:
>
> > I now have (on a SH7785-based board). My patch fixes a few more failures
> > than yours. ;-P
>
> This will require more study and discussion then.
Heh, indeed. :-)
> I tested against
> the simulator using the default multilib. I compared results using
> each of our patches to an unpatched sh-tdep.c.
Thanks, and I will likewise do such testing in the next days.
> Here are the FAILs that my patch fixed. There are 246 of them. [...]
>
> Here is a list of FAILs that my patch introduced (regressions). There
> are 9 of them: [...]
> Here are the FAILs that your patch fixed. There are 127 of them. [...]
> Your patch did not introduce any regressions.
> So my testing showed that my patch fixed more failures, but introduced
> regressions. I find it conceivable, however, that my patch might not
> fare as well on some other target. (That's what your testing
> demonstrates, right?)
Yeah, apparently.
> > tbreak add_charest
> > Temporary breakpoint 10 at 0x400720: file /scratch/tschwing/FM_sh-linux-gnu/src/gdb-mainline/gdb/testsuite/gdb.base/store.c, line 14.
> > So the ``tbreak add_charest'' chose line 14 instead of 15.
>
> I took a look at this regression. [...]
Many thanks for the analysis (omitted here) as well as your comments to
my comments; I will go throught this in the next days.
So, we do seem to agree that something like the patch I posted initially
(and as it is incorporated in a similar fashion in your patch, too) does
already move us forward; is it reasonable that we commit that one now,
and then continue to look on how to further improve the situation based
on your patch? This will let us point out more easily which are the
additional cases your patch improves/regresses on. (I'd offer to port
your patch over to the new baseline, if you want me to.)
Grüße,
Thomas
[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-02-20 16:19 ` Thomas Schwinge
@ 2012-02-21 5:25 ` Kevin Buettner
2012-02-24 11:09 ` Thomas Schwinge
0 siblings, 1 reply; 34+ messages in thread
From: Kevin Buettner @ 2012-02-21 5:25 UTC (permalink / raw)
To: gdb-patches
Hi Thomas,
On Mon, 20 Feb 2012 17:15:54 +0100
Thomas Schwinge <thomas@codesourcery.com> wrote:
> So, we do seem to agree that something like the patch I posted initially
> (and as it is incorporated in a similar fashion in your patch, too) does
> already move us forward; is it reasonable that we commit that one now,
> and then continue to look on how to further improve the situation based
> on your patch? This will let us point out more easily which are the
> additional cases your patch improves/regresses on. (I'd offer to port
> your patch over to the new baseline, if you want me to.)
Yeah, as a first step, I think it makes sense for you to commit
the patch that you came up with.
Kevin
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-02-17 2:30 ` Kevin Buettner
2012-02-20 16:19 ` Thomas Schwinge
@ 2012-02-21 15:23 ` Thomas Schwinge
2012-02-22 14:54 ` Simulator testing for sh and sh64 (was: [PATCH] [SH] Prologue skipping if there is none) Thomas Schwinge
2 siblings, 0 replies; 34+ messages in thread
From: Thomas Schwinge @ 2012-02-21 15:23 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 33780 bytes --]
Hi Kevin!
On Thu, 16 Feb 2012 18:25:44 -0700, Kevin Buettner <kevinb@redhat.com> wrote:
> On Thu, 16 Feb 2012 17:32:18 +0100
> Thomas Schwinge <thomas@codesourcery.com> wrote:
>
> > I now have (on a SH7785-based board). My patch fixes a few more failures
> > than yours. ;-P
>
> This will require more study and discussion then. I tested against
> the simulator using the default multilib. I compared results using
> each of our patches to an unpatched sh-tdep.c.
I'm now comparing your findings to mine, based on my 2012-02-16 test
results.
> Here are the FAILs that my patch fixed. There are 246 of them.
> FAIL: gdb.arch/gdb1291.exp: set breakpoint
> FAIL: gdb.arch/gdb1291.exp: get to sub1 (the program exited)
> FAIL: gdb.arch/gdb1291.exp: backtrace with local variable less than or equal to 256 bytes
> FAIL: gdb.arch/gdb1291.exp: set breakpoint
> FAIL: gdb.arch/gdb1291.exp: get to sub2 (the program is no longer running)
> FAIL: gdb.arch/gdb1291.exp: backtrace with local variable larger than 256 bytes
> FAIL: gdb.arch/gdb1431.exp: get to sub1
> FAIL: gdb.arch/gdb1431.exp: advance returns from sub1 frame
> FAIL: gdb.arch/gdb1431.exp: get to sub2
Are you sure these are releated to the patch you've sent? For me, these
failures look the same with and without your patch; and that doesn't very
much look like a prologue analysis issue:
Running /scratch/tschwing/FM_sh-linux-gnu/src/gdb-mainline/gdb/testsuite/gdb.arch/gdb1291.exp ...
gdb compile failed, /scratch/tschwing/FM_sh-linux-gnu/install/bin/../sh-linux-gnu/libc/usr/lib/crt1.o: In function `L_main':
(.text+0x1c): undefined reference to `main'
/tmp/ccmGUZOl.o: In function `_main':
gdb1291.c:(.text+0x2c): undefined reference to `_printf'
collect2: error: ld returned 1 exit status
UNTESTED: gdb.arch/gdb1291.exp: gdb1291.exp
> FAIL: gdb.base/arrayidx.exp: Print array with array-indexes off
> FAIL: gdb.base/arrayidx.exp: Print array with array-indexes on
> FAIL: gdb.base/break-always.exp: continue to breakpoint: bar
Ack.
> FAIL: gdb.base/break-inline.exp: start
Again, (at least in my case) not related to the patch we're discussing:
Running /scratch/tschwing/FM_sh-linux-gnu/src/gdb-mainline/gdb/testsuite/gdb.base/break-inline.exp ...
ERROR: tcl error sourcing /scratch/tschwing/FM_sh-linux-gnu/src/gdb-mainline/gdb/testsuite/gdb.base/break-inline.exp.
ERROR: gdbserver does not support start without extended-remote
> FAIL: gdb.base/break.exp: next over recursive call
> FAIL: gdb.base/break.exp: backtrace from factorial(5.1)
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tc
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tc
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-ts
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-ts
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-ti
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-ti
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tl
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tl
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tll
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tll
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tf
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tf
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-td
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-td
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tld
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tld
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-te
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-te
> FAIL: gdb.base/chng-syms.exp: running to stop_here first time
> FAIL: gdb.base/chng-syms.exp: continue until exit at breakpoint first time through (the program is no longer running)
> FAIL: gdb.base/gdb11531.exp: watchpoint variable triggers at next
Ack.
At this point, your patch also causes this progression:
-FAIL: gdb.base/gdb1250.exp: setting breakpoint at abort
+PASS: gdb.base/gdb1250.exp: backtrace from abort
> FAIL: gdb.base/longjmp.exp: next over setjmp (1)
> FAIL: gdb.base/longjmp.exp: next to longjmp (1)
> FAIL: gdb.base/longjmp.exp: next over setjmp (2)
> FAIL: gdb.base/nodebug.exp: backtrace from inner in nodebug.exp
> FAIL: gdb.base/nodebug.exp: backtrace from middle in nodebug.exp
These still FAIL for me with your patch.
> FAIL: gdb.base/pc-fp.exp: get hexadecimal valueof "$fp" (timeout)
> FAIL: gdb.base/pc-fp.exp: display/i $pc
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.base/reread.exp: breakpoint foo in first file
> FAIL: gdb.base/reread.exp: run to foo()
Ack.
> FAIL: gdb.base/return-nodebug.exp: signed-char: return from function with no debug info with a cast
> FAIL: gdb.base/return-nodebug.exp: signed-char: full width of the returned result
> FAIL: gdb.base/return-nodebug.exp: short: return from function with no debug info with a cast
> FAIL: gdb.base/return-nodebug.exp: short: full width of the returned result
> FAIL: gdb.base/return-nodebug.exp: int: return from function with no debug info with a cast
> FAIL: gdb.base/return-nodebug.exp: int: full width of the returned result
> FAIL: gdb.base/return-nodebug.exp: long: return from function with no debug info with a cast
> FAIL: gdb.base/return-nodebug.exp: long: full width of the returned result
> FAIL: gdb.base/return-nodebug.exp: long-long: return from function with no debug info with a cast
> FAIL: gdb.base/return-nodebug.exp: long-long: full width of the returned result
These still FAIL for me with your patch.
> FAIL: gdb.base/return.exp: continue to return of -5
> FAIL: gdb.base/return.exp: continue to return of -5.0
> FAIL: gdb.base/return2.exp: void function returned successfully
> FAIL: gdb.base/scope.exp: print funclocal at bar
> FAIL: gdb.base/scope.exp: print funclocal_bss at bar
Ack.
> FAIL: gdb.base/sepdebug.exp: next over recursive call
> FAIL: gdb.base/sepdebug.exp: backtrace from factorial(5.1)
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.base/skip.exp: step after deleting 1 (3)
> FAIL: gdb.base/skip.exp: step after disabling 3 (5)
> FAIL: gdb.base/skip.exp: step after enable 3 (3)
> FAIL: gdb.base/step-resume-infcall.exp: step
> FAIL: gdb.base/step-test.exp: step into
Ack.
> FAIL: gdb.base/step-test.exp: large struct by value
This one still FAILs for me with your patch.
> FAIL: gdb.base/store.exp: var struct 2 u; next to add_struct_2 call
> FAIL: gdb.base/store.exp: var struct 2 u; print old u, expecting {s = \{0, 0}}
> FAIL: gdb.base/store.exp: var struct 2 u; set u to s_2
> FAIL: gdb.base/store.exp: var struct 2 u; print new u, expecting {s = \{1, 2}}
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.base/store.exp: var struct 3 u; next to add_struct_3 call
> FAIL: gdb.base/store.exp: var struct 3 u; print old u, expecting {s = \{0, 0, 0}}
> FAIL: gdb.base/store.exp: var struct 3 u; set u to s_3
> FAIL: gdb.base/store.exp: var struct 3 u; print new u, expecting {s = \{1, 2, 3}}
These still FAIL for me with your patch.
> FAIL: gdb.base/store.exp: var struct 4 u; next to add_struct_4 call
> FAIL: gdb.base/store.exp: var struct 4 u; print old u, expecting {s = \{0, 0, 0, 0}}
> FAIL: gdb.base/store.exp: var struct 4 u; set u to s_4
> FAIL: gdb.base/store.exp: var struct 4 u; print new u, expecting {s = \{1, 2, 3, 4}}
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tc
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tc
Ack.
> FAIL: gdb.base/structs.exp: return foo<n>; return 2 structs-tc
This one still FAILs for me with your patch.
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tc
> FAIL: gdb.base/structs.exp: zed L<n> for finish; return 2 structs-tc
> FAIL: gdb.base/structs.exp: finish foo<n>; return 2 structs-tc
> FAIL: gdb.base/structs.exp: value foo<n> finished; return 2 structs-tc
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.base/structs.exp: return foo<n>; return 3 structs-tc
This one still FAILs for me with your patch.
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 3 structs-tc
> FAIL: gdb.base/structs.exp: zed L<n> for finish; return 3 structs-tc
> FAIL: gdb.base/structs.exp: finish foo<n>; return 3 structs-tc
> FAIL: gdb.base/structs.exp: value foo<n> finished; return 3 structs-tc
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.base/structs.exp: return foo<n>; return 4 structs-tc
This one still FAILs for me with your patch.
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 4 structs-tc
> FAIL: gdb.base/structs.exp: zed L<n> for finish; return 4 structs-tc
> FAIL: gdb.base/structs.exp: finish foo<n>; return 4 structs-tc
> FAIL: gdb.base/structs.exp: value foo<n> finished; return 4 structs-tc
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.base/structs.exp: return foo<n>; return 5 structs-tc
This one still FAILs for me with your patch.
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 5 structs-tc
> FAIL: gdb.base/structs.exp: zed L<n> for finish; return 5 structs-tc
> FAIL: gdb.base/structs.exp: finish foo<n>; return 5 structs-tc
> FAIL: gdb.base/structs.exp: value foo<n> finished; return 5 structs-tc
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.base/structs.exp: return foo<n>; return 6 structs-tc
This one still FAILs for me with your patch.
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 6 structs-tc
> FAIL: gdb.base/structs.exp: zed L<n> for finish; return 6 structs-tc
> FAIL: gdb.base/structs.exp: finish foo<n>; return 6 structs-tc
> FAIL: gdb.base/structs.exp: value foo<n> finished; return 6 structs-tc
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.base/structs.exp: return foo<n>; return 7 structs-tc
This one still FAILs for me with your patch.
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 7 structs-tc
> FAIL: gdb.base/structs.exp: zed L<n> for finish; return 7 structs-tc
> FAIL: gdb.base/structs.exp: finish foo<n>; return 7 structs-tc
> FAIL: gdb.base/structs.exp: value foo<n> finished; return 7 structs-tc
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.base/structs.exp: return foo<n>; return 8 structs-tc
This one still FAILs for me with your patch.
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 8 structs-tc
> FAIL: gdb.base/structs.exp: zed L<n> for finish; return 8 structs-tc
> FAIL: gdb.base/structs.exp: finish foo<n>; return 8 structs-tc
> FAIL: gdb.base/structs.exp: value foo<n> finished; return 8 structs-tc
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-ts
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-ts
Ack.
> FAIL: gdb.base/structs.exp: return foo<n>; return 2 structs-ts
This one still FAILs for me with your patch.
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-ts
> FAIL: gdb.base/structs.exp: zed L<n> for finish; return 2 structs-ts
> FAIL: gdb.base/structs.exp: finish foo<n>; return 2 structs-ts
> FAIL: gdb.base/structs.exp: value foo<n> finished; return 2 structs-ts
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.base/structs.exp: return foo<n>; return 3 structs-ts
This one still FAILs for me with your patch.
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 3 structs-ts
> FAIL: gdb.base/structs.exp: zed L<n> for finish; return 3 structs-ts
> FAIL: gdb.base/structs.exp: finish foo<n>; return 3 structs-ts
> FAIL: gdb.base/structs.exp: value foo<n> finished; return 3 structs-ts
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.base/structs.exp: return foo<n>; return 4 structs-ts
This one still FAILs for me with your patch.
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 4 structs-ts
> FAIL: gdb.base/structs.exp: zed L<n> for finish; return 4 structs-ts
> FAIL: gdb.base/structs.exp: finish foo<n>; return 4 structs-ts
> FAIL: gdb.base/structs.exp: value foo<n> finished; return 4 structs-ts
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-ti
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-ti
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-ti
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-ti
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tl
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tl
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tl
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tl
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tll
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tll
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tf
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tf
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tf
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tf
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-td
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-td
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tld
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tld
Ack.
> FAIL: gdb.base/structs.exp: return foo<n>; return 2 structs-ts-tc
This one still FAILs for me with your patch.
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-ts-tc
> FAIL: gdb.base/structs.exp: zed L<n> for finish; return 2 structs-ts-tc
> FAIL: gdb.base/structs.exp: finish foo<n>; return 2 structs-ts-tc
> FAIL: gdb.base/structs.exp: value foo<n> finished; return 2 structs-ts-tc
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-ti-tc
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-ti-tc
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tl-tc
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tl-tc
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tf-tc
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tf-tc
Ack.
> FAIL: gdb.base/structs.exp: return foo<n>; return 2 structs-tc-ts
This one still FAILs for me with your patch.
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tc-ts
> FAIL: gdb.base/structs.exp: zed L<n> for finish; return 2 structs-tc-ts
> FAIL: gdb.base/structs.exp: finish foo<n>; return 2 structs-tc-ts
> FAIL: gdb.base/structs.exp: value foo<n> finished; return 2 structs-tc-ts
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tc-ti
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tc-ti
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tc-tl
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tc-tl
> FAIL: gdb.base/value-double-free.exp: continue
Ack.
> FAIL: gdb.base/watch-cond-infcall.exp: hw: continue
This is is not tested in my configuration/with my target (no hardware
watchpoints).
> FAIL: gdb.base/watch-cond-infcall.exp: sw: continue
Ack.
> FAIL: gdb.base/watchpoint-cond-gone.exp: Place the watchpoint
> FAIL: gdb.base/watchpoint-cond-gone.exp: Catch the no longer valid watchpoint
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.base/watchpoint.exp: global_ptr next
> FAIL: gdb.base/watchpoint.exp: next over ptr init
> FAIL: gdb.base/watchpoint.exp: next over buffer set
> FAIL: gdb.base/watchpoint.exp: global_ptr_ptr next
> FAIL: gdb.base/watchpoint.exp: next over global_ptr_ptr init
> FAIL: gdb.base/watchpoint.exp: next over global_ptr_ptr buffer set
> FAIL: gdb.base/watchpoint.exp: next over global_ptr_ptr pointer advance
These still FAIL for me with your patch.
> FAIL: gdb.base/watchpoint.exp: no-hw: global_ptr next
> FAIL: gdb.base/watchpoint.exp: no-hw: next over ptr init
> FAIL: gdb.base/watchpoint.exp: no-hw: next over buffer set
> FAIL: gdb.base/watchpoint.exp: no-hw: global_ptr_ptr next
> FAIL: gdb.base/watchpoint.exp: no-hw: next over global_ptr_ptr init
> FAIL: gdb.base/watchpoint.exp: no-hw: next over global_ptr_ptr buffer set
> FAIL: gdb.base/watchpoint.exp: no-hw: next over global_ptr_ptr pointer advance
Actually, these are the ones that still FAIL for me with your patch; the
other ones (with ``no-hw'' prefix) are with hardware watchpoints, which
is not applicable for my target.
> FAIL: gdb.base/watchpoints.exp: watchpoint ival1 hit, second time
> FAIL: gdb.base/watchpoints.exp: Watchpoint ival1 hit count is 2
> FAIL: gdb.base/watchpoints.exp: watchpoint hit, second time
> FAIL: gdb.base/watchpoints.exp: Watchpoint hit count is 2
> FAIL: gdb.base/watchpoints.exp: watchpoint ival1 hit, third time
> FAIL: gdb.base/watchpoints.exp: Watchpoint ival1 hit count is 3
> FAIL: gdb.base/watchpoints.exp: watchpoint hit, third time
> FAIL: gdb.base/watchpoints.exp: Watchpoint hit count is 3
> FAIL: gdb.base/watchpoints.exp: watchpoint hit, fourth time
> FAIL: gdb.base/watchpoints.exp: Watchpoint hit count is 4
> FAIL: gdb.base/watchpoints.exp: watchpoint hit, fifth time
> FAIL: gdb.base/watchpoints.exp: Watchpoint hit count is 5
> FAIL: gdb.cp/expand-sals.exp: continue to breakpoint: func
> FAIL: gdb.cp/expand-sals.exp: continue to breakpoint: next caller func
> FAIL: gdb.cp/extern-c.exp: continue to breakpoint: c_func
> FAIL: gdb.cp/extern-c.exp: continue to breakpoint: c_funcs_1
> FAIL: gdb.cp/extern-c.exp: continue to breakpoint: c_funcs_2
> FAIL: gdb.cp/namespace-nested-import.exp: print C::x
Ack.
> FAIL: gdb.dwarf2/dw2-cp-infcall-ref-static.exp: p f()
> FAIL: gdb.dwarf2/dw2-ref-missing-frame.exp: func_nofb backtrace
> FAIL: gdb.dwarf2/dw2-ref-missing-frame.exp: func_loopfb backtrace
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> FAIL: gdb.linespec/linespec.exp: set breakpoint at lspec.cc instance of NameSpace::overload
> FAIL: gdb.linespec/linespec.exp: set breakpoint at specific instance of NameSpace::overload
Ack.
> FAIL: gdb.mi/dw2-ref-missing-frame.exp: test func_nofb_marker
> FAIL: gdb.mi/mi-var-display.exp: print FP register
> FAIL: gdb.mi/mi2-var-display.exp: print FP register
> FAIL: gdb.opt/inline-bt.exp: continue to bar (1)
> FAIL: gdb.opt/inline-bt.exp: continue to bar (2)
> FAIL: gdb.opt/inline-bt.exp: continue to bar (3)
> FAIL: gdb.opt/inline-cmds.exp: continue to bar (1)
> FAIL: gdb.opt/inline-cmds.exp: continue to bar (2)
> FAIL: gdb.opt/inline-cmds.exp: continue to marker
> FAIL: gdb.opt/inline-cmds.exp: next over inlined functions
> FAIL: gdb.opt/inline-cmds.exp: next past inlined func1
> FAIL: gdb.opt/inline-cmds.exp: step into finish marker
> FAIL: gdb.opt/inline-cmds.exp: enter inlined_fn from noinline
> FAIL: gdb.opt/inline-cmds.exp: backtrace at inlined_fn from noinline
> FAIL: gdb.opt/inline-cmds.exp: inlined_fn from noinline inlined
> FAIL: gdb.opt/inline-cmds.exp: up to noinline
> FAIL: gdb.opt/inline-cmds.exp: noinline from outer_inline1 not inlined
> FAIL: gdb.opt/inline-cmds.exp: up to outer_inline1
> FAIL: gdb.opt/inline-cmds.exp: outer_inline1 inlined
> FAIL: gdb.opt/inline-cmds.exp: up to outer_inline2
> FAIL: gdb.opt/inline-cmds.exp: outer_inline2 inlined
> FAIL: gdb.opt/inline-cmds.exp: up from outer_inline2
> FAIL: gdb.opt/inline-cmds.exp: main not inlined
> FAIL: gdb.opt/inline-locals.exp: continue to bar (1)
> FAIL: gdb.opt/inline-locals.exp: continue to bar (2)
> FAIL: gdb.opt/inline-locals.exp: continue to bar (3)
> FAIL: gdb.stabs/gdb11479.exp: Stop at first breakpoint forced_stabs (the program exited)
> FAIL: gdb.stabs/gdb11479.exp: Inspect t in test2 forced_stabs
> FAIL: gdb.stabs/gdb11479.exp: sizeof (e) in test2 forced_stabs
> FAIL: gdb.stabs/gdb11479.exp: Stop at first breakpoint forced_stabs (the program is no longer running)
> FAIL: gdb.stabs/gdb11479.exp: Inspect t in test forced_stabs
> FAIL: gdb.stabs/gdb11479.exp: sizeof (e) in test forced_stabs
These didn't originally FAIL for me, but already PASSed; no change with
your patch.
> Here is a list of FAILs that my patch introduced (regressions). There
> are 9 of them:
> 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
Ack. Additional regressions for my testing with your patch:
FAIL: gdb.base/store.exp: continue to add_float
FAIL: gdb.base/store.exp: continue to add_double
FAIL: gdb.base/store.exp: continue to add_doublest
> 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
Ack.
Additional regressions for my testing with your patch:
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
> Here are the FAILs that your patch fixed. There are 127 of them.
> FAIL: gdb.base/arrayidx.exp: Print array with array-indexes off
> FAIL: gdb.base/arrayidx.exp: Print array with array-indexes on
> FAIL: gdb.base/break-always.exp: continue to breakpoint: bar
Ack.
> FAIL: gdb.base/break-inline.exp: start
As above, I'm still seeing this:
Running /scratch/tschwing/FM_sh-linux-gnu/src/gdb-mainline/gdb/testsuite/gdb.base/break-inline.exp ...
ERROR: tcl error sourcing /scratch/tschwing/FM_sh-linux-gnu/src/gdb-mainline/gdb/testsuite/gdb.base/break-inline.exp.
ERROR: gdbserver does not support start without extended-remote
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tc
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tc
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-ts
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-ts
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-ti
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-ti
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tl
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tl
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tll
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tll
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tf
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tf
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-td
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-td
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tld
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tld
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-te
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-te
> FAIL: gdb.base/chng-syms.exp: running to stop_here first time
> FAIL: gdb.base/chng-syms.exp: continue until exit at breakpoint first time through (the program is no longer running)
> FAIL: gdb.base/gdb11531.exp: watchpoint variable triggers at next
> FAIL: gdb.base/reread.exp: breakpoint foo in first file
> FAIL: gdb.base/reread.exp: run to foo()
> FAIL: gdb.base/return.exp: continue to return of -5
> FAIL: gdb.base/return.exp: continue to return of -5.0
> FAIL: gdb.base/return2.exp: void function returned successfully
> FAIL: gdb.base/scope.exp: print funclocal at bar
> FAIL: gdb.base/scope.exp: print funclocal_bss at bar
> FAIL: gdb.base/skip.exp: step after deleting 1 (3)
> FAIL: gdb.base/skip.exp: step after disabling 3 (5)
> FAIL: gdb.base/skip.exp: step after enable 3 (3)
> FAIL: gdb.base/step-resume-infcall.exp: step
> FAIL: gdb.base/step-test.exp: step into
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tc
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tc
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-ts
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-ts
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-ti
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-ti
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-ti
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-ti
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tl
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tl
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tl
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tl
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tll
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tll
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tf
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tf
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tf
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tf
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-td
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-td
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 1 structs-tld
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 1 structs-tld
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-ti-tc
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-ti-tc
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tl-tc
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tl-tc
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tf-tc
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tf-tc
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tc-ti
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tc-ti
> FAIL: gdb.base/structs.exp: advance to fun<n> for return; return 2 structs-tc-tl
> FAIL: gdb.base/structs.exp: advance to fun<n> for finish; return 2 structs-tc-tl
> FAIL: gdb.base/value-double-free.exp: continue
Ack.
> FAIL: gdb.base/watch-cond-infcall.exp: hw: continue
As above, this is is not tested in my configuration/with my target (no
hardware watchpoints).
> FAIL: gdb.base/watch-cond-infcall.exp: sw: continue
Ack.
> FAIL: gdb.base/watchpoint.exp: global_ptr next
> FAIL: gdb.base/watchpoint.exp: next over ptr init
> FAIL: gdb.base/watchpoint.exp: next over buffer set
> FAIL: gdb.base/watchpoint.exp: global_ptr_ptr next
> FAIL: gdb.base/watchpoint.exp: next over global_ptr_ptr init
> FAIL: gdb.base/watchpoint.exp: next over global_ptr_ptr buffer set
> FAIL: gdb.base/watchpoint.exp: next over global_ptr_ptr pointer advance
As above, these still FAIL for me with your patch.
> FAIL: gdb.base/watchpoint.exp: no-hw: global_ptr next
> FAIL: gdb.base/watchpoint.exp: no-hw: next over ptr init
> FAIL: gdb.base/watchpoint.exp: no-hw: next over buffer set
> FAIL: gdb.base/watchpoint.exp: no-hw: global_ptr_ptr next
> FAIL: gdb.base/watchpoint.exp: no-hw: next over global_ptr_ptr init
> FAIL: gdb.base/watchpoint.exp: no-hw: next over global_ptr_ptr buffer set
> FAIL: gdb.base/watchpoint.exp: no-hw: next over global_ptr_ptr pointer advance
As above, actually, these are the ones that still FAIL for me with your
patch; the other ones (with ``no-hw'' prefix) are with hardware
watchpoints, which is not applicable for my target.
> FAIL: gdb.base/watchpoints.exp: watchpoint ival1 hit, second time
> FAIL: gdb.base/watchpoints.exp: Watchpoint ival1 hit count is 2
> FAIL: gdb.base/watchpoints.exp: watchpoint hit, second time
> FAIL: gdb.base/watchpoints.exp: Watchpoint hit count is 2
> FAIL: gdb.base/watchpoints.exp: watchpoint ival1 hit, third time
> FAIL: gdb.base/watchpoints.exp: Watchpoint ival1 hit count is 3
> FAIL: gdb.base/watchpoints.exp: watchpoint hit, third time
> FAIL: gdb.base/watchpoints.exp: Watchpoint hit count is 3
> FAIL: gdb.base/watchpoints.exp: watchpoint hit, fourth time
> FAIL: gdb.base/watchpoints.exp: Watchpoint hit count is 4
> FAIL: gdb.base/watchpoints.exp: watchpoint hit, fifth time
> FAIL: gdb.base/watchpoints.exp: Watchpoint hit count is 5
> FAIL: gdb.cp/expand-sals.exp: continue to breakpoint: func
> FAIL: gdb.cp/expand-sals.exp: continue to breakpoint: next caller func
> FAIL: gdb.cp/extern-c.exp: continue to breakpoint: c_func
> FAIL: gdb.cp/extern-c.exp: continue to breakpoint: c_funcs_1
> FAIL: gdb.cp/extern-c.exp: continue to breakpoint: c_funcs_2
> FAIL: gdb.cp/namespace-nested-import.exp: print C::x
> FAIL: gdb.linespec/linespec.exp: set breakpoint at lspec.cc instance of NameSpace::overload
> FAIL: gdb.linespec/linespec.exp: set breakpoint at specific instance of NameSpace::overload
Ack.
> FAIL: gdb.opt/inline-bt.exp: continue to bar (1)
> FAIL: gdb.opt/inline-bt.exp: continue to bar (2)
> FAIL: gdb.opt/inline-bt.exp: continue to bar (3)
> FAIL: gdb.opt/inline-cmds.exp: continue to bar (1)
> FAIL: gdb.opt/inline-cmds.exp: continue to bar (2)
> FAIL: gdb.opt/inline-cmds.exp: continue to marker
> FAIL: gdb.opt/inline-cmds.exp: step into finish marker
> FAIL: gdb.opt/inline-cmds.exp: enter inlined_fn from noinline
> FAIL: gdb.opt/inline-cmds.exp: backtrace at inlined_fn from noinline
> FAIL: gdb.opt/inline-cmds.exp: inlined_fn from noinline inlined
> FAIL: gdb.opt/inline-cmds.exp: up to noinline
> FAIL: gdb.opt/inline-cmds.exp: noinline from outer_inline1 not inlined
> FAIL: gdb.opt/inline-cmds.exp: up to outer_inline1
> FAIL: gdb.opt/inline-cmds.exp: outer_inline1 inlined
> FAIL: gdb.opt/inline-cmds.exp: up to outer_inline2
> FAIL: gdb.opt/inline-cmds.exp: outer_inline2 inlined
> FAIL: gdb.opt/inline-cmds.exp: up from outer_inline2
> FAIL: gdb.opt/inline-cmds.exp: main not inlined
> FAIL: gdb.opt/inline-locals.exp: continue to bar (1)
> FAIL: gdb.opt/inline-locals.exp: continue to bar (2)
> FAIL: gdb.opt/inline-locals.exp: continue to bar (3)
As above, these didn't originally FAIL for me, but already PASSed; no
change with my patch.
> Your patch did not introduce any regressions.
Ack.
Grüße,
Thomas
[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Simulator testing for sh and sh64 (was: [PATCH] [SH] Prologue skipping if there is none)
2012-02-17 2:30 ` Kevin Buettner
2012-02-20 16:19 ` Thomas Schwinge
2012-02-21 15:23 ` Thomas Schwinge
@ 2012-02-22 14:54 ` Thomas Schwinge
2012-02-22 16:56 ` Kevin Buettner
2 siblings, 1 reply; 34+ messages in thread
From: Thomas Schwinge @ 2012-02-22 14:54 UTC (permalink / raw)
To: Kevin Buettner, gdb-patches
[-- Attachment #1: Type: text/plain, Size: 632 bytes --]
Hi Kevin!
On Thu, 16 Feb 2012 18:25:44 -0700, Kevin Buettner <kevinb@redhat.com> wrote:
> On Thu, 16 Feb 2012 17:32:18 +0100
> Thomas Schwinge <thomas@codesourcery.com> wrote:
>
> > I now have (on a SH7785-based board). My patch fixes a few more failures
> > than yours. ;-P
>
> This will require more study and discussion then. I tested against
> the simulator using the default multilib.
How do you configure the toolchain's components for the sim testing?
And, any quick suggestion for a sh64 sim testing configuration, too? My
attempt so far only results in a series of SIGILL...
Grüße,
Thomas
[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Simulator testing for sh and sh64 (was: [PATCH] [SH] Prologue skipping if there is none)
2012-02-22 14:54 ` Simulator testing for sh and sh64 (was: [PATCH] [SH] Prologue skipping if there is none) Thomas Schwinge
@ 2012-02-22 16:56 ` Kevin Buettner
2012-02-22 19:33 ` Simulator testing for sh and sh64 Thomas Schwinge
0 siblings, 1 reply; 34+ messages in thread
From: Kevin Buettner @ 2012-02-22 16:56 UTC (permalink / raw)
To: gdb-patches
On Wed, 22 Feb 2012 15:52:03 +0100
Thomas Schwinge <thomas@codesourcery.com> wrote:
> How do you configure the toolchain's components for the sim testing?
I use --target=sh-elf .
When it comes time to run the tests, do:
make check RUNTESTFLAGS="--target_board=sh-sim"
> And, any quick suggestion for a sh64 sim testing configuration, too? My
> attempt so far only results in a series of SIGILL...
I haven't tried this in a while. I'll give it a go and let you know
what I find.
Kevin
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Simulator testing for sh and sh64
2012-02-22 16:56 ` Kevin Buettner
@ 2012-02-22 19:33 ` Thomas Schwinge
2012-02-23 0:35 ` Kaz Kojima
2012-02-23 19:55 ` Thomas Schwinge
0 siblings, 2 replies; 34+ messages in thread
From: Thomas Schwinge @ 2012-02-22 19:33 UTC (permalink / raw)
To: Kaz Kojima, gcc; +Cc: Kevin Buettner, gdb-patches
[-- Attachment #1: Type: text/plain, Size: 4357 bytes --]
Hi!
This is about sh and sh64 GDB sim testing for the whole toolchain. (I
also do have sh4a hardware available, where testing is working fine.)
Kaz, could you please have a look whether this looks basically sane, that
my assumptions and the results I'm getting are about right, etc.?
On Wed, 22 Feb 2012 09:39:29 -0700, Kevin Buettner <kevinb@redhat.com> wrote:
> On Wed, 22 Feb 2012 15:52:03 +0100
> Thomas Schwinge <thomas@codesourcery.com> wrote:
>
> > How do you configure the toolchain's components for the sim testing?
>
> I use --target=sh-elf .
>
> When it comes time to run the tests, do:
>
> make check RUNTESTFLAGS="--target_board=sh-sim"
OK, that matches what I'm doing (simple enough), and that works for me,
too.
With all-current sources (CVS HEAD, svn trunk, Git master, as
appropriate), I get 707 unexpected failures in g++ testing (a lot of
execution tests, as it seems), 204 in gcc, 434 in gdb (I'm currently
working on improving that), 41 in ld (seems that some test harness
problem is involved; get a lot of: ``sh-elf-ld: cannot find sh-unknown.o:
No such file or directory''), 322 in libstdc++, 3 in newlib. So far, so
good.
> > And, any quick suggestion for a sh64 sim testing configuration, too? My
> > attempt so far only results in a series of SIGILL...
Kaz, is my understanding correct, that I simply use sh64-elf as target,
and again the sh-sim board? Should I be setting a specific CPU when
configuring GCC, or any other customization?
Building all-current sources comes to a halt as follows:
/scratch/tschwing/FM_sh64-elf/obj/gcc-first-mainline-0-sh64-elf-i686-pc-linux-gnu/./gcc/xgcc -B/scratch/tschwing/FM_sh64-elf/obj/gcc-first-mainline-0-sh64-elf-i686-pc-linux-gnu/./gcc/ -B/scratch/tschwing/FM_sh64-elf/install/sh64-elf/bin/ -B/scratch/tschwing/FM_sh64-elf/install/sh64-elf/lib/ -isystem /scratch/tschwing/FM_sh64-elf/install/sh64-elf/include -isystem /scratch/tschwing/FM_sh64-elf/install/sh64-elf/sys-include --sysroot=/scratch/tschwing/FM_sh64-elf/install/sh64-elf -g -O2 -ml -O2 -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -W -Wall -Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -isystem ./include -mieee -g -DIN_LIBGCC2 -fbuilding-libgcc -fno-stack-protector -Dinhibit_libc -mieee -I. -I. -I../../.././gcc -I/scratch/tschwing/FM_sh64-elf/src/gcc-mainline/libgcc -I/scratch/tschwing/FM_sh64-elf/src/gcc-mainline/libgcc/. -I/scratch/tschwing/FM_sh64-elf/src/gcc-mainline/libgcc/../gcc -I/scratch/tschwing/FM_sh64-elf/src/gcc-mainline/libgcc/../include -DHAVE_CC_TLS -DUSE_EMUTLS -o _powisf2.o -MT _powisf2.o -MD -MP -MF _powisf2.dep -DL_powisf2 -c /scratch/tschwing/FM_sh64-elf/src/gcc-mainline/libgcc/libgcc2.c
/scratch/tschwing/FM_sh64-elf/src/gcc-mainline/libgcc/libgcc2.c: In function '__powisf2':
/scratch/tschwing/FM_sh64-elf/src/gcc-mainline/libgcc/libgcc2.c:1779:1: error: unrecognizable insn:
(insn 10 9 11 3 (set (reg:SI 162 [ D.2769 ])
(abs:SI (reg/v:SI 168 [ m ]))) /scratch/tschwing/FM_sh64-elf/src/gcc-mainline/libgcc/libgcc2.c:1770 -1
(nil))
/scratch/tschwing/FM_sh64-elf/src/gcc-mainline/libgcc/libgcc2.c:1779:1: internal compiler error: in extract_insn, at recog.c:2123
Stepping back to using the 4.5 GCC branch and otherwise all-current
sources, it compiles, and I get 76 unexpected failures in g++ (a lot of
``ld: section .stack loaded at [0000000000080000,0000000000080003]
overlaps section .text loaded at [0000000000001060,00000000000ec0bf]''),
119 in gcc, 41 in ld, 1185 in libstdc++ (the section overlap issue again,
it seems), 3 in newlib, and GDB testing totally breaks down: I'm
receiving a lot of ``Program received signal SIGILL, Illegal
instruction''; from a quick investigation, it seems that GDB is patching
the breakpoints at addresses that are 2 bytes offset from where they
meant to go. I'll have a look at this.
Moving a bit forward by using the 4.6 GCC branch and otherwise
all-current sources, it compiles, and the test results look similar to
GCC 4.5's.
This means, for sh-elf sim testing, we have a bit too many failures in
GCC and GDB, and some ld test harness issue. For sh64-elf we have a GCC
trunk ICE, some section overlap issue, and even more GDB issues.
Grüße,
Thomas
[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Simulator testing for sh and sh64
2012-02-22 19:33 ` Simulator testing for sh and sh64 Thomas Schwinge
@ 2012-02-23 0:35 ` Kaz Kojima
2012-02-24 21:38 ` Thomas Schwinge
2012-02-23 19:55 ` Thomas Schwinge
1 sibling, 1 reply; 34+ messages in thread
From: Kaz Kojima @ 2012-02-23 0:35 UTC (permalink / raw)
To: thomas; +Cc: gcc, kevinb, gdb-patches
Thomas Schwinge <thomas@codesourcery.com> wrote:
> This is about sh and sh64 GDB sim testing for the whole toolchain. (I
> also do have sh4a hardware available, where testing is working fine.)
> Kaz, could you please have a look whether this looks basically sane, that
> my assumptions and the results I'm getting are about right, etc.?
You are right, I think.
> Kaz, is my understanding correct, that I simply use sh64-elf as target,
> and again the sh-sim board? Should I be setting a specific CPU when
> configuring GCC, or any other customization?
I used sh64-sim board for sh64-elf. sh64-sim.exp baseboard can
be seen in
http://lists.gnu.org/archive/html/dejagnu/2008-02/msg00056.html
> This means, for sh-elf sim testing, we have a bit too many failures in
> GCC and GDB, and some ld test harness issue. For sh64-elf we have a GCC
> trunk ICE, some section overlap issue, and even more GDB issues.
Yep. About sh64, I had used sh64-linux as my testing target, but
unfortunately that real sh64 system stopped working after the earthquake.
Regards,
kaz
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Simulator testing for sh and sh64
2012-02-22 19:33 ` Simulator testing for sh and sh64 Thomas Schwinge
2012-02-23 0:35 ` Kaz Kojima
@ 2012-02-23 19:55 ` Thomas Schwinge
2012-02-23 22:53 ` Kevin Buettner
2012-02-23 23:57 ` Kevin Buettner
1 sibling, 2 replies; 34+ messages in thread
From: Thomas Schwinge @ 2012-02-23 19:55 UTC (permalink / raw)
To: gdb-patches; +Cc: Kevin Buettner, Kaz Kojima, gcc
[-- Attachment #1: Type: text/plain, Size: 2065 bytes --]
Hi!
On Wed, 22 Feb 2012 20:29:11 +0100, I wrote:
> On Wed, 22 Feb 2012 09:39:29 -0700, Kevin Buettner <kevinb@redhat.com> wrote:
> > On Wed, 22 Feb 2012 15:52:03 +0100
> > Thomas Schwinge <thomas@codesourcery.com> wrote:
> > > And, any quick suggestion for a sh64 sim testing configuration, too? My
> > > attempt so far only results in a series of SIGILL...
> GDB testing totally breaks down: I'm
> receiving a lot of ``Program received signal SIGILL, Illegal
> instruction''; from a quick investigation, it seems that GDB is patching
> the breakpoints at addresses that are 2 bytes offset from where they
> meant to go. I'll have a look at this.
I did, but it's turning into a rat's nest -- sh64-tdep needs several
years of catch-up, as it seems. The SIGILL issue that I briefly
described is what Kevin (hello, hello!) :-) fixed for MIPS in
486ee7f3437358941f0762ace2550170ef474de1,
<http://sourceware.org/ml/gdb-patches/2010-12/msg00202.html>; basically
the issue is that setting PC's bit 0 in sh64_elf_make_msymbol_special for
ISA32 (SHmedia) code will confuse GDB's msymbol machinery, resulting
first in a 1-byte offset, which is later ``fixed'' into the 2-byte offset
that I mentioned. And patching a 4-byte breakpoint instruction into the
middle of two 4-byte instructions is very likely to cause a SIGILL.
I have begun hacking into sh64-tdep the same kind of changes that Kevin
did for MIPS, but there's more to do, as I'm now seeing ``odd addresses''
for symbols coming out of GDB's symbol table lookup, find_pc_line.
Again, the addresses point into the middle of 4-byte instructions. This
may be an issue with the debugging information GCC generates, or it may
be something else. For the time being, I'm stopping at this point.
Anyway, the patch for sh-tdep that I posted in
<http://sourceware.org/ml/gdb-patches/2012-02/msg00299.html> (at the end)
also applies to sh64-tdep -- shall I commit the equivalent sh64-tdep
change without any testsuite testing, or let it bit-rot some more?
Grüße,
Thomas
[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Simulator testing for sh and sh64
2012-02-23 19:55 ` Thomas Schwinge
@ 2012-02-23 22:53 ` Kevin Buettner
2012-02-24 11:12 ` Thomas Schwinge
2012-02-23 23:57 ` Kevin Buettner
1 sibling, 1 reply; 34+ messages in thread
From: Kevin Buettner @ 2012-02-23 22:53 UTC (permalink / raw)
To: gdb-patches
On Thu, 23 Feb 2012 20:49:50 +0100
Thomas Schwinge <thomas@codesourcery.com> wrote:
> Anyway, the patch for sh-tdep that I posted in
> <http://sourceware.org/ml/gdb-patches/2012-02/msg00299.html> (at the end)
> also applies to sh64-tdep -- shall I commit the equivalent sh64-tdep
> change without any testsuite testing, or let it bit-rot some more?
I would like you to apply that patch to sh-tdep.c. (So consider
that patch approved for sh-tdep.c.)
I'd also like to see some version of my patch go in if it makes
sense. But I'd like to see your patch applied as a first step.
(I sent a similar reply on Monday.)
With regard to sh64-tdep.c... Are you able to do any testing at all
to make sure that your patch basically works for sh64? If you're
able to get partial results with the other changes that you've made,
I think that's good enough. Even hand testing on something like
the gdb.base/break.exp test case would be okay.
So... if you're able to test it at all so that you know it basically
works, then it can go in. If not, I'd prefer to have sh64-tdep.c left
in its current state until you are able to do some testing.
Kevin
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Simulator testing for sh and sh64
2012-02-23 19:55 ` Thomas Schwinge
2012-02-23 22:53 ` Kevin Buettner
@ 2012-02-23 23:57 ` Kevin Buettner
1 sibling, 0 replies; 34+ messages in thread
From: Kevin Buettner @ 2012-02-23 23:57 UTC (permalink / raw)
To: gdb-patches
On Thu, 23 Feb 2012 20:49:50 +0100
Thomas Schwinge <thomas@codesourcery.com> wrote:
> I did, but it's turning into a rat's nest -- sh64-tdep needs several
> years of catch-up, as it seems. The SIGILL issue that I briefly
> described is what Kevin (hello, hello!) :-) fixed for MIPS in
> 486ee7f3437358941f0762ace2550170ef474de1,
> <http://sourceware.org/ml/gdb-patches/2010-12/msg00202.html>; basically
> the issue is that setting PC's bit 0 in sh64_elf_make_msymbol_special for
> ISA32 (SHmedia) code will confuse GDB's msymbol machinery, resulting
> first in a 1-byte offset, which is later ``fixed'' into the 2-byte offset
> that I mentioned. And patching a 4-byte breakpoint instruction into the
> middle of two 4-byte instructions is very likely to cause a SIGILL.
For what it's worth, I used arm as a model when I made those MIPS
changes. I think I referenced one other architecture too, but I
don't recall what it was.
Kevin
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-02-21 5:25 ` Kevin Buettner
@ 2012-02-24 11:09 ` Thomas Schwinge
2012-02-24 22:21 ` Kevin Buettner
0 siblings, 1 reply; 34+ messages in thread
From: Thomas Schwinge @ 2012-02-24 11:09 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 3806 bytes --]
Hi Kevin!
On Mon, 20 Feb 2012 16:20:29 -0700, Kevin Buettner <kevinb@redhat.com> wrote:
> On Mon, 20 Feb 2012 17:15:54 +0100
> Thomas Schwinge <thomas@codesourcery.com> wrote:
>
> > So, we do seem to agree that something like the patch I posted initially
> > (and as it is incorporated in a similar fashion in your patch, too) does
> > already move us forward; is it reasonable that we commit that one now,
> > and then continue to look on how to further improve the situation based
> > on your patch? This will let us point out more easily which are the
> > additional cases your patch improves/regresses on. (I'd offer to port
> > your patch over to the new baseline, if you want me to.)
>
> Yeah, as a first step, I think it makes sense for you to commit
> the patch that you came up with.
Thanks; here is the patch I committed in the end:
2012-02-24 Thomas Schwinge <thomas@codesourcery.com>
* sh-tdep.c (sh_skip_prologue): Use skip_prologue_using_sal.
(after_prologue): Remove.
Index: gdb/sh-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sh-tdep.c,v
retrieving revision 1.237
diff -u -p -r1.237 sh-tdep.c
--- gdb/sh-tdep.c 17 Feb 2012 08:39:57 -0000 1.237
+++ gdb/sh-tdep.c 24 Feb 2012 10:59:08 -0000
@@ -712,55 +712,29 @@ sh_analyze_prologue (struct gdbarch *gdb
}
/* 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)
+sh_skip_prologue (struct gdbarch *gdbarch, 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 post_prologue_pc, func_addr;
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. */
- pc = after_prologue (start_pc);
+ 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);
+ }
- /* 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);
+ /* Can't determine prologue from the symbol table, need to examine
+ instructions. */
cache.sp_offset = -4;
- pc = sh_analyze_prologue (gdbarch, start_pc, (CORE_ADDR) -1, &cache, 0);
- if (!cache.uses_fp)
- return start_pc;
+ post_prologue_pc = sh_analyze_prologue (gdbarch, pc, (CORE_ADDR) -1, &cache, 0);
+ if (cache.uses_fp)
+ pc = post_prologue_pc;
return pc;
}
Working on your patch is the next step.
Grüße,
Thomas
[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Simulator testing for sh and sh64
2012-02-23 22:53 ` Kevin Buettner
@ 2012-02-24 11:12 ` Thomas Schwinge
0 siblings, 0 replies; 34+ messages in thread
From: Thomas Schwinge @ 2012-02-24 11:12 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches, Kaz Kojima
[-- Attachment #1: Type: text/plain, Size: 1248 bytes --]
Hi!
On Thu, 23 Feb 2012 15:48:46 -0700, Kevin Buettner <kevinb@redhat.com> wrote:
> On Thu, 23 Feb 2012 20:49:50 +0100
> Thomas Schwinge <thomas@codesourcery.com> wrote:
>
> > Anyway, the patch for sh-tdep that I posted in
> > <http://sourceware.org/ml/gdb-patches/2012-02/msg00299.html> (at the end)
> > also applies to sh64-tdep -- shall I commit the equivalent sh64-tdep
> > change without any testsuite testing, or let it bit-rot some more?
> With regard to sh64-tdep.c... Are you able to do any testing at all
> to make sure that your patch basically works for sh64? If you're
> able to get partial results with the other changes that you've made,
> I think that's good enough. Even hand testing on something like
> the gdb.base/break.exp test case would be okay.
Not really, I'm afraid.
> So... if you're able to test it at all so that you know it basically
> works, then it can go in. If not, I'd prefer to have sh64-tdep.c left
> in its current state until you are able to do some testing.
OK.
Is there any actual interest (other than ``nice to have'') in getting
sh64 back into a functional state, given that its GDB port has very much
been broken for several years already?
Grüße,
Thomas
[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Simulator testing for sh and sh64
2012-02-23 0:35 ` Kaz Kojima
@ 2012-02-24 21:38 ` Thomas Schwinge
0 siblings, 0 replies; 34+ messages in thread
From: Thomas Schwinge @ 2012-02-24 21:38 UTC (permalink / raw)
To: Kaz Kojima; +Cc: gcc, kevinb, gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1111 bytes --]
Hi!
On Thu, 23 Feb 2012 08:42:53 +0900, Kaz Kojima <kkojima@rr.iij4u.or.jp> wrote:
> Thomas Schwinge <thomas@codesourcery.com> wrote:
> > Kaz, is my understanding correct, that I simply use sh64-elf as target,
> > and again the sh-sim board? Should I be setting a specific CPU when
> > configuring GCC, or any other customization?
>
> I used sh64-sim board for sh64-elf. sh64-sim.exp baseboard can
> be seen in
>
> http://lists.gnu.org/archive/html/dejagnu/2008-02/msg00056.html
I gave both sh-sim and sh64-sim a try, and -- if I'm reading correctly
between all the noise -- there isn't really much difference in the
testresults depending on which of the two is being used.
> > This means, for sh-elf sim testing, we have a bit too many failures in
> > GCC and GDB, and some ld test harness issue. For sh64-elf we have a GCC
> > trunk ICE, some section overlap issue, and even more GDB issues.
>
> Yep. About sh64, I had used sh64-linux as my testing target, but
> unfortunately that real sh64 system stopped working after the earthquake.
Ouch. :-/
Grüße,
Thomas
[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-02-24 11:09 ` Thomas Schwinge
@ 2012-02-24 22:21 ` Kevin Buettner
2012-02-29 13:51 ` Thomas Schwinge
0 siblings, 1 reply; 34+ messages in thread
From: Kevin Buettner @ 2012-02-24 22:21 UTC (permalink / raw)
To: gdb-patches
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...)
Kevin
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-02-24 22:21 ` Kevin Buettner
@ 2012-02-29 13:51 ` Thomas Schwinge
2012-03-01 0:13 ` Kevin Buettner
2012-03-01 9:00 ` Thomas Schwinge
0 siblings, 2 replies; 34+ messages in thread
From: Thomas Schwinge @ 2012-02-29 13:51 UTC (permalink / raw)
To: Kevin Buettner, gdb-patches
[-- 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:
Grüße,
Thomas
[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-02-29 13:51 ` Thomas Schwinge
@ 2012-03-01 0:13 ` Kevin Buettner
2012-03-01 9:03 ` Thomas Schwinge
2012-03-01 9:00 ` Thomas Schwinge
1 sibling, 1 reply; 34+ messages in thread
From: Kevin Buettner @ 2012-03-01 0:13 UTC (permalink / raw)
To: gdb-patches
On Wed, 29 Feb 2012 12:05:01 +0100
Thomas Schwinge <thomas@schwinge.name> wrote:
> 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
I could live with these if there were a significant number of progressions
elsewhere.
> 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).
This is bad. I think this could be fixed (by stopping the scan if a
return instruction is seen), but I'm not sure it's worth it at the
moment.
> 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>.
It turns out that I do. It's unrelated to prologue analysis. I'll
post it separately in a few minutes. (I'll CC you on it.)
It's a small patch that I thought I had removed from my tree. Sorry
about the noise caused by that patch.
> I'll now try to carry over your ``more advanced limit_pc/func_end
> mechanism'' separately.
I wouldn't make it a high priority. Your patch provides very good
results and my additions (on the prologue analysis front) hurt more
than they help.
Thanks for all of your work on this matter.
Kevin
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-02-29 13:51 ` Thomas Schwinge
2012-03-01 0:13 ` Kevin Buettner
@ 2012-03-01 9:00 ` Thomas Schwinge
2012-03-02 0:19 ` Kevin Buettner
1 sibling, 1 reply; 34+ messages in thread
From: Thomas Schwinge @ 2012-03-01 9:00 UTC (permalink / raw)
To: Kevin Buettner, gdb-patches
[-- 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 --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-03-01 0:13 ` Kevin Buettner
@ 2012-03-01 9:03 ` Thomas Schwinge
0 siblings, 0 replies; 34+ messages in thread
From: Thomas Schwinge @ 2012-03-01 9:03 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 890 bytes --]
Hi Kevin!
On Wed, 29 Feb 2012 17:12:47 -0700, Kevin Buettner <kevinb@redhat.com> wrote:
> On Wed, 29 Feb 2012 12:05:01 +0100
> Thomas Schwinge <thomas@schwinge.name> wrote:
> > 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>.
>
> It turns out that I do. It's unrelated to prologue analysis. I'll
> post it separately in a few minutes. (I'll CC you on it.)
>
> It's a small patch that I thought I had removed from my tree. Sorry
> about the noise caused by that patch.
Ah good, and no problem -- so at least we know there's no magic involved
causing the differences betweeen our two's testing setups.
> Thanks for all of your work on this matter.
Thanks to your for helping, reviewing, etc.!
Grüße,
Thomas
[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-03-01 9:00 ` Thomas Schwinge
@ 2012-03-02 0:19 ` Kevin Buettner
2012-03-02 11:18 ` Thomas Schwinge
0 siblings, 1 reply; 34+ messages in thread
From: Kevin Buettner @ 2012-03-02 0:19 UTC (permalink / raw)
To: gdb-patches
Hi Thomas,
Please see my comments on your patch below...
On Thu, 01 Mar 2012 10:00:00 +0100
Thomas Schwinge <thomas@codesourcery.com> wrote:
> @@ -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);
> }
FIXME and TODO comments are generally frowned upon. All too often,
they end up hanging about for many years.
If you want to check the validity of the address, you should call
something like target_read_memory() and examine the return status.
You may want to just keep that TODO comment in your tree or in
some other TODO list on the side.
> @@ -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. */
Ditto.
> 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))
For this, my preference would be for the limit check to
be moved a bit later on...
> {
> 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;
I.e. put the test here and break if the limit has been exceeded.
Also, is there a good reason for moving the increment further on? I
think it reads better to increment first and then fetch from the
pc instead of 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 */
Either get rid of that comment or put something meaningful in its place.
(I'd sort of like to see an explanation of how 2*6 was decided upon.
If it's historical and you don't know why, then just get rid of the
comment.)
> +
> 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))
Again, I'd like to see that limit check moved further on.
The reason for this is that I'd like to have a case for the JSR
instruction. I'd rather have the details regarding what might happen
if some limit is exceeded to be contained in the code implementing
that case.
(This is just my preference. Some other maintainer may disagree with
me.)
> {
> /* 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 */
I assume this is the limit that had been used before? If so, just say
so in a comment - if you like, you can also state that you don't know
how this number was derived.
FWIW, I'm not very fond of doing it this way. I think it'd be preferable
to make the prologue analyzer bail when it hits a branch, call, or
return instruction. It shouldn't be too hard to encode these cases
in the prologue analyzer.
We do need some limit though. I'm just concerned about debugging leaf
functions where that limit will put us into the next function. (This
was one of the problems with my earlier patch - it didn't handle that
case.)
> +
> + /* 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;
Kevin
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-03-02 0:19 ` Kevin Buettner
@ 2012-03-02 11:18 ` Thomas Schwinge
2012-03-02 12:01 ` Pedro Alves
2012-03-03 1:18 ` Kevin Buettner
0 siblings, 2 replies; 34+ messages in thread
From: Thomas Schwinge @ 2012-03-02 11:18 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 11710 bytes --]
Hi Kevin!
Thanks for reviewing; here are my comments.
On Thu, 1 Mar 2012 17:18:47 -0700, Kevin Buettner <kevinb@redhat.com> wrote:
> On Thu, 01 Mar 2012 10:00:00 +0100
> Thomas Schwinge <thomas@codesourcery.com> wrote:
>
> > @@ -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);
> > }
>
> FIXME and TODO comments are generally frowned upon. All too often,
> they end up hanging about for many years.
> You may want to just keep that TODO comment in your tree or in
> some other TODO list on the side.
Hmm, I don't agree. I think it's better to have such comments in a
central place, instead of each developer having their own set of them in
their own files. I do agree that source code comments are not useful for
more *general* ``work to be done'', but this is a very local issue, where
the comment applies directly to the next line. Anyway, I'm not the one
to set the rules here; I've taken these out.
> > - else if (IS_MOVI20 (inst))
> > + else if (IS_MOVI20 (inst)
> > + && (pc + 2 < limit_pc))
>
> For this, my preference would be for the limit check to
> be moved a bit later on...
>
>
> > {
> > 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;
>
> I.e. put the test here and break if the limit has been exceeded.
Here is my reasoning for moving the check early: for having a valid
movi20 instruction at pc (and thus this if-case to match), it is required
that four bytes can be read, that is, the words at pc as well as pc + 2
are within the limit. If that is not the case (because the limit is
hit), we can't analyze this as a movi20 instruction. Also, no other
if-case can then match anyway (if the movi20 one had matched here), and
we will drop out of the loop. Does that make sense now? Would a comment
clarify this?
> Also, is there a good reason for moving the increment further on? I
> think it reads better to increment first and then fetch from the
> pc instead of 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;
> > }
This (non-functional) change was my attempt to make the code easier to
grasp: first handle the full 32-bit instruction (the part at pc, and the
one at pc + 2), then advance over it. I don't feel strongly about this;
I backed it out.
> > @@ -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 */
>
> Either get rid of that comment or put something meaningful in its place.
> (I'd sort of like to see an explanation of how 2*6 was decided upon.
> If it's historical and you don't know why, then just get rid of the
> comment.)
> > - pc += 2;
> > - for (opc = pc + 12; pc < opc; pc += 2)
> > + for (; pc < limit_pc; pc += 2)
I don't like silently hiding constant values (that nobody knows and/or
researched where they're coming from). What I calculate as 2 * 6 was
hidden in the value 12 quoted above. Anyway, I added a bit of a comment.
> > - else if (IS_JSR (inst))
> > + else if (IS_JSR (inst)
> > + && (pc + 2 < limit_pc))
>
> Again, I'd like to see that limit check moved further on.
>
> The reason for this is that I'd like to have a case for the JSR
> instruction. I'd rather have the details regarding what might happen
> if some limit is exceeded to be contained in the code implementing
> that case.
>
> (This is just my preference. Some other maintainer may disagree with
> me.)
In this case, I can be convinced; jsr is not a 32-bit instruction, so
this if-case may well trigger also if only two bytes are within the
limit. But, if won't ever matter in practice, as no complying function
is going to end with an jsr and nothing after it. It may only be that we
have a jsr as the last instruction before we hit limit_pc, and in that
case things are a bit dubious anyway. I changed this.
> > @@ -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 */
>
> I assume this is the limit that had been used before? If so, just say
> so in a comment - if you like, you can also state that you don't know
> how this number was derived.
Same as with the 2 * 6 case above. Also, a lot of other architectures'
tdep files that I looked at have similar comments, with ``Magic'' being a
very prominent one. We're now clearly improving upon that!
> FWIW, I'm not very fond of doing it this way.
(I'm assuming ``this'' is the 28 instructions limit.) This is what many
other architectures' tdep files are doing, too, and it triggers only as a
safeguard if we fail to find any other limit. Also, a similar thing is
used in sh_in_function_epilogue_p.
> I think it'd be preferable
> to make the prologue analyzer bail when it hits a branch, call, or
> return instruction. It shouldn't be too hard to encode these cases
> in the prologue analyzer.
I consider your suggestion to be an additional improvement, but still
think that my patch (more specifically here, the 28 instructions limit)
remains valid, too.
> We do need some limit though. I'm just concerned about debugging leaf
> functions where that limit will put us into the next function. (This
> was one of the problems with my earlier patch - it didn't handle that
> case.)
As I said, this limit is only a safeguard if everything else fails.
Before that, the end of the function will have tried to be determined
with the symbol table (find_pc_partial_function), or debug information
(skip_prologue_using_sal), which will typically trigger (but not in PLT
slots).
How's this version?
* 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 2 Mar 2012 11:12:15 -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. */
@@ -614,7 +610,8 @@ sh_analyze_prologue (struct gdbarch *gdb
}
}
}
- else if (IS_MOVI20 (inst))
+ else if (IS_MOVI20 (inst)
+ && (pc + 2 < limit_pc))
{
if (sav_reg < 0)
{
@@ -656,14 +653,17 @@ sh_analyze_prologue (struct gdbarch *gdb
}
else if (IS_MOV_SP_FP (inst))
{
+ pc += 2;
+ /* Don't go any further than six more instructions. */
+ limit_pc = min (limit_pc, pc + (2 * 6));
+
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))
@@ -695,10 +695,13 @@ sh_analyze_prologue (struct gdbarch *gdb
jsr, which will be very confusing. Most likely the next
instruction is going to be IS_MOV_SP_FP in the delay slot. If
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;
- break;
+ if (pc + 2 < limit_pc)
+ {
+ inst = read_memory_integer (pc + 2, 2, byte_order);
+ if (IS_MOV_SP_FP (inst))
+ cache->uses_fp = 1;
+ }
+ break;
}
#if 0 /* This used to just stop when it found an instruction
that was not considered part of the prologue. Now,
@@ -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,21 @@ 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)
+ /* Don't go any further than 28 instructions. */
+ limit_pc = pc + (2 * 28);
+
+ /* 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 --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-03-02 11:18 ` Thomas Schwinge
@ 2012-03-02 12:01 ` Pedro Alves
2012-03-02 14:15 ` Thomas Schwinge
2012-03-03 1:18 ` Kevin Buettner
1 sibling, 1 reply; 34+ messages in thread
From: Pedro Alves @ 2012-03-02 12:01 UTC (permalink / raw)
To: Thomas Schwinge; +Cc: Kevin Buettner, gdb-patches
On 03/02/2012 11:17 AM, Thomas Schwinge wrote:
> On Thu, 1 Mar 2012 17:18:47 -0700, Kevin Buettner <kevinb@redhat.com> wrote:
>> > On Thu, 01 Mar 2012 10:00:00 +0100
>> > Thomas Schwinge <thomas@codesourcery.com> wrote:
>> >
>>> > > @@ -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);
>>> > > }
>> >
>> > FIXME and TODO comments are generally frowned upon. All too often,
>> > they end up hanging about for many years.
>> > You may want to just keep that TODO comment in your tree or in
>> > some other TODO list on the side.
> Hmm, I don't agree. I think it's better to have such comments in a
> central place, instead of each developer having their own set of them in
> their own files. I do agree that source code comments are not useful for
> more *general* ``work to be done'', but this is a very local issue, where
> the comment applies directly to the next line. Anyway, I'm not the one
> to set the rules here; I've taken these out.
FWIW, I agree that FIXMEs and TODOs in the code can be helpful. I've learnt
immensely about gdb's intended direction from the FIXME's in place. However,
there should be a high barrier to adding new FIXMEs. If the fix is
known, and it doesn't involve e.g., deep design level changes, then we should
just get it fixed before the change lands in the tree. In this case, what's
necessary to just fix that particular issue?
--
Pedro Alves
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-03-02 12:01 ` Pedro Alves
@ 2012-03-02 14:15 ` Thomas Schwinge
2012-03-06 19:08 ` Pedro Alves
0 siblings, 1 reply; 34+ messages in thread
From: Thomas Schwinge @ 2012-03-02 14:15 UTC (permalink / raw)
To: Pedro Alves; +Cc: Kevin Buettner, gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1618 bytes --]
Hi!
On Fri, 2 Mar 2012 12:00:36 +0000, Pedro Alves <palves@redhat.com> wrote:
> On 03/02/2012 11:17 AM, Thomas Schwinge wrote:
> > On Thu, 1 Mar 2012 17:18:47 -0700, Kevin Buettner <kevinb@redhat.com> wrote:
> >> > On Thu, 01 Mar 2012 10:00:00 +0100
> >> > Thomas Schwinge <thomas@codesourcery.com> wrote:
> >> >
> >>> > > @@ -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);
> >>> > > }
> In this case, what's
> necessary to just fix that particular issue?
The issue here is that external data (a malicious executable that is
being debugged) might possibly cause GDB to do arbitrary things due to
corrupting its internal state. (I don't know if GDB development is
generally paying attention to such ``detail'', but it certainly is an
attack vector if you're debugging a binary that has been provided by a
third party.)
For inaccessible addresses, target_read_memory returns EIO, which causes
read_memory to invoke throw_error: ``Cannot access memory at address
0xfffffffe''; that's fine.
For improper but accessible addresses, it is more difficult to predict
what might happen in the following. The value will be propagated into a
frame cache's sp_offset and saved_sp. From there on, we have to rely on
the frame unwinding machinery to reliably detect any failures or
inconsistencies.
Grüße,
Thomas
[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-03-02 11:18 ` Thomas Schwinge
2012-03-02 12:01 ` Pedro Alves
@ 2012-03-03 1:18 ` Kevin Buettner
2012-03-05 15:16 ` Thomas Schwinge
1 sibling, 1 reply; 34+ messages in thread
From: Kevin Buettner @ 2012-03-03 1:18 UTC (permalink / raw)
To: gdb-patches; +Cc: Thomas Schwinge
On Fri, 02 Mar 2012 12:17:39 +0100
Thomas Schwinge <thomas@codesourcery.com> wrote:
> > > - else if (IS_MOVI20 (inst))
> > > + else if (IS_MOVI20 (inst)
> > > + && (pc + 2 < limit_pc))
> >
> > For this, my preference would be for the limit check to
> > be moved a bit later on...
> >
> >
> > > {
> > > 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;
> >
> > I.e. put the test here and break if the limit has been exceeded.
>
> Here is my reasoning for moving the check early: for having a valid
> movi20 instruction at pc (and thus this if-case to match), it is required
> that four bytes can be read, that is, the words at pc as well as pc + 2
> are within the limit. If that is not the case (because the limit is
> hit), we can't analyze this as a movi20 instruction. Also, no other
> if-case can then match anyway (if the movi20 one had matched here), and
> we will drop out of the loop. Does that make sense now? Would a comment
> clarify this?
What you say makes sense, but is still not my preference. However, I
don't see this as that big of a deal and am willing to let it go.
I'd actually prefer not to see a comment about this. IMO, sometimes
it's just better to let the code speak for itself. A lot of times
comments become out of date and what's in the comment doesn't match up
with what the code is actually doing. Such comments are worse than
having no comment at all because sometimes the reader will just accept
what the comment is saying. And, then, if a mismatch if found, the
question becomes, "Is the code wrong, or is the comment wrong?"
> > Also, is there a good reason for moving the increment further on? I
> > think it reads better to increment first and then fetch from the
> > pc instead of 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;
> > > }
>
> This (non-functional) change was my attempt to make the code easier to
> grasp: first handle the full 32-bit instruction (the part at pc, and the
> one at pc + 2), then advance over it. I don't feel strongly about this;
> I backed it out.
Okay, thanks.
[...]
> > > @@ -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 */
> >
> > I assume this is the limit that had been used before? If so, just say
> > so in a comment - if you like, you can also state that you don't know
> > how this number was derived.
>
> Same as with the 2 * 6 case above. Also, a lot of other architectures'
> tdep files that I looked at have similar comments, with ``Magic'' being a
> very prominent one. We're now clearly improving upon that!
>
> > FWIW, I'm not very fond of doing it this way.
>
> (I'm assuming ``this'' is the 28 instructions limit.) This is what many
> other architectures' tdep files are doing, too, and it triggers only as a
> safeguard if we fail to find any other limit. Also, a similar thing is
> used in sh_in_function_epilogue_p.
In the past, these limits were often calculated by hand coding a
"maximal prologue", i.e. an instruction sequence which made the
necessary frame adjustments (counting a potential alloca in the
function body), saved all possible callee-saved registers and wrote
all possible register arguments to the stack. One simply counted the
instructions in this sequence and that was your limit.
A lot of prologue analyzers had this kind of limit in them at one
time. Perhaps many of them still do; I haven't made a recent survey.
The problem with this is that often times instructions from the
function body get moved into the prologue, especially for optimized
code. But, these days, it's happening even for unoptimized code too.
It's not clear to me if that constant is still large enough.
And sometimes, especially for leaf functions, there is no identifiable
prologue in which case the limit is much too high.
As I said before, my preference is to scan until some instruction is
reached that we know cannot be part of the prologue or until some other
limit is hit such as the address of the instruction pointer. An
instruction which changes the control flow is often an excellent
place to stop. Sometimes though, special provisions must be made
for accomodating PIC code. In such a case, it's not uncommon for
there to be a JSR or some such to a nearby address just so that
the current PC value is known. I don't know if this happens on sh
or not.
With all that said, if you're preserving existing functionality, I'm
not going to hold up your patch. But enhancements which remove those
limits would be welcome. (You may want to revisit the patch that I
posted earlier.) Or, alternately, if you can find a compelling case
for why those limits should be preserved, then a comment describing
such would be appreciated too.
> > I think it'd be preferable
> > to make the prologue analyzer bail when it hits a branch, call, or
> > return instruction. It shouldn't be too hard to encode these cases
> > in the prologue analyzer.
>
> I consider your suggestion to be an additional improvement, but still
> think that my patch (more specifically here, the 28 instructions limit)
> remains valid, too.
Yes, subject to my comments above.
> > We do need some limit though. I'm just concerned about debugging leaf
> > functions where that limit will put us into the next function. (This
> > was one of the problems with my earlier patch - it didn't handle that
> > case.)
>
> As I said, this limit is only a safeguard if everything else fails.
> Before that, the end of the function will have tried to be determined
> with the symbol table (find_pc_partial_function), or debug information
> (skip_prologue_using_sal), which will typically trigger (but not in PLT
> slots).
Hmm. Have you tried it when only minimal symbols are present? (It is
useful to have this stuff working so that you can get decent stack
traces when only linker symbols are present.)
> How's this version?
>
> * 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.
Okay. (You can check it in.)
Kevin
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-03-03 1:18 ` Kevin Buettner
@ 2012-03-05 15:16 ` Thomas Schwinge
2012-03-05 19:40 ` Kevin Buettner
0 siblings, 1 reply; 34+ messages in thread
From: Thomas Schwinge @ 2012-03-05 15:16 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1523 bytes --]
Hi!
On Fri, 2 Mar 2012 18:18:10 -0700, Kevin Buettner <kevinb@redhat.com> wrote:
> On Fri, 02 Mar 2012 12:17:39 +0100
> Thomas Schwinge <thomas@codesourcery.com> wrote:
> > (I'm assuming ``this'' is the 28 instructions limit.) This is what many
> > other architectures' tdep files are doing, too, and it triggers only as a
> > safeguard if we fail to find any other limit. Also, a similar thing is
> > used in sh_in_function_epilogue_p.
>
> In the past, [...]
Thanks for telling the history of that.
> enhancements which remove those limits would be welcome.
Should we file a bug in bugzilla for that?
> > > We do need some limit though. I'm just concerned about debugging leaf
> > > functions where that limit will put us into the next function. (This
> > > was one of the problems with my earlier patch - it didn't handle that
> > > case.)
> >
> > As I said, this limit is only a safeguard if everything else fails.
> > Before that, the end of the function will have tried to be determined
> > with the symbol table (find_pc_partial_function), or debug information
> > (skip_prologue_using_sal), which will typically trigger (but not in PLT
> > slots).
>
> Hmm. Have you tried it when only minimal symbols are present? (It is
> useful to have this stuff working so that you can get decent stack
> traces when only linker symbols are present.)
Yes, I had a look at gdb.base's arrayidx, advance and nodebug examples,
and they're working as before.
Grüße,
Thomas
[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-03-05 15:16 ` Thomas Schwinge
@ 2012-03-05 19:40 ` Kevin Buettner
0 siblings, 0 replies; 34+ messages in thread
From: Kevin Buettner @ 2012-03-05 19:40 UTC (permalink / raw)
To: gdb-patches
On Mon, 05 Mar 2012 16:16:11 +0100
Thomas Schwinge <thomas@codesourcery.com> wrote:
> > enhancements which remove those limits would be welcome.
>
> Should we file a bug in bugzilla for that?
Sure, if you know of some test case which isn't handled by the current
code. (It doesn't have to be a part of the current test suite.)
Kevin
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [SH] Prologue skipping if there is none
2012-03-02 14:15 ` Thomas Schwinge
@ 2012-03-06 19:08 ` Pedro Alves
0 siblings, 0 replies; 34+ messages in thread
From: Pedro Alves @ 2012-03-06 19:08 UTC (permalink / raw)
To: Thomas Schwinge; +Cc: Kevin Buettner, gdb-patches
On 03/02/2012 02:14 PM, Thomas Schwinge wrote:
> Hi!
>
> On Fri, 2 Mar 2012 12:00:36 +0000, Pedro Alves <palves@redhat.com> wrote:
>> On 03/02/2012 11:17 AM, Thomas Schwinge wrote:
>>> On Thu, 1 Mar 2012 17:18:47 -0700, Kevin Buettner <kevinb@redhat.com> wrote:
>>>>> On Thu, 01 Mar 2012 10:00:00 +0100
>>>>> Thomas Schwinge <thomas@codesourcery.com> wrote:
>>>>>
>>>>>>> @@ -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);
>>>>>>> }
>
>> In this case, what's
>> necessary to just fix that particular issue?
>
> The issue here is that external data (a malicious executable that is
> being debugged) might possibly cause GDB to do arbitrary things due to
> corrupting its internal state. (I don't know if GDB development is
> generally paying attention to such ``detail'', but it certainly is an
> attack vector if you're debugging a binary that has been provided by a
> third party.)
>
> For inaccessible addresses, target_read_memory returns EIO, which causes
> read_memory to invoke throw_error: ``Cannot access memory at address
> 0xfffffffe''; that's fine.
>
> For improper but accessible addresses, it is more difficult to predict
> what might happen in the following. The value will be propagated into a
> frame cache's sp_offset and saved_sp. From there on, we have to rely on
> the frame unwinding machinery to reliably detect any failures or
> inconsistencies.
I really have trouble understanding the point, unless you're talking about
GDB ending up touching random volatile memory mapped registers in the inferior
it should not, and that affecting the system. Considering something like
this is a bigger problem that applies to every access, so it doesn't justify
an isolated and vague comment in the code like that, in my view.
--
Pedro Alves
^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2012-03-06 19:08 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-15 14:00 [PATCH] [SH] Prologue skipping if there is none Thomas Schwinge
2012-02-15 14:54 ` Pedro Alves
2012-02-16 15:27 ` [PATCH] [SH] GDB crash in sh_is_renesas_calling_convention, TYPE_CALLING_CONVENTION (was: Prologue skipping if there is none) Thomas Schwinge
2012-02-16 19:38 ` [PATCH] [SH] GDB crash in sh_is_renesas_calling_convention, TYPE_CALLING_CONVENTION Tom Tromey
2012-02-15 16:09 ` [PATCH] [SH] Prologue skipping if there is none Kevin Buettner
2012-02-16 0:13 ` Kevin Buettner
2012-02-16 16:59 ` Thomas Schwinge
2012-02-17 2:30 ` Kevin Buettner
2012-02-20 16:19 ` Thomas Schwinge
2012-02-21 5:25 ` Kevin Buettner
2012-02-24 11:09 ` Thomas Schwinge
2012-02-24 22:21 ` Kevin Buettner
2012-02-29 13:51 ` Thomas Schwinge
2012-03-01 0:13 ` Kevin Buettner
2012-03-01 9:03 ` Thomas Schwinge
2012-03-01 9:00 ` Thomas Schwinge
2012-03-02 0:19 ` Kevin Buettner
2012-03-02 11:18 ` Thomas Schwinge
2012-03-02 12:01 ` Pedro Alves
2012-03-02 14:15 ` Thomas Schwinge
2012-03-06 19:08 ` Pedro Alves
2012-03-03 1:18 ` Kevin Buettner
2012-03-05 15:16 ` Thomas Schwinge
2012-03-05 19:40 ` Kevin Buettner
2012-02-21 15:23 ` Thomas Schwinge
2012-02-22 14:54 ` Simulator testing for sh and sh64 (was: [PATCH] [SH] Prologue skipping if there is none) Thomas Schwinge
2012-02-22 16:56 ` Kevin Buettner
2012-02-22 19:33 ` Simulator testing for sh and sh64 Thomas Schwinge
2012-02-23 0:35 ` Kaz Kojima
2012-02-24 21:38 ` Thomas Schwinge
2012-02-23 19:55 ` Thomas Schwinge
2012-02-23 22:53 ` Kevin Buettner
2012-02-24 11:12 ` Thomas Schwinge
2012-02-23 23:57 ` Kevin Buettner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox