Re: GDB breakpoints are broken in new threads -- here's why

[Sergey Bugaev]

On Tue, Apr 11, 2023 at 1:50 AM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> Sergey Bugaev, le dim. 02 avril 2023 15:22:33 +0300, a ecrit:
> > I propose the following: before resetting the exception port, glibc
> > would fetch the previous one, and if it's non-null, it will perform a
> > special synchronous RPC on it, both passing the new exception port
> > that it would set to the tracer (so there's no need to actually set
> > it), and telling the tracer what its signal thread is (currently GDB
> > just tries to guess that the second thread is the one, except this
> > again doesn't work for the very same reason, there's not yet a second
> > thread when the task is at its very _start).
> >
> > routine name_to_be_bikeshedded (
> >         tracer_exc_port: mach_port_move_send_t;
> >         my_exc_port: mach_port_make_send_t;
> >         signal_thread: thread_t);
>
> Yes, that seems much more cooperative.
>
> I would say call it exception_set_exception_port?

Maybe. I was thinking more like tracee_check_in, since it sends more
than just the exception port (the signal thread port, and potentially
we may want to pack something else in here?).

There's also a question of which subsystem we should put it in. I was
tentatively thinking into msg.defs, even though it's not a real
msgport RPC. But maybe it deserves its own subsystem?

> > Any ideas how to fix the first issue (thread and subcode info getting
> > lost when forwarding signals to the tracer)?
>
> I'd say just add an extended proc_wait RPC indeed. proc can easily
> implement both, and new callers can revert to the old version.

Yes, so:

routine proc_wait_ext (
    process: process_t;
    sreplyport reply_port: sreply_port_t;
    waitee: task_t; /* XXX */
    options: int;
    out status: int; /* encodes signo */
    out thread: thread_t;
    out sigdetail: hurd_signal_detail);

This needs to accept a task port (not just a PID or even negative PID
or some such), since it returns the thread port, so we need to make
sure the caller already has full access to the task (which GDB of
course does). So this won't fully replace proc_wait / proc_waitid.

We also need these, which do supersede the existing ones:

routine proc_mark_stop2 (
    process: process_t;
    thread: thread_t; /* null if this was sent to the process as a whole */
    signo: int;
    sigdetail: hurd_signal_detail);

And a new routine to post/forward the signal to the task while
preserving thread/detail info:

routine msg_sig_post2 (
    process: mach_port_t;
    sreplyport reply_port: sreply_port_t;
    thread: thread_t; /* null to deliver to any thread */
    signo: int;
    sigdetail: hurd_signal_detail;
    untraced: bool;
    refport: mach_port_t);

And ideally, I'd take the chance to add a better public API for
posting signals in glibc, i.e. introduce _hurd_kill, or own analog of
pidfd_send_signal:

/* Send signal SIGNO to a process or process group.  If PROC is non-null,
   the signal is reliably sent to that process, even in the face of PID
   reuse, and even if the current process otherwise is not allowed to gain
   full access to the target task.  In this case PID can still be provided
   as a hint, to save on the number of RPCs.  If PROC is null, PID can
   designate either a single process or a process group, like in kill(2).
   If SIGDETAIL is passed, it will be sent to the target as-is, otherwise
   one will be synthesized.  If REFPORT is non-null, it will be used to
   authenticate ourselves to the target; otherwise this function will try
   to find a suitable refport itself.  */
error_t
_hurd_kill (process_t proc, pid_t pid, int signo,
            const hurd_signal_detail *__restrict sigdetail,
            mach_port_t refport);

Super ideally, we'd also find a way to implement si_pid/si_uid while
doing this all, but it's complicated. But in the simple & common case
when the signal sender has full access to the task (i.e. refport is
task or auth port), we can just trust whatever the caller says. So
maybe we just need to add pid/uid to hurd_signal_detail.

Thoughts?

Sergey