Re: [PATCH 02/15 v2] Remove some GDBSERVER checks from linux-ptrace

Pedro Alves
2014-07-17T16:35:23Z

On 07/16/2014 03:17 PM, Gary Benson wrote:
> This patch removes some GDBSERVER checks from nat/linux-ptrace.c.
> Currently the code uses a compile-time check to decide whether some
> flags should be used. This changes the code to instead let users of
> the module specify an additional set of flags; and then changes gdb's
> linux-nat.c to call this function.

> At some later date, when the back
> ends are fully merged, we will be able to remove this function again.

> @@ -37,6 +37,10 @@
> there are no supported features. */
> static int current_ptrace_options = -1;
>
> +/* Additional flags to test. */
> +
> +static int additional_flags;
> +
> /* Find all possible reasons we could fail to attach PID and append
> these as strings to the already initialized BUFFER. '\0'
> termination of BUFFER must be done by the caller. */
> @@ -359,16 +363,15 @@ linux_check_ptrace_features (void)
> static void
> linux_test_for_tracesysgood (int child_pid)
> {
> -#ifdef GDBSERVER
> - /* gdbserver does not support PTRACE_O_TRACESYSGOOD. */
> -#else
> - int ret;
> + if ((additional_flags & PTRACE_O_TRACESYSGOOD) != 0)
> + {

Please avoid this indenting unless necessary, by
reversing the logic:

if ((additional_flags & PTRACE_O_TRACESYSGOOD) == 0)
return;

As this will end up being undone once additional_flags is
removed again later on, this way avoid re-indenting back,
and complicating history / git blame.

> + int ret;
>
> - ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
> - (PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD);
> - if (ret == 0)
> - current_ptrace_options |= PTRACE_O_TRACESYSGOOD;
> -#endif
> + ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
> + (PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD);
> + if (ret != 0)
> + additional_flags &= ~PTRACE_O_TRACESYSGOOD;

This isn't right. The function should still update
current_ptrace_options (the effectively enabled options).

Otherwise ...

> + }
> }
>
> /* Determine if PTRACE_O_TRACEFORK can be used to follow fork
> @@ -388,16 +391,15 @@ linux_test_for_tracefork (int child_pid)
> if (ret != 0)
> return;
>
> -#ifdef GDBSERVER
> - /* gdbserver does not support PTRACE_O_TRACEVFORKDONE yet. */
> -#else
> - /* 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)
> - current_ptrace_options |= PTRACE_O_TRACEVFORKDONE;
> -#endif
> + if ((additional_flags & PTRACE_O_TRACEVFORKDONE) != 0)
> + {
> + /* 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)
> + additional_flags &= ~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
> @@ -433,18 +435,7 @@ linux_test_for_tracefork (int child_pid)
>
> /* We got the PID from the grandchild, which means fork
> tracing is supported. */
> -#ifdef GDBSERVER
> - /* Do not enable all the options for now since gdbserver does not
> - properly support them. This restriction will be lifted when
> - gdbserver is augmented to support them. */
> - current_ptrace_options |= PTRACE_O_TRACECLONE;
> -#else
> - current_ptrace_options |= PTRACE_O_TRACEFORK | PTRACE_O_TRACEVFORK
> - | PTRACE_O_TRACECLONE | PTRACE_O_TRACEEXEC;
> -
> - /* Do not enable PTRACE_O_TRACEEXIT until GDB is more prepared to
> - support read-only process state. */
> -#endif
> + current_ptrace_options |= PTRACE_O_TRACECLONE | additional_flags;

... note how PTRACE_O_TRACESYSGOOD is lost if PTRACE_O_TRACECLONE
isn't supported.

So don't clear additional_flags. Instead add to current_ptrace_options
as options required in additional_flags are detected to be supported.

--
Pedro Alves