gdbstub initial code, v8

[Oleg Nesterov]

Changes:

- fix UTRACE_SIGNAL_HOLD logic

- don't assume QPassSignals: passes valid_signal()

- implement qXfer:siginfo:read

- implement continue with signal.

Note: it is still not clear to me what "signal SIG" should
do if the tracee did not report a signal (T00 and no signal
context). Currently, in this case ugdb just sends this sig
to the tracee, this means another TSIG report.

Another corner case. Consider

(gdb) signal SIG &
(gdb) interrupt

it is possible (not in practice, I think) that "interrupt"
will try to stop the tracee before it handles SIG, in this
case the tracee can report TSIG too.

Oleg.

[Jan Kratochvil]

Post by Oleg Nesterov
- implement qXfer:siginfo:read
- implement continue with signal.

OK, thanks, just it was a bit premature to ask for it I see. I miss at least
memory writes (also to put in breakpoints):

(gdb) ptype done
type = int
(gdb) p done=1
$6 = 0
:-)

Sending packet: $M7f00e6a378e0,1:cc#da...Packet received:
- without much effect.

Using some:
killall -9 nc sigtest;gcc -o sigtest sigtest.c -Wall -g;nice -n20 ./sigtest&p=$!;nc -l 2000 <>/proc/ugdb >&0& ../gdb -nx -ex 'set architecture i386:x86-64' -ex 'set debug remote 0' -ex 'set debug solib 1' -ex 'set target-async on' -ex 'set non-stop on' -ex 'target extended-remote :2000' -ex 'file ./sigtest' -ex "attach $p" -ex 'info shared'


Thanks,
Jan

[Oleg Nesterov]

Post by Jan Kratochvil

Post by Oleg Nesterov
- implement qXfer:siginfo:read
- implement continue with signal.

OK, thanks, just it was a bit premature to ask for it I see. I miss at least
memory writes

Yes. This is simple.
And this is not clear to me, I need your help ;)

What should ugdb know about breakpoints? I played with the real
gdbserver, and afaics gdb just changes the tracee's memory and
inserts 0xcc (int 3). But gdb.info mentions "Z TYPE,ADDR,KIND"
packets.

So, what should ugdb do? Just implement memory write (M or X)
and then report SIGTRAP like gdbserver does?

Oleg.

[Jan Kratochvil]

Post by Oleg Nesterov

Post by Jan Kratochvil
memory writes

Yes. This is simple.
And this is not clear to me, I need your help ;)

Sorry, I just meant that by implementing the memory writes breakpoints
automatically start to work.

Post by Oleg Nesterov
What should ugdb know about breakpoints? I played with the real
gdbserver, and afaics gdb just changes the tracee's memory and
inserts 0xcc (int 3). But gdb.info mentions "Z TYPE,ADDR,KIND"
packets.

I believe it is described by:
/* If GDB wanted this thread to single step, we always want to
report the SIGTRAP, and let GDB handle it. Watchpoints should
always be reported. So should signals we can't explain. A
SIGTRAP we can't explain could be a GDB breakpoint --- we may or
not support Z0 breakpoints. If we do, we're be able to handle
GDB breakpoints on top of internal breakpoints, by handling the
internal breakpoint and still reporting the event to GDB. If we
don't, we're out of luck, GDB won't see the breakpoint hit. */

Post by Oleg Nesterov
So, what should ugdb do? Just implement memory write (M or X)
and then report SIGTRAP like gdbserver does?

Therefore until you track some ugdb-specific software(*) breakpoints ugdb does
not need to support Z0 IMO. I guess ugdb will never have to support these:
thread-related(?) and tracepoint ones.

