[reverse RFA] no singlestep-over-BP in reverse

[reverse RFA] no singlestep-over-BP in reverse

[Michael Snyder]

[-- Attachment #1: Type: text/plain, Size: 452 bytes --]

When we're stopped at a breakpoint and we want to
continue in reverse, we're not actually going to
execute the instruction at the breakpoint -- we're
going to de-execute the previous instruction.

Therefore there's no need to singlestep before
inserting breakpoints.  In fact it would be a bad
idea to do so, because if there is a breakpoint at
the previous instruction, we WANT to hit it.

Note that this patch is to be applied to the reverse branch.

[-- Attachment #2: bpstep.txt --]
[-- Type: text/plain, Size: 1217 bytes --]

2008-09-15  Michael Snyder  <msnyder@vmware.com>

	* infrun.c (proceed): No need to singlestep over a breakpoint
	when resuming in reverse.

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.300.2.5
diff -u -p -r1.300.2.5 infrun.c
--- infrun.c	5 Sep 2008 03:37:10 -0000	1.300.2.5
+++ infrun.c	15 Sep 2008 18:28:29 -0000
@@ -1226,11 +1226,17 @@ proceed (CORE_ADDR addr, enum target_sig
 
   if (addr == (CORE_ADDR) -1)
     {
-      if (pc == stop_pc && breakpoint_here_p (pc))
+      if (pc == stop_pc && breakpoint_here_p (pc) 
+	  && target_get_execution_direction () == EXEC_FORWARD)
 	/* There is a breakpoint at the address we will resume at,
 	   step one instruction before inserting breakpoints so that
 	   we do not stop right away (and report a second hit at this
-	   breakpoint).  */
+	   breakpoint).
+
+	   Note, we don't do this in reverse, because we won't
+	   actually be executing the breakpoint insn anyway.
+	   We'll be (un-)executing the previous instruction.  */
+
 	oneproc = 1;
       else if (gdbarch_single_step_through_delay_p (gdbarch)
 	       && gdbarch_single_step_through_delay (gdbarch,

Re: [reverse RFA] no singlestep-over-BP in reverse

[Daniel Jacobowitz]

On Mon, Sep 15, 2008 at 11:31:33AM -0700, Michael Snyder wrote:
> When we're stopped at a breakpoint and we want to
> continue in reverse, we're not actually going to
> execute the instruction at the breakpoint -- we're
> going to de-execute the previous instruction.
>
> Therefore there's no need to singlestep before
> inserting breakpoints.  In fact it would be a bad
> idea to do so, because if there is a breakpoint at
> the previous instruction, we WANT to hit it.
>
> Note that this patch is to be applied to the reverse branch.

If there is a breakpoint on the previous instruction, will you hit it
before or after de-executing that instruction?  It seems like this
logic should be somehow still necessary... but I can't put my finger
on when.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [reverse RFA] no singlestep-over-BP in reverse

[Michael Snyder]

Daniel Jacobowitz wrote:
> On Mon, Sep 15, 2008 at 11:31:33AM -0700, Michael Snyder wrote:
>> When we're stopped at a breakpoint and we want to
>> continue in reverse, we're not actually going to
>> execute the instruction at the breakpoint -- we're
>> going to de-execute the previous instruction.
>>
>> Therefore there's no need to singlestep before
>> inserting breakpoints.  In fact it would be a bad
>> idea to do so, because if there is a breakpoint at
>> the previous instruction, we WANT to hit it.
>>
>> Note that this patch is to be applied to the reverse branch.
> 
> If there is a breakpoint on the previous instruction, will you hit it
> before or after de-executing that instruction? 

Well, if trap insns are in place, that's what will be
executed (not the shadow insn).

> It seems like this
> logic should be somehow still necessary... but I can't put my finger
> on when.

Good point -- there is a test in the testsuite for consecutive
breakpoints.  I'll try to reversify that.

Re: [reverse RFA] no singlestep-over-BP in reverse

[Daniel Jacobowitz]

On Mon, Sep 15, 2008 at 12:05:25PM -0700, Michael Snyder wrote:
>> If there is a breakpoint on the previous instruction, will you hit it
>> before or after de-executing that instruction? 
>
> Well, if trap insns are in place, that's what will be
> executed (not the shadow insn).

Presumably it'd be a hardware breakpoint.  Software breakpoints in
reverse execution seem... awkward.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [reverse RFA] no singlestep-over-BP in reverse

[Michael Snyder]

Daniel Jacobowitz wrote:
> On Mon, Sep 15, 2008 at 12:05:25PM -0700, Michael Snyder wrote:
>>> If there is a breakpoint on the previous instruction, will you hit it
>>> before or after de-executing that instruction?
>> Well, if trap insns are in place, that's what will be
>> executed (not the shadow insn).
> 
> Presumably it'd be a hardware breakpoint.  Software breakpoints in
> reverse execution seem... awkward.

Right.  I've just been re-thinking the issue.

I don't know how it might work with SW bp, but the three
implementations that I know of right now all use HW bps.

In that case, the target just has to guarantee to un-execute
the instruction before stopping, and then everything will be
OK.

I posted some tests, those all work, but I will go ahead
and try to port (and post) the consecutive.exp test anyway.

Re: [reverse RFA] no singlestep-over-BP in reverse

[Michael Snyder]

[-- Attachment #1: Type: text/plain, Size: 969 bytes --]

Daniel Jacobowitz wrote:
> On Mon, Sep 15, 2008 at 11:31:33AM -0700, Michael Snyder wrote:
>> When we're stopped at a breakpoint and we want to
>> continue in reverse, we're not actually going to
>> execute the instruction at the breakpoint -- we're
>> going to de-execute the previous instruction.
>>
>> Therefore there's no need to singlestep before
>> inserting breakpoints.  In fact it would be a bad
>> idea to do so, because if there is a breakpoint at
>> the previous instruction, we WANT to hit it.
>>
>> Note that this patch is to be applied to the reverse branch.
> 
> If there is a breakpoint on the previous instruction, will you hit it
> before or after de-executing that instruction?  It seems like this
> logic should be somehow still necessary... but I can't put my finger
> on when.

The attached test does just that -- and it passes on three
targets that can go in reverse (teawater's, gdb-freeplay,
and the as yet unreleased VMware implementation).


[-- Attachment #2: consecutive-reverse.exp --]
[-- Type: text/plain, Size: 3284 bytes --]

#   Copyright 2001, 2007, 2008 Free Software Foundation, Inc.

# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program.  If not, see <http://www.gnu.org/licenses/>.

# Please email any bugs, comments, and/or additions to this file to:
# bug-gdb@prep.ai.mit.edu

# Based on a file written by Michael Snyder. (msnyder@redhat.com)

#
# Test breakpoints at consecutive instruction addresses.
#

set testfile "consecutive-reverse"
set srcfile ${testfile}.c
set binfile ${objdir}/${subdir}/${testfile}

if { [prepare_for_testing ${testfile}.exp $testfile $srcfile] } {
    untested "${testfile}.exp"
    return -1
}

runto main

# FIXME 'record' command should say something so we know it's working.
gdb_test "record" "" "start recording"

gdb_breakpoint foo
gdb_test "continue" "Breakpoint $decimal, foo .*" \
	"continue to breakpoint in foo"

set foo1_addr 0
set foo2_addr 0
set stop_addr 0

send_gdb "x /2i \$pc\n"
gdb_expect {
    global hex
    global foo1_addr
    global foo2_addr
    global gdb_prompt

    -re "($hex).*\[\r\n\]+($hex).*$gdb_prompt $" {
	set foo1_addr $expect_out(1,string)
	set foo2_addr $expect_out(2,string)
	pass "get breakpoint address for foo"
    }
    -re ".*$gdb_prompt $" {
	fail "get breakpoint address for foo"
	return 0;
    }
    timeout {
	fail "get breakpoint address for foo (timeout)"
	return 0;
    }
}

gdb_test "break \*$foo2_addr" "Breakpoint $decimal at $foo2_addr: file .*" \
	"set bp, 2nd instr"

send_gdb "step\n"
gdb_expect {
    -re "Breakpoint $decimal, ($hex) in foo.*$gdb_prompt $" {
	set stop_addr $expect_out(1,string)
	if [eval expr "$foo2_addr == $stop_addr"] then {
	    pass "stopped at bp, 2nd instr"
	} else {
	    fail "stopped at bp, 2nd instr (wrong address)"
	}
    }
    -re ".*$gdb_prompt $" {
	fail "stopped at bp, 2nd instr"
    }
    timeout {
	fail "stopped at bp, 2nd instr (timeout)"

    }
}

###
###
###

# Set reverse execution direction
# FIXME: command needs to acknowledge, so we can test if it succeeded.

gdb_test "set exec-dir reverse" "" "set reverse execution"

# Now step backward and hope to hit the first breakpoint.

set test_msg "stopped at bp in reverse, 1st instr"
gdb_test_multiple "step" "$test_msg" {
    -re "Breakpoint $decimal, ($hex) in foo.*$gdb_prompt $" {
	set stop_addr $expect_out(1,string)
	if [eval expr "$foo1_addr == $stop_addr"] then {
	    pass "$test_msg"
	} else {
	    fail "$test_msg (wrong address)"
	}
    }
    -re "Breakpoint $decimal, foo.*$gdb_prompt $" {
	send_gdb "print \$pc == $foo1_addr\n"
	gdb_expect {
	    -re "$decimal = 1\[\r\n\]+$gdb_prompt $" {
		pass "$test_msg"
	    }
	    -re "$decimal = 0\[\r\n\]+$gdb_prompt $" {
		fail "$test_msg (wrong address)"
	    }
	}
    }
    -re ".*$gdb_prompt $" {
	fail "$test_msg"
    }
}

Re: [reverse RFA] no singlestep-over-BP in reverse

[teawater]

I think is before exec the next instruction, the GDB will get breakpoint trap.

But I found that there is a bug in inside record replay mode, I stop
the GDB after exec the instruction.
Michael, how do you deal with the breakpoint in gdb-freeplay and vmware record?

Hui

On Tue, Sep 16, 2008 at 02:42, Daniel Jacobowitz <drow@false.org> wrote:
>
> On Mon, Sep 15, 2008 at 11:31:33AM -0700, Michael Snyder wrote:
> > When we're stopped at a breakpoint and we want to
> > continue in reverse, we're not actually going to
> > execute the instruction at the breakpoint -- we're
> > going to de-execute the previous instruction.
> >
> > Therefore there's no need to singlestep before
> > inserting breakpoints.  In fact it would be a bad
> > idea to do so, because if there is a breakpoint at
> > the previous instruction, we WANT to hit it.
> >
> > Note that this patch is to be applied to the reverse branch.
>
> If there is a breakpoint on the previous instruction, will you hit it
> before or after de-executing that instruction?  It seems like this
> logic should be somehow still necessary... but I can't put my finger
> on when.
>
> --
> Daniel Jacobowitz
> CodeSourcery

Re: [reverse RFA] no singlestep-over-BP in reverse

[teawater]

I think maybe some reverse target (in the future?) need it.
Someone already know it already deal the breakpoint. But the others
will not know. Maybe I will change inside record to second type.


If this single step affect some target, how about let target choice it
with itself?

Thanks,
Hui




On Tue, Sep 16, 2008 at 02:31, Michael Snyder <msnyder@vmware.com> wrote:
> When we're stopped at a breakpoint and we want to
> continue in reverse, we're not actually going to
> execute the instruction at the breakpoint -- we're
> going to de-execute the previous instruction.
>
> Therefore there's no need to singlestep before
> inserting breakpoints.  In fact it would be a bad
> idea to do so, because if there is a breakpoint at
> the previous instruction, we WANT to hit it.
>
> Note that this patch is to be applied to the reverse branch.
>
> 2008-09-15  Michael Snyder  <msnyder@vmware.com>
>
>        * infrun.c (proceed): No need to singlestep over a breakpoint
>        when resuming in reverse.
>
> Index: infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.300.2.5
> diff -u -p -r1.300.2.5 infrun.c
> --- infrun.c    5 Sep 2008 03:37:10 -0000       1.300.2.5
> +++ infrun.c    15 Sep 2008 18:28:29 -0000
> @@ -1226,11 +1226,17 @@ proceed (CORE_ADDR addr, enum target_sig
>
>   if (addr == (CORE_ADDR) -1)
>     {
> -      if (pc == stop_pc && breakpoint_here_p (pc))
> +      if (pc == stop_pc && breakpoint_here_p (pc)
> +         && target_get_execution_direction () == EXEC_FORWARD)
>        /* There is a breakpoint at the address we will resume at,
>           step one instruction before inserting breakpoints so that
>           we do not stop right away (and report a second hit at this
> -          breakpoint).  */
> +          breakpoint).
> +
> +          Note, we don't do this in reverse, because we won't
> +          actually be executing the breakpoint insn anyway.
> +          We'll be (un-)executing the previous instruction.  */
> +
>        oneproc = 1;
>       else if (gdbarch_single_step_through_delay_p (gdbarch)
>               && gdbarch_single_step_through_delay (gdbarch,
>
>

Re: [reverse RFA] no singlestep-over-BP in reverse

[teawater]

Hi guys,

I think we need to decide the behavior of reverse mode breakpoint.

In forward mode, it's clear that CPU stop in address of breakpoint.
The instruction in this address will not be executed.

In reverse mode, If CPU stop in address of breakpoint, I think we have
2 choices:

1. The instruction in this address already reverse executed, it make
program status back to before forward execute this instruction.
Good for it is:
This way make reverse breakpoint same with simple BP.
If program break in reverse mode and got forward execute command. It
don't need special process.
Bad for it is:
This instruction already reverse executed, I think it's not the
original idea of breakpoint. I think the original idea of breakpoint
is stop the program before the instruction forward and reverse
execute.
And for record and replay type reverse target such as inside record
and gdb-freeplay(Sorry I am not very clear), to implement this BP are
not hard. But maybe not easy for others.

2. The instr in this address will not reverse executed, it make
program status in after forward execute this instruction.
Good for it is:
Maybe this is the original idea of breakpoint.
Bad for it is:
If program break in reverse mode and got forward execute command. It
need special process.
Maybe some reverse target can't support it.

Maybe we need support both of them. Target choice what it want, maybe
both of them. And target need to decide if he want a single step to
jump a BP or not.

How do you think about it?


Thanks,
Hui




On Tue, Sep 16, 2008 at 12:21, teawater <teawater@gmail.com> wrote:
> I think maybe some reverse target (in the future?) need it.
> Someone already know it already deal the breakpoint. But the others
> will not know. Maybe I will change inside record to second type.
>
>
> If this single step affect some target, how about let target choice it
> with itself?
>
> Thanks,
> Hui
>
>
>
>
> On Tue, Sep 16, 2008 at 02:31, Michael Snyder <msnyder@vmware.com> wrote:
>> When we're stopped at a breakpoint and we want to
>> continue in reverse, we're not actually going to
>> execute the instruction at the breakpoint -- we're
>> going to de-execute the previous instruction.
>>
>> Therefore there's no need to singlestep before
>> inserting breakpoints.  In fact it would be a bad
>> idea to do so, because if there is a breakpoint at
>> the previous instruction, we WANT to hit it.
>>
>> Note that this patch is to be applied to the reverse branch.
>>
>> 2008-09-15  Michael Snyder  <msnyder@vmware.com>
>>
>>        * infrun.c (proceed): No need to singlestep over a breakpoint
>>        when resuming in reverse.
>>
>> Index: infrun.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/infrun.c,v
>> retrieving revision 1.300.2.5
>> diff -u -p -r1.300.2.5 infrun.c
>> --- infrun.c    5 Sep 2008 03:37:10 -0000       1.300.2.5
>> +++ infrun.c    15 Sep 2008 18:28:29 -0000
>> @@ -1226,11 +1226,17 @@ proceed (CORE_ADDR addr, enum target_sig
>>
>>   if (addr == (CORE_ADDR) -1)
>>     {
>> -      if (pc == stop_pc && breakpoint_here_p (pc))
>> +      if (pc == stop_pc && breakpoint_here_p (pc)
>> +         && target_get_execution_direction () == EXEC_FORWARD)
>>        /* There is a breakpoint at the address we will resume at,
>>           step one instruction before inserting breakpoints so that
>>           we do not stop right away (and report a second hit at this
>> -          breakpoint).  */
>> +          breakpoint).
>> +
>> +          Note, we don't do this in reverse, because we won't
>> +          actually be executing the breakpoint insn anyway.
>> +          We'll be (un-)executing the previous instruction.  */
>> +
>>        oneproc = 1;
>>       else if (gdbarch_single_step_through_delay_p (gdbarch)
>>               && gdbarch_single_step_through_delay (gdbarch,
>>
>>
>

Re: [reverse RFA] no singlestep-over-BP in reverse

[teawater]

On Tue, Sep 16, 2008 at 23:03, teawater <teawater@gmail.com> wrote:
> Hi guys,
>
> I think we need to decide the behavior of reverse mode breakpoint.
>
> In forward mode, it's clear that CPU stop in address of breakpoint.
> The instruction in this address will not be executed.
>
> In reverse mode, If CPU stop in address of breakpoint, I think we have
> 2 choices:
>
> 1. The instruction in this address already reverse executed, it make
> program status back to before forward execute this instruction.
> Good for it is:
> This way make reverse breakpoint same with simple BP.
> If program break in reverse mode and got forward execute command. It
> don't need special process.
> Bad for it is:
> This instruction already reverse executed, I think it's not the
> original idea of breakpoint. I think the original idea of breakpoint
> is stop the program before the instruction forward and reverse
> execute.
> And for record and replay type reverse target such as inside record
> and gdb-freeplay(Sorry I am not very clear), to implement this BP are
> not hard. But maybe not easy for others.
>
> 2. The instr in this address will not reverse executed, it make
> program status in after forward execute this instruction.
> Good for it is:
> Maybe this is the original idea of breakpoint.
> Bad for it is:
> If program break in reverse mode and got forward execute command. It
> need special process.
> Maybe some reverse target can't support it.
>
> Maybe we need support both of them. Target choice what it want, maybe
> both of them. And target need to decide if he want a single step to
> jump a BP or not.

If we want support both of them. Maybe we can support a command make
user can get the type and set the type(Maybe some target just support
1 tyoe).


>
> How do you think about it?
>
>
> Thanks,
> Hui
>
>
>
>
> On Tue, Sep 16, 2008 at 12:21, teawater <teawater@gmail.com> wrote:
>> I think maybe some reverse target (in the future?) need it.
>> Someone already know it already deal the breakpoint. But the others
>> will not know. Maybe I will change inside record to second type.
>>
>>
>> If this single step affect some target, how about let target choice it
>> with itself?
>>
>> Thanks,
>> Hui
>>
>>
>>
>>
>> On Tue, Sep 16, 2008 at 02:31, Michael Snyder <msnyder@vmware.com> wrote:
>>> When we're stopped at a breakpoint and we want to
>>> continue in reverse, we're not actually going to
>>> execute the instruction at the breakpoint -- we're
>>> going to de-execute the previous instruction.
>>>
>>> Therefore there's no need to singlestep before
>>> inserting breakpoints.  In fact it would be a bad
>>> idea to do so, because if there is a breakpoint at
>>> the previous instruction, we WANT to hit it.
>>>
>>> Note that this patch is to be applied to the reverse branch.
>>>
>>> 2008-09-15  Michael Snyder  <msnyder@vmware.com>
>>>
>>>        * infrun.c (proceed): No need to singlestep over a breakpoint
>>>        when resuming in reverse.
>>>
>>> Index: infrun.c
>>> ===================================================================
>>> RCS file: /cvs/src/src/gdb/infrun.c,v
>>> retrieving revision 1.300.2.5
>>> diff -u -p -r1.300.2.5 infrun.c
>>> --- infrun.c    5 Sep 2008 03:37:10 -0000       1.300.2.5
>>> +++ infrun.c    15 Sep 2008 18:28:29 -0000
>>> @@ -1226,11 +1226,17 @@ proceed (CORE_ADDR addr, enum target_sig
>>>
>>>   if (addr == (CORE_ADDR) -1)
>>>     {
>>> -      if (pc == stop_pc && breakpoint_here_p (pc))
>>> +      if (pc == stop_pc && breakpoint_here_p (pc)
>>> +         && target_get_execution_direction () == EXEC_FORWARD)
>>>        /* There is a breakpoint at the address we will resume at,
>>>           step one instruction before inserting breakpoints so that
>>>           we do not stop right away (and report a second hit at this
>>> -          breakpoint).  */
>>> +          breakpoint).
>>> +
>>> +          Note, we don't do this in reverse, because we won't
>>> +          actually be executing the breakpoint insn anyway.
>>> +          We'll be (un-)executing the previous instruction.  */
>>> +
>>>        oneproc = 1;
>>>       else if (gdbarch_single_step_through_delay_p (gdbarch)
>>>               && gdbarch_single_step_through_delay (gdbarch,
>>>
>>>
>>
>

Re: [reverse RFA] no singlestep-over-BP in reverse

[Daniel Jacobowitz]

On Tue, Sep 16, 2008 at 11:03:42PM +0800, teawater wrote:
> 1. The instruction in this address already reverse executed, it make
> program status back to before forward execute this instruction.

I like this approach; I believe it's what we used in the qemu reverse
implementation also.  It means that you have the same state when
you're pointing at the start of a source line: it has not yet executed.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [reverse RFA] no singlestep-over-BP in reverse

[teawater]

Maybe we can make user have more choices.
Of course, some of target can just support one choice.

On Tue, Sep 16, 2008 at 23:21, Daniel Jacobowitz <drow@false.org> wrote:
> On Tue, Sep 16, 2008 at 11:03:42PM +0800, teawater wrote:
>> 1. The instruction in this address already reverse executed, it make
>> program status back to before forward execute this instruction.
>
> I like this approach; I believe it's what we used in the qemu reverse
> implementation also.  It means that you have the same state when
> you're pointing at the start of a source line: it has not yet executed.
>
> --
> Daniel Jacobowitz
> CodeSourcery
>

Re: [reverse RFA] no singlestep-over-BP in reverse

[teawater]

Maybe we can make user have more choices.
Of course, some of target can just support one choice.

On Tue, Sep 16, 2008 at 23:21, Daniel Jacobowitz <drow@false.org> wrote:
> On Tue, Sep 16, 2008 at 11:03:42PM +0800, teawater wrote:
>> 1. The instruction in this address already reverse executed, it make
>> program status back to before forward execute this instruction.
>
> I like this approach; I believe it's what we used in the qemu reverse
> implementation also.  It means that you have the same state when
> you're pointing at the start of a source line: it has not yet executed.
>
> --
> Daniel Jacobowitz
> CodeSourcery
>

Re: [reverse RFA] no singlestep-over-BP in reverse

[Michael Snyder]

teawater wrote:
> 
> 
> On Tue, Sep 16, 2008 at 02:42, Daniel Jacobowitz <drow@false.org 
> <mailto:drow@false.org>> wrote:
> 
> 
>     If there is a breakpoint on the previous instruction, will you hit it
>     before or after de-executing that instruction?  It seems like this
>     logic should be somehow still necessary... but I can't put my finger
>     on when.
> 
> 
> I think is before exec the next instruction, the GDB will get breakpoint 
> trap.
> 
> But I found that there is a bug in inside record replay mode, I stop the 
> GDB after exec the instruction.

Aha, let's figure out a test for this question.
It's rather important.   ;-)

> Michael, how do you deal with the breakpoint in gdb-freeplay and vmware 
> record?

Both use simulated hardware breakpoints.  That is, they both
accept the Z0 message and emulate hardware breakpoint semantics
by watching the address of execution (one instruction at a time).

Re: [reverse RFA] no singlestep-over-BP in reverse

[Michael Snyder]

teawater wrote:
> I think maybe some reverse target (in the future?) need it.
> Someone already know it already deal the breakpoint. But the others
> will not know. Maybe I will change inside record to second type.
> 
> 
> If this single step affect some target, how about let target choice it
> with itself?

At the moment, my opinion is that gdb needs to be able
to expect consistent behavior from the target(s).

And I believe that consistent behavior / semantics should be:

    If you tell me that you are stopped at instruction 1000,
    regardless of whether you were going forward or backward
    when you got there, then I will expect that if I tell you
    to execute forward, you will execute the instruction at
    1000.

Therefore the machine state at this point should reflect
the state BEFORE any side effects of the forward execution
of that instruction.

This is how it is for forward execution, and it only makes
sense to expect the same for reverse execution.  Any variables
that are changed by the instruction should have their pre-
execution values.

Re: [reverse RFA] no singlestep-over-BP in reverse

[Michael Snyder]

Daniel Jacobowitz wrote:
> On Tue, Sep 16, 2008 at 11:03:42PM +0800, teawater wrote:
>> 1. The instruction in this address already reverse executed, it make
>> program status back to before forward execute this instruction.
> 
> I like this approach; I believe it's what we used in the qemu reverse
> implementation also.  It means that you have the same state when
> you're pointing at the start of a source line: it has not yet executed.

Yes, this is also the behavior of the Simics (Virtutech)
reverse engine, and VMware's implementation, and gdb-freeplay.

I think it's the only correct choice, frankly.

Suppose my source statement is:

    x = -1;

and suppose my architecture implements that in a single instruction.

If x == -1 after the statement, but x == 0 before the statement,
then reverse-executing the statement has no meaning unles it
changes the value of x back to its pre-execution value.

Re: [reverse RFA] no singlestep-over-BP in reverse

[Joel Brobecker]

> And I believe that consistent behavior / semantics should be:
> 
>    If you tell me that you are stopped at instruction 1000,
>    regardless of whether you were going forward or backward
>    when you got there, then I will expect that if I tell you
>    to execute forward, you will execute the instruction at
>    1000.

This makes total sense to me. I think I would be very confused
by the debugger if I started going back and forth with a debugger
that didn't follow the semantics above.

-- 
Joel

Re: [reverse RFA] no singlestep-over-BP in reverse

[Michael Snyder]

[-- Attachment #1: Type: text/plain, Size: 699 bytes --]

Joel Brobecker wrote:
>> And I believe that consistent behavior / semantics should be:
>>
>>    If you tell me that you are stopped at instruction 1000,
>>    regardless of whether you were going forward or backward
>>    when you got there, then I will expect that if I tell you
>>    to execute forward, you will execute the instruction at
>>    1000.
> 
> This makes total sense to me. I think I would be very confused
> by the debugger if I started going back and forth with a debugger
> that didn't follow the semantics above.

Thanks.  By the way I've revised this patch slightly,
as shown below.  Use "== reverse" instead of "!= forward".

Makes it do the right thing in the "unknown" case.


[-- Attachment #2: bpstep.txt --]
[-- Type: text/plain, Size: 1217 bytes --]

2008-09-15  Michael Snyder  <msnyder@vmware.com>

	* infrun.c (proceed): No need to singlestep over a breakpoint
	when resuming in reverse.

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.300.2.5
diff -u -p -r1.300.2.5 infrun.c
--- infrun.c	5 Sep 2008 03:37:10 -0000	1.300.2.5
+++ infrun.c	17 Sep 2008 00:54:00 -0000
@@ -1226,11 +1226,17 @@ proceed (CORE_ADDR addr, enum target_sig
 
   if (addr == (CORE_ADDR) -1)
     {
-      if (pc == stop_pc && breakpoint_here_p (pc))
+      if (pc == stop_pc && breakpoint_here_p (pc) 
+	  && target_get_execution_direction () != EXEC_REVERSE)
 	/* There is a breakpoint at the address we will resume at,
 	   step one instruction before inserting breakpoints so that
 	   we do not stop right away (and report a second hit at this
-	   breakpoint).  */
+	   breakpoint).
+
+	   Note, we don't do this in reverse, because we won't
+	   actually be executing the breakpoint insn anyway.
+	   We'll be (un-)executing the previous instruction.  */
+
 	oneproc = 1;
       else if (gdbarch_single_step_through_delay_p (gdbarch)
 	       && gdbarch_single_step_through_delay (gdbarch,

Re: [reverse RFA] no singlestep-over-BP in reverse

[teawater]

Agree with you.

And I think Maybe need add a test for it.
I think is:
1. There is a statement that set the value of a variable.
For example:
a = 1;
2. Before this statement, the value of this variable is not same with new value.
3. Set a breakpoint on this statement, And check the value of this
variable when forward execute and reverse execute program break by
this breakpoint. If the value is the new value, fail. If is the old
value, pass.


Thanks,
Hui


On Wed, Sep 17, 2008 at 02:42, Michael Snyder <msnyder@vmware.com> wrote:
> teawater wrote:
>>
>> I think maybe some reverse target (in the future?) need it.
>> Someone already know it already deal the breakpoint. But the others
>> will not know. Maybe I will change inside record to second type.
>>
>>
>> If this single step affect some target, how about let target choice it
>> with itself?
>
> At the moment, my opinion is that gdb needs to be able
> to expect consistent behavior from the target(s).
>
> And I believe that consistent behavior / semantics should be:
>
>   If you tell me that you are stopped at instruction 1000,
>   regardless of whether you were going forward or backward
>   when you got there, then I will expect that if I tell you
>   to execute forward, you will execute the instruction at
>   1000.
>
> Therefore the machine state at this point should reflect
> the state BEFORE any side effects of the forward execution
> of that instruction.
>
> This is how it is for forward execution, and it only makes
> sense to expect the same for reverse execution.  Any variables
> that are changed by the instruction should have their pre-
> execution values.
>
>

Re: [reverse RFA] no singlestep-over-BP in reverse

[teawater]

Looks good. Please check it in.

And I think we have decided the how to deal with breakpoint.

On Wed, Sep 17, 2008 at 08:54, Michael Snyder <msnyder@vmware.com> wrote:
> Joel Brobecker wrote:
>>>
>>> And I believe that consistent behavior / semantics should be:
>>>
>>>   If you tell me that you are stopped at instruction 1000,
>>>   regardless of whether you were going forward or backward
>>>   when you got there, then I will expect that if I tell you
>>>   to execute forward, you will execute the instruction at
>>>   1000.
>>
>> This makes total sense to me. I think I would be very confused
>> by the debugger if I started going back and forth with a debugger
>> that didn't follow the semantics above.
>
> Thanks.  By the way I've revised this patch slightly,
> as shown below.  Use "== reverse" instead of "!= forward".
>
> Makes it do the right thing in the "unknown" case.
>
>
> 2008-09-15  Michael Snyder  <msnyder@vmware.com>
>
>        * infrun.c (proceed): No need to singlestep over a breakpoint
>        when resuming in reverse.
>
> Index: infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.300.2.5
> diff -u -p -r1.300.2.5 infrun.c
> --- infrun.c    5 Sep 2008 03:37:10 -0000       1.300.2.5
> +++ infrun.c    17 Sep 2008 00:54:00 -0000
> @@ -1226,11 +1226,17 @@ proceed (CORE_ADDR addr, enum target_sig
>
>   if (addr == (CORE_ADDR) -1)
>     {
> -      if (pc == stop_pc && breakpoint_here_p (pc))
> +      if (pc == stop_pc && breakpoint_here_p (pc)
> +         && target_get_execution_direction () != EXEC_REVERSE)
>        /* There is a breakpoint at the address we will resume at,
>           step one instruction before inserting breakpoints so that
>           we do not stop right away (and report a second hit at this
> -          breakpoint).  */
> +          breakpoint).
> +
> +          Note, we don't do this in reverse, because we won't
> +          actually be executing the breakpoint insn anyway.
> +          We'll be (un-)executing the previous instruction.  */
> +
>        oneproc = 1;
>       else if (gdbarch_single_step_through_delay_p (gdbarch)
>               && gdbarch_single_step_through_delay (gdbarch,
>
>

Re: [reverse RFA] no singlestep-over-BP in reverse

[Michael Snyder]

teawater wrote:
> Agree with you.
> 
> And I think Maybe need add a test for it.
> I think is:
> 1. There is a statement that set the value of a variable.
> For example:
> a = 1;
> 2. Before this statement, the value of this variable is not same with new value.
> 3. Set a breakpoint on this statement, And check the value of this
> variable when forward execute and reverse execute program break by
> this breakpoint. If the value is the new value, fail. If is the old
> value, pass.

That's the idea behind this test.
Do you think this test does what you want?
If not, what would you add to it?

http://sourceware.org/ml/gdb-patches/2008-09/msg00365.html

Re: [reverse RFA] no singlestep-over-BP in reverse

[Michael Snyder]

Committed to branch.

teawater wrote:
> Looks good. Please check it in.
> 
> And I think we have decided the how to deal with breakpoint.
> 
> On Wed, Sep 17, 2008 at 08:54, Michael Snyder <msnyder@vmware.com> wrote:
>> Joel Brobecker wrote:
>>>> And I believe that consistent behavior / semantics should be:
>>>>
>>>>   If you tell me that you are stopped at instruction 1000,
>>>>   regardless of whether you were going forward or backward
>>>>   when you got there, then I will expect that if I tell you
>>>>   to execute forward, you will execute the instruction at
>>>>   1000.
>>> This makes total sense to me. I think I would be very confused
>>> by the debugger if I started going back and forth with a debugger
>>> that didn't follow the semantics above.
>> Thanks.  By the way I've revised this patch slightly,
>> as shown below.  Use "== reverse" instead of "!= forward".
>>
>> Makes it do the right thing in the "unknown" case.
>>
>>
>> 2008-09-15  Michael Snyder  <msnyder@vmware.com>
>>
>>        * infrun.c (proceed): No need to singlestep over a breakpoint
>>        when resuming in reverse.
>>
>> Index: infrun.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/infrun.c,v
>> retrieving revision 1.300.2.5
>> diff -u -p -r1.300.2.5 infrun.c
>> --- infrun.c    5 Sep 2008 03:37:10 -0000       1.300.2.5
>> +++ infrun.c    17 Sep 2008 00:54:00 -0000
>> @@ -1226,11 +1226,17 @@ proceed (CORE_ADDR addr, enum target_sig
>>
>>   if (addr == (CORE_ADDR) -1)
>>     {
>> -      if (pc == stop_pc && breakpoint_here_p (pc))
>> +      if (pc == stop_pc && breakpoint_here_p (pc)
>> +         && target_get_execution_direction () != EXEC_REVERSE)
>>        /* There is a breakpoint at the address we will resume at,
>>           step one instruction before inserting breakpoints so that
>>           we do not stop right away (and report a second hit at this
>> -          breakpoint).  */
>> +          breakpoint).
>> +
>> +          Note, we don't do this in reverse, because we won't
>> +          actually be executing the breakpoint insn anyway.
>> +          We'll be (un-)executing the previous instruction.  */
>> +
>>        oneproc = 1;
>>       else if (gdbarch_single_step_through_delay_p (gdbarch)
>>               && gdbarch_single_step_through_delay (gdbarch,
>>
>>

Re: [reverse RFA] no singlestep-over-BP in reverse

[teawater]

On Thu, Sep 18, 2008 at 02:13, Michael Snyder <msnyder@vmware.com> wrote:
> teawater wrote:
>>
>> Agree with you.
>>
>> And I think Maybe need add a test for it.
>> I think is:
>> 1. There is a statement that set the value of a variable.
>> For example:
>> a = 1;
>> 2. Before this statement, the value of this variable is not same with new
>> value.
>> 3. Set a breakpoint on this statement, And check the value of this
>> variable when forward execute and reverse execute program break by
>> this breakpoint. If the value is the new value, fail. If is the old
>> value, pass.
>
> That's the idea behind this test.
> Do you think this test does what you want?
> If not, what would you add to it?
>
> http://sourceware.org/ml/gdb-patches/2008-09/msg00365.html
>
>

I think it's OK. Sorry I send this mail before read this test RFC.

Thanks,
Hui