From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13258 invoked by alias); 18 Oct 2008 01:26:08 -0000 Received: (qmail 13249 invoked by uid 22791); 18 Oct 2008 01:26:07 -0000 X-Spam-Check-By: sourceware.org Received: from smtp-outbound-2.vmware.com (HELO smtp-outbound-2.vmware.com) (65.115.85.73) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 18 Oct 2008 01:25:32 +0000 Received: from mailhost4.vmware.com (mailhost4.vmware.com [10.16.67.124]) by smtp-outbound-2.vmware.com (Postfix) with ESMTP id E23A25001; Fri, 17 Oct 2008 18:25:29 -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 CD477C9A30; Fri, 17 Oct 2008 18:25:29 -0700 (PDT) Message-ID: <48F93A22.7070409@vmware.com> Date: Sat, 18 Oct 2008 01:26: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/msg00453.txt.bz2 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... 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);