(*) That is the memory-type one. There are also `hbreak' breakpoints via the
CPU debug registers.


Thanks,
Jan

[Oleg Nesterov]

Post by Jan Kratochvil

Post by Oleg Nesterov
And this is not clear to me, I need your help ;)

Sorry, I just meant that by implementing the memory writes breakpoints
automatically start to work.

Post by Oleg Nesterov
What should ugdb know about breakpoints? I played with the real
gdbserver, and afaics gdb just changes the tracee's memory and
inserts 0xcc (int 3). But gdb.info mentions "Z TYPE,ADDR,KIND"
packets.

/* If GDB wanted this thread to single step, we always want to
report the SIGTRAP, and let GDB handle it. Watchpoints should
always be reported. So should signals we can't explain. A
SIGTRAP we can't explain could be a GDB breakpoint --- we may or
not support Z0 breakpoints. If we do, we're be able to handle
GDB breakpoints on top of internal breakpoints, by handling the
internal breakpoint and still reporting the event to GDB. If we
don't, we're out of luck, GDB won't see the breakpoint hit. */

Post by Oleg Nesterov
So, what should ugdb do? Just implement memory write (M or X)
and then report SIGTRAP like gdbserver does?

Therefore until you track some ugdb-specific software(*) breakpoints ugdb does
thread-related(?) and tracepoint ones.

Good! I thought ugdb should somehow handle this all "transparently"
for gdb. I thought (I don't know why) that writing "int 3" from gdb
side should be avoided in favour of some "better" method unknown to me.

Thanks Jan.

Oleg.

[fche-H+wXaHxf7aLQT0dZR+ (Frank Ch. Eigler)]

Post by Oleg Nesterov
[...]

Post by Jan Kratochvil
Therefore until you track some ugdb-specific software(*)
breakpoints ugdb does not need to support Z0 IMO. I guess ugdb
will never have to support these: thread-related(?) and tracepoint
ones.

Good! I thought ugdb should somehow handle this all "transparently"
for gdb. I thought (I don't know why) that writing "int 3" from gdb
side should be avoided in favour of some "better" method unknown to me.

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.

- FChE

[Oleg Nesterov]

Post by fche-H+wXaHxf7aLQT0dZR+ (Frank Ch. Eigler)

Post by Oleg Nesterov
[...]

Post by Jan Kratochvil
Therefore until you track some ugdb-specific software(*)
breakpoints ugdb does not need to support Z0 IMO. I guess ugdb
will never have to support these: thread-related(?) and tracepoint
ones.

Good! I thought ugdb should somehow handle this all "transparently"
for gdb. I thought (I don't know why) that writing "int 3" from gdb
side should be avoided in favour of some "better" method unknown to me.

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.

Yes, agreed.

Oleg.

[Roland McGrath]

Post by fche-H+wXaHxf7aLQT0dZR+ (Frank Ch. Eigler)
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

[Oleg Nesterov]

Post by Oleg Nesterov
- implement continue with signal.

There is something wrong. I tried to debug the ptraced task via ugdb,
and the result is not good. I am not surprised ugdb confuses ptrace,
but I didn't expect that ptrace can break ugdb.

I am still trying to understand what is wrong, it should be something
simple but somehow I can't explain the problem so far...

Oleg.

[Oleg Nesterov]

Post by Oleg Nesterov

Post by Oleg Nesterov
- implement continue with signal.

There is something wrong. I tried to debug the ptraced task via ugdb,
and the result is not good. I am not surprised ugdb confuses ptrace,
but I didn't expect that ptrace can break ugdb.
I am still trying to understand what is wrong, it should be something
simple but somehow I can't explain the problem so far...

OK, I am stupid. Indeed, say, "return UTRACE_STOP | UTRACE_SIGNAL_IGN"
from under "case UTRACE_SIGNAL_REPORT" changes utrace_report->result,
and confuses other tracers.

Probably we can ignore ptrace, but this also means ugdb conflicts
with itself and should be fixed.

Oleg.

[Oleg Nesterov]

Post by Oleg Nesterov
OK, I am stupid. Indeed, say, "return UTRACE_STOP | UTRACE_SIGNAL_IGN"
from under "case UTRACE_SIGNAL_REPORT" changes utrace_report->result,
and confuses other tracers.
Probably we can ignore ptrace, but this also means ugdb conflicts
with itself and should be fixed.

Yes.

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.
But it needs to 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.

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

I need to re-read utrace_get_signal() with a fresh head.

Oleg.

[Roland McGrath]

Post by Oleg Nesterov
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.

Post by Oleg Nesterov
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.

Post by Oleg Nesterov
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.

Post by Oleg Nesterov
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

[Oleg Nesterov]

Post by Roland McGrath

Post by Oleg Nesterov
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,

Post by Roland McGrath

Post by Oleg Nesterov
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().

Post by Roland McGrath

Post by Oleg Nesterov
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.

Post by Roland McGrath

Post by Oleg Nesterov
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.

[Roland McGrath]

Post by Oleg Nesterov
But. Suppose we have to attached engines. The first engine gets
UTRACE_SIGNAL_REPORT and returns "UTRACE_STOP | UTRACE_SIGNAL_IGN".

Right, that's what it would do. I see, you're saying that the
report.result passed on to the next engine would appear like it had
been a real signal that the previous engine decided to ignore (or
whose sa_handler==SIG_IGN or default action was to ignore). Hmm.

Well, it's also distinguished by having orig_ka==NULL in the callback.
Any real signal has a non-null orig_ka argument.

Post by Oleg Nesterov
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.

At least in the UTRACE_SIGNAL_DELIVER case, that's really the true
thing for it to see. The previous engine decided to do a signal
delivery (as directed by return_ka), so the next engine needs to see
this to know what the prevailing state of play is now.

Post by Oleg Nesterov
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.

Well, it *can* do that. If UTRACE_SIGNAL_REPORT (or anything else
random) is the ultimate report.result from the whole callback loop, we
treat it just like UTRACE_SIGNAL_IGN.

Post by Oleg Nesterov
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.

I see. This is indeed the only way for any engine to know that it's
getting the UTRACE_SIGNAL_HANDLER specific notification rather than
some random fallout of someone else's UTRACE_REPORT request or whatnot.

Post by Oleg Nesterov

Post by Roland McGrath

Post by Oleg Nesterov
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.

Right. But this is in fact just the same for either
UTRACE_SIGNAL_REPORT or UTRACE_SIGNAL_HANDLER.

Post by Oleg Nesterov

Post by Roland McGrath

Post by Oleg Nesterov
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.

I don't follow this. If we're stopping, then we don't "receive"
(dequeue) any other signal until we get resumed. That should be fine
from gdb's point of view. The next signal is just pending for later.
The signal that we just reported was a) in fact reported to gdb and
b) is still sitting in the siginfo_t on stack and the engine can
examine/modify that before letting the thread resume. (The kerneldoc
paragraph about @report_signal in utrace.h makes this guarantee.)


Thanks,
Roland

[Oleg Nesterov]

Post by Roland McGrath

Post by Oleg Nesterov
But. Suppose we have to attached engines. The first engine gets
UTRACE_SIGNAL_REPORT and returns "UTRACE_STOP | UTRACE_SIGNAL_IGN".

Right, that's what it would do. I see, you're saying that the
report.result passed on to the next engine would appear like it had
been a real signal that the previous engine decided to ignore (or
whose sa_handler==SIG_IGN or default action was to ignore). Hmm.
Well, it's also distinguished by having orig_ka==NULL in the callback.
Any real signal has a non-null orig_ka argument.

Yes, this is what ugdb does.

ptrace doesn't, I didn't realize this problem when I was writing
that code.

Post by Roland McGrath

Post by Oleg Nesterov
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.

At least in the UTRACE_SIGNAL_DELIVER case, that's really the true
thing for it to see. The previous engine decided to do a signal
delivery (as directed by return_ka), so the next engine needs to see
this to know what the prevailing state of play is now.

Agreed. Currently ugdb doesn't report a signal injected by another
engine.

Post by Roland McGrath

Post by Oleg Nesterov

Post by Roland McGrath
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.

Right. But this is in fact just the same for either
UTRACE_SIGNAL_REPORT or UTRACE_SIGNAL_HANDLER.

Now I am not sure what you mean...

Yes, if the callback returns UTRACE_SIGNAL_REPORT then the next
callback will see "orig_ka == NULL" too, sure. But I guess you meant
something else.

Post by Roland McGrath

Post by Oleg Nesterov

Post by Roland McGrath

Post by Oleg Nesterov
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.

I don't follow this. If we're stopping, then we don't "receive"
(dequeue) any other signal until we get resumed. That should be fine
from gdb's point of view. The next signal is just pending for later.

Hmm. Yes.

If a callback ever returned UTRACE_STOP, utrace_get_signal() should
notice utrace->resume <= UTRACE_REPORT. OK, so ugdb doesn't need
this hack.

Thanks.

Now I am wondering how I managed to test this UTRACE_SIGNAL_HOLD code.
I recall, initially it returned "UTRACE_STOP | UTRACE_SIGNAL_HOLD",
then the testing showed it doesn't work and I added UTRACE_SIGNAL_REPORT.
Probably I added some hacks to simulate the problem which, as you showed,
doesn't exist.

Oleg.