Step over clone syscall w/ breakpoint, TARGET_WAITKIND_THREAD_CLONED

(A good chunk of the problem statement in the commit log below is
Andrew's, adjusted for a different solution, and for covering
displaced stepping too.  The testcase is mostly Andrew's too.)

This commit addresses bugs gdb/19675 and gdb/27830, which are about
stepping over a breakpoint set at a clone syscall instruction, one is
about displaced stepping, and the other about in-line stepping.

Currently, when a new thread is created through a clone syscall, GDB
sets the new thread running.  With 'continue' this makes sense
(assuming no schedlock):

 - all-stop mode, user issues 'continue', all threads are set running,
   a newly created thread should also be set running.

 - non-stop mode, user issues 'continue', other pre-existing threads
   are not affected, but as the new thread is (sort-of) a child of the
   thread the user asked to run, it makes sense that the new threads
   should be created in the running state.

Similarly, if we are stopped at the clone syscall, and there's no
software breakpoint at this address, then the current behaviour is
fine:

 - all-stop mode, user issues 'stepi', stepping will be done in place
   (as there's no breakpoint to step over).  While stepping the thread
   of interest all the other threads will be allowed to continue.  A
   newly created thread will be set running, and then stopped once the
   thread of interest has completed its step.

 - non-stop mode, user issues 'stepi', stepping will be done in place
   (as there's no breakpoint to step over).  Other threads might be
   running or stopped, but as with the continue case above, the new
   thread will be created running.  The only possible issue here is
   that the new thread will be left running after the initial thread
   has completed its stepi.  The user would need to manually select
   the thread and interrupt it, this might not be what the user
   expects.  However, this is not something this commit tries to
   change.

The problem then is what happens when we try to step over a clone
syscall if there is a breakpoint at the syscall address.

- For both all-stop and non-stop modes, with in-line stepping:

   + user issues 'stepi',
   + [non-stop mode only] GDB stops all threads.  In all-stop mode all
     threads are already stopped.
   + GDB removes s/w breakpoint at syscall address,
   + GDB single steps just the thread of interest, all other threads
     are left stopped,
   + New thread is created running,
   + Initial thread completes its step,
   + [non-stop mode only] GDB resumes all threads that it previously
     stopped.

There are two problems in the in-line stepping scenario above:

  1. The new thread might pass through the same code that the initial
     thread is in (i.e. the clone syscall code), in which case it will
     fail to hit the breakpoint in clone as this was removed so the
     first thread can single step,

  2. The new thread might trigger some other stop event before the
     initial thread reports its step completion.  If this happens we
     end up triggering an assertion as GDB assumes that only the
     thread being stepped should stop.  The assert looks like this:

     infrun.c:5899: internal-error: int finish_step_over(execution_control_state*): Assertion `ecs->event_thread->control.trap_expected' failed.

- For both all-stop and non-stop modes, with displaced stepping:

   + user issues 'stepi',
   + GDB starts the displaced step, moves thread's PC to the
     out-of-line scratch pad, maybe adjusts registers,
   + GDB single steps the thread of interest, [non-stop mode only] all
     other threads are left as they were, either running or stopped.
     In all-stop, all other threads are left stopped.
   + New thread is created running,
   + Initial thread completes its step, GDB re-adjusts its PC,
     restores/releases scratchpad,
   + [non-stop mode only] GDB resumes the thread, now past its
     breakpoint.
   + [all-stop mode only] GDB resumes all threads.

There is one problem with the displaced stepping scenario above:

  3. When the parent thread completed its step, GDB adjusted its PC,
     but did not adjust the child's PC, thus that new child thread
     will continue execution in the scratch pad, invoking undefined
     behavior.  If you're lucky, you see a crash.  If unlucky, the
     inferior gets silently corrupted.

What is needed is for GDB to have more control over whether the new
thread is created running or not.  Issue #1 above requires that the
new thread not be allowed to run until the breakpoint has been
reinserted.  The only way to guarantee this is if the new thread is
held in a stopped state until the single step has completed.  Issue #3
above requires that GDB is informed of when a thread clones itself,
and of what is the child's ptid, so that GDB can fixup both the parent
and the child.

When looking for solutions to this problem I considered how GDB
handles fork/vfork as these have some of the same issues.  The main
difference between fork/vfork and clone is that the clone events are
not reported back to core GDB.  Instead, the clone event is handled
automatically in the target code and the child thread is immediately
set running.

Note we have support for requesting thread creation events out of the
target (TARGET_WAITKIND_THREAD_CREATED).  However, those are reported
for the new/child thread.  That would be sufficient to address in-line
stepping (issue #1), but not for displaced-stepping (issue #3).  To
handle displaced-stepping, we need an event that is reported to the
_parent_ of the clone, as the information about the displaced step is
associated with the clone parent.  TARGET_WAITKIND_THREAD_CREATED
includes no indication of which thread is the parent that spawned the
new child.  In fact, for some targets, like e.g., Windows, it would be
impossible to know which thread that was, as thread creation there
doesn't work by "cloning".

The solution implemented here is to model clone on fork/vfork, and
introduce a new TARGET_WAITKIND_THREAD_CLONED event.  This event is
similar to TARGET_WAITKIND_FORKED and TARGET_WAITKIND_VFORKED, except
that we end up with a new thread in the same process, instead of a new
thread of a new process.  Like FORKED and VFORKED, THREAD_CLONED
waitstatuses have a child_ptid property, and the child is held stopped
until GDB explicitly resumes it.  This addresses the in-line stepping
case (issues #1 and #2).

The infrun code that handles displaced stepping fixup for the child
after a fork/vfork event is thus reused for THREAD_CLONE, with some
minimal conditions added, addressing the displaced stepping case
(issue #3).

The native Linux backend is adjusted to unconditionally report
TARGET_WAITKIND_THREAD_CLONED events to the core.

Following the follow_fork model in core GDB, we introduce a
target_follow_clone target method, which is responsible for making the
new clone child visible to the rest of GDB.

Subsequent patches will add clone events support to the remote
protocol and gdbserver.

displaced_step_in_progress_thread becomes unused with this patch, but
a new use will reappear later in the series.  To avoid deleting it and
readding it back, this patch marks it with attribute unused, and the
latter patch removes the attribute again.  We need to do this because
the function is static, and with no callers, the compiler would warn,
(error with -Werror), breaking the build.

This adds a new gdb.threads/stepi-over-clone.exp testcase, which
exercises stepping over a clone syscall, with displaced stepping vs
inline stepping, and all-stop vs non-stop.  We already test stepping
over clone syscalls with gdb.base/step-over-syscall.exp, but this test
uses pthreads, while the other test uses raw clone, and this one is
more thorough.  The testcase passes on native GNU/Linux, but fails
against GDBserver.  GDBserver will be fixed by a later patch in the
series.

Co-authored-by: Andrew Burgess <aburgess@redhat.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=19675
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27830
Change-Id: I95c06024736384ae8542a67ed9fdf6534c325c8e
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
This commit is contained in:
Pedro Alves
2021-11-12 20:50:29 +00:00
parent 6a534f85cb
commit 0d36baa9af
10 changed files with 789 additions and 206 deletions

View File

@@ -1280,6 +1280,68 @@ get_detach_signal (struct lwp_info *lp)
return 0;
}
/* If LP has a pending fork/vfork/clone status, return it. */
static gdb::optional<target_waitstatus>
get_pending_child_status (lwp_info *lp)
{
LINUX_NAT_SCOPED_DEBUG_ENTER_EXIT;
linux_nat_debug_printf ("lwp %s (stopped = %d)",
lp->ptid.to_string ().c_str (), lp->stopped);
/* Check in lwp_info::status. */
if (WIFSTOPPED (lp->status) && linux_is_extended_waitstatus (lp->status))
{
int event = linux_ptrace_get_extended_event (lp->status);
if (event == PTRACE_EVENT_FORK
|| event == PTRACE_EVENT_VFORK
|| event == PTRACE_EVENT_CLONE)
{
unsigned long child_pid;
int ret = ptrace (PTRACE_GETEVENTMSG, lp->ptid.lwp (), 0, &child_pid);
if (ret == 0)
{
target_waitstatus ws;
if (event == PTRACE_EVENT_FORK)
ws.set_forked (ptid_t (child_pid, child_pid));
else if (event == PTRACE_EVENT_VFORK)
ws.set_vforked (ptid_t (child_pid, child_pid));
else if (event == PTRACE_EVENT_CLONE)
ws.set_thread_cloned (ptid_t (lp->ptid.pid (), child_pid));
else
gdb_assert_not_reached ("unhandled");
return ws;
}
else
{
perror_warning_with_name (_("Failed to retrieve event msg"));
return {};
}
}
}
/* Check in lwp_info::waitstatus. */
if (is_new_child_status (lp->waitstatus.kind ()))
return lp->waitstatus;
thread_info *tp = linux_target->find_thread (lp->ptid);
/* Check in thread_info::pending_waitstatus. */
if (tp->has_pending_waitstatus ()
&& is_new_child_status (tp->pending_waitstatus ().kind ()))
return tp->pending_waitstatus ();
/* Check in thread_info::pending_follow. */
if (is_new_child_status (tp->pending_follow.kind ()))
return tp->pending_follow;
return {};
}
/* Detach from LP. If SIGNO_P is non-NULL, then it points to the
signal number that should be passed to the LWP when detaching.
Otherwise pass any pending signal the LWP may have, if any. */
@@ -1287,62 +1349,16 @@ get_detach_signal (struct lwp_info *lp)
static void
detach_one_lwp (struct lwp_info *lp, int *signo_p)
{
LINUX_NAT_SCOPED_DEBUG_ENTER_EXIT;
linux_nat_debug_printf ("lwp %s (stopped = %d)",
lp->ptid.to_string ().c_str (), lp->stopped);
int lwpid = lp->ptid.lwp ();
int signo;
gdb_assert (lp->status == 0 || WIFSTOPPED (lp->status));
/* If the lwp/thread we are about to detach has a pending fork/clone
event, there is a process/thread GDB is attached to that the core
of GDB doesn't know about. Detach from it. */
/* If the lwp/thread we are about to detach has a pending fork event,
there is a process GDB is attached to that the core of GDB doesn't know
about. Detach from it. */
/* Check in lwp_info::status. */
if (WIFSTOPPED (lp->status) && linux_is_extended_waitstatus (lp->status))
{
int event = linux_ptrace_get_extended_event (lp->status);
if (event == PTRACE_EVENT_FORK || event == PTRACE_EVENT_VFORK)
{
unsigned long child_pid;
int ret = ptrace (PTRACE_GETEVENTMSG, lp->ptid.lwp (), 0, &child_pid);
if (ret == 0)
detach_one_pid (child_pid, 0);
else
perror_warning_with_name (_("Failed to detach fork child"));
}
}
/* Check in lwp_info::waitstatus. */
if (lp->waitstatus.kind () == TARGET_WAITKIND_VFORKED
|| lp->waitstatus.kind () == TARGET_WAITKIND_FORKED)
detach_one_pid (lp->waitstatus.child_ptid ().pid (), 0);
/* Check in thread_info::pending_waitstatus. */
thread_info *tp = linux_target->find_thread (lp->ptid);
if (tp->has_pending_waitstatus ())
{
const target_waitstatus &ws = tp->pending_waitstatus ();
if (ws.kind () == TARGET_WAITKIND_VFORKED
|| ws.kind () == TARGET_WAITKIND_FORKED)
detach_one_pid (ws.child_ptid ().pid (), 0);
}
/* Check in thread_info::pending_follow. */
if (tp->pending_follow.kind () == TARGET_WAITKIND_VFORKED
|| tp->pending_follow.kind () == TARGET_WAITKIND_FORKED)
detach_one_pid (tp->pending_follow.child_ptid ().pid (), 0);
if (lp->status != 0)
linux_nat_debug_printf ("Pending %s for %s on detach.",
strsignal (WSTOPSIG (lp->status)),
lp->ptid.to_string ().c_str ());
gdb::optional<target_waitstatus> ws = get_pending_child_status (lp);
if (ws.has_value ())
detach_one_pid (ws->child_ptid ().lwp (), 0);
/* If there is a pending SIGSTOP, get rid of it. */
if (lp->signalled)
@@ -1836,6 +1852,55 @@ linux_handle_syscall_trap (struct lwp_info *lp, int stopping)
return 1;
}
/* See target.h. */
void
linux_nat_target::follow_clone (ptid_t child_ptid)
{
lwp_info *new_lp = add_lwp (child_ptid);
new_lp->stopped = 1;
/* If the thread_db layer is active, let it record the user
level thread id and status, and add the thread to GDB's
list. */
if (!thread_db_notice_clone (inferior_ptid, new_lp->ptid))
{
/* The process is not using thread_db. Add the LWP to
GDB's list. */
add_thread (linux_target, new_lp->ptid);
}
/* We just created NEW_LP so it cannot yet contain STATUS. */
gdb_assert (new_lp->status == 0);
if (!pull_pid_from_list (&stopped_pids, child_ptid.lwp (), &new_lp->status))
internal_error (_("no saved status for clone lwp"));
if (WSTOPSIG (new_lp->status) != SIGSTOP)
{
/* This can happen if someone starts sending signals to
the new thread before it gets a chance to run, which
have a lower number than SIGSTOP (e.g. SIGUSR1).
This is an unlikely case, and harder to handle for
fork / vfork than for clone, so we do not try - but
we handle it for clone events here. */
new_lp->signalled = 1;
/* Save the wait status to report later. */
linux_nat_debug_printf
("waitpid of new LWP %ld, saving status %s",
(long) new_lp->ptid.lwp (), status_to_str (new_lp->status).c_str ());
}
else
{
new_lp->status = 0;
if (report_thread_events)
new_lp->waitstatus.set_thread_created ();
}
}
/* Handle a GNU/Linux extended wait response. If we see a clone
event, we need to add the new LWP to our list (and not report the
trap to higher layers). This function returns non-zero if the
@@ -1876,11 +1941,9 @@ linux_handle_extended_wait (struct lwp_info *lp, int status)
internal_error (_("wait returned unexpected status 0x%x"), status);
}
ptid_t child_ptid (new_pid, new_pid);
if (event == PTRACE_EVENT_FORK || event == PTRACE_EVENT_VFORK)
{
open_proc_mem_file (child_ptid);
open_proc_mem_file (ptid_t (new_pid, new_pid));
/* The arch-specific native code may need to know about new
forks even if those end up never mapped to an
@@ -1917,66 +1980,18 @@ linux_handle_extended_wait (struct lwp_info *lp, int status)
}
if (event == PTRACE_EVENT_FORK)
ourstatus->set_forked (child_ptid);
ourstatus->set_forked (ptid_t (new_pid, new_pid));
else if (event == PTRACE_EVENT_VFORK)
ourstatus->set_vforked (child_ptid);
ourstatus->set_vforked (ptid_t (new_pid, new_pid));
else if (event == PTRACE_EVENT_CLONE)
{
struct lwp_info *new_lp;
ourstatus->set_ignore ();
linux_nat_debug_printf
("Got clone event from LWP %d, new child is LWP %ld", pid, new_pid);
new_lp = add_lwp (ptid_t (lp->ptid.pid (), new_pid));
new_lp->stopped = 1;
new_lp->resumed = 1;
/* Save the status again, we'll use it in follow_clone. */
add_to_pid_list (&stopped_pids, new_pid, status);
/* If the thread_db layer is active, let it record the user
level thread id and status, and add the thread to GDB's
list. */
if (!thread_db_notice_clone (lp->ptid, new_lp->ptid))
{
/* The process is not using thread_db. Add the LWP to
GDB's list. */
add_thread (linux_target, new_lp->ptid);
}
/* Even if we're stopping the thread for some reason
internal to this module, from the perspective of infrun
and the user/frontend, this new thread is running until
it next reports a stop. */
set_running (linux_target, new_lp->ptid, true);
set_executing (linux_target, new_lp->ptid, true);
if (WSTOPSIG (status) != SIGSTOP)
{
/* This can happen if someone starts sending signals to
the new thread before it gets a chance to run, which
have a lower number than SIGSTOP (e.g. SIGUSR1).
This is an unlikely case, and harder to handle for
fork / vfork than for clone, so we do not try - but
we handle it for clone events here. */
new_lp->signalled = 1;
/* We created NEW_LP so it cannot yet contain STATUS. */
gdb_assert (new_lp->status == 0);
/* Save the wait status to report later. */
linux_nat_debug_printf
("waitpid of new LWP %ld, saving status %s",
(long) new_lp->ptid.lwp (), status_to_str (status).c_str ());
new_lp->status = status;
}
else if (report_thread_events)
{
new_lp->waitstatus.set_thread_created ();
new_lp->status = status;
}
return 1;
ourstatus->set_thread_cloned (ptid_t (lp->ptid.pid (), new_pid));
}
return 0;
@@ -3562,59 +3577,56 @@ kill_wait_callback (struct lwp_info *lp)
return 0;
}
/* Kill the fork children of any threads of inferior INF that are
stopped at a fork event. */
/* Kill the fork/clone child of LP if it has an unfollowed child. */
static void
kill_unfollowed_fork_children (struct inferior *inf)
static int
kill_unfollowed_child_callback (lwp_info *lp)
{
for (thread_info *thread : inf->non_exited_threads ())
gdb::optional<target_waitstatus> ws = get_pending_child_status (lp);
if (ws.has_value ())
{
struct target_waitstatus *ws = &thread->pending_follow;
ptid_t child_ptid = ws->child_ptid ();
int child_pid = child_ptid.pid ();
int child_lwp = child_ptid.lwp ();
if (ws->kind () == TARGET_WAITKIND_FORKED
|| ws->kind () == TARGET_WAITKIND_VFORKED)
{
ptid_t child_ptid = ws->child_ptid ();
int child_pid = child_ptid.pid ();
int child_lwp = child_ptid.lwp ();
kill_one_lwp (child_lwp);
kill_wait_one_lwp (child_lwp);
kill_one_lwp (child_lwp);
kill_wait_one_lwp (child_lwp);
/* Let the arch-specific native code know this process is
gone. */
linux_target->low_forget_process (child_pid);
}
/* Let the arch-specific native code know this process is
gone. */
if (ws->kind () != TARGET_WAITKIND_THREAD_CLONED)
linux_target->low_forget_process (child_pid);
}
return 0;
}
void
linux_nat_target::kill ()
{
/* If we're stopped while forking and we haven't followed yet,
kill the other task. We need to do this first because the
ptid_t pid_ptid (inferior_ptid.pid ());
/* If we're stopped while forking/cloning and we haven't followed
yet, kill the child task. We need to do this first because the
parent will be sleeping if this is a vfork. */
kill_unfollowed_fork_children (current_inferior ());
iterate_over_lwps (pid_ptid, kill_unfollowed_child_callback);
if (forks_exist_p ())
linux_fork_killall ();
else
{
ptid_t ptid = ptid_t (inferior_ptid.pid ());
/* Stop all threads before killing them, since ptrace requires
that the thread is stopped to successfully PTRACE_KILL. */
iterate_over_lwps (ptid, stop_callback);
iterate_over_lwps (pid_ptid, stop_callback);
/* ... and wait until all of them have reported back that
they're no longer running. */
iterate_over_lwps (ptid, stop_wait_callback);
iterate_over_lwps (pid_ptid, stop_wait_callback);
/* Kill all LWP's ... */
iterate_over_lwps (ptid, kill_callback);
iterate_over_lwps (pid_ptid, kill_callback);
/* ... and wait until we've flushed all events. */
iterate_over_lwps (ptid, kill_wait_callback);
iterate_over_lwps (pid_ptid, kill_wait_callback);
}
target_mourn_inferior (inferior_ptid);