[PATCH] gdb/linux: remove ptrace support check for exec, fork, vfork, vforkdone, clone, sysgood

[PATCH] gdb/linux: remove ptrace support check for exec, fork, vfork, vforkdone, clone, sysgood

[Simon Marchi via Gdb-patches]

I think it's safe to remove checking support for these ptrace features,
they have all been added in what is now ancient times (around the
beginning of Linux 2.6).  This allows removing a bit of complexity in
linux-nat.c and nat/linux-ptrace.c.

It also allows saving one extra fork every time we start debugging on
Linux: linux_check_ptrace_features forks a child process to test if some
ptrace features are supported.  That child process forks a grand-child,
to test whether ptrace reports an event for the fork by the child.  This
is no longer needed, if we assume the kernel supports reporting forks.

PTRACE_O_TRACEVFORKDONE was introduced in Linux in this change, in 2003:

  https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=45c1a159b85b3b30afd26a77b4be312226bba416

PTRACE_O_TRACESYSGOOD was supported at least as of this change, in 2002:

  https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=acc7088569c8eef04eeed0eff51d23bb5bcff964

PTRACE_O_TRACEFORK, PTRACE_O_TRACEVFORK, PTRACE_O_TRACEEXEC and
PTRACE_O_TRACECLONE were introduced in this change, in 2002:

  https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=a0691b116f6a4473f0fa264210ab9b95771a2b46

Change-Id: Iffb906549a89cc6b619427f976ec044706ab1e8d
---
 gdb/linux-nat.c        |  77 ++---------------
 gdb/nat/linux-ptrace.c | 186 ++---------------------------------------
 gdb/nat/linux-ptrace.h |   5 --
 gdbserver/linux-low.cc |   9 +-
 4 files changed, 21 insertions(+), 256 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index b926fb5eba9e..39eb96e5b77a 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -535,70 +535,12 @@ linux_nat_target::follow_fork (inferior *child_inf, ptid_t child_ptid,
 
       if (has_vforked)
 	{
-	  struct lwp_info *parent_lp;
+	  lwp_info *parent_lp = find_lwp_pid (parent_ptid);
+	  linux_nat_debug_printf ("waiting for VFORK_DONE on %d", parent_pid);
+	  parent_lp->stopped = 1;
 
-	  parent_lp = find_lwp_pid (parent_ptid);
-	  gdb_assert (linux_supports_tracefork () >= 0);
-
-	  if (linux_supports_tracevforkdone ())
-	    {
-	      linux_nat_debug_printf ("waiting for VFORK_DONE on %d",
-				      parent_pid);
-	      parent_lp->stopped = 1;
-
-	      /* We'll handle the VFORK_DONE event like any other
-		 event, in target_wait.  */
-	    }
-	  else
-	    {
-	      /* We can't insert breakpoints until the child has
-		 finished with the shared memory region.  We need to
-		 wait until that happens.  Ideal would be to just
-		 call:
-		 - ptrace (PTRACE_SYSCALL, parent_pid, 0, 0);
-		 - waitpid (parent_pid, &status, __WALL);
-		 However, most architectures can't handle a syscall
-		 being traced on the way out if it wasn't traced on
-		 the way in.
-
-		 We might also think to loop, continuing the child
-		 until it exits or gets a SIGTRAP.  One problem is
-		 that the child might call ptrace with PTRACE_TRACEME.
-
-		 There's no simple and reliable way to figure out when
-		 the vforked child will be done with its copy of the
-		 shared memory.  We could step it out of the syscall,
-		 two instructions, let it go, and then single-step the
-		 parent once.  When we have hardware single-step, this
-		 would work; with software single-step it could still
-		 be made to work but we'd have to be able to insert
-		 single-step breakpoints in the child, and we'd have
-		 to insert -just- the single-step breakpoint in the
-		 parent.  Very awkward.
-
-		 In the end, the best we can do is to make sure it
-		 runs for a little while.  Hopefully it will be out of
-		 range of any breakpoints we reinsert.  Usually this
-		 is only the single-step breakpoint at vfork's return
-		 point.  */
-
-	      linux_nat_debug_printf ("no VFORK_DONE support, sleeping a bit");
-
-	      usleep (10000);
-
-	      /* Pretend we've seen a PTRACE_EVENT_VFORK_DONE event,
-		 and leave it pending.  The next linux_nat_resume call
-		 will notice a pending event, and bypasses actually
-		 resuming the inferior.  */
-	      parent_lp->status = 0;
-	      parent_lp->waitstatus.set_vfork_done ();
-	      parent_lp->stopped = 1;
-
-	      /* If we're in async mode, need to tell the event loop
-		 there's something here to process.  */
-	      if (target_is_async_p ())
-		async_file_mark ();
-	    }
+	  /* We'll handle the VFORK_DONE event like any other
+	     event, in target_wait.  */
 	}
     }
   else
