Re: gdbstub initial code, v11

2010-09-22 Thread Tom Tromey
Oleg But what about features? What should I do next? all-stop,
Oleg thread-specific breakpoints (currently I have no idea what
Oleg this means), or what?

I think it would be good to implement a feature that shows how this
approach is an improvement over the current state of gdb+ptrace or
gdb+gdbserver.

Exactly what feature this should be... I don't know :-)
I would imagine something performance-related.

There was previously some discussion about some watchpoint-related
thing, I forget the details of that.

I don't think thread-specific breakpoints are exposed outside of gdb
yet.  If that is true, then implementing that would mean adding remote
protocol features and also other stuff inside gdb.  So, I would suggest
not tackling this yet.

Tom



Re: gdbstub initial code, v11

2010-09-22 Thread Jan Kratochvil
On Wed, 22 Sep 2010 21:09:12 +0200, Tom Tromey wrote:
 I think it would be good to implement a feature that shows how this
 approach is an improvement over the current state of gdb+ptrace or
 gdb+gdbserver.
 
 Exactly what feature this should be... I don't know :-)
 I would imagine something performance-related.

I would bet on a massive threads creating/deleting testcase signalling tasks
around, together with watchpoints.  There are races in the linux-nat code and
IIRC even gdbserver code.

OTOH if one tries hard one can probably manage one day to fix all the corner
cases in the ptrace based linux-nat and gdbserver.


Regards,
Jan



Re: gdbstub initial code, v11

2010-09-22 Thread Oleg Nesterov
On 09/22, Frank Ch. Eigler wrote:

 oleg wrote:

  [...]  Honestly, I don't really know how do the right thing here.
  Anyway, most probably this code will be changed. Like ptrace, ugdb
  uses -report_syscall_exit() to synthesize a trap. Unlike ptrace,
  ugdb_report_signal() doesn't send SIGTRAP to itself but reports
  SIGTRAP using siginfo_t we have. In any case, whatever we do,
  multiple tracers can confuse each other.

 (It seems to me that a pure gdb report, without a synthetic
 self-injected SIGTRAP, should be fine.)

What do you mean?

  Next: fully implement g/G/p/P, currently I am a bit confused...
  But what about features? [...]

 You could dig out the old fishing plan.  One demonstrated
 improvement was from simulating (software) watchpoints within the
 gdb stub, instead of having gdb fall back to issing countless
 single-steps with memory-fetch inquiries in between.

When I do 'watch', gdb sends '$Z2'. I am a bit confused, iirc it
was decided I shouldn't play with Z packets now. But I won't
argue.

Oleg.



Re: gdbstub initial code, v11

2010-09-22 Thread Frank Ch. Eigler
Hi -

On Thu, Sep 23, 2010 at 01:14:51AM +0200, Oleg Nesterov wrote:
  (It seems to me that a pure gdb report, without a synthetic
  self-injected SIGTRAP, should be fine.)
 
 What do you mean?

