Re: gdbstub initial code, v8

2010-09-10 Thread Roland McGrath
 Please note that last year's gdbstub prototype used kernel uprobes as
 an optional gdb breakpoint implementation (i.e., a backend for the Z
 packets).  When/if the lkml uprobes patches actually get merged, ugdb
 should also use them.

That's something for later, and it's not quite so simple.  If a utrace
engine ever uses uprobes, it probably would need to use the utrace-based
version of uprobes.  If something different goes in upstream, it remains
to be seen how it would interact with utrace, and there would be
specific work required for that.  There are many more issues about that
too.  At any rate, this is all a distraction at the moment, and Oleg
doesn't need any more of those!


Thanks,
Roland



Re: gdbstub initial code, v8 ptrace

2010-09-10 Thread Roland McGrath
 I am a bit confused... OK, ugdb is wrong wrt multitracing.
 UTRACE_SIGNAL_REPORT case shouldn't return UTRACE_STOP | UTRACE_SIGNAL_IGN,
 it should return UTRACE_STOP | UTRACE_SIGNAL_REPORT to keep report-result.

No, UTRACE_SIGNAL_REPORT is not meant for a return value.  Its only use is
in the incoming argument to tell you that a given report_signal call is
standing in for a report_quiesce(0) call.

 But it needs to return UTRACE_SIGNAL_DELIVER?

That's what you return when you want the signal delivered.
When you are stopping the tracee to decide about the signal,
that's not what you want.

UTRACE_STOP | UTRACE_SIGNAL_IGN is correct to not deliver the signal right
now, and stop instead.  If you want to deliver the signal later, then
you'll resume with UTRACE_INTERRUPT to ensure you get back to report_signal
and that can fill in the details and return UTRACE_SIGNAL_DELIVER.

 Probably we can check orig_ka != NULL and treat orig_ka == NULL as
 UTRACE_SIGNAL_REPORT. Hopefully this can't be confused with
 UTRACE_SIGNAL_HANDLER.

I'm not really sure what you mean here.

 Not sure about UTRACE_SIGNAL_HOLD, but this is very unlikely race.

You probably don't want to use that, but I'm not entirely sure.  ptrace
doesn't use it, and the signal interception model is pretty much the same.


Thanks,
Roland



Re: gdbstub initial code, v7