@@ -615,7 +557,7 @@ linux_nat_target::follow_fork (inferior *child_inf, ptid_t child_ptid,
 int
 linux_nat_target::insert_fork_catchpoint (int pid)
 {
-  return !linux_supports_tracefork ();
+  return 0;
 }
 
 int
@@ -627,7 +569,7 @@ linux_nat_target::remove_fork_catchpoint (int pid)
 int
 linux_nat_target::insert_vfork_catchpoint (int pid)
 {
-  return !linux_supports_tracefork ();
+  return 0;
 }
 
 int
@@ -639,7 +581,7 @@ linux_nat_target::remove_vfork_catchpoint (int pid)
 int
 linux_nat_target::insert_exec_catchpoint (int pid)
 {
-  return !linux_supports_tracefork ();
+  return 0;
 }
 
 int
@@ -652,9 +594,6 @@ int
 linux_nat_target::set_syscall_catchpoint (int pid, bool needed, int any_count,
 					  gdb::array_view<const int> syscall_counts)
 {
-  if (!linux_supports_tracesysgood ())
-    return 1;
-
   /* On GNU/Linux, we ignore the arguments.  It means that we only
      enable the syscall catchpoints, but do not disable them.
 
diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
index 7258f4a0f07d..5b3086e76a6c 100644
--- a/gdb/nat/linux-ptrace.c
+++ b/gdb/nat/linux-ptrace.c
@@ -300,20 +300,6 @@ linux_fork_to_function (gdb_byte *child_stack, int (*function) (void *))
   return child_pid;
 }
 
-/* A helper function for linux_check_ptrace_features, called after
-   the child forks a grandchild.  */
-
-static int
-linux_grandchild_function (void *child_stack)
-{
-  /* Free any allocated stack.  */
-  xfree (child_stack);
-
-  /* This code is only reacheable by the grandchild (child's child)
-     process.  */
-  _exit (0);
-}
-
 /* A helper function for linux_check_ptrace_features, called after
    the parent process forks a child.  The child allows itself to
    be traced by its parent.  */
@@ -324,16 +310,11 @@ linux_child_function (void *child_stack)
   ptrace (PTRACE_TRACEME, 0, (PTRACE_TYPE_ARG3) 0, (PTRACE_TYPE_ARG4) 0);
   kill (getpid (), SIGSTOP);
 
-  /* Fork a grandchild.  */
-  linux_fork_to_function ((gdb_byte *) child_stack, linux_grandchild_function);
-
   /* This code is only reacheable by the child (grandchild's parent)
      process.  */
   _exit (0);
 }
 
-static void linux_test_for_tracesysgood (int child_pid);
-static void linux_test_for_tracefork (int child_pid);
 static void linux_test_for_exitkill (int child_pid);
 
 /* Determine ptrace features available on this target.  */
@@ -343,8 +324,15 @@ linux_check_ptrace_features (void)
 {
   int child_pid, ret, status;
 
-  /* Initialize the options.  */
-  supported_ptrace_options = 0;
+  /* Initialize the options.  We consider that these options are always
+     supported.  */
+  supported_ptrace_options
+    = (PTRACE_O_TRACESYSGOOD
+       | PTRACE_O_TRACECLONE
+       | PTRACE_O_TRACEFORK
+       | PTRACE_O_TRACEVFORK
+       | PTRACE_O_TRACEVFORKDONE
+       | PTRACE_O_TRACEEXEC);
 
   /* Fork a child so we can do some testing.  The child will call
      linux_child_function and will get traced.  The child will
@@ -362,104 +350,12 @@ linux_check_ptrace_features (void)
     error (_("linux_check_ptrace_features: waitpid: unexpected status %d."),
 	   status);
 
-  linux_test_for_tracesysgood (child_pid);
-
-  linux_test_for_tracefork (child_pid);
-
   linux_test_for_exitkill (child_pid);
 
   /* Kill child_pid.  */
   kill_child (child_pid, "linux_check_ptrace_features");
 }
 
-/* Determine if PTRACE_O_TRACESYSGOOD can be used to catch
-   syscalls.  */
-
-static void
-linux_test_for_tracesysgood (int child_pid)
-{
-  int ret;
-
-  ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
-		(PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD);
-
-  if (ret == 0)
-    supported_ptrace_options |= PTRACE_O_TRACESYSGOOD;
-}
-
-/* Determine if PTRACE_O_TRACEFORK can be used to follow fork
-   events.  */
-
-static void
-linux_test_for_tracefork (int child_pid)
-{
-  int ret, status;
-  long second_pid;
-
-  /* First, set the PTRACE_O_TRACEFORK option.  If this fails, we
-     know for sure that it is not supported.  */
-  ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
-		(PTRACE_TYPE_ARG4) PTRACE_O_TRACEFORK);
-
-  if (ret != 0)
-    return;
-
-  /* Check if the target supports PTRACE_O_TRACEVFORKDONE.  */
-  ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
-		(PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK
-				    | PTRACE_O_TRACEVFORKDONE));
-  if (ret == 0)
-    supported_ptrace_options |= PTRACE_O_TRACEVFORKDONE;
-
-  /* Setting PTRACE_O_TRACEFORK did not cause an error, however we
-     don't know for sure that the feature is available; old
-     versions of PTRACE_SETOPTIONS ignored unknown options.
-     Therefore, we attach to the child process, use PTRACE_SETOPTIONS
-     to enable fork tracing, and let it fork.  If the process exits,
-     we assume that we can't use PTRACE_O_TRACEFORK; if we get the
-     fork notification, and we can extract the new child's PID, then
-     we assume that we can.
-
-     We do not explicitly check for vfork tracing here.  It is
-     assumed that vfork tracing is available whenever fork tracing
-     is available.  */
-  ret = ptrace (PTRACE_CONT, child_pid, (PTRACE_TYPE_ARG3) 0,
-		(PTRACE_TYPE_ARG4) 0);
-  if (ret != 0)
-    warning (_("linux_test_for_tracefork: failed to resume child"));
-
-  ret = my_waitpid (child_pid, &status, 0);
-
-  /* Check if we received a fork event notification.  */
-  if (ret == child_pid && WIFSTOPPED (status)
-      && linux_ptrace_get_extended_event (status) == PTRACE_EVENT_FORK)
-    {
-      /* We did receive a fork event notification.  Make sure its PID
-	 is reported.  */
-      second_pid = 0;
-      ret = ptrace (PTRACE_GETEVENTMSG, child_pid, (PTRACE_TYPE_ARG3) 0,
-		    (PTRACE_TYPE_ARG4) &second_pid);
-      if (ret == 0 && second_pid != 0)
-	{
-	  int second_status;
-
-	  /* We got the PID from the grandchild, which means fork
-	     tracing is supported.  */
-	  supported_ptrace_options |= PTRACE_O_TRACECLONE;
-	  supported_ptrace_options |= (PTRACE_O_TRACEFORK
-				       | PTRACE_O_TRACEVFORK
-				       | PTRACE_O_TRACEEXEC);
-
-	  /* Do some cleanup and kill the grandchild.  */
-	  my_waitpid (second_pid, &second_status, 0);
-	  kill_child (second_pid, "linux_test_for_tracefork");
-	}
-    }
-  else
-    warning (_("linux_test_for_tracefork: unexpected result from waitpid "
-	     "(%d, status 0x%x)"), ret, status);
-}
-
 /* Determine if PTRACE_O_EXITKILL can be used.  */
 
 static void