(Never mind, I'm probably just confused about what you were asking.)


   Next: fully implement g/G/p/P, currently I am a bit confused...
   But what about features? [...]
 
  You could dig out the old fishing plan.  One demonstrated
  improvement was from simulating (software) watchpoints within the
  gdb stub, instead of having gdb fall back to issing countless
  single-steps with memory-fetch inquiries in between.
 
 When I do 'watch', gdb sends '$Z2'. I am a bit confused, iirc it
 was decided I shouldn't play with Z packets now. But I won't
 argue.

There are Z packets and then there are Z packets.  The ones Roland
told you not to worry about are Z0/Z1 related to (code) breakpoints,
which should be implemented via uprobes at some point.

The ones I'm talking about are Z2/Z3 for (data) watchpoints.

- FChE



Re: [PATCH] utrace: utrace_reset() should clear TIF_SINGLESTEP if no more engines

2010-09-22 Thread Oleg Nesterov
On 09/21, Roland McGrath wrote:

  OK, so what should we do for now?

 If we can't come to a straightforward answer for the general case
 fairly quickly, then we can do the simple change to start with.

I think we should start with changing utrace_control(DETACH) anyway,
then try to improve. I'll ressend the one-liner I already showed.

  Without the multitracing utrace_resume()-user_disable_single_step()
  fixes the problem. Otherwise we probably need ENGINE_WANTS_SINGLESTEP.

 No, no, we don't want that.  We don't need more state.  And, remember,
 we really don't need to microoptimize cases when single-step was used
 recently--the cost of taking one more single-step trap and percolating
 through the signal paths was going to be pretty large anyway.

OK,

 It's better to have a spurious report (preferably just spurious
 TIF_NOTIFY_RESUME with no actual callbacks) following any detach,
 or whatever it takes to ensure user_disable_single_step() always
 runs if user_enable_*_step did before and there is no engine around
 to see a report_quiesce callback pass.  If there is a report_quiesce
 or report_signal callback pass where nobody returns UTRACE_*STEP,
 then we should never leave stepping enabled when we return to user mode.

Hmm. I'll try to think more. Right now I don't really understand
how to do this correctly.

OK, finish_callback_report() and utrace_control(DETACH) can set
TIF_NOTIFY_RESUME. But what if there are no more attached engines?
Looks like, utrace_resume(UTRACE_RESUME) needs to handle this special
case. And utrace_reset() shouldn't clear task-utrace_flags, otherwise
utrace_resume/utrace_get_signal won't be called.

So, probably detach should set TIF_NOTIFY_RESUME, but utrace_reset()
should do user_disable_single_step() too if no more engines. Confused.

And in fact I don't understand why this is important. When it comes
to multiracing, any engine can hit the unwanted/unexpected trap
because another engine can ask for UTRACE_*STEP. The only really
important (I think) case is when the last engine detaches.

IOW. Suppose that eninge E does utrace_control(STEP) or its callback
returns UTRACE_*STEP. If we do not detach this engine, other engines
will see the trap. So why it is so important to clear X86_EFLAGS_TF
if we detach E ?

Oleg.



Re: [PATCH] utrace: utrace_reset() should clear TIF_SINGLESTEP if no more engines

2010-09-22 Thread Oleg Nesterov
On 09/23, Oleg Nesterov wrote:

 On 09/21, Roland McGrath wrote:
 
  It's better to have a spurious report (preferably just spurious
  TIF_NOTIFY_RESUME with no actual callbacks) following any detach,
  or whatever it takes to ensure user_disable_single_step() always
  runs if user_enable_*_step did before and there is no engine around
  to see a report_quiesce callback pass.  If there is a report_quiesce
  or report_signal callback pass where nobody returns UTRACE_*STEP,
  then we should never leave stepping enabled when we return to user mode.

 Hmm. I'll try to think more. Right now I don't really understand
 how to do this correctly.

 OK, finish_callback_report() and utrace_control(DETACH) can set
 TIF_NOTIFY_RESUME. But what if there are no more attached engines?
 Looks like, utrace_resume(UTRACE_RESUME) needs to handle this special
 case. And utrace_reset() shouldn't clear task-utrace_flags, otherwise
 utrace_resume/utrace_get_signal won't be called.

 So, probably detach should set TIF_NOTIFY_RESUME, but utrace_reset()
 should do user_disable_single_step() too if no more engines. Confused.

Or, detach can always do user_disable_single_step() and set
TIF_NOTIFY_RESUME. If another engine wants stepping its report_quiesce()
should re-ack UTRACE_SINGLESTEP anyway, otherwise it is buggy.

 And in fact I don't understand why this is important. When it comes
 to multiracing, any engine can hit the unwanted/unexpected trap
 because another engine can ask for UTRACE_*STEP. The only really
 important (I think) case is when the last engine detaches.

Aaah. please ignore. Somehow I assumed every engine should hook
SIGTRAP.

Oleg.