Some ideas of displaced step function

Some ideas of displaced step function

[teawater]

Hi all,

I read the source code of displaced step and found some fail with it
(http://sourceware.org/ml/gdb/2008-08/msg00231.html).

I have some ideas on it:

1. I think this function just for asynchronous debug. But in acutally,
it be used for any inferior:
/* Return non-zero if displaced stepping is enabled, and can be used
   with GDBARCH.  */
static int
use_displaced_stepping (struct gdbarch *gdbarch)
{
  return (can_use_displaced_stepping
	  && gdbarch_displaced_step_copy_insn_p (gdbarch));
}
Why not let it just work with asynchronous debug?

2. This function will affect another code cause it change pc change
memory, but it not supply a get and set interfaces? Most of its
variables are set to static. Just have a can_use_displaced_stepping
can control it disable or enable, but user can set it too. If other
code want know the orig PC or another things, How should they do?

3.  If get memory fail, it will just output a memory fail directly.
Simple user will not know what happen. But in actually, he can use
"maintenance set can-use-displaced-stepping 0" to jump off this
problem directly.
So I think maybe can add some clear fail message for example:
Displaced step write memory fail in address 0x....., Maybe you can
jump off with close displaced step function in "maintenance set
can-use-displaced-stepping 0".


Thanks,
Hui

Re: Some ideas of displaced step function

[Michael Snyder]

teawater wrote:
> Hi all,
> 
> I read the source code of displaced step and found some fail with it
> (http://sourceware.org/ml/gdb/2008-08/msg00231.html).
> 
> I have some ideas on it:
> 
> 1. I think this function just for asynchronous debug. But in acutally,
> it be used for any inferior:
> /* Return non-zero if displaced stepping is enabled, and can be used
>    with GDBARCH.  */
> static int
> use_displaced_stepping (struct gdbarch *gdbarch)
> {
>   return (can_use_displaced_stepping
>           && gdbarch_displaced_step_copy_insn_p (gdbarch));
> }
> Why not let it just work with asynchronous debug?
> 
> 2. This function will affect another code cause it change pc change
> memory, but it not supply a get and set interfaces? Most of its
> variables are set to static. Just have a can_use_displaced_stepping
> can control it disable or enable, but user can set it too. If other
> code want know the orig PC or another things, How should they do?
> 
> 3.  If get memory fail, it will just output a memory fail directly.
> Simple user will not know what happen. But in actually, he can use
> "maintenance set can-use-displaced-stepping 0" to jump off this
> problem directly.

Yes, in fact I have run into this myself.  In replay debugging
(for instance both Teawater's and VMware's implementation), you
can't write to an arbitrary memory location when you're
debugging a replay log.  Actually it's true with gdb-freeplay
too.  So I always have to remember to do

    maint set can-use-displaced-step off

before I begin.

Why are we using displaced-stepping when we're not async?
Isn't it unnecessarily expensive?

> So I think maybe can add some clear fail message for example:
> Displaced step write memory fail in address 0x....., Maybe you can
> jump off with close displaced step function in "maintenance set
> can-use-displaced-stepping 0".
> 
> 
> Thanks,
> Hui

Re: Some ideas of displaced step function

[Pedro Alves]

On Friday 03 October 2008 01:31:54, Michael Snyder wrote:

> Why are we using displaced-stepping when we're not async?

s/async/non-stop mode/g.  All-stop + async doesn't need
it either.  This feature was added for non-stop, as a way to
avoid lifting breakpoints from the inferior when stepping over
a breakpoint, otherwise, other running threads could miss them.

No other reason to have it always on other than for more
exposure, I guess.  Time to pull the plug?

I'd still like to have a way to enable displaced-stepping
in all-stop mode, as it's very useful for testing.

-- 
Pedro Alves

Re: Some ideas of displaced step function

[Michael Snyder]

Pedro Alves wrote:
> On Friday 03 October 2008 01:31:54, Michael Snyder wrote:
> 
>> Why are we using displaced-stepping when we're not async?
> 
> s/async/non-stop mode/g.  All-stop + async doesn't need
> it either.  This feature was added for non-stop, as a way to
> avoid lifting breakpoints from the inferior when stepping over
> a breakpoint, otherwise, other running threads could miss them.
> 
> No other reason to have it always on other than for more
> exposure, I guess.  Time to pull the plug?
> 
> I'd still like to have a way to enable displaced-stepping
> in all-stop mode, as it's very useful for testing.

I certainly don't mind 'enable', but maybe it doesn't
need to always be enabled by default?

Re: Some ideas of displaced step function

[Pedro Alves]

On Friday 03 October 2008 19:28:31, Michael Snyder wrote:
> Pedro Alves wrote:
> > On Friday 03 October 2008 01:31:54, Michael Snyder wrote:
> > 
> >> Why are we using displaced-stepping when we're not async?
> > 
> > s/async/non-stop mode/g.  All-stop + async doesn't need
> > it either.  This feature was added for non-stop, as a way to
> > avoid lifting breakpoints from the inferior when stepping over
> > a breakpoint, otherwise, other running threads could miss them.
> > 

> > No other reason to have it always on other than for more
> > exposure, I guess.  Time to pull the plug?
> > 
> > I'd still like to have a way to enable displaced-stepping
> > in all-stop mode, as it's very useful for testing.
> 
> I certainly don't mind 'enable', but maybe it doesn't
> need to always be enabled by default?
> 

There, you lost me.  :-)  Perhaps we're saying the same thing.  What
I meant was that sneaking a `&& non_stop' here:

 static int
 use_displaced_stepping (struct gdbarch *gdbarch)
 {
   return (can_use_displaced_stepping
+           && non_stop
           && gdbarch_displaced_step_copy_insn_p (gdbarch));
 }

... would make us use displaced stepping only in non-stop mode,
so the problems we've been hearing about are sidestepped.  This
puts the onus of making displaced stepping work on
targets/systems/modes that need it.

But, then one can't use displaced stepping in all-stop mode, without
hacking the code, that is.  It feels like if we make this change, we
should either give new meaning to the current
`maint set can-use-displaced-stepping' command, replace it with
some other command, or plainly add a new command in addition to
the current.

-- 
Pedro Alves