@@ -507,70 +403,6 @@ linux_disable_event_reporting (pid_t pid)
   ptrace (PTRACE_SETOPTIONS, pid, (PTRACE_TYPE_ARG3) 0, 0);
 }
 
-/* Returns non-zero if PTRACE_OPTIONS is contained within
-   SUPPORTED_PTRACE_OPTIONS, therefore supported.  Returns 0
-   otherwise.  */
-
-static int
-ptrace_supports_feature (int ptrace_options)
-{
-  if (supported_ptrace_options == -1)
-    linux_check_ptrace_features ();
-
-  return ((supported_ptrace_options & ptrace_options) == ptrace_options);
-}
-
-/* Returns non-zero if PTRACE_EVENT_FORK is supported by ptrace,
-   0 otherwise.  Note that if PTRACE_EVENT_FORK is supported so is
-   PTRACE_EVENT_CLONE, PTRACE_EVENT_EXEC and PTRACE_EVENT_VFORK,
-   since they were all added to the kernel at the same time.  */
-
-int
-linux_supports_tracefork (void)
-{
-  return ptrace_supports_feature (PTRACE_O_TRACEFORK);
-}
-
-/* Returns non-zero if PTRACE_EVENT_EXEC is supported by ptrace,
-   0 otherwise.  Note that if PTRACE_EVENT_FORK is supported so is
-   PTRACE_EVENT_CLONE, PTRACE_EVENT_FORK and PTRACE_EVENT_VFORK,
-   since they were all added to the kernel at the same time.  */
-
-int
-linux_supports_traceexec (void)
-{
-  return ptrace_supports_feature (PTRACE_O_TRACEEXEC);
-}
-
-/* Returns non-zero if PTRACE_EVENT_CLONE is supported by ptrace,
-   0 otherwise.  Note that if PTRACE_EVENT_CLONE is supported so is
-   PTRACE_EVENT_FORK, PTRACE_EVENT_EXEC and PTRACE_EVENT_VFORK,
-   since they were all added to the kernel at the same time.  */
-
-int
-linux_supports_traceclone (void)
-{
-  return ptrace_supports_feature (PTRACE_O_TRACECLONE);
-}
-
-/* Returns non-zero if PTRACE_O_TRACEVFORKDONE is supported by
-   ptrace, 0 otherwise.  */
-
-int
-linux_supports_tracevforkdone (void)
-{
-  return ptrace_supports_feature (PTRACE_O_TRACEVFORKDONE);
-}
-
-/* Returns non-zero if PTRACE_O_TRACESYSGOOD is supported by ptrace,
-   0 otherwise.  */
-
-int
-linux_supports_tracesysgood (void)
-{
-  return ptrace_supports_feature (PTRACE_O_TRACESYSGOOD);
-}
-
 /* Display possible problems on this system.  Display them only once per GDB
    execution.  */
 
diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
index 2ac59a7317f0..4694046e1e8d 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -187,11 +187,6 @@ extern void linux_ptrace_init_warnings (void);
 extern void linux_check_ptrace_features (void);
 extern void linux_enable_event_reporting (pid_t pid, int attached);
 extern void linux_disable_event_reporting (pid_t pid);
-extern int linux_supports_tracefork (void);
-extern int linux_supports_traceexec (void);
-extern int linux_supports_traceclone (void);
-extern int linux_supports_tracevforkdone (void);
-extern int linux_supports_tracesysgood (void);
 extern int linux_ptrace_get_extended_event (int wstat);
 extern int linux_is_extended_waitstatus (int wstat);
 extern int linux_wstatus_maybe_breakpoint (int wstat);
diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
index d7c7336ff1f5..6bac1ced6931 100644
--- a/gdbserver/linux-low.cc
+++ b/gdbserver/linux-low.cc
@@ -6114,7 +6114,7 @@ linux_process_target::supports_multi_process ()
 bool
 linux_process_target::supports_fork_events ()
 {
-  return linux_supports_tracefork ();
+  return true;
 }
 
 /* Check if vfork events are supported.  */
@@ -6122,7 +6122,7 @@ linux_process_target::supports_fork_events ()
 bool
 linux_process_target::supports_vfork_events ()
 {
-  return linux_supports_tracefork ();
+  return true;
 }
 
 /* Check if exec events are supported.  */
@@ -6130,7 +6130,7 @@ linux_process_target::supports_vfork_events ()
 bool
 linux_process_target::supports_exec_events ()
 {
-  return linux_supports_traceexec ();
+  return true;
 }
 
 /* Target hook for 'handle_new_gdb_connection'.  Causes a reset of the
@@ -6329,8 +6329,7 @@ linux_process_target::read_loadmap (const char *annex, CORE_ADDR offset,
 bool
 linux_process_target::supports_catch_syscall ()
 {
-  return (low_supports_catch_syscall ()
-	  && linux_supports_tracesysgood ());
+  return low_supports_catch_syscall ();
 }
 
 bool

base-commit: f61defbc986e4ed6da99a3cc061e4134552f4a21
-- 
2.34.1

Re: [PATCH] gdb/linux: remove ptrace support check for exec, fork, vfork, vforkdone, clone, sysgood

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> I think it's safe to remove checking support for these ptrace features,
Simon> they have all been added in what is now ancient times (around the
Simon> beginning of Linux 2.6).  This allows removing a bit of complexity in
Simon> linux-nat.c and nat/linux-ptrace.c.

FWIW this makes sense to me.

thanks,
Tom

Re: [PATCH] gdb/linux: remove ptrace support check for exec, fork, vfork, vforkdone, clone, sysgood

[Simon Marchi via Gdb-patches]

On 2022-02-10 15:56, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> I think it's safe to remove checking support for these ptrace features,
> Simon> they have all been added in what is now ancient times (around the
> Simon> beginning of Linux 2.6).  This allows removing a bit of complexity in
> Simon> linux-nat.c and nat/linux-ptrace.c.
> 
> FWIW this makes sense to me.
> 
> thanks,
> Tom

Thanks, pushed.

Simon