2010-09-10 Thread Roland McGrath
 But I meant another case, when the stopped tracee doesn't have siginfo.
 Currently ugdb just sends this signal to tracee, and then it will be
 reported to gdb. Not sure if this is right or not, I can change this.
 (or perhap this doesn't matter, I dunno).

What do you mean by doesn't have siginfo?  You mean non-signal stops?
What non-signal stops does ugdb report?


Thanks,
Roland



Re: [PATCH 0/3] UTRACE_DETACH fixes

2010-09-10 Thread Roland McGrath
 I misunderstood the UTRACE_INTERRUPT without UTRACE_EVENT(QUIESCE) above
 as if you mean it is possible to get UTRACE_SIGNAL_REPORT without QUIESCE.

No, I meant the opposite: since you won't get UTRACE_SIGNAL_REPORT without
UTRACE_EVENT(QUIESCE), it might seem that UTRACE_INTERRUPT should not be
permitted, as UTRACE_REPORT is not.  But because that is not it's *only*
effect, we don't do permit it even though you won't get UTRACE_SIGNAL_REPORT.

 OK. This means ugdb should set QUIESCE, just to ensure its -report_signal()
 will be called.

If you ever want UTRACE_SIGNAL_REPORT calls, yes.

 OK, please forget. I must admit, this still looks a bit, well, unnatural
 to me. May be it makes sense to add
 
   _UTRACE_EVENT_SIGNAL_REPORT,
   _UTRACE_EVENT_SIGNAL_HANDLER,
 
 into utrace_events. Then ugdb could ask for UTRACE_SIGNAL_REPORT and
 avoid the unnecessary -report_quiesce() calls.

If anything, I think UTRACE_EVENT(RESUME) would make sense, perhaps with a
separate -report_resume callback.  That would request only the cases that
now get report_quiesce(0).  But, it really does not cost much to have 

if (event) return UTRACE_RESUME;

or similar in your report_quiesce callback.  It's beyond refinement of the
utrace API (that we're not doing now), it's just micro-optimization.

 Roland, could you also comment this patch?
 
   https://www.redhat.com/archives/utrace-devel/2010-August/msg00108.html
 
 Again, this looks like a bug to me, but I won't insist if it is not.

Sorry, that is still on my queue.  I haven't forgotten it.  
I just haven't gotten to reviewing it yet because of other
work and it needing a lot of hard thinking.


Thanks,
Roland



Re: gdbstub initial code, v9

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

   Oleg == Oleg Nesterov o...@redhat.com writes:
 
  Oleg   (gdb) set var 0
 
  You need:  set variable var = 0
  The variable can be abbreviated.

 I've always just used:

   (gdb) set var=0

No, I tried this too, doesn't work.

(gdb) set var=0
A syntax error in expression, near `=0'.

But, it turns out I choosed a bad name for the variable when
I tested the fix in unxex().

(gdb) set xxx=0

This works.

(gdb) set var var=0

This works too. I guess, when gdb sees set var it expects the
full set variable ... command.

Oleg.



Re: gdbstub initial code, v7

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

  But I meant another case, when the stopped tracee doesn't have siginfo.
  Currently ugdb just sends this signal to tracee, and then it will be
  reported to gdb. Not sure if this is right or not, I can change this.
  (or perhap this doesn't matter, I dunno).

 What do you mean by doesn't have siginfo?  You mean non-signal stops?

Yes.

 What non-signal stops does ugdb report?

(gdb) interrupt

ugdb sets please stop flag and does utrace_control(INTERRUPT). However,
in unlikely case the tracee can stop before -report_signal() reporting
loop (especially in multitracing case). Or it can be already stopped
(note: this needs a separate discussion, currently ugdb intentionally
doesn't handle this case).

And. With the current implementation, even if the tracee stops after
ugdb_report_signal() was called, it doesn't setup -t_siginfo.

IOW. If the tracee actually recieves a signal, then

- qXfer:siginfo:read works

- signal SIG works as expected (delivered to tracee)

Otherwise

- qXfer:siginfo:read reports E01

- signal XX means TXX report.

Once again, this can be changed (fixed?), but I am not sure this
should be changed.

Oleg.



Re: gdbstub initial code, v7

2010-09-10 Thread Roland McGrath
 ugdb sets please stop flag and does utrace_control(INTERRUPT). However,
 in unlikely case the tracee can stop before -report_signal() reporting

I don't think this is the right thing to do.  When the intent is explicitly
to interrupt, there is no reason to stop before the interruption is
complete, i.e. report_signal.  If you only stop there, then you can always
process a signal injection with complete flexibility.  The gdb model and
the remote protocol doesn't currently have any concept of requesting a stop
that is not an interruption.


Thanks,
Roland



Re: gdbstub initial code, v8 ptrace

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

  I am a bit confused... OK, ugdb is wrong wrt multitracing.
  UTRACE_SIGNAL_REPORT case shouldn't return UTRACE_STOP | 
  UTRACE_SIGNAL_IGN,
  it should return UTRACE_STOP | UTRACE_SIGNAL_REPORT to keep 
  report-result.

 No, UTRACE_SIGNAL_REPORT is not meant for a return value.  Its only use is
 in the incoming argument to tell you that a given report_signal call is
 standing in for a report_quiesce(0) call.

Yes, that is why initially ugdb returned  | UTRACE_SIGNAL_IGN. But
you misunerstood my concerns (or me your reply ;)

But. Suppose we have to attached engines. The first engine gets
UTRACE_SIGNAL_REPORT and returns UTRACE_STOP | UTRACE_SIGNAL_IGN.

Or,

  But it needs to return UTRACE_SIGNAL_DELIVER?

 That's what you return when you want the signal delivered.

or yes, it returns UTRACE_SIGNAL_DELIVER after gdb does signal XX.

Now. The second engine gets UTRACE_SIGNAL_DELIVER or UTRACE_SIGNAL_IGN,
not UTRACE_SIGNAL_REPORT.


That is why ugdb_signal_report(UTRACE_SIGNAL_REPORT) tries to return
UTRACE_STOP | utrace_signal_action(action) to not change report-result
(passed to the next tracee) inside the reporting loop.

The worst case is UTRACE_SIGNAL_HANDLER. Single-stepping should know
about this case. This means that any engine should always return
UTRACE_resume_action | UTRACE_SIGNAL_HANDLER.

Unless we are going to change utrace_get_signal().

  Probably we can check orig_ka != NULL and treat orig_ka == NULL as
  UTRACE_SIGNAL_REPORT. Hopefully this can't be confused with
  UTRACE_SIGNAL_HANDLER.

 I'm not really sure what you mean here.

If -report_signal(orig_ka) was calles with orig_ka == NULL, then,
whatever utrace_signal_action(action) we see, originally it was
either UTRACE_SIGNAL_HANDLER or UTRACE_SIGNAL_REPORT but another engine
returned, say, UTRACE_SIGNAL_DELIVER/UTRACE_SIGNAL_IGN to deliver/stop.

  Not sure about UTRACE_SIGNAL_HOLD, but this is very unlikely race.

 You probably don't want to use that, but I'm not entirely sure.  ptrace
 doesn't use it, and the signal interception model is pretty much the same.

Yes, agreed. But there is one corner case. Until we generalize
utrace_stop()-ptrace_notify_stop() hook, the tracee can be reported as
stopped to gdb, but before it actually stops it can recieve a signal.

The remote protocol doesn't allow to send TSIG after we alreay sent
T00 (at least this actually confuses gdb), and we can't just stop, the
tracee should report this signal to debugger.

So, currently ugdb stops but uses UTRACE_SIGNAL_HOLD to report this
signal later.

Oleg.



Re: gdbstub initial code, v7

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

  ugdb sets please stop flag and does utrace_control(INTERRUPT). However,
  in unlikely case the tracee can stop before -report_signal() reporting

 I don't think this is the right thing to do.  When the intent is explicitly
 to interrupt, there is no reason to stop before the interruption is
 complete, i.e. report_signal.

This means that ugdb_report_quiesce() should never return UTRACE_STOP,
and that is all.

But what about multitracing? Suppose that (gdb) interrupt happens
just before, say, do_report_syscall_entry() and another engine wants
to stop. If ugdb_report_quiesce() doesn't return UTRACE_STOP, then
gdb will wait until another debugger resumes the tracee.

What do you think?

 If you only stop there, then you can always
 process a signal injection with complete flexibility.

Yes, sure (again, currently ugdb does not injection a signal even
if the tracee was stopped in report_signal, but of course we can
change this).

Oleg.



Re: gdbstub initial code, v9

2010-09-10 Thread Tom Tromey
 Oleg == Oleg Nesterov o...@redhat.com writes:

 I've always just used:
 (gdb) set var=0

Oleg No, I tried this too, doesn't work.
Oleg   (gdb) set var=0
Oleg   A syntax error in expression, near `=0'.

Yeah, it is ambiguous if the actual variable name conflicts with any gdb
set subcommand.

I typically just use call or print.

Tom



Re: ugdb breakpoints

2010-09-10 Thread Tom Tromey
Oleg   Now, to continue the tracee, gdb does not restore the
Oleg   original instruction. Instead, it
Oleg   - writes this insn into _start code
Oleg   - changes regs-ip to point to this insn
Oleg   - does single-step to execute this insn
Oleg   - changes regs-ip again

This is what is done for non-stop.
I believe it is called displaced stepping in gdb.

I think eventually we would like it if uprobes did this work, instead of
gdb doing it.  Presumably that would yield better performance.  E.g., if
we have a thread-specific breakpoint, then other threads hitting that
breakpoint could simply do the displaced stepping via uprobes, and not
report a breakpoint hit to gdb at all.

For all-stop, breakpoints are handled differently, though I don't
remember how offhand.

Tom