gdb: fix handling of vfork by multi-threaded program (follow-fork-mode=parent, detach...
authorSimon Marchi <simon.marchi@efficios.com>
Fri, 14 Jan 2022 20:40:59 +0000 (15:40 -0500)
committerSimon Marchi <simon.marchi@polymtl.ca>
Tue, 5 Apr 2022 02:11:51 +0000 (22:11 -0400)
commitd8bbae6ea080249c05ca90b1f8640fde48a18301
tree963f15c025a7bed3d81f18787ae3558bf19b89b5
parent05d65a7a6c888a0b80a7591a72059fec520c6a2e
gdb: fix handling of vfork by multi-threaded program (follow-fork-mode=parent, detach-on-fork=on)

There is a problem with how GDB handles a vfork happening in a
multi-threaded program.  This problem was reported to me by somebody not
using vfork directly, but using system(3) in a multi-threaded program,
which may be implemented using vfork.

This patch only deals about the follow-fork-mode=parent,
detach-on-fork=on case, because it would be too much to chew at once to
fix the bugs in the other cases as well (I tried).

The problem
-----------

When a program vforks, the parent thread is suspended by the kernel
until the child process exits or execs.  Specifically, in a
multi-threaded program, only the thread that called vfork is suspended,
other threads keep running freely. This is documented in the vfork(2)
man page ("Caveats" section).

Let's suppose GDB is handling a vfork and the user's desire is to detach
from the child. Before detaching the child, GDB must remove the software
breakpoints inserted in the shared parent/child address space, in case
there's a breakpoint in the path the child is going to take before
exec'ing or exit'ing (unlikely, but possible). Otherwise the child could
hit a breakpoint instruction while running outside the control of GDB,
which would make it crash.  GDB must also avoid re-inserting breakpoints
in the parent as long as it didn't receive the "vfork done" event (that
is, when the child has exited or execed): since the address space is
shared with the child, that would re-insert breakpoints in the child
process also. So what GDB does is:

  1. Receive "vfork" event for the parent
  2. Remove breakpoints from the (shared) address space and set
     program_space::breakpoints_not_allowed to avoid re-inserting them
  3. Detach from the child thread
  4. Resume the parent
  5. Wait for and receive "vfork done" event for the parent
  6. Clean program_space::breakpoints_not_allowed and re-insert
     breakpoints
  7. Resume the parent

Resuming the parent at step 4 is necessary in order for the kernel to
report the "vfork done" event.  The kernel won't report a ptrace event
for a thread that is ptrace-stopped.  But the theory behind this is that
between steps 4 and 5, the parent won't actually do any progress even
though it is ptrace-resumed, because the kernel keeps it suspended,
waiting for the child to exec or exit.  So it doesn't matter for that
thread if breakpoints are not inserted.

The problem is when the program is multi-threaded.  In step 4, GDB
resumes all threads of the parent. The thread that did the vfork stays
suspended by the kernel, so that's fine. But other threads are running
freely while breakpoints are removed, which is a problem because they
could miss a breakpoint that they should have hit.

The problem is present with all-stop and non-stop targets.  The only
difference is that with an all-stop targets, the other threads are
stopped by the target when it reports the vfork event and are resumed by
the target when GDB resumes the parent.  With a non-stop target, the
other threads are simply never stopped.

The fix
-------

There many combinations of settings to consider (all-stop/non-stop,
target-non-stop on/off, follow-fork-mode parent/child, detach-on-fork
on/off, schedule-multiple on/off), but for this patch I restrict the
scope to follow-fork-mode=parent, detach-on-fork=on.  That's the
"default" case, where we detach the child and keep debugging the
parent.  I tried to fix them all, but it's just too much to do at once.
The code paths and behaviors for when we don't detach the child are
completely different.

The guiding principle for this patch is that all threads of the vforking
inferior should be stopped as long as breakpoints are removed.  This is
similar to handling in-line step-overs, in a way.

For non-stop targets (the default on Linux native), this is what
happens:

 - In follow_fork, we call stop_all_threads to stop all threads of the
   inferior
 - In follow_fork_inferior, we record the vfork parent thread in
   inferior::thread_waiting_for_vfork_done
 - Back in handle_inferior_event, we call keep_going, which resumes only
   the event thread (this is already the case, with a non-stop target).
   This is the thread that will be waiting for vfork-done.
 - When we get the vfork-done event, we go in the (new) handle_vfork_done
   function to restart the previously stopped threads.

In the same scenario, but with an all-stop target:

 - In follow_fork, no need to stop all threads of the inferior, the
   target has stopped all threads of all its inferiors before returning
   the event.
 - In follow_fork_inferior, we record the vfork parent thread in
   inferior::thread_waiting_for_vfork_done.
 - Back in handle_inferior_event, we also call keep_going.  However, we
   only want to resume the event thread here, not all inferior threads.
   In internal_resume_ptid (called by resume_1), we therefore now check
   whether one of the inferiors we are about to resume has
   thread_waiting_for_vfork_done set.  If so, we only resume that
   thread.

   Note that when resuming multiple inferiors, one vforking and one not
   non-vforking, we could resume the vforking thread from the vforking
   inferior plus all threads from the non-vforking inferior.  However,
   this is not implemented, it would require more work.
 - When we get the vfork-done event, the existing call to keep_going
   naturally resumes all threads.

Testing-wise, add a test that tries to make the main thread hit a
breakpoint while a secondary thread calls vfork.  Without the fix, the
main thread keeps going while breakpoints are removed, resulting in a
missed breakpoint and the program exiting.

Change-Id: I20eb78e17ca91f93c19c2b89a7e12c382ee814a1
gdb/infrun.c
gdb/testsuite/gdb.threads/vfork-multi-thread.c [new file with mode: 0644]
gdb/testsuite/gdb.threads/vfork-multi-thread.exp [new file with mode: 0644]