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

[Pedro Alves <pedro@codesourcery.com>]

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