Re: [reverse/record] adjust_pc_after_break in reverse execution mode?

teawater
2008-10-18T03:18:39Z

Agree with it.

And I think we don't need worry about it very much.
breakpoint_pc = regcache_read_pc (regcache)
- gdbarch_decr_pc_after_break (gdbarch);
Cause most of arch are RISC that have same size insn set (Or most of
them are same such as MIPS16).

In this test, we need use a insn that size is same with breakpint insn.
X86's nop is same breakpint insn. How about other CISC?

On Sat, Oct 18, 2008 at 11:08, Pedro Alves <pedro@codesourcery.com> wrote:
> A Saturday 18 October 2008 02:21:38, Michael Snyder wrote:
>> Grump grump -- there's a "consecutive.exp" test in the testsuite,
>> but your example shows it to be inadequate -- it places two bps
>> on consecutive instructions, but doesn't make sure that they are
>> one byte in size.
>>
>> So naturally, my derived "consecutive-reverse.exp" test
>> (see the branch) has the same failing.
>>
>> Got any ideas how we could address this, testsuite-wise?
>> Ideally we'd like it to be arch-independent...
>>
>
> I think asm ("nop") like below is your best bet. Is there any assembler
> that doesn't understand "nop"? I believe it should pretty much be a safe
> bet that nop will be the smallest possible sized instruction on
> variable sized instruction archs, and the same size of a decr_pc_after_break
> (or of a breakpoint insn). Else, the .exp could -DNOP_ASM depending on target,
> and the code could use 'asm (NOP_ASM);'. NOP_ASM could be any instruction
> other than jmps and branches, doesn't really have to be a "nop".
>
>> Pedro Alves wrote:
>> > Just noticed this, while looking at the code, so I tried it out against
>> > the record target (x86) on the reverse-20080930-branch branch.
>> >
>> > 4 int main ()
>> > 5 {
>> > 6 asm ("nop");
>> > 7 asm ("nop");
>> > 8 asm ("nop");
>> > 9 asm ("nop");
>> > 10 }
>> >
>> > (gdb) disassemble
>> > Dump of assembler code for function main:
>> > 0x08048344 <main+0>: lea 0x4(%esp),%ecx
>> > 0x08048348 <main+4>: and $0xfffffff0,%esp
>> > 0x0804834b <main+7>: pushl -0x4(%ecx)
>> > 0x0804834e <main+10>: push %ebp
>> > 0x0804834f <main+11>: mov %esp,%ebp
>> > 0x08048351 <main+13>: push %ecx
>> > 0x08048352 <main+14>: nop
>> > 0x08048353 <main+15>: nop
>> > 0x08048354 <main+16>: nop
>> > 0x08048355 <main+17>: nop
>> > 0x08048356 <main+18>: pop %ecx
>> >
>> > Now let's try reverse continuing until hitting a breakpoint at 0x8048353 (line 7):
>> >
>> > (gdb) b 7
>> > Breakpoint 1 at 0x8048353: file nop.c, line 7.
>> > (gdb) start
>> > Temporary breakpoint 2 at 0x8048352: file nop.c, line 6.
>> > Starting program: /home/pedro/gdb/reverse-20080930-branch/build32/gdb/nop
>> >
>> > Temporary breakpoint 2, main () at nop.c:6
>> > 6 asm ("nop");
>> > (gdb) record
>> > (gdb) n
>> >
>> > Breakpoint 1, main () at nop.c:7
>> > 7 asm ("nop");
>> > (gdb) n
>> > 8 asm ("nop");
>> > (gdb) n
>> > 9 asm ("nop");
>> > (gdb) p $pc
>> > $1 = (void (*)()) 0x8048355 <main+17>
>> > (gdb) reverse-continue
>> > Continuing.
>> >
>> > Breakpoint 1, main () at nop.c:7
>> > 7 asm ("nop");
>> > (gdb) p $pc
>> > $1 = (void (*)()) 0x8048353 <main+15>
>> > (gdb)
>> >
>> > Now, let's try reverse continuing to a breakpoint at 0x8048353 (line 6),
>> > but this time, let's also sneak a breakpoint at 0x8048352 (line 6):
>> >
>> > (gdb) start
>> > Temporary breakpoint 1 at 0x8048352: file nop.c, line 6.
>> > Starting program: /home/pedro/gdb/reverse-20080930-branch/build32/gdb/nop
>> >
>> > Temporary breakpoint 1, main () at nop.c:6
>> > 6 asm ("nop");
>> > (gdb) b 6
>> > Breakpoint 2 at 0x8048352: file nop.c, line 6.
>> > (gdb) b 7
>> > Breakpoint 3 at 0x8048353: file nop.c, line 7.
>> > (gdb) record
>> > (gdb) n
>> >
>> > Breakpoint 3, main () at nop.c:7
>> > 7 asm ("nop");
>> > (gdb) n
>> > 8 asm ("nop");
>> > (gdb) n
>> > 9 asm ("nop");
>> > (gdb) p $pc
>> > $1 = (void (*)()) 0x8048355 <main+17>
>> > (gdb) reverse-continue
>> > Continuing.
>> >
>> > Breakpoint 2, main () at nop.c:6
>> > 6 asm ("nop");
>> > (gdb) p $pc
>> > $1 = (void (*)()) 0x8048352 <main+14>
>> >
>> > Oh-oh. Not good.
>> >
>> > So, in the second example, reverse execution should continue until
>> > breakpoint 3, but, adjust_pc_after_break finds a breakpoint
>> > at `PC - decr_pc_after_break' (1 on x86), adjusts the PC, and then we
>> > report breakpoint 2 being hit. The first example didn't trip on the
>> > problem, because there was no breakpoint at `PC - 1' when GDB went to
>> > look if adjustment was needed.
>> >
>> > I'm guessing the attached patch should be correct for all
>> > targets/archs, or could it be your targets are behaving differently?
>> >
>> > --
>> > Pedro Alves
>> >
>> >
>> > ------------------------------------------------------------------------
>> >
>> > 2008-10-18 Pedro Alves <pedro@codesourcery.com>
>> >
>> > * infrun.c (adjust_pc_after_break): Do nothing if executing in
>> > reverse.
>> >
>> > ---
>> > gdb/infrun.c | 27 +++++++++++++++++++++++++++
>> > 1 file changed, 27 insertions(+)
>> >
>> > Index: src/gdb/infrun.c
>> > ===================================================================
>> > --- src.orig/gdb/infrun.c 2008-10-18 02:06:15.000000000 +0100
>> > +++ src/gdb/infrun.c 2008-10-18 02:09:36.000000000 +0100
>> > @@ -1787,6 +1787,33 @@ adjust_pc_after_break (struct execution_
>> > if (ecs->ws.value.sig != TARGET_SIGNAL_TRAP)
>> > return;
>> >
>> > + /* In reverse execution, when a breakpoint is hit, the instruction
>> > + under it has already been de-executed. The reported PC always
>> > + points at the breakpoint address, so adjusting it further would
>> > + be wrong. E.g., consider:
>> > +
>> > + B1 0x08000000 : INSN1
>> > + B2 0x08000001 : INSN2
>> > + 0x08000002 : INSN3
>> > + PC -> 0x08000003 : INSN4
>> > +
>> > + Say you're stopped at 0x08000003 as above. Reverse continuing
>> > + from that point should hit B2 as below. Reading the PC when the
>> > + SIGTRAP is reported should read 0x08000001 and INSN2 should have
>> > + been de-executed already.
>> > +
>> > + B1 0x08000000 : INSN1
>> > + B2 PC -> 0x08000001 : INSN2
>> > + 0x08000002 : INSN3
>> > + 0x08000003 : INSN4
>> > +
>> > + If we tried to adjust the PC on for example, a
>> > + decr_pc_after_break == 1 architecture, we would wrongly further
>> > + adjust the PC to 0x08000000 and report a hit on B1, although the
>> > + INSN1 effects hadn't been de-executed yet. */
>> > + if (execution_direction == EXEC_REVERSE)
>> > + return;
>> > +
>> > /* If this target does not decrement the PC after breakpoints, then
>> > we have nothing to do. */
>> > regcache = get_thread_regcache (ecs->ptid);
>>
>>
>
>
>
> --
> Pedro Alves
>