From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7509 invoked by alias); 18 Oct 2008 03:18:39 -0000 Received: (qmail 7501 invoked by uid 22791); 18 Oct 2008 03:18:38 -0000 X-Spam-Check-By: sourceware.org Received: from ti-out-0910.google.com (HELO ti-out-0910.google.com) (209.85.142.189) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 18 Oct 2008 03:18:02 +0000 Received: by ti-out-0910.google.com with SMTP id d10so443670tib.12 for ; Fri, 17 Oct 2008 20:17:59 -0700 (PDT) Received: by 10.110.16.9 with SMTP id 9mr3477292tip.54.1224299879190; Fri, 17 Oct 2008 20:17:59 -0700 (PDT) Received: by 10.110.42.9 with HTTP; Fri, 17 Oct 2008 20:17:59 -0700 (PDT) Message-ID: Date: Sat, 18 Oct 2008 03:18:00 -0000 From: teawater To: "Pedro Alves" Subject: Re: [reverse/record] adjust_pc_after_break in reverse execution mode? Cc: "Michael Snyder" , "gdb-patches@sourceware.org" In-Reply-To: <200810180408.42297.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <200810180210.16346.pedro@codesourcery.com> <48F93A22.7070409@vmware.com> <200810180408.42297.pedro@codesourcery.com> 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/msg00460.txt.bz2 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 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 : 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); >> >> > > > > -- > Pedro Alves >