Re: gdbstub initial code, v7
On 10/13, Roland McGrath wrote: 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. I'm not sure about this. Ignoring the problems below, why? 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. Yes, I do think that's a problem. We want gdb to report back promptly. One possibility is to have report_quiesce notice its argument is UTRACE_EVENT(SYSCALL_ENTRY) and roll back to before the syscall. That is, it enables SYSCALL_ENTRY and SYSCALL_EXIT reporting, then its report_syscall_entry uses UTRACE_SIGNAL_ABORT, report_syscall_exit does syscall_set_return_value(-ERESTARTNOHAND, 0) and then returns UTRACE_INTERRUPT. Now, we'll reenter a UTRACE_SIGNAL_REPORT callback before the system call, and we can stop there without being in any sticky situation. Well, but this doesn't look friendly to other engines... And at first glance this looks a bit too hairy. And, this doesn't cover another case: gdb asks to stop the tracee when it is already stopped by another engine and sleeps in utrace_resume() path. So, I think ugdb should be changed so that signal SIG always works (without reporing this signal) even when the stopped tracee doesn't have the signal context. As for $_siginfo (qXfer:siginfo:read::), I do not know what ugdb should do. Probably it can just report the all-zeroes siginfo or report si_signo = SIGSTOP. Oleg.
Re: gdbstub initial code, v8 ptrace
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. 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. 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. 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. 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. 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
Re: gdbstub initial code, v13
On 10/08, Oleg Nesterov wrote: - full vCont support. Well, this was easy, except even after 3 hours of debugging I can't understand why this change breaks the stepping over pthread_create :/ Oh, it doesn't break. I did a lot of mistakes when I was trying to understand what happens. In short, GDBCAT was buggy, it used the wrong socket for TCP_NODELAY. Damn, it took me almost 2 days. Oleg.
Re: BUG: gdb notification packets (Was: gdbstub initial code, v12)
On Thursday 07 October 2010 23:59:22, Oleg Nesterov wrote: Hmm. Not sure I understand this... gdb could issue a series of Hc+c after s to do step a thread and resume all others. But this doesn't matter. Obviously vCont is better and more handy. Not in all-stop mode. GDB can not send any other command to the stub until the stub returns a stop reply to the first 's'. Remember, there's no vStopped+notifications in the all-stop mode protocol. -- Pedro Alves
Re: BUG: gdb notification packets (Was: gdbstub initial code, v12)
On 10/05, Pedro Alves wrote:
(reordered)
On Tuesday 05 October 2010 18:27:29, Oleg Nesterov wrote:
So, I strongly believe gdb is buggy and should be fixed.
Fix your stub to implement vCont;s/c(/S/C).
First of all, I confirm that when I added the (incomplete right
now) support for vCont;s the problem goes away, gdb never forgets
to send the necessary vStopped to consume all stop-reply packets.
Thanks Pedro.
The more or less typical transcript is:
[... snip ...]
= s
This is already wrong.
The stub must support @samp{vCont} if it reports support for
multiprocess extensions (@pxref{multiprocess extensions}).
Cough. Previously I was told here (on arc...@sourceware.org) that
Hc + s/c is enough and I shouldn't worry about vCont;s/c ;)
Currently ugdb only supports vCont;t because otherwise there is
obviously no way to stop all threads.
The stub must also support vCont for non-stop, though I'll give you
that it doesn't appear to be mentioned in the manual,
Yes, the manual doesn't explain this. Quite contrary, the decsription
of 'vCont?' definitely looks as if the stub is not obliged to implement
all vCont commands.
And, if the stub must support vCont for non-stop, then why gdb
doesn't complain after 'vCont?' but falls back to '$s' ?
Look at remote.c:remote_resume, and you'll see that gdb does not
wait for the OK after 'c'/'s'/'S'/'C' in non-stop mode.
Then gdbserver should be fixed? It does send OK in response to '$s',
that is why ugdb does this.
And what should be replied back to '$s', nothing? Very strange.
But seems to work... And yes, this explains the hang, gdb thinks
that this OK is connected to vStopped.
Again, the documentation is very confusing. Looking at
remote_resume()-remote_vcont_resume()-getpkt() I think that
vCont;s needs OK. Looking at D.3 Stop Reply Packets in
gdb.info I do not see any difference between `s' and `vCont'.
So. I still belive that something is wrong with gdb/gdbserever
but I don't care.
In any case ugdb should fully support vCont, hopefully I'll finish
this tomorrow. Could you answer a couple of questions?
1. Say, $vCont;s or $vCont;s:p-1.-1
I assume, this should ignore the running threads, correct?
IOW, iiuc this 's' applies to all threads which we already
reported as stopped.
2. Say, $vCont;c:pPID.TID;s:p-1.-1
Can I assume that gdb can never send this request as
$vCont;s:p-1.-1;c:pPID.TID ?
If yes, then the implementation will be much simpler, I can
add something like gencounters to ugdb_thread/process. Otherwise
this needs more complications to figure out what should be done
with each tracee.
Thanks,
Oleg.
Re: BUG: gdb notification packets (Was: gdbstub initial code, v12)
On Wednesday 06 October 2010 18:19:53, Oleg Nesterov wrote:
On 10/05, Pedro Alves wrote:
The stub must support @samp{vCont} if it reports support for
multiprocess extensions (@pxref{multiprocess extensions}).
Cough. Previously I was told here (on arc...@sourceware.org) that
Hc + s/c is enough and I shouldn't worry about vCont;s/c ;)
vCont was introduced because with only 'Hc', 's' and 'c', there's
no way to distinguish step a thread and resume all others vs step
a thread and leave others stopped (scheduler-locking, in gdb lingo).
This was added way before non-stop was added, back in 2002/2003,
I believe. vCont;t was added much later, when non-stop was
introduced.
The stub must also support vCont for non-stop, though I'll give you
that it doesn't appear to be mentioned in the manual,
Yes, the manual doesn't explain this. Quite contrary, the decsription
of 'vCont?' definitely looks as if the stub is not obliged to implement
all vCont commands.
And, if the stub must support vCont for non-stop, then why gdb
doesn't complain after 'vCont?' but falls back to '$s' ?
Because nobody took the trouble to made it complain. As I said,
I'll give you that gdb could be noisier about that...
Look at remote.c:remote_resume, and you'll see that gdb does not
wait for the OK after 'c'/'s'/'S'/'C' in non-stop mode.
Then gdbserver should be fixed? It does send OK in response to '$s',
that is why ugdb does this.
Think of it as undefined behavior. It could be made to
error out instead, if somebody cared. Not sure how you got gdb to
send gdbserver 's' or 'c' (well, unless you used
set remote verbose-resume-packet off, or started gdbserver
with --disable-packet=vCont).
Again, the documentation is very confusing. Looking at
remote_resume()-remote_vcont_resume()-getpkt() I think that
vCont;s needs OK. Looking at D.3 Stop Reply Packets in
gdb.info I do not see any difference between `s' and `vCont'.
Yeah. It's the problem that those that are very familiar with
the thing get to write docs for it, so may have missed spelling
out things that were obvious to them.
It goes without saying, but ... patches to improve the docs are
always welcome.
In any case ugdb should fully support vCont, hopefully I'll finish
this tomorrow. Could you answer a couple of questions?
1. Say, $vCont;s or $vCont;s:p-1.-1
I assume, this should ignore the running threads, correct?
IOW, iiuc this 's' applies to all threads which we already
reported as stopped.
Yes.
2. Say, $vCont;c:pPID.TID;s:p-1.-1
This would be effectively
$vCont;c:pPID.TID;s
Can I assume that gdb can never send this request as
$vCont;s:p-1.-1;c:pPID.TID ?
If yes, then the implementation will be much simpler, I can
add something like gencounters to ugdb_thread/process. Otherwise
this needs more complications to figure out what should be done
with each tracee.
All GDB currently sends is in gdb/remote.c:remote_vcont_resume.
All vCont packets GDB sends today have the actions ordered
from more specific to less specific --- the most complicated
is something like vCont;s:pPID.TID;c (step PID.TID, continue
all others). It will probably make sense to maintain that
ordering, if we ever make a single vCont contain more
actions.
--
Pedro Alves
Re: BUG: gdb notification packets (Was: gdbstub initial code, v12)
On Tuesday 05 October 2010 19:30:38, Pedro Alves wrote: Now, given this, I won't be surprised if you're seeing races with -s, -OK, -vCont sequences, as GDB may well be thinking that the OK is a reply to the vCont. I meant -s, -OK, -vStopped sequences. -- Pedro Alves
Re: gdbstub initial code, v11
The ones I'm talking about are Z2/Z3 for (data) watchpoints. Ah, OK, thanks. I'll try to understand how this works. In theory these will map to uses of the hw_breakpoint interface. Thanks, Roland
Re: gdbstub initial code, v11
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. My vague notion was that we'd get it working well enough to have basic parity with native or gdbserver on some realish test cases, and then just look at the protocol interaction log to see cases where we could reduce round-trips between gdb and the stub. Some of those are bound to entail protocol extensions and gdb changes to exploit them. Off hand, the Z cases might be the only things that won't need that. Thanks, Roland
Re: gdbstub initial code, v11
Hi - On Thu, Sep 23, 2010 at 02:21:38PM -0700, Roland McGrath wrote: The ones I'm talking about are Z2/Z3 for (data) watchpoints. Ah, OK, thanks. I'll try to understand how this works. In theory these will map to uses of the hw_breakpoint interface. Not quite. The hw_breakpoint widget is only useful for the first few active watchpoints. The rest, which gdb calls software watchpoints, can be implemented in ugdb by local singlestep + polling, without gdb's live involvement. - FChE
Re: gdbstub initial code, v11
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
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
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
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
gdbstub initial code, v11
Changes: syscall stepping + minor cleanups.
Seems to work, more or less, but surely there are some bugs.
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.
Next: fully implement g/G/p/P, currently I am a bit confused...
But what about features? What should I do next? all-stop,
thread-specific breakpoints (currently I have no idea what
this means), or what?
Oleg.
#include linux/module.h
#include linux/proc_fs.h
#include linux/utrace.h
#include linux/poll.h
#include linux/mm.h
#include linux/regset.h
#include asm/uaccess.h
static int o_remote_debug;
module_param_named(echo, o_remote_debug, bool, 0);
#define BUFFER_SIZE 1024
#define PACKET_SIZE 1024
struct pbuf {
char*cur, *pkt;
charbuf[BUFFER_SIZE];
};
static inline void pb_init(struct pbuf *pb)
{
pb-cur = pb-buf;
pb-pkt = NULL;
}
enum {
U_STOP_IDLE = 0,
U_STOP_PENDING,
U_STOP_SENT,
};
struct ugdb {
struct list_headu_processes;
struct list_headu_stopped;
int u_stop_state;
struct mutexu_mutex;
spinlock_t u_slock;
struct ugdb_thread
*u_cur_tinfo,
*u_cur_hg,
*u_cur_hc;
wait_queue_head_t u_wait;
int u_err;
struct pbuf u_pbuf;
charu_cbuf[PACKET_SIZE];
int u_clen;
sigset_tu_sig_ign;
unsigned int
u_no_ack:1,
u_allstop:1;
};
static inline void ugdb_ck_stopped(struct ugdb *ugdb)
{
spin_lock(ugdb-u_slock);
WARN_ON(!list_empty(ugdb-u_stopped)
ugdb-u_stop_state == U_STOP_IDLE);
WARN_ON(list_empty(ugdb-u_stopped)
ugdb-u_stop_state == U_STOP_PENDING);
spin_unlock(ugdb-u_slock);
}
static struct ugdb *ugdb_create(void)
{
struct ugdb *ugdb;
int err;
err = -ENODEV;
// XXX: ugly. proc_reg_open() should take care.
if (!try_module_get(THIS_MODULE))
goto out;
err = -ENOMEM;
ugdb = kzalloc(sizeof(*ugdb), GFP_KERNEL);
if (!ugdb)
goto put_module;
INIT_LIST_HEAD(ugdb-u_processes);
INIT_LIST_HEAD(ugdb-u_stopped);
mutex_init(ugdb-u_mutex);
spin_lock_init(ugdb-u_slock);
init_waitqueue_head(ugdb-u_wait);
pb_init(ugdb-u_pbuf);
return ugdb;
put_module:
module_put(THIS_MODULE);
out:
return ERR_PTR(err);
}
#define P_DETACHING (1 1)
#define P_ZOMBIE(1 2)
struct ugdb_process {
int p_pid;
int p_state;
struct list_headp_threads;
struct ugdb *p_ugdb;
struct list_headp_processes;
};
static inline bool process_alive(struct ugdb_process *process)
{
return !(process-p_state P_ZOMBIE);
}
static inline void mark_process_dead(struct ugdb_process *process)
{
process-p_state |= P_ZOMBIE;
}
static struct ugdb_process *ugdb_create_process(struct ugdb *ugdb, int pid_nr)
{
struct ugdb_process *process;
process = kzalloc(sizeof(*process), GFP_KERNEL);
if (!process)
return NULL;
process-p_pid = pid_nr;
process-p_ugdb = ugdb;
INIT_LIST_HEAD(process-p_threads);
list_add_tail(process-p_processes, ugdb-u_processes);
return process;
}
#define T_STOP_RUN 0
#define T_STOP_REQ (1 0)/* requested by gdb */
#define T_STOP_ALL (1 1)/* vCont;c:pX.-1, for report_clone */
#define T_STOP_ACK (1 2)/* visible to vStopped */
#define T_STOP_STOPPED (1 3)/* reported as stopped to gdb */
/* TASK_TRACED + deactivated ? */
#define T_EV_NONE 0
#define T_EV_EXIT (1 24)
#define T_EV_SIGN (2 24)
#define T_EV_TYPE(event)((0xff 24) (event))
#define T_EV_DATA(event)(~(0xff 24) (event))
struct ugdb_thread {
int t_tid;
int t_stop_state;
int t_stop_event;
boolt_step; // XXX: move me into
t_stop_event
siginfo_t *t_siginfo;
struct ugdb *t_ugdb;
struct ugdb_process *t_process;
struct
gdbstub initial code, v10
Back to ugdb.
Changes: implement stepi, this also means breakpoints work too.
Notes:
- almost untested, probably needs more fixes
- syscall-stepping doesn't work correctly (should be simple)
- don't look at handle_setregs/handle_set_one_reg, I did
this on purpose to be sure I really understand what gdb
actually does
Well. This wasn't simple because I nearly got heart attack twice
when I was writing this change ;)
However, it turns out that ptrace-utrace (old or recently changed)
is innocent.
I found 2 problems, the 1st one is /usr/bin/gdb bug, another one
(I believe) is utrace core bug. I'll report tomorrow.
Oleg.
#include linux/module.h
#include linux/proc_fs.h
#include linux/utrace.h
#include linux/poll.h
#include linux/mm.h
#include linux/regset.h
#include asm/uaccess.h
static int o_remote_debug;
module_param_named(echo, o_remote_debug, bool, 0);
#define BUFFER_SIZE 1024
#define PACKET_SIZE 1024
struct pbuf {
char*cur, *pkt;
charbuf[BUFFER_SIZE];
};
static inline void pb_init(struct pbuf *pb)
{
pb-cur = pb-buf;
pb-pkt = NULL;
}
enum {
U_STOP_IDLE = 0,
U_STOP_PENDING,
U_STOP_SENT,
};
struct ugdb {
struct list_headu_processes;
struct list_headu_stopped;
int u_stop_state;
struct mutexu_mutex;
spinlock_t u_slock;
struct ugdb_thread
*u_cur_tinfo,
*u_cur_hg,
*u_cur_hc;
wait_queue_head_t u_wait;
int u_err;
struct pbuf u_pbuf;
charu_cbuf[PACKET_SIZE];
int u_clen;
sigset_tu_sig_ign;
unsigned int
u_no_ack:1,
u_allstop:1;
};
static inline void ugdb_ck_stopped(struct ugdb *ugdb)
{
spin_lock(ugdb-u_slock);
WARN_ON(!list_empty(ugdb-u_stopped)
ugdb-u_stop_state == U_STOP_IDLE);
WARN_ON(list_empty(ugdb-u_stopped)
ugdb-u_stop_state == U_STOP_PENDING);
spin_unlock(ugdb-u_slock);
}
static struct ugdb *ugdb_create(void)
{
struct ugdb *ugdb;
int err;
err = -ENODEV;
// XXX: ugly. proc_reg_open() should take care.
if (!try_module_get(THIS_MODULE))
goto out;
err = -ENOMEM;
ugdb = kzalloc(sizeof(*ugdb), GFP_KERNEL);
if (!ugdb)
goto put_module;
INIT_LIST_HEAD(ugdb-u_processes);
INIT_LIST_HEAD(ugdb-u_stopped);
mutex_init(ugdb-u_mutex);
spin_lock_init(ugdb-u_slock);
init_waitqueue_head(ugdb-u_wait);
pb_init(ugdb-u_pbuf);
return ugdb;
put_module:
module_put(THIS_MODULE);
out:
return ERR_PTR(err);
}
#define P_DETACHING (1 1)
#define P_ZOMBIE(1 2)
struct ugdb_process {
int p_pid;
int p_state;
struct list_headp_threads;
struct ugdb *p_ugdb;
struct list_headp_processes;
};
static inline bool process_alive(struct ugdb_process *process)
{
return !(process-p_state P_ZOMBIE);
}
static inline void mark_process_dead(struct ugdb_process *process)
{
process-p_state |= P_ZOMBIE;
}
static struct ugdb_process *ugdb_create_process(struct ugdb *ugdb, int pid_nr)
{
struct ugdb_process *process;
process = kzalloc(sizeof(*process), GFP_KERNEL);
if (!process)
return NULL;
process-p_pid = pid_nr;
process-p_ugdb = ugdb;
INIT_LIST_HEAD(process-p_threads);
list_add_tail(process-p_processes, ugdb-u_processes);
return process;
}
#define T_STOP_RUN 0
#define T_STOP_REQ (1 0)/* requested by gdb */
#define T_STOP_ALL (1 1)/* vCont;c:pX.-1, for report_clone */
#define T_STOP_ACK (1 2)/* visible to vStopped */
#define T_STOP_STOPPED (1 3)/* reported as stopped to gdb */
/* TASK_TRACED + deactivated ? */
#define T_EV_NONE 0
#define T_EV_EXIT (1 24)
#define T_EV_SIGN (2 24)
#define T_EV_TYPE(event)((0xff 24) (event))
#define T_EV_DATA(event)(~(0xff 24) (event))
struct ugdb_thread {
int t_tid;
int t_stop_state;
int t_stop_event;
boolt_step; // XXX: move me into
t_stop_event
siginfo_t *t_siginfo;
struct ugdb *t_ugdb;
struct ugdb_process *t_process;
struct
Re: gdbstub initial code, v8
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
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
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: gdbstub initial code, v9
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
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
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
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
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
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: gdbstub initial code, v9
Hi Oleg, kernel-devel-2.6.34.6-54.fc13.x86_64 (real F13) says: ugdb.c:1988: error: implicit declaration of function ‘hex_to_bin’ Jan
Re: gdbstub initial code, v9
On 09/09, Frank Ch. Eigler wrote: Oleg Nesterov o...@redhat.com writes: [...] But, Jan. Implementing the memory writes does not mean breakpoints automatically start to work! It approximately should though. Yes, gdb writes cc, and yes the tracee reports SIGTRAP. But after that continue does nothing except $c, and the tracee naturally gets SIGILL. I expected that, since ugdb doesn't even know the code was changed, gdb should write the original byte back before continue, but this doesn't happen. In normal all-stop mode, Currently ugdb only supports non-stop gdb does normally replace the old instruction, in order to single-step over it with the 's' packet. Yes, probably single-stepping is needed... I am still trying to understand how this works with gdbserver, but I see vCont:s packets. Perhaps you're testing some buggy non-stop aspect that only works with 'Z' breakpoint management packets? No. Just a trivial test-case which printfs in a loop. A fuller packet trace would help explain. Please see below. But the only important part is: $M4005ba,1:cc --- set bp $c --- resume of course, this can't work. Full trace: = qSupported:multiprocess+ = PacketSize=400;QStartNoAckMode+;QNonStop+;multiprocess+;QPassS... = QStartNoAckMode = OK = ! = OK = Hgp0.0 = E01 = QNonStop:1 = OK = qfThreadInfo = E01 = ? = OK = qSymbol:: = = vAttach;95b = OK = qfThreadInfo = mp95b.95b = qsThreadInfo = l = Hgp95b.95b = OK = vCont? = vCont;t = vCont;t:p95b.-1 = OK = %Stop:T00thread:p95b.95b; = vStopped = OK = g = fcfd90ad5329ff7f00... = m600880,8 = 403c6d7d007f = m7f007d6d3c48,8 = 00106d7d007f = m7f007d6d1000,28 = f6e04c7d007fe807600080156d7d007f00... = m7f007d6d1580,28 = 00f0ef29ff7ff6e04c7d007f50f45f29ff7f00c06c7d007f00... = m7f007d4ce0f4,4 = 090a0069 = m7f007d6cc000,28 = 0030167d007f781f6d7d007f400b4b7d007fe8346d7d007f00... = m7f007d6d1f78,4 = 2f6c6962 = m7f007d6d1f7c,4 = 2f6c6962 = m7f007d6d1f80,4 = 632e736f = m7f007d6d1f84,4 = 2e36 = m7f007d6d34e8,28 = 00704b7d007f0002482e6d7d007f00... = m400200,4 = 2f6c6962 = m400204,4 = 2f6c642d = m400208,4 = 6c696e75 = m40020c,4 = 782d7838 = m400210,4 = 362d3634 = m400214,4 = 2e736f2e = m400218,4 = 3200 = m7f007d6d3c40,4 = 0100 = m7f007d6d3c48,8 = 00106d7d007f = m7f007d6d3c50,8 = c04e4c7d007f = Z0,7f007d4c4ec0,1 = = m7f007d4c4ec0,1 = f3 = X7f007d4c4ec0,0: = = M7f007d4c4ec0,1:cc = OK = m600880,8 = 403c6d7d007f = m7f007d6d3c48,8 = 00106d7d007f = m7f007d6d1000,28 = f6e04c7d007fe807600080156d7d007f00... = m7f007d6d1580,28 = 00f0ef29ff7ff6e04c7d007f50f45f29ff7f00c06c7d007f00... = m7f007d4ce0f4,4 = 090a0069 = m7f007d6cc000,28 = 0030167d007f781f6d7d007f400b4b7d007fe8346d7d007f00... = m7f007d6d1f78,4 = 2f6c6962 = m7f007d6d1f7c,4 = 2f6c6962 = m7f007d6d1f80,4 = 632e736f = m7f007d6d1f84,4 = 2e36 = m7f007d6d34e8,28 = 00704b7d007f0002482e6d7d007f00... = m400200,4 = 2f6c6962 = m400204,4 = 2f6c642d = m400208,4 = 6c696e75 = m40020c,4 = 782d7838 = m400210,4 = 362d3634 = m400214,4 = 2e736f2e = m400218,4 = 3200 = m7f007d6d3c40,4 = 0100 = vCont;t:p95b.-1 = OK = m7f007d201f40,1 = 48 = m7f007d201f40,1 = 48 = g = fcfd90ad5329ff7f00... = m7f007d201f40,1 = 48 = m7f007d201f40,1 = 48 = m40056c,12 = 554889e5e8e3fe89c6ba0700bfdc = m40056c,1 = 55 = m40056d,3 = 4889e5 = m40056c,12 = 554889e5e8e3fe89c6ba0700bfdc = m40056c,1 = 55 = m40056d,3 = 4889e5 = m4005ba,1 = e8 = m4005ba,1 = e8 (gdb) b BP.c:13 Breakpoint 1 at 0x4005ba: file BP.c,
Re: gdbstub initial code, v9
On 09/09, Oleg Nesterov wrote: the tracee hits this bp and reports SIGTRAP = vStopped = OK = g = 00064000401f207d007f00... = P10=ba054000 = = G00064000401f207d007f0... = May be this can explain... Probably I need to implement G/P first, otherwise gdb can't change ip. Still, I'd appreciate if someone can explain me what gdb needs/expects to handle breakpoints before I start to read the sources. Oleg.
Re: gdbstub initial code, v9
Hi - On Thu, Sep 09, 2010 at 05:50:47PM +0200, Oleg Nesterov wrote: Probably I need to implement G/P first, otherwise gdb can't change ip. Perhaps. Still, I'd appreciate if someone can explain me what gdb needs/expects to handle breakpoints before I start to read the sources. It'd be simpler if the normal all-stop mode was what you first focused on. That mode works fine with Z/z and with M/m based breakpoint insertion/removal and c/s continue/singlestep. (This stuff was all working in the earlier gdbstub code.) Re. non-stop mode, see http://www.codesourcery.com/publications/non_stop_multi_threaded_debugging_in_gdb.pdf - FChE
Re: gdbstub initial code, v9
On Thu, 09 Sep 2010 18:30:31 +0200, Oleg Nesterov wrote: OOPS! indeed, unhex() confuses lo and hi. It works for 0xcc, though. Cough... could you tell me how can I change the variable done without printing it? (gdb) help set variable Evaluate expression EXP and assign result to variable VAR, using assignment syntax appropriate for the current language (VAR = EXP or VAR := EXP for example). VAR may be a debugger convenience variable (names starting with $), a register (a few standard names starting with $), or an actual variable in the program being debugged. EXP is any valid expression. This may usually be abbreviated to simply set. Regards, Jan
Re: gdbstub initial code, v9
Oleg == Oleg Nesterov o...@redhat.com writes: Oleg (gdb) set var 0 You need: set variable var = 0 The variable can be abbreviated. FWIW, print, set variable, and call are basically aliases. print just happens to print the result of the expression. Tom
gdbstub initial code, v9
Changes:
- partly fix the multitracing problems.
ugdb still can't work with ptrace, ptrace-utrace.c needs
changes. But at least multiple udgb tracers can coexist,
more or less.
But of course they can confuse each other anyway, no matter
what ugdb does.
- implement memory writes ($M).
- refactor memory reads to avoid the Too late to report the
error case.
But, Jan. Implementing the memory writes does not mean breakpoints
automatically start to work!
Yes, gdb writes cc, and yes the tracee reports SIGTRAP. But after
that continue does nothing except $c, and the tracee naturally
gets SIGILL. I expected that, since ugdb doesn't even know the code
was changed, gdb should write the original byte back before continue,
but this doesn't happen.
Tried to understand how this works with gdbserver, but failed so far.
Will continue, but any hint is very much appreciated ;)
Oleg.
#include linux/module.h
#include linux/proc_fs.h
#include linux/utrace.h
#include linux/poll.h
#include linux/mm.h
#include linux/regset.h
#include asm/uaccess.h
static int o_remote_debug;
module_param_named(echo, o_remote_debug, bool, 0);
#define BUFFER_SIZE 1024
#define PACKET_SIZE 1024
struct pbuf {
char*cur, *pkt;
charbuf[BUFFER_SIZE];
};
static inline void pb_init(struct pbuf *pb)
{
pb-cur = pb-buf;
pb-pkt = NULL;
}
enum {
U_STOP_IDLE = 0,
U_STOP_PENDING,
U_STOP_SENT,
};
struct ugdb {
struct list_headu_processes;
struct list_headu_stopped;
int u_stop_state;
struct mutexu_mutex;
spinlock_t u_slock;
struct ugdb_thread
*u_cur_tinfo,
*u_cur_hg,
*u_cur_hc;
wait_queue_head_t u_wait;
int u_err;
struct pbuf u_pbuf;
charu_cbuf[PACKET_SIZE];
int u_clen;
sigset_tu_sig_ign;
unsigned int
u_no_ack:1,
u_allstop:1;
};
static inline void ugdb_ck_stopped(struct ugdb *ugdb)
{
spin_lock(ugdb-u_slock);
WARN_ON(!list_empty(ugdb-u_stopped)
ugdb-u_stop_state == U_STOP_IDLE);
WARN_ON(list_empty(ugdb-u_stopped)
ugdb-u_stop_state == U_STOP_PENDING);
spin_unlock(ugdb-u_slock);
}
static struct ugdb *ugdb_create(void)
{
struct ugdb *ugdb;
int err;
err = -ENODEV;
// XXX: ugly. proc_reg_open() should take care.
if (!try_module_get(THIS_MODULE))
goto out;
err = -ENOMEM;
ugdb = kzalloc(sizeof(*ugdb), GFP_KERNEL);
if (!ugdb)
goto put_module;
INIT_LIST_HEAD(ugdb-u_processes);
INIT_LIST_HEAD(ugdb-u_stopped);
mutex_init(ugdb-u_mutex);
spin_lock_init(ugdb-u_slock);
init_waitqueue_head(ugdb-u_wait);
pb_init(ugdb-u_pbuf);
return ugdb;
put_module:
module_put(THIS_MODULE);
out:
return ERR_PTR(err);
}
#define P_DETACHING (1 1)
#define P_ZOMBIE(1 2)
struct ugdb_process {
int p_pid;
int p_state;
struct list_headp_threads;
struct ugdb *p_ugdb;
struct list_headp_processes;
};
static inline bool process_alive(struct ugdb_process *process)
{
return !(process-p_state P_ZOMBIE);
}
static inline void mark_process_dead(struct ugdb_process *process)
{
process-p_state |= P_ZOMBIE;
}
static struct ugdb_process *ugdb_create_process(struct ugdb *ugdb, int pid_nr)
{
struct ugdb_process *process;
process = kzalloc(sizeof(*process), GFP_KERNEL);
if (!process)
return NULL;
process-p_pid = pid_nr;
process-p_ugdb = ugdb;
INIT_LIST_HEAD(process-p_threads);
list_add_tail(process-p_processes, ugdb-u_processes);
return process;
}
#define T_STOP_RUN 0
#define T_STOP_REQ (1 0)/* requested by gdb */
#define T_STOP_ALL (1 1)/* vCont;c:pX.-1, for report_clone */
#define T_STOP_ACK (1 2)/* visible to vStopped */
#define T_STOP_STOPPED (1 3)/* reported as stopped to gdb */
/* TASK_TRACED + deactivated ? */
#define T_EV_NONE 0
#define T_EV_EXIT (1 24)
#define T_EV_SIGN (2 24)
#define T_EV_TYPE(event)((0xff 24) (event))
#define T_EV_DATA(event)(~(0xff 24) (event))
struct ugdb_thread {
int t_tid;
int
Re: gdbstub initial code, v8
On 09/06, Frank Ch. Eigler wrote: Oleg Nesterov o...@redhat.com writes: [...] 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.
Re: gdbstub initial code, v8
On 09/05, Jan Kratochvil wrote: On Sat, 04 Sep 2010 00:40:47 +0200, Oleg Nesterov wrote: - 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. (also to put in breakpoints): 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.
Re: gdbstub initial code, v8
On 09/06, Jan Kratochvil wrote: On Mon, 06 Sep 2010 20:18:08 +0200, Oleg Nesterov wrote: On 09/05, Jan Kratochvil wrote: (also to put in breakpoints): 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. 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. */ 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. 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.
Re: gdbstub initial code, v7
On Thu, 02 Sep 2010 22:06:32 +0200, Oleg Nesterov wrote: I assume that qXfer:siginfo:read always mean Hg thread. It seems so. It is not clear to me what should ugdb report if there is no a valid siginfo. linux_xfer_siginfo() return E01, but gdbserver uses SIGSTOP to stop the tracee, I find error more appropriate in such case. Likewise, it is not clear what should ugdb do if gdb sends $CSIG in this case. Currently GDB does not do anything special, that is if there is siginfo for signal SIGUSR1 but one does $C0B (SIGSEGV) does ptrace reset the siginfo or is left the SIGUSR1 siginfo for SIGSEGV? But this all is minor, I think. As this is being discussed for GDB I would find enough to just make $_siginfo accessible without these details. Thanks, Jan
Re: gdbstub initial code, v7
Oleg Nesterov o...@redhat.com writes: [...] To the point, I had to add printk's to utrace.c to understand what is wrong. Hopefully tomorrow. You might wish to try out systemtap for such purposes. It's easy to insert printk-like tracing at almost any point; monitor variables for changes, pretty-print structs, etc. No recompilation/reboot. - FChE
Re: gdbstub initial code, v7
Currently GDB does not do anything special, that is if there is siginfo for signal SIGUSR1 but one does $C0B (SIGSEGV) does ptrace reset the siginfo or is left the SIGUSR1 siginfo for SIGSEGV? The kernel considers this sloppy behavior on the debugger's part. If you inject a different signal, we expect you should PTRACE_SETSIGINFO to something appropriate, or else that you really didn't care about the bits being accurate. If the resumption signal does not match the siginfo_t.si_signo, then the kernel resets the siginfo as if the debugger had just used kill with the new signal (i.e. si_pid, si_uid point to the ptracer). Thanks, Roland
Re: gdbstub initial code, v7
On Fri, 03 Sep 2010 21:59:06 +0200, Roland McGrath wrote: Currently GDB does not do anything special, that is if there is siginfo for signal SIGUSR1 but one does $C0B (SIGSEGV) does ptrace reset the siginfo or is left the SIGUSR1 siginfo for SIGSEGV? The kernel considers this sloppy behavior on the debugger's part. If you inject a different signal, we expect you should PTRACE_SETSIGINFO to something appropriate, or else that you really didn't care about the bits being accurate. If the resumption signal does not match the siginfo_t.si_signo, then the kernel resets the siginfo as if the debugger had just used kill with the new signal (i.e. si_pid, si_uid point to the ptracer). OK, that seems to me as the best choice. Sorry I did not test/read it. Thanks, Jan
Re: gdbstub initial code, v7
Sorry for the delay, I was distracted. Trying to switch back to ugdb. On 08/31, Jan Kratochvil wrote: ugdb should support qXfer:siginfo, currently accessible only via $_siginfo print/set, though. Still sure this feature should be also implemented one day. Yes sure. This should be simple, although I didn't expect qXfer needs remote_escape_output() and x86_siginfo_fixup(). I assume that qXfer:siginfo:read always mean Hg thread. It is not clear to me what should ugdb report if there is no a valid siginfo. linux_xfer_siginfo() return E01, but gdbserver uses SIGSTOP to stop the tracee, so it always has something to report. But ugdb stop the tracee somewhere else, not in get_signal_to_deliver() path. Likewise, it is not clear what should ugdb do if gdb sends $CSIG in this case. But this all is minor, I think. I was going to send v8 which implements qXfer:siginfo:read and continue with signal, but (oh, as always) hit the unexpected problems. To the point, I had to add printk's to utrace.c to understand what is wrong. Hopefully tomorrow. Oleg.
Re: gdbstub initial code, v7
On Mon, 30 Aug 2010 21:20:40 +0200, Jan Kratochvil wrote: On Mon, 30 Aug 2010 20:58:50 +0200, Oleg Nesterov wrote: - report signals. A bit more code changes than I expected. BTW not sure if it is already the right time for it but to keep ugdb on-par with my linux-nat's re-post today (still not accepted in FSF GDB) That's not true, this functionality needs no gdb/remote.c changes and its correctnes relies just on ugdb (and it is probably not a problem for ugdb). ugdb should support qXfer:siginfo, currently accessible only via $_siginfo print/set, though. Still sure this feature should be also implemented one day. Thanks, Jan
gdbstub initial code, v7
Changes:
- report signals. A bit more code changes than I expected.
- implement QPassSignals, trivial.
Note: $CSIG is not supported yet, and I am not sure I understand
how it should work. Next time...
#include linux/module.h
#include linux/proc_fs.h
#include linux/utrace.h
#include linux/poll.h
#include linux/mm.h
#include linux/regset.h
#include asm/uaccess.h
static int o_remote_debug;
module_param_named(echo, o_remote_debug, bool, 0);
#define BUFFER_SIZE 1024
#define PACKET_SIZE 1024
struct pbuf {
char*cur, *pkt;
charbuf[BUFFER_SIZE];
};
static inline void pb_init(struct pbuf *pb)
{
pb-cur = pb-buf;
pb-pkt = NULL;
}
enum {
U_STOP_IDLE = 0,
U_STOP_PENDING,
U_STOP_SENT,
};
struct ugdb {
struct list_headu_processes;
struct list_headu_stopped;
int u_stop_state;
struct mutexu_mutex;
spinlock_t u_slock;
struct ugdb_thread
*u_cur_tinfo,
*u_cur_hg,
*u_cur_hc;
wait_queue_head_t u_wait;
int u_err;
struct pbuf u_pbuf;
charu_cbuf[PACKET_SIZE];
int u_clen;
sigset_tu_sig_ign;
unsigned int
u_no_ack:1,
u_allstop:1;
};
static inline void ugdb_ck_stopped(struct ugdb *ugdb)
{
spin_lock(ugdb-u_slock);
WARN_ON(!list_empty(ugdb-u_stopped)
ugdb-u_stop_state == U_STOP_IDLE);
WARN_ON(list_empty(ugdb-u_stopped)
ugdb-u_stop_state == U_STOP_PENDING);
spin_unlock(ugdb-u_slock);
}
static struct ugdb *ugdb_create(void)
{
struct ugdb *ugdb;
int err;
err = -ENODEV;
// XXX: ugly. proc_reg_open() should take care.
if (!try_module_get(THIS_MODULE))
goto out;
err = -ENOMEM;
ugdb = kzalloc(sizeof(*ugdb), GFP_KERNEL);
if (!ugdb)
goto put_module;
INIT_LIST_HEAD(ugdb-u_processes);
INIT_LIST_HEAD(ugdb-u_stopped);
mutex_init(ugdb-u_mutex);
spin_lock_init(ugdb-u_slock);
init_waitqueue_head(ugdb-u_wait);
pb_init(ugdb-u_pbuf);
return ugdb;
put_module:
module_put(THIS_MODULE);
out:
return ERR_PTR(err);
}
#define P_DETACHING (1 1)
#define P_ZOMBIE(1 2)
struct ugdb_process {
int p_pid;
int p_state;
struct list_headp_threads;
struct ugdb *p_ugdb;
struct list_headp_processes;
};
static inline bool process_alive(struct ugdb_process *process)
{
return !(process-p_state P_ZOMBIE);
}
static inline void mark_process_dead(struct ugdb_process *process)
{
process-p_state |= P_ZOMBIE;
}
static struct ugdb_process *ugdb_create_process(struct ugdb *ugdb, int pid_nr)
{
struct ugdb_process *process;
process = kzalloc(sizeof(*process), GFP_KERNEL);
if (!process)
return NULL;
process-p_pid = pid_nr;
process-p_ugdb = ugdb;
INIT_LIST_HEAD(process-p_threads);
list_add_tail(process-p_processes, ugdb-u_processes);
return process;
}
#define T_STOP_RUN 0
#define T_STOP_REQ (1 0)/* requested by gdb */
#define T_STOP_ALL (1 1)/* vCont;c:pX.-1, for report_clone */
#define T_STOP_ACK (1 2)/* visible to vStopped */
#define T_STOP_STOPPED (1 3)/* reported as stopped to gdb */
/* TASK_TRACED + deactivated ? */
#define T_EV_NONE 0
#define T_EV_EXIT (1 24)
#define T_EV_SIGN (2 24)
#define T_EV_TYPE(event)((0xff 24) (event))
#define T_EV_DATA(event)(~(0xff 24) (event))
struct ugdb_thread {
int t_tid;
int t_stop_state;
int t_stop_event;
struct ugdb *t_ugdb;
struct ugdb_process *t_process;
struct list_headt_threads;
struct list_headt_stopped;
struct pid *t_spid;
struct utrace_engine*t_engine;
};
static inline bool thread_alive(struct ugdb_thread *thread)
{
WARN_ON((thread-t_tid != 0) != process_alive(thread-t_process));
return thread-t_tid != 0;
}
static inline void mark_thread_dead(struct ugdb_thread *thread)
{
mark_process_dead(thread-t_process);
thread-t_tid = 0;
}
/*
* The thread should be alive, and it can't pass ugdb_report_death()
* if the caller holds ugdb-u_mutex. However
Re: gdbstub initial code, v7
On Mon, 30 Aug 2010 20:58:50 +0200, Oleg Nesterov wrote: - report signals. A bit more code changes than I expected. BTW not sure if it is already the right time for it but to keep ugdb on-par with my linux-nat's re-post today (still not accepted in FSF GDB) [0/9]#2 Fix lost siginfo_t http://sourceware.org/ml/gdb-patches/2010-08/msg00480.html ugdb should support qXfer:siginfo, currently accessible only via $_siginfo print/set, though. Thanks, Jan
gdbstub initial code, v6
Changes:
- adapt to no more utrace changes. Whatever I think
about utrace I have to agree, it is not practical to
change utrace now
- attach to the thread-group with the dead leader
- fix the double-attaching bug
- do not assume we can trust pid_task(), the tracee can
be reaped even if it can't pass -report_death()
- size_t is unsigned! -EAGAIN from -read() makes this
fd unusable from rw_verify_area() pov
Next: report signals.
#include linux/module.h
#include linux/proc_fs.h
#include linux/utrace.h
#include linux/poll.h
#include linux/mm.h
#include linux/regset.h
#include asm/uaccess.h
static int o_remote_debug;
module_param_named(echo, o_remote_debug, bool, 0);
#define BUFFER_SIZE 1024
#define PACKET_SIZE 1024
struct pbuf {
char*cur, *pkt;
charbuf[BUFFER_SIZE];
};
static inline void pb_init(struct pbuf *pb)
{
pb-cur = pb-buf;
pb-pkt = NULL;
}
enum {
U_STOP_IDLE = 0,
U_STOP_PENDING,
U_STOP_SENT,
};
struct ugdb {
struct list_headu_processes;
struct list_headu_stopped;
int u_stop_state;
struct mutexu_mutex;
spinlock_t u_slock;
struct ugdb_thread
*u_cur_tinfo,
*u_cur_hg,
*u_cur_hc;
wait_queue_head_t u_wait;
int u_err;
struct pbuf u_pbuf;
charu_cbuf[PACKET_SIZE];
int u_clen;
unsigned int
u_no_ack:1,
u_allstop:1;
};
static inline void ugdb_ck_stopped(struct ugdb *ugdb)
{
spin_lock(ugdb-u_slock);
WARN_ON(!list_empty(ugdb-u_stopped)
ugdb-u_stop_state == U_STOP_IDLE);
WARN_ON(list_empty(ugdb-u_stopped)
ugdb-u_stop_state == U_STOP_PENDING);
spin_unlock(ugdb-u_slock);
}
static struct ugdb *ugdb_create(void)
{
struct ugdb *ugdb;
int err;
err = -ENODEV;
// XXX: ugly. proc_reg_open() should take care.
if (!try_module_get(THIS_MODULE))
goto out;
err = -ENOMEM;
ugdb = kzalloc(sizeof(*ugdb), GFP_KERNEL);
if (!ugdb)
goto put_module;
INIT_LIST_HEAD(ugdb-u_processes);
INIT_LIST_HEAD(ugdb-u_stopped);
mutex_init(ugdb-u_mutex);
spin_lock_init(ugdb-u_slock);
init_waitqueue_head(ugdb-u_wait);
pb_init(ugdb-u_pbuf);
return ugdb;
put_module:
module_put(THIS_MODULE);
out:
return ERR_PTR(err);
}
#define P_DETACHING (1 1)
#define P_ZOMBIE(1 2)
struct ugdb_process {
int p_pid;
int p_state;
struct list_headp_threads;
struct ugdb *p_ugdb;
struct list_headp_processes;
};
static inline bool process_alive(struct ugdb_process *process)
{
return !(process-p_state P_ZOMBIE);
}
static inline void mark_process_dead(struct ugdb_process *process)
{
process-p_state |= P_ZOMBIE;
}
static struct ugdb_process *ugdb_create_process(struct ugdb *ugdb, int pid_nr)
{
struct ugdb_process *process;
process = kzalloc(sizeof(*process), GFP_KERNEL);
if (!process)
return NULL;
process-p_pid = pid_nr;
process-p_ugdb = ugdb;
INIT_LIST_HEAD(process-p_threads);
list_add_tail(process-p_processes, ugdb-u_processes);
return process;
}
#define T_STOP_RUN 0
#define T_STOP_REQ (1 0)/* requested by gdb */
#define T_STOP_ALL (1 1)/* vCont;c:pX.-1, for report_clone */
#define T_STOP_ACK (1 2)/* visible to vStopped */
#define T_STOP_STOPPED (1 3)/* reported as stopped to gdb */
/* TASK_TRACED + deactivated ? */
struct ugdb_thread {
int t_tid;
int t_stop_state;
int t_stop_code;
struct ugdb *t_ugdb;
struct ugdb_process *t_process;
struct list_headt_threads;
struct list_headt_stopped;
struct pid *t_spid;
struct utrace_engine*t_engine;
};
static inline bool thread_alive(struct ugdb_thread *thread)
{
WARN_ON((thread-t_tid != 0) != process_alive(thread-t_process));
return thread-t_tid != 0;
}
static inline void mark_thread_dead(struct ugdb_thread *thread)
{
mark_process_dead(thread-t_process);
thread-t_tid = 0;
}
/*
* The thread should be alive, and it can't pass ugdb_report_death()
* if the caller
Re: gdbstub initial code, v5
On 08/23, Oleg Nesterov wrote: However. I spent all Monday trying to resolve the new bug, and so far I do not understand what happens. Extremely hard to reproduce, and the kernel just hangs silently, without any message. So far I suspect the proble in utrace.c, but this time I am not sure. Solved. This was scheduler bug fixed in 2.6.35, but I used 2.6.34. This is really funny. This bug (PF_STARTING lockup) was found and fixed by me Peter. Oh. But I hit yet another problem, BUG_ON() in __utrace_engine_release(). Again, it is not reproducible, I saw it only once in dmesg and I do not even know for sure what I was doing. I'll contiue tomorrow, but if I won't be able to quickly resolve this problem I am going to ignore it for now. This time I think ugdb is wrong. Oleg.
Re: gdbstub initial code, v5
When the main thread exits, gdbserver still exposes it to gdb as a running process. It is visible via info threads, you can switch to this thread, $Tp or $Hx result in OK as if this thread is alive. gdbserver even pretends that $vCont;x:DEAD_THEAD works, although this thread obviously can never report something. This is sort of consistent with the kernel treatment. The main thread stays around as a zombie, acting as a moniker for the whole process. But indeed that is not actually useful for any thread-granularity control or information (well, there is the dead thread's usage stats, but that's all). I don't think this is really right. This just confuses the user, and imho this should be considered like the minor bug. I tend to agree, but don't think it's a big issue either way, really. ugdb doesn't do this. If the main thread exits - it exits like any other thread. I played with gdb, it seems to handle this case fine. Sounds good to me! - The exit code (Wxx) can be wrong in mt-case. The problem is, -report_death can't safely access -group_exit_code with kernel 2.6.35. This is solveable. Don't even worry about it. If there is something trivial to do that makes it better for earlier kernels, then go ahead. But if the easy thing to do gives correct results on =2.6.35 and racily wrong or random results on older kernels, then we can just live with that. Thanks, Roland
Re: gdbstub initial code, v5
Just a small report to explain what I am doing... On 08/20, Oleg Nesterov wrote: - I forgot to implement the attach to the thread group with the dead leader. Next time. Almost done, but we should avoid the races with exec somehow. But this is minor. I tried to test this code as much as I can. Again, I do not use gdb at all, I am using the scripts which try to really stress ugdb. Found 2 bugs in ugdb.ko, the second one is not nice but at least I have the temporary fix. However. I spent all Monday trying to resolve the new bug, and so far I do not understand what happens. Extremely hard to reproduce, and the kernel just hangs silently, without any message. So far I suspect the proble in utrace.c, but this time I am not sure. Will continue tomorrow... Oleg.
gdbstub initial code, v5
On 08/19, Oleg Nesterov wrote:
Next step: handle exit correctly and report W/S. I misunderstood
what gdbserver does when the main thread exits, it is not stupid
as I wrongly thought.
Yes, in non-stop mode gdbserver reports W/X;process:PID when the
last thread exits. This makes sense, so does ugdb.
But,
==
All, please ack/nack the behavioral difference!
When the main thread exits, gdbserver still exposes it to gdb as
a running process. It is visible via info threads, you can switch
to this thread, $Tp or $Hx result in OK as if this thread is alive.
gdbserver even pretends that $vCont;x:DEAD_THEAD works, although
this thread obviously can never report something.
I don't think this is really right. This just confuses the user, and
imho this should be considered like the minor bug.
ugdb doesn't do this. If the main thread exits - it exits like any
other thread. I played with gdb, it seems to handle this case fine.
==
Problems:
- I forgot to implement the attach to the thread group
with the dead leader. Next time.
- The exit code (Wxx) can be wrong in mt-case.
The problem is, -report_death can't safely access
-group_exit_code with kernel 2.6.35. This is
solveable.
Roland, sorry, I ignored your emails for today. It is not easy to me
to switch between ugdb an utrace ;)
Oleg.
#include linux/module.h
#include linux/proc_fs.h
#include linux/utrace.h
#include linux/poll.h
#include linux/mm.h
#include linux/regset.h
#include asm/uaccess.h
static int o_remote_debug;
module_param_named(echo, o_remote_debug, bool, 0);
#define BUFFER_SIZE 1024
#define PACKET_SIZE 1024
struct pbuf {
char*cur, *pkt;
charbuf[BUFFER_SIZE];
};
static inline void pb_init(struct pbuf *pb)
{
pb-cur = pb-buf;
pb-pkt = NULL;
}
enum {
U_STOP_IDLE = 0,
U_STOP_PENDING,
U_STOP_SENT,
};
struct ugdb {
struct list_headu_processes;
struct list_headu_stopped;
int u_stop_state;
struct mutexu_mutex;
spinlock_t u_slock;
struct ugdb_thread
*u_cur_tinfo,
*u_cur_hg,
*u_cur_hc;
wait_queue_head_t u_wait;
int u_err;
struct pbuf u_pbuf;
charu_cbuf[PACKET_SIZE];
int u_clen;
unsigned int
u_no_ack:1,
u_allstop:1;
};
static inline void ugdb_ck_stopped(struct ugdb *ugdb)
{
// XXX: temporary racy check
WARN_ON(!list_empty(ugdb-u_stopped)
ugdb-u_stop_state == U_STOP_IDLE);
WARN_ON(list_empty(ugdb-u_stopped)
ugdb-u_stop_state == U_STOP_PENDING);
}
static struct ugdb *ugdb_create(void)
{
struct ugdb *ugdb;
int err;
err = -ENODEV;
// XXX: ugly. proc_reg_open() should take care.
if (!try_module_get(THIS_MODULE))
goto out;
err = -ENOMEM;
ugdb = kzalloc(sizeof(*ugdb), GFP_KERNEL);
if (!ugdb)
goto put_module;
INIT_LIST_HEAD(ugdb-u_processes);
INIT_LIST_HEAD(ugdb-u_stopped);
mutex_init(ugdb-u_mutex);
spin_lock_init(ugdb-u_slock);
init_waitqueue_head(ugdb-u_wait);
pb_init(ugdb-u_pbuf);
return ugdb;
put_module:
module_put(THIS_MODULE);
out:
return ERR_PTR(err);
}
#define P_DETACHING (1 1)
#define P_ZOMBIE(1 2)
struct ugdb_process {
int p_pid;
int p_state;
struct list_headp_threads;
struct ugdb *p_ugdb;
struct list_headp_processes;
};
static inline bool process_alive(struct ugdb_process *process)
{
return !(process-p_state P_ZOMBIE);
}
static inline void mark_process_dead(struct ugdb_process *process)
{
process-p_state |= P_ZOMBIE;
}
static struct ugdb_process *ugdb_create_process(struct ugdb *ugdb, int pid)
{
struct ugdb_process *process;
process = kzalloc(sizeof(*process), GFP_KERNEL);
if (!process)
return NULL;
process-p_pid = pid;
process-p_ugdb = ugdb;
INIT_LIST_HEAD(process-p_threads);
list_add_tail(process-p_processes, ugdb-u_processes);
return process;
}
#define T_STOP_RUN 0
#define T_STOP_REQ (1 0)/* requested by gdb */
#define T_STOP_ALL (1 1)/* vCont;c:pX.-1, for report_clone */
#define T_STOP_ACK (1 2)/* visible to vStopped */
#define T_STOP_STOPPED (1
Re: gdbstub initial code, v4
On 08/19, Roland McGrath wrote: Note the second attachment, GDBCAT. It is just the simple perl script which connects /proc/ugdb to tcp port. It can be used for remote debugging via tcp, or with (unpatched) gdb which can't do select() on /proc/ugdb. bash$ nc -l 2000 /proc/ugdb Yes, this works, Actually, it is more convenient to use it in any case, at least for logging purposes. (gdb) set remote debug 1 set debug remote 1. Yes, I tried this. Very, very inconvenient. In this case the debugging output is mixed with the normal output, Also: you can't save to file, can't filter packets, can't add the packet for debugging (not implemented yet). But yes, it is not strictly necessary, nc should work too. Oleg.
Re: gdbstub initial code, v4
Note the second attachment, GDBCAT. It is just the simple perl script which connects /proc/ugdb to tcp port. It can be used for remote debugging via tcp, or with (unpatched) gdb which can't do select() on /proc/ugdb. bash$ nc -l 2000 /proc/ugdb Actually, it is more convenient to use it in any case, at least for logging purposes. (gdb) set remote debug 1 Thanks, Roland
Re: problems with v3 (Was: gdbstub initial code, v3)
I seem to understand the problem(s). I am a bit surprized. Basically I have the questions about almost every utrace_ function. I'll try to recheck and summarize my concerns tomorrow with a fresh head, I hope I missed something. Ok. That stuff about pure kernel implementation issues doesn't need to go to the archer list and Tom, only to utrace-devel. Thanks, Roland
Re: gdbstub initial code, v3
oleg wrote: [...] - It doesn't support all-stop mode. Please tell me if this is needed. I hope not, this needs a lot of nasty complications :/ [...] As opposed to non-stop mode? I'm pretty sure all-stop will be needed as that is the common default gdb usage model. - FChE
problems with v3 (Was: gdbstub initial code, v3)
On 08/11, Tom Tromey wrote: Oleg == Oleg Nesterov o...@redhat.com writes: Oleg So, the patch below fixes the problem, and gdb + /proc/ugdb seems Oleg to work. Oleg Indeed, gdb sees that this fd is not pipe/tcp and uses the hardwire Oleg serial_ops, but hardwire_readchar() doesn't play well with select(). Oleg Please teach gdb to use poll/select ? I looked at this a little bit. It seems to me that the hardwire stuff is for talking to ttys, and we instead want gdb to be using the pipe code. I didn't verify this, but I don't think so. Please look at pipe_open(). Perhaps it makes sense to serial_add_interface(ugdb), I dunno. OK. I was going to add some cleanups and send the new version today. But after some testing (without gdb) I hit the kernel crashes. This makes me think that probably something is wrong ;) As usual, I can't blame my code. I am still investigating, the crash is not easy to reproduce, but so far I _suspect_ the problems in utrace code. At least utrace_barrier()-signal_pending() is definitely not right. Will continue tomorrow. Oleg.
Re: gdbstub initial code, v3
Indeed, gdb sees that this fd is not pipe/tcp and uses the hardwire serial_ops, but hardwire_readchar() doesn't play well with select(). Please teach gdb to use poll/select ? If it makes it easier, could use: bash$ nc -l -U /tmp/socket /proc/ugdb (gdb) target remote |nc -U /tmp/socket for the moment. Silly of course, but just not to be blocked on cleaning up gdb's serial-device handling to be more nicely nonblocking. Thanks, Roland
gdbstub initial code, v3
Please see the attached ugdb.c.
It supports multiple inferiors/threads, stop/cont, clone/exit.
It doesn't report W/X when the process exits yet, but this is
only because I'd like to discuss the current problems with the
exited leader first, then implement this as a separate change.
Any code review is very much appreciated.
Problems:
- It doesn't support all-stop mode.
Please tell me if this is needed. I hope not, this needs
a lot of nasty complications :/
- We should discuss utrace limitations: UTRACE_RESUME
races/problems, utrace_prepare_examine() issues.
- I just finished this code, and it doesn't work with gdb ;)
I will investigate tomorrow, but I am almost sure gdb is
wrong.
Oleg.
#include linux/module.h
#include linux/proc_fs.h
#include linux/utrace.h
#include linux/poll.h
#include linux/mm.h
#include linux/regset.h
#include asm/uaccess.h
static int o_remote_debug;
module_param_named(echo, o_remote_debug, bool, 0);
#define BUFFER_SIZE 1024
#define PACKET_SIZE 1024
struct pbuf {
char*cur, *pkt;
charbuf[BUFFER_SIZE];
};
static inline void pb_init(struct pbuf *pb)
{
pb-cur = pb-buf;
pb-pkt = NULL;
}
enum {
U_STOP_IDLE = 0,
U_STOP_PENDING,
U_STOP_SENT,
};
struct ugdb {
struct list_headu_processes;
struct list_headu_stopped;
int u_stop_state;
struct mutexu_mutex;
spinlock_t u_slock;
struct ugdb_thread
*u_cur_tinfo,
*u_cur_hg,
*u_cur_hc;
wait_queue_head_t u_wait;
int u_err;
struct pbuf u_pbuf;
charu_cbuf[PACKET_SIZE];
int u_clen;
unsigned int
u_no_ack:1,
u_allstop:1;
};
static inline void ugdb_ck_stopped(struct ugdb *ugdb)
{
// XXX: temporary racy check
WARN_ON(!list_empty(ugdb-u_stopped)
ugdb-u_stop_state == U_STOP_IDLE);
WARN_ON(list_empty(ugdb-u_stopped)
ugdb-u_stop_state == U_STOP_PENDING);
}
static struct ugdb *ugdb_create(void)
{
struct ugdb *ugdb;
int err;
err = -ENODEV;
// XXX: ugly. proc_reg_open() should take care.
if (!try_module_get(THIS_MODULE))
goto out;
err = -ENOMEM;
ugdb = kzalloc(sizeof(*ugdb), GFP_KERNEL);
if (!ugdb)
goto put_module;
INIT_LIST_HEAD(ugdb-u_processes);
INIT_LIST_HEAD(ugdb-u_stopped);
mutex_init(ugdb-u_mutex);
spin_lock_init(ugdb-u_slock);
init_waitqueue_head(ugdb-u_wait);
pb_init(ugdb-u_pbuf);
return ugdb;
put_module:
module_put(THIS_MODULE);
out:
return ERR_PTR(err);
}
#define P_DETACHING (1 1)
struct ugdb_process {
int p_pid;
int p_state;
struct list_headp_threads;
struct ugdb *p_ugdb;
struct list_headp_processes;
};
static struct ugdb_process *ugdb_create_process(struct ugdb *ugdb, int pid)
{
struct ugdb_process *process;
process = kzalloc(sizeof(*process), GFP_KERNEL);
if (!process)
return NULL;
process-p_pid = pid;
process-p_ugdb = ugdb;
INIT_LIST_HEAD(process-p_threads);
list_add_tail(process-p_processes, ugdb-u_processes);
return process;
}
#define T_STOP_RUN 0
#define T_STOP_REQ (1 0)/* requested by gdb */
#define T_STOP_ALL (1 1)/* vCont;c:pX.-1, for report_clone */
#define T_STOP_ACK (1 2)/* visible to vStopped */
#define T_STOP_STOPPED (1 3)/* reported as stopped to gdb */
/* TASK_TRACED + deactivated ? */
struct ugdb_thread {
int t_tid;
int t_stop_state;
struct ugdb *t_ugdb;
struct ugdb_process *t_process;
struct list_headt_threads;
struct list_headt_stopped;
struct pid *t_spid;
// create/attach border
struct utrace_engine*t_engine;
};
static inline struct task_struct *thread_to_task(struct ugdb_thread *thread)
{
return pid_task(thread-t_spid, PIDTYPE_PID);
}
static struct ugdb_thread *ugdb_create_thread(struct ugdb_process *process,
struct pid *spid)
{
struct ugdb_thread *thread;
thread = kzalloc(sizeof(*thread), GFP_KERNEL);
if (!thread)
return NULL;
thread-t_tid =
Re: gdbstub initial code, another approach
On 07/28, Oleg Nesterov wrote: - currently it only supports attach, stop, cont, detach and exit. OK, I am a bit stuck. I am trying to implement attach-to-the-thread-group, and I'd like to invent something simple without O(n**2) and semaphores in -report_clone(). There are other problems with process-wide ops which should be addressed somehow. Will continue tomorrow... Oleg.
Re: gdbstub initial code, another approach
On Wed, 28 Jul 2010 20:17:02 +0200, Oleg Nesterov wrote: - the testing was very limited. I played with it about an hour and didn't find any problems, vut that is all. [...] Btw, gdb crashes very often right after (gdb) set target-async on (gdb) set non-stop (gdb) file mt-program (gdb) target extended-remote :port (gdb) attach its_pid I didn't even try to investigate (this doesn't happen when it works with the real gdbserver). Just retry, gdb is buggy. Trying it with both /bin/sleep and a threaded testcase and I never got a crash (kernel-2.6.33.6-147.fc13.x86_64 as both host and KVM guest OS). $ killall gdbstub;~/redhat/threaditp=$!;~/redhat/gdbstub ~/redhat/outsleep 0.1;./gdb -nx -ex 'set target-async on' -ex 'set non-stop' -ex file $HOME/redhat/threadit -ex 'target extended-remote :2000' -ex attach $p -ex 'set confirm no';kill $p; gdbstub: no process killed [6] 22822 [7] 22823 GNU gdb (GDB) 7.2.50.20100802-cvs Copyright (C) 2010 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type show copying and show warranty for details. This GDB was configured as x86_64-unknown-linux-gnu. For bug reporting instructions, please see: http://www.gnu.org/software/gdb/bugs/. Reading symbols from /home/jkratoch/redhat/threadit...done. Remote debugging using :2000 Attached to process 22822 [New Thread 22822.22822] [New Thread 22822.22825] Reading symbols from /lib64/libpthread.so.0...Reading symbols from /usr/lib/debug/lib64/libpthread-2.12.so.debug...done. done. Loaded symbols for /lib64/libpthread.so.0 Reading symbols from /lib64/libc.so.6...Reading symbols from /usr/lib/debug/lib64/libc-2.12.so.debug...done. done. Loaded symbols for /lib64/libc.so.6 Reading symbols from /lib64/ld-linux-x86-64.so.2...Reading symbols from /usr/lib/debug/lib64/ld-2.12.so.debug...done. done. Loaded symbols for /lib64/ld-linux-x86-64.so.2 0x7fead8db6fbd in pthread_join (threadid=140646633633552, thread_return=0x0) at pthread_join.c:89 89 lll_wait_tid (pd-tid); (gdb) [Thread 22822.22825] #2 stopped. 0x7fead8ad6a6d in nanosleep () at ../sysdeps/unix/syscall-template.S:82 82 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) Current language: auto The current source language is auto; currently asm. info threads 2 Thread 22822.22825 0x7fead8ad6a6d in nanosleep () at ../sysdeps/unix/syscall-template.S:82 * 1 Thread 22822.22822 0x7fead8db6fbd in pthread_join (threadid=140646633633552, thread_return=0x0) at pthread_join.c:89 (gdb) q [7]+ Done~/redhat/gdbstub ~/redhat/out [6]+ Terminated ~/redhat/threadit Thanks, Jan
Re: gdbstub initial code, another approach
On 07/29, Frank Ch. Eigler wrote: Oleg Nesterov o...@redhat.com writes: [...] - ugdb.c The kernel module which implements the basic user-space API on top of utrace. Of course, this API should be discussed. - gdbstub The simple user-space gdbserver written in perl which works with ugdb API. [...] To the extent that the problems with an in-kernel gdbstub are weaknesses in the protocol - or gdb's implementation thereof - how would this split improve that situation? I have the strong desire to ask you by turn why do you think that in-kernel gdbstub can help in any way ;) Yes, I never liked the idea of in-kernel gdbstub. Apart from too-highlevel and vague it is also a bit limited. And some things, say, register renumbering, doesn't belong to kernel. Or vRun. Many other things. The only advantage is that we already have the great tool which works with this protocol - gdb. Ok, it is easy to criticize, and my opinion doesn't really matter. We can put it in kernel later, when we have something more than just the proof of concept. But I do not see how in-kernel gdbstub can help even to prototype things. In my opinion it only complicates this. If nothing else, it is not easy to test even the simple things. Just imagine the simple tests like ptrace-tests rewritten to work via remote protocol. IIUK, the main goal is prototype the new generic API, while the remote protocol (in my opinion) is obviously can't be considered as such. With this split it is possible to try to add some API and test it with or without gdb. Also, it is much more easy to play with the the protocol extensions (which I believe it needs) this way. It would be (I think) much easier to teach the real gdbserver and/or gdb to use this new API if we already had the userspace aplication which actually works using this API. OTOH, with this split we still have the same advantage: we can use gdb to prove that this code can do something useful. Oleg.
Re: gdbstub initial code, another approach
Hi, Oleg - [...] But I do not see how in-kernel gdbstub can help even to prototype things. In my opinion it only complicates this. If nothing else, it is not easy to test even the simple things. Just imagine the simple tests like ptrace-tests rewritten to work via remote protocol. (One could use a new user-space library. There is not that much complexity difference between a write/read syscall pair and a complex ioctl.) IIUK, the main goal is prototype the new generic API [...] It would be (I think) much easier to teach the real gdbserver and/or gdb to use this new API if we already had the userspace aplication which actually works using this API. To an extent, it's all a SMOP. But the key is the level of abstraction provided by any new API. ptrace(2) is low, the gdb-wire-protocol is high, and both are pretty well established. A brand new API aiming into some new middle point will be harder to validate. OTOH, with this split we still have the same advantage: we can use gdb to prove that this code can do something useful. Not if you run into the exact same multithreading protocol glitches, but this time with three separate interacting bodies of code instead of two. - FChE
Re: gdbstub initial code, another approach
On Fri, 30 Jul 2010 16:41:24 +0200, Oleg Nesterov wrote: IOW, you think that it is better to shift gdbserver into kernel-space than port the existing one to the new API or write the new one in user space ? So far I just assumed kernel-space ugdb is the plan. As I wrote before I do not know gdbserver too much. If you check gdb/gdbserver/linux-low.c it is just one big ptrace/wait/\/proc interface. I would guess it could be more simple with the utrace API at hand. Catching up with systemtap's 200x higher software-watchpoint performance over current (local) gdb (described in [debug-list] Utrace Discussion Notes off this list) could be easier with in-kernel gdb I thought. Thanks, Jan
Re: gdbstub initial code, another approach
Jan == Jan Kratochvil jan.kratoch...@redhat.com writes: Jan gdb linux-nat.c (=local gdb) should be deprecated. There is Jan definitely a need for remote target and actively maintaining two Jan modes is not effective, we can run gdbserver even during Jan single-host debugging. I think we should differentiate a bit between Oleg's project and projects internal to gdb. I agree that the current approach of writing all linux-nat code twice -- once for gdb and once for gdbserver -- is no good. Also, I think Oleg's recent questions and investigations have shown that perhaps gdbserver is currently a bit lacking for local debugging. But, a lot of this is a problem specific to gdb. We could, for example, remove linux-nat.c and move to only allowing gdbserver. Or, we could have gdb and gdbserver share code. But either of these would be independent of whatever interface the kernel provides. Tom
Re: gdbstub initial code, another approach
Oleg Nesterov o...@redhat.com writes: [...] - ugdb.c The kernel module which implements the basic user-space API on top of utrace. Of course, this API should be discussed. - gdbstub The simple user-space gdbserver written in perl which works with ugdb API. [...] To the extent that the problems with an in-kernel gdbstub are weaknesses in the protocol - or gdb's implementation thereof - how would this split improve that situation? - FChE
gdbstub initial code, another approach
On 07/26, Oleg Nesterov wrote:
I decided to take a bit different approach, we will see if it
makes sense in the longer term.
Please see the attached files,
- ugdb.c
The kernel module which implements the basic
user-space API on top of utrace. Of course,
this API should be discussed.
- gdbstub
The simple user-space gdbserver written in
perl which works with ugdb API.
Limitations:
- this is just initial code, again.
- doesn't work in all-stop mode (should be simple to
implement).
- currently it only supports attach, stop, cont, detach
and exit.
- the testing was very limited. I played with it about
an hour and didn't find any problems, vut that is all.
However, please note that this time the code is clearly opened
for improvements.
I stronly believe this is the only sane approach. Even for
prototyping. No, _especially_ for prototyping!
Btw, gdb crashes very often right after
(gdb) set target-async on
(gdb) set non-stop
(gdb) file mt-program
(gdb) target extended-remote :port
(gdb) attach its_pid
I didn't even try to investigate (this doesn't happen when
it works with the real gdbserver). Just retry, gdb is buggy.
What do you think?
Oleg.
#include linux/module.h
#include linux/proc_fs.h
#include linux/utrace.h
#include linux/poll.h
#include linux/mm.h
#include linux/regset.h
#include asm/uaccess.h
struct ugdb_thread {
int t_tid;
int t_stop;
int t_exit;
struct pid *t_spid;
struct ugdb *t_ugdb;
struct utrace_engine*t_engine;
struct list_headt_threads;
struct list_headt_stopped;
};
struct ugdb {
spinlock_t u_llock;
struct list_headu_threads;
struct list_headu_stopped;
wait_queue_head_t u_wait;
};
// XXX: gdb is single-thread, no locking currently.
#define T printk(KERN_INFO TRACE: %s:%d\n, __FUNCTION__, __LINE__)
static struct ugdb_thread *ugdb_create_thread(struct ugdb *ugdb, int tid)
{
struct pid *spid;
struct ugdb_thread *thread;
int err;
err = -ESRCH;
spid = find_get_pid(tid);
if (!spid)
goto err;
err = -ENOMEM;
thread = kzalloc(sizeof(*thread), GFP_KERNEL);
if (!thread)
goto free_pid;
thread-t_tid = tid;
thread-t_spid = spid;
thread-t_ugdb = ugdb;
INIT_LIST_HEAD(thread-t_stopped);
spin_lock(ugdb-u_llock);
list_add_tail(thread-t_threads, ugdb-u_threads);
spin_unlock(ugdb-u_llock);
return thread;
free_pid:
put_pid(spid);
err:
return ERR_PTR(err);
}
static void ugdb_destroy_thread(struct ugdb_thread *thread)
{
struct ugdb *ugdb = thread-t_ugdb;
spin_lock(ugdb-u_llock);
list_del(thread-t_stopped);
list_del(thread-t_threads);
spin_unlock(ugdb-u_llock);
put_pid(thread-t_spid);
kfree(thread);
}
static struct ugdb_thread *ugdb_find_thread(struct ugdb *ugdb, int tid)
{
struct ugdb_thread *thread;
spin_lock(ugdb-u_llock);
list_for_each_entry(thread, ugdb-u_threads, t_threads) {
if (thread-t_tid == tid)
goto found;
}
thread = NULL;
found:
spin_unlock(ugdb-u_llock);
return thread;
}
static int ugdb_set_events(struct ugdb_thread *thread,
unsigned long events)
{
return utrace_set_events_pid(thread-t_spid, thread-t_engine,
events);
}
static int ugdb_control(struct ugdb_thread *thread,
enum utrace_resume_action action)
{
return utrace_control_pid(thread-t_spid, thread-t_engine,
action);
}
static const struct utrace_engine_ops ugdb_utrace_ops;
static struct ugdb_thread *ugdb_attach_thread(struct ugdb *ugdb, int tid)
{
struct ugdb_thread *thread;
void *errp;
int err;
thread = ugdb_create_thread(ugdb, tid);
if (IS_ERR(thread)) {
errp = thread;
goto err;
}
thread-t_engine = utrace_attach_pid(thread-t_spid,
UTRACE_ATTACH_CREATE,
ugdb_utrace_ops,
thread);
if (IS_ERR(thread-t_engine)) {
errp = thread-t_engine;
goto free_thread;
}
err = ugdb_set_events(thread,
UTRACE_EVENT(QUIESCE) | UTRACE_EVENT(DEATH));
if (err) {
errp = ERR_PTR(-ESRCH);
goto
Re: gdbstub initial code
At first I tried to support both multiprocess and !multiprocess modes, but this needs too many unnecessary code which I'd like to avoid, at least now. So this version requires multiprocess+ in qSupported, otherwise target extended-remote fails. Please let me know if we need both modes. No, that is fine. We aren't trying to make it a universal implementation of the gdb remote protocol in all variants to work with all gdb versions of the past. We're just trying to make it work well with current/future gdb. Thanks, Roland
Re: gdbstub initial code
Note that currently I am not even trying to do something meaningful with utrace. My only goal for now is to implement the very basic things like attach/detach/stop/cont/exit correctly from the remote protocol pov. And I want to do this rightly, then we will discuss utrace issues. Ok. I think you might need select to work on the pseudofile for gdb to be happy. I'm not sure on the situation about that. Thanks, Roland
Re: gdbstub initial code
On 07/16, Roland McGrath wrote:
Note that currently I am not even trying to do something meaningful
with utrace. My only goal for now is to implement the very basic things
like attach/detach/stop/cont/exit correctly from the remote protocol
pov. And I want to do this rightly, then we will discuss utrace
issues.
Ok. I think you might need select to work on the pseudofile for gdb to be
happy. I'm not sure on the situation about that.
Ah, it is easy to implement ugdb_f_ops-poll(). I think. The only problem
is that gdb doesn't try to use select. Perhaps it sees S_IFREG, I didn't
check.
But, it turns out, the multithreading is not trivial, and nasty. Because,
unless I missed something a) remote protocol is very stupid, and b) gdb
is buggy.
I was going to add the threading support today, but I guess I have to
really ask the questions before I can do this. So I am attaching the
yesterday's version.
Changes:
- cleanups/fixes to make cont/^C really work
- handle tracee's exit
Oleg.
#include linux/module.h
#include linux/proc_fs.h
#include linux/utrace.h
#include linux/regset.h
#include linux/mm.h
#include asm/uaccess.h
static int d_echo;
module_param_named(echo, d_echo, bool, 0);
#define PACKET_SIZE 1024
#define BUFFER_SIZE 1024
struct pbuf {
char*cur, *pkt;
charbuf[BUFFER_SIZE];
};
static inline void pb_init(struct pbuf *pb)
{
pb-cur = pb-buf;
pb-pkt = NULL;
}
static inline int pb_size(struct pbuf *pb)
{
return pb-cur - pb-buf;
}
static inline int pb_room(struct pbuf *pb)
{
return pb-buf + BUFFER_SIZE - pb-cur;
}
static inline void pb_putc(struct pbuf *pb, char c)
{
if (WARN_ON(pb-cur = pb-buf + BUFFER_SIZE-1))
return;
*pb-cur++ = c;
}
static void pb_memcpy(struct pbuf *pb, const void *data, int size)
{
if (WARN_ON(size pb_room(pb)))
return;
memcpy(pb-cur, data, size);
pb-cur += size;
}
static inline void pb_puts(struct pbuf *pb, const char *s)
{
pb_memcpy(pb, s, strlen(s));
}
static inline void pb_putb(struct pbuf *pb, unsigned char val)
{
static char hex[] = 0123456789abcdef;
pb_putc(pb, hex[(val 0xf0) 4]);
pb_putc(pb, hex[(val 0x0f) 0]);
}
static void pb_putbs(struct pbuf *pb, const char *data, int size)
{
while (size--)
pb_putb(pb, *data++);
}
static inline void pb_start(struct pbuf *pb)
{
WARN_ON(pb-pkt);
pb_putc(pb, '$');
pb-pkt = pb-cur;
}
static inline void pb_cancel(struct pbuf *pb)
{
if (WARN_ON(!pb-pkt))
return;
pb-cur = pb-pkt - 1;
pb-pkt = NULL;
}
static void pb_end(struct pbuf *pb)
{
unsigned char csm = 0;
char *pkt = pb-pkt;
pb-pkt = NULL;
if (WARN_ON(!pkt))
return;
while (pkt pb-cur) {
WARN_ON(*pkt == '$' || *pkt == '#');
csm += (unsigned char)*pkt++;
}
pb_putc(pb, '#');
pb_putb(pb, csm);
}
static inline void pb_packs(struct pbuf *pb, const char *s)
{
pb_start(pb);
pb_puts(pb, s);
pb_end(pb);
}
static void __attribute__ ((format(printf, 3, 4)))
__pb_format(struct pbuf *pb, bool whole_pkt, const char *fmt, ...)
{
int room = pb_room(pb), size;
va_list args;
if (whole_pkt)
pb_start(pb);
va_start(args, fmt);
size = vsnprintf(pb-cur, room, fmt, args);
va_end(args);
if (WARN_ON(size room))
return;
pb-cur += size;
if (whole_pkt)
pb_end(pb);
}
#define pb_printf(pb, args...) __pb_format((pb), false, args)
#define pb_packf(pb, args...) __pb_format((pb), true, args)
static inline void *pb_alloc_bs(struct pbuf *pb, int size)
{
if (unlikely(pb_room(pb) 2 * size + 4))
return NULL;
return pb-cur + size;
}
static inline void *pb_alloc_tmp(struct pbuf *pb, int size)
{
if (unlikely(pb_room(pb) size))
return NULL;
return pb-cur + BUFFER_SIZE - size;
}
static inline void pb_flush(struct pbuf *pb, int size)
{
int keep = pb_size(pb) - size;
if (keep)
memmove(pb-buf, pb-buf + size, keep);
pb-cur -= size;
}
static int pb_copy_to_user(struct pbuf *pb, char __user *ubuf, int size)
{
int copy = min(size, pb_size(pb));
if (d_echo)
printk(KERN_INFO = %.*s\n, copy, pb-buf);
if (copy_to_user(ubuf, pb-buf, copy))
return -EFAULT;
pb_flush(pb, copy);
return copy;
}
//
// XXX: TODO: gdb is single-thread, no locking currently.
//
#define T printk(KERN_INFO TRACE: %s:%d\n, __FUNCTION__, __LINE__)
enum {
STOP_RUN,
STOP_REQ,
Re: gdbstub initial code
On 07/13, Oleg Nesterov wrote:
And now I am going to rewrite this code without adding the new
functionality (this shouldn't take much time, hopefully today).
Next iteration. Misc changes, but still in the initial stage. The
only visible change is that cont/ctrl-c pretends to work.
Note that currently I am not even trying to do something meaningful
with utrace. My only goal for now is to implement the very basic things
like attach/detach/stop/cont/exit correctly from the remote protocol
pov. And I want to do this rightly, then we will discuss utrace
issues.
Oleg.
#include linux/module.h
#include linux/proc_fs.h
#include linux/utrace.h
#include linux/regset.h
#include linux/mm.h
#include asm/uaccess.h
static int d_echo;
module_param_named(echo, d_echo, bool, 0);
#define PACKET_SIZE 1024
#define BUFFER_SIZE 1024
struct pbuf {
char*cur, *pkt;
charbuf[BUFFER_SIZE];
};
static inline void pb_init(struct pbuf *pb)
{
pb-cur = pb-buf;
pb-pkt = NULL;
}
static inline int pb_size(struct pbuf *pb)
{
return pb-cur - pb-buf;
}
static inline int pb_room(struct pbuf *pb)
{
return pb-buf + BUFFER_SIZE - pb-cur;
}
static inline void pb_putc(struct pbuf *pb, char c)
{
if (WARN_ON(pb-cur = pb-buf + BUFFER_SIZE))
return;
*pb-cur++ = c;
}
static void pb_memcpy(struct pbuf *pb, const void *data, int size)
{
if (WARN_ON(size pb_room(pb)))
return;
memcpy(pb-cur, data, size);
pb-cur += size;
}
static inline void pb_puts(struct pbuf *pb, const char *s)
{
pb_memcpy(pb, s, strlen(s));
}
static inline void pb_putb(struct pbuf *pb, unsigned char val)
{
static char hex[] = 0123456789abcdef;
pb_putc(pb, hex[(val 0xf0) 4]);
pb_putc(pb, hex[(val 0x0f) 0]);
}
static void pb_putbs(struct pbuf *pb, const char *data, int size)
{
while (size--)
pb_putb(pb, *data++);
}
static inline void pb_start(struct pbuf *pb)
{
WARN_ON(pb-pkt);
pb_putc(pb, '$');
pb-pkt = pb-cur;
}
static inline void pb_cancel(struct pbuf *pb)
{
if (WARN_ON(!pb-pkt))
return;
pb-cur = pb-pkt - 1;
pb-pkt = NULL;
}
static void pb_end(struct pbuf *pb)
{
unsigned char csm = 0;
char *pkt = pb-pkt;
pb-pkt = NULL;
if (WARN_ON(!pkt))
return;
while (pkt pb-cur) {
WARN_ON(*pkt == '$' || *pkt == '#');
csm += (unsigned char)*pkt++;
}
pb_putc(pb, '#');
pb_putb(pb, csm);
}
static inline void pb_packs(struct pbuf *pb, const char *s)
{
pb_start(pb);
pb_puts(pb, s);
pb_end(pb);
}
static void __attribute__ ((format(printf, 3, 4)))
__pb_format(struct pbuf *pb, bool whole_pkt, const char *fmt, ...)
{
int room = pb_room(pb), size;
va_list args;
if (whole_pkt)
pb_start(pb);
va_start(args, fmt);
size = vsnprintf(pb-cur, room, fmt, args);
va_end(args);
if (WARN_ON(size room))
return;
pb-cur += size;
if (whole_pkt)
pb_end(pb);
}
#define pb_printf(pb, args...) __pb_format((pb), false, args)
#define pb_packf(pb, args...) __pb_format((pb), true, args)
static inline void *pb_alloc_bs(struct pbuf *pb, int size)
{
if (unlikely(pb_room(pb) 2 * size + 4))
return NULL;
return pb-cur + size;
}
static inline void *pb_alloc_tmp(struct pbuf *pb, int size)
{
if (unlikely(pb_room(pb) size))
return NULL;
return pb-cur + BUFFER_SIZE - size;
}
static inline void pb_flush(struct pbuf *pb, int size)
{
int keep = pb_size(pb) - size;
if (keep)
memmove(pb-buf, pb-buf + size, keep);
pb-cur -= size;
}
static int pb_copy_to_user(struct pbuf *pb, char __user *ubuf, int size)
{
int copy = min(size, pb_size(pb));
if (d_echo)
printk(KERN_INFO = %.*s\n, copy, pb-buf);
if (copy_to_user(ubuf, pb-buf, copy))
return -EFAULT;
pb_flush(pb, copy);
return copy;
}
//
// XXX: TODO: gdb is single-thread, no locking currently.
//
#define T printk(KERN_INFO TRACE: %s:%d\n, __FUNCTION__, __LINE__)
enum {
NO_STOP,
STOP_REQ,
STOP_ACK,
};
struct ugdb_context {
int c_stop;
const char *fake_rc;
struct pid *pid;
struct utrace_engine*engine;
struct ugdb *c_ugdb;
struct list_headnode;
};
struct ugdb {
charg_ibuf[PACKET_SIZE];
int g_ilen;
struct pbuf g_pbuf;
bool
Re: gdbstub initial code
* Roland McGrath rol...@redhat.com [2010-07-12 22:59:05]: When I had posted a prototype of a gdbstub which Frank and I had worked on. http://lkml.org/lkml/2009/11/30/173, Peter and Ingo showed a preference for a combined gdbstub in kernel, i.e kgdb and the newer stub should use only one stub in kernel. Do we have plans to handle that? Their actual idea there largely represents a misunderstanding of the problem space. But regardless, it's a distraction from the prototype work that Oleg is doing now. The actual possibilities for code sharing between kgdb and something at all like what we're doing now are quite small. It's just not a problem at all to get prototyping progress done with a new implementation of the fairly trivial gdb remote protocol decoder, and contemplate consolidation later on. Agree, my concern was that if they keep repeating it after we have done the prototyping. Yes, that shouldnt stop us from prototyping. -- Thanks and Regards Srikar
Re: gdbstub initial code
Given these requirements, and given that the 'new' uprobes is close to -tip, would it be more useful to pursue an alternate syscall approach rather than gdbstub? Feel free to pursue whatever you like. For our own time allocation, we see an effort along those lines now as a distraction from work that will really make a qualitative difference in the debugging experience. Any new interface is an instant source of endless discussions (at best) about many details that are ultimately trivial in the greater scheme of things. Aside from flaming its details a priori, no new interface is of any interest to anyone unless its use is integrated into real, non-toy userland debugging tools and it enables their delivery of qualitatively significant, new or better aspects of the debugging experience. Starting with a whole new interface inevitably involves spending most of the time on the combination of LKML flames about the interface trivia and work on toy userland libraries and utilities to demonstrate using the new kernel features. That's a whole lot more time and effort and friction to come around to doing the toy version of what real userland debugging tools do today, and maybe then start on doing anything that's actually new or different beyond cleaning up pet-peeve interface trivia, if you don't get too side-tracked filling in practical holes in your toy tools along the way first. What Oleg is embarking on now is a prototyping exercise. We're not trying to find a new kind of backwards to bend over to have upstream people like any new interface layers. We're trying to get quickly to the experimental baseline from which we can try to come up with some of those qualitatively significant new things. That means having a full, adult-sized, real-world debugging tool plugged into new and unencumbered kernel code paths and doing approximately its normal thing at least as well (approximately) as normal. In the end, more of the work is on the userland side (or in fritter about what the user-kernel interface details should be) than the actual guts of the kernel-side work. We've chosen the gdb remote protocol as a prototype vehicle because we start with about 95% complete support in our closest-to-hand real-world tool (gdb) for that baseline. (We also happen to have some gdb-hacking colleagues nearby to help us experiment with anything that might rely on that remaining 5% or otherwise on teaching gdb new tricks.) Hence we hope to get very quickly to that baseline for experimentation. From there we can start trying out the things that could really make a big difference in what a debugger tool can do (or how well/fast it can do them). What matters for that isn't the little details of encodings or syscall interfaces, but the large-granularity issues about how the interface is used, like how many context switches back and forth to the debugger it takes to get a task done, etc. IMHO it would be pointless to try to design any new interface before knowing concretely what kinds of things on the big-idea scale make an important difference to the actual debugging experience with a real-world tool like gdb. When we've shown what key features and characteristics deliver a big tangible payoff, we can worry about how to formulate new interfaces or extensions that both achieve those essential goods and meet with upstream tastes. Thanks, Roland
Re: gdbstub initial code
What is the reasoning for selecting /proc/ugdb instead of something like /proc/pid/ugdb? The protocol, and gdb, support dealing with many processes over one control channel. For normal the debugger model to work where it can attach to a process on demand, with your model it would have to open another fd for every process (or perhaps thread), maintain perhaps thousands of fds, do a select dance, never be able to pipeline notifications from multiple processes into a single receiving call in the debugger, etc. Moreover, that's just not what the protocol is and we already have a protocol with a client that we can just support as is. Thanks, Roland
Re: gdbstub initial code
On 07/13, Ananth N Mavinakayanahalli wrote: Given these requirements, and given that the 'new' uprobes is close to -tip, would it be more useful to pursue an alternate syscall approach rather than gdbstub? Roland has already answered, and I agree. The problem is that we do not have the new and nice API. gdbstub was chosen for prototyping because gdb (at least) is already here and can use it immediately. Oleg.
Re: gdbstub initial code
On 07/12, Roland McGrath wrote: Please see the attachment. Don't take this code seriously, this is the early prototype and everything should be rewritten. It barely uses utrace, only to stop the target. You've got to start somewhere! Yes ;) And now I am going to rewrite this code without adding the new functionality (this shouldn't take much time, hopefully today). I greatly misunderstood how should this stub communicate with gdb when I started the coding, struct cbuf is absolutely ugly. I still can't understand what utrace_xxx_pid() buys us, and I still think that utrace_prepare_examine() can't protect the task even for regset calls. Please start a separate thread here about each of those issues. The first one is fairly simple, but what that means in practice depends on the resolution of the second question, which is a more complex subject. Yes. Oleg.
Re: gdbstub initial code
Ananth == Ananth N Mavinakayanahalli ana...@in.ibm.com writes: Ananth OTOH, the Tom Tromey alluded on lkml that if kernel provides Ananth infrastructure that allows for breakpoint assistance and 'displaced' Ananth stepping, with a suitable user interface, preferably a ptrace variant Ananth that is fd based, they'll migrate gdb to using the interface. (The bp Ananth assistance and displaced stepping can be provided with extensions to Ananth the current uprobes set under review). Just to be clear, the interface doesn't matter to gdb. We'll port gdb to use any new interface that the kernel provides, assuming it provides some benefit. Tom
gdbstub initial code
Hello.
Please see the attachment. Don't take this code seriously, this is
the early prototype and everything should be rewritten. It barely
uses utrace, only to stop the target.
(gdb) file /path/to/binary
(gdb) target extended-remote /proc/ugdb
(gdb) attach PID
(gdb) disassemble _start
(gdb) bt
(gdb) info registers
(gdb) info threads
(gdb) detach
This seems to work, but I had to export access_process_vm().
Currently it only attaches to the single thread. vCont or ^C doesn't
work.
I still can't understand what utrace_xxx_pid() buys us, and I still
think that utrace_prepare_examine() can't protect the task even for
regset calls.
Oleg.
#include linux/module.h
#include linux/proc_fs.h
#include linux/utrace.h
#include linux/regset.h
#include linux/mm.h
#include asm/uaccess.h
#define static
#define PACKET_SIZE 1024
#define BUFFER_SIZE 1024
struct cbuf {
char*w_ptr;
char*r_ptr;
charbuf[BUFFER_SIZE];
};
static inline void cb_init(struct cbuf *cb)
{
cb-w_ptr = cb-r_ptr = cb-buf;
}
static int cb_rsz(struct cbuf *cb)
{
int rsz = cb-w_ptr - cb-r_ptr;
if (rsz 0)
rsz += BUFFER_SIZE;
return rsz;
}
static int cb_wsz(struct cbuf *cb)
{
int wsz = cb-r_ptr - cb-w_ptr - 1;
if (wsz 0)
wsz += BUFFER_SIZE;
return wsz;
}
static int cb_l_wsz(struct cbuf *cb)
{
char *lim = cb-r_ptr - 1;
if (cb-w_ptr = cb-r_ptr) {
lim = cb-buf + BUFFER_SIZE;
if (cb-r_ptr == cb-buf)
lim--;
}
return lim - cb-w_ptr;
}
static void __cb_padd(struct cbuf *cb, char **pptr, int cnt)
{
char *p = *pptr + cnt;
if (p = cb-buf + BUFFER_SIZE)
p -= BUFFER_SIZE;
*pptr = p;
}
#define cb_pinc(cb, pptr) __cb_padd((cb), (pptr), 1)
#define cb_padd(cb, pptr, cnt) __cb_padd((cb), (pptr), (cnt))
static inline void put_hex(unsigned char val, char to[2])
{
static char hex[] = 0123456789abcdef;
to[0] = hex[(val 0xf0) 4];
to[1] = hex[(val 0x0f) 0];
}
static int cb_copy_to_user(struct cbuf *cb, unsigned char *pcsum,
char __user *uptr, int size)
{
unsigned char csum = *pcsum;
char *kptr = cb-r_ptr;
int total, copied = 0;
if (!access_ok(VERIFY_WRITE, uptr, size))
goto efault;
for (total = cb_rsz(cb); total; --total) {
unsigned char c = *kptr;
int sz = (c == '#') ? 3 : 1;
char csh[2];
size -= sz;
if (size 0)
break;
if (__put_user(c, uptr))
goto efault;
switch (c) {
case '#':
put_hex(csum, csh);
if (__copy_to_user(uptr + 1, csh, sizeof csh))
goto efault;
case '$':
case '%':
csum = 0;
break;
default:
csum += (unsigned char)c;
}
cb_pinc(cb, kptr);
copied += sz;
uptr += sz;
}
ret:
*pcsum = csum;
cb-r_ptr = kptr;
return copied ?: -EPIPE;
efault:
copied = copied ?: -EFAULT;
goto ret;
}
static int cb_copy_from(struct cbuf *cb, const char *data, int size)
{
int flat, i;
if (size cb_wsz(cb))
return -EOVERFLOW;
for (i = 0; size i 2; ++i) {
flat = cb_l_wsz(cb);
if (flat size)
flat = size;
memcpy(cb-w_ptr, data, flat);
cb_padd(cb, cb-w_ptr, flat);
data += flat;
size -= flat;
}
BUG_ON(size);
return 0;
}
static int __attribute__ ((format(printf, 2, 3)))
cb_printf(struct cbuf *cb, const char *fmt, ...)
{
char sbuf[64];
va_list args;
int size;
va_start(args, fmt);
size = vsnprintf(sbuf, sizeof(sbuf), fmt, args);
va_end(args);
if (size = sizeof(sbuf))
return -EINVAL;
return cb_copy_from(cb, sbuf, size);
}
static void cb_hexcopy_from(struct cbuf *cb, const char *data, int size)
{
if (WARN_ON(2 * size cb_wsz(cb)))
return;
while (size--) {
char byte[2];
put_hex(*data++, byte);
*cb-w_ptr = byte[0];
cb_pinc(cb, cb-w_ptr);
*cb-w_ptr = byte[1];
cb_pinc(cb, cb-w_ptr);
}
}
#define cb_puthex(cb, val) \
cb_hexcopy_from((cb), (val), sizeof(val))
static void cb_putc(struct cbuf *cb, char c)
{
if
Re: gdbstub initial code
Hi Oleg, Hello. Please see the attachment. Don't take this code seriously, this is the early prototype and everything should be rewritten. It barely uses utrace, only to stop the target. (gdb) file /path/to/binary (gdb) target extended-remote /proc/ugdb (gdb) attach PID (gdb) disassemble _start (gdb) bt (gdb) info registers (gdb) info threads (gdb) detach This seems to work, but I had to export access_process_vm(). Currently it only attaches to the single thread. vCont or ^C doesn't work. I still can't understand what utrace_xxx_pid() buys us, and I still think that utrace_prepare_examine() can't protect the task even for regset calls. When I had posted a prototype of a gdbstub which Frank and I had worked on. http://lkml.org/lkml/2009/11/30/173, Peter and Ingo showed a preference for a combined gdbstub in kernel, i.e kgdb and the newer stub should use only one stub in kernel. Do we have plans to handle that? -- Thanks and Regards Srikar
Re: gdbstub initial code
On Mon, Jul 12, 2010 at 08:37:29PM +0200, Oleg Nesterov wrote: Hello. Please see the attachment. Don't take this code seriously, this is the early prototype and everything should be rewritten. It barely uses utrace, only to stop the target. (gdb) file /path/to/binary (gdb) target extended-remote /proc/ugdb (gdb) attach PID (gdb) disassemble _start (gdb) bt (gdb) info registers (gdb) info threads (gdb) detach This seems to work, but I had to export access_process_vm(). Currently it only attaches to the single thread. vCont or ^C doesn't work. I still can't understand what utrace_xxx_pid() buys us, and I still think that utrace_prepare_examine() can't protect the task even for regset calls. IMHO, if this is the start of another stab at getting utrace in the upstream kernel, you may want to consider Linus' problem statement for utrace -- infrastructure that will allow strace and gdb on the same thread at the same time. OTOH, the Tom Tromey alluded on lkml that if kernel provides infrastructure that allows for breakpoint assistance and 'displaced' stepping, with a suitable user interface, preferably a ptrace variant that is fd based, they'll migrate gdb to using the interface. (The bp assistance and displaced stepping can be provided with extensions to the current uprobes set under review). In any case, its good to see a restart of this effort. Though there has been support for gdbstub in the past, an overwhelming majority of people would like to see a 'user interface', be it ptrace2 or PTRACE_ATTACH2 or whatever it needs to be called. Given these requirements, and given that the 'new' uprobes is close to -tip, would it be more useful to pursue an alternate syscall approach rather than gdbstub? Ananth
Re: gdbstub initial code
Hello. Please see the attachment. Don't take this code seriously, this is the early prototype and everything should be rewritten. It barely uses utrace, only to stop the target. (gdb) file /path/to/binary (gdb) target extended-remote /proc/ugdb (gdb) attach PID (gdb) disassemble _start What is the reasoning for selecting /proc/ugdb instead of something like /proc/pid/ugdb? We had a discussion sometime back in utrace_devel https://www.redhat.com/archives/utrace-devel/2008-June/msg00070.html where we thought having /proc/utrace was a bad choice. -- Thanks and Regards Srikar
Re: gdbstub initial code
Please see the attachment. Don't take this code seriously, this is the early prototype and everything should be rewritten. It barely uses utrace, only to stop the target. You've got to start somewhere! Thanks, Oleg. It's great to see this get underway. This seems to work, but I had to export access_process_vm(). Yeah, that's a known issue. We can discuss how to either work around or change it at some point, but it's just a distraction at the moment. I still can't understand what utrace_xxx_pid() buys us, and I still think that utrace_prepare_examine() can't protect the task even for regset calls. Please start a separate thread here about each of those issues. The first one is fairly simple, but what that means in practice depends on the resolution of the second question, which is a more complex subject. Thanks, Roland
Re: gdbstub initial code
When I had posted a prototype of a gdbstub which Frank and I had worked on. http://lkml.org/lkml/2009/11/30/173, Peter and Ingo showed a preference for a combined gdbstub in kernel, i.e kgdb and the newer stub should use only one stub in kernel. Do we have plans to handle that? Their actual idea there largely represents a misunderstanding of the problem space. But regardless, it's a distraction from the prototype work that Oleg is doing now. The actual possibilities for code sharing between kgdb and something at all like what we're doing now are quite small. It's just not a problem at all to get prototyping progress done with a new implementation of the fairly trivial gdb remote protocol decoder, and contemplate consolidation later on. Thanks, Roland
