From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5002 invoked by alias); 19 Oct 2008 22:44:09 -0000 Received: (qmail 4993 invoked by uid 22791); 19 Oct 2008 22:44:08 -0000 X-Spam-Check-By: sourceware.org Received: from smtp-outbound-1.vmware.com (HELO smtp-outbound-1.vmware.com) (65.115.85.69) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sun, 19 Oct 2008 22:43:33 +0000 Received: from mailhost4.vmware.com (mailhost4.vmware.com [10.16.67.124]) by smtp-outbound-1.vmware.com (Postfix) with ESMTP id 350FA19001; Sun, 19 Oct 2008 15:43:32 -0700 (PDT) Received: from [10.20.92.59] (promb-2s-dhcp59.eng.vmware.com [10.20.92.59]) by mailhost4.vmware.com (Postfix) with ESMTP id 287F1C9A17; Sun, 19 Oct 2008 15:43:32 -0700 (PDT) Message-ID: <48FBB718.4040706@vmware.com> Date: Sun, 19 Oct 2008 22:44:00 -0000 From: Michael Snyder User-Agent: Thunderbird 1.5.0.12 (X11/20080411) MIME-Version: 1.0 To: Pedro Alves CC: "gdb-patches@sourceware.org" , teawater Subject: Re: [reverse/record] adjust_pc_after_break in reverse execution mode? References: <200810180210.16346.pedro@codesourcery.com> In-Reply-To: <200810180210.16346.pedro@codesourcery.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-10/txt/msg00472.txt.bz2 After codgitating for a bit (that's "thinking" when you're over 50), I've decided that you're right. However, I have a new concern -- I'm worried about what it will do when it's replaying but going forward. Could you possibly revisit your test and see what it does if you record all the way to line 9 or 10, then back up to line 6, then continue with breakpoints at 6 and 7? 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 : lea 0x4(%esp),%ecx > 0x08048348 : and $0xfffffff0,%esp > 0x0804834b : pushl -0x4(%ecx) > 0x0804834e : push %ebp > 0x0804834f : mov %esp,%ebp > 0x08048351 : push %ecx > 0x08048352 : nop > 0x08048353 : nop > 0x08048354 : nop > 0x08048355 : nop > 0x08048356 : 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 > (gdb) reverse-continue > Continuing. > > Breakpoint 1, main () at nop.c:7 > 7 asm ("nop"); > (gdb) p $pc > $1 = (void (*)()) 0x8048353 > (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 > (gdb) reverse-continue > Continuing. > > Breakpoint 2, main () at nop.c:6 > 6 asm ("nop"); > (gdb) p $pc > $1 = (void (*)()) 0x8048352 > > 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 > > * 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);