This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Fix disp-step-syscall.exp on some i386 targets
On Tue, 28 Feb 2012 08:50:38 +0100, Yao Qi wrote:
> On 02/28/2012 03:22 AM, Jan Kratochvil wrote:
> > + 'int $0x80; ret' and i386_displaced_step_fixup would keep PC at the displaced
> > + location expecting it just executed 'ret' despite it finished the syscall.
> > + Hide the 'ret' instruction by 'nop'. */
> > +
>
> We write 'ret' into scratchpad, but return 'nop' in closure. I am
> afraid it is a little risky.
Yes, it is a hack. I find the most correct way to have a flag in i386-linux
struct displaced_step_closure to mark it. But we would have then to
complicate i386-linux-tdep by wrapping all the three displaced vector methods
in i386-linux-tdep which I did not find worth it.
> > +struct displaced_step_closure *
> > +i386_linux_displaced_step_copy_insn (struct gdbarch *gdbarch,
> > + CORE_ADDR from, CORE_ADDR to,
> > + struct regcache *regs)
> > +{
> > + struct displaced_step_closure *closure;
> > +
> > + closure = i386_displaced_step_copy_insn (gdbarch, from, to, regs);
> > +
> > + if (i386_linux_get_syscall_number_from_regcache (regs) != -1)
>
> I don't understand this. If we are doing displaced stepping on
> arbitrary instruction, the condition is always true?, I guess.
Not in the first case.
0:int 0x80
2:ret
A:
PC == 0, orig_eax == -1, GDB in i386_displaced_step_copy_insn
PTRACE_SINGLESTEP, waitpid == SIGTRAP | PTRACE_EVENT_FORK
PC == 2, orig_eax == __NR_fork, GDB in i386_displaced_step_fixup
B:
PC == 2, orig_eax == __NR_fork, GDB in i386_displaced_step_copy_insn
PTRACE_SINGLESTEP, waitpid == SIGTRAP
PC == 2, orig_eax == -1, GDB in i386_displaced_step_fixup
C:
PC == 2, orig_eax == -1, initiate standard step over 'ret'
-> PC == return address
The extra flag in struct displaced_step_closure would remember in the B case
for i386_displaced_step_fixup that this step is special due to %orig_rax.
Without special i386_displaced_step_copy_insn the function
i386_displaced_step_fixup in the B case can no longer find anything unusual,
it can no longer see 'int $0x80' anywhere and %orig_eax is also normal.
Only PC did not change but that may happen for various regular instructions.
> > + {
> > + /* Since we use simple_displaced_step_copy_insn, our closure is a
> > + copy of the instruction. */
> > + gdb_byte *insn = (gdb_byte *) closure;
> > +
> > + /* Fake nop. */
> > + insn[0] = 0x90;
> > + }
> > +
> > + return closure;
> > +}
>
> I am afraid this fix is not correct.
It works, it is a hack, I find your patch another kind of hack.
> So, the proper fix would be defining i386_linux_displaced_step_fixup in
> i386-linux-tdep.c to check whether pc is changed (equal to the address
> of scratchpad). If pc is not changed, this means pc has been updated in
> previous syscall event, and restore pc to its original place.
> Otherwise, call i386_displaced_step_fixup. The attached patch is to do
> what I said here. WDYT?
I do not mind much but it makes some assumption if PC did not change it was by
a syscall without checking it really was a syscall at all. There could be for
example some "jmp *%ebx" with %ebx == _start and it would be falsely relocated
by your patch back to its code location, ignoring its intended jump. The
patch of mine would not relocate it as %orig_eax remained 0.
But any code messing with the entry point address may confuse this
autodetection anyway so these countercases are more hypothetical.
What do you think about the %orig_eax verification?
I do not mind in general, displaced stepping is not used by default yet.
I just had some kernel ptrace bug suspection and the FAIL was unstable while
regression testing across archs.
Regards,
Jan