Re: [PATCH v2 1/2] gdbserver: Set Linux ptrace options ASAP

[Josh Stone <jistone at redhat dot com>]

On 11/30/2015 10:50 AM, Josh Stone wrote:
> On 11/26/2015 02:34 AM, Pedro Alves wrote:
>> On 11/26/2015 02:53 AM, Josh Stone wrote:
>>> The ptrace options should be set as soon as we know a thread is stopped,
>>> so no events can be missed.  There's an arch-setup early return that was
>>> effectively delaying this update before, and I found for instance that
>>> the first syscall event wouldn't be properly reported with TRACESYSGOOD.
>>> It's now more similar to the way that gdb/linux-nat.c handles it.
>>
>> Hmm, I'm confused on how this resulted in the first syscall being misssed.
>> That early return happens when we're not executing the real inferior
>> yet -- the process is still running the "gdbserver --wrapper WRAPPER"
>> binary.
> 
> My memory of this is admittedly hazy by now.  IIRC the first syscall
> wasn't *completely* missed, just reported without TRACESYSGOOD in
> effect, so it looked like a plain SIGTRAP.
> 
> I will try to dig in and characterize the problem I had better,
> especially with your explanation of exec startup at hand.  Thanks!

OK, I think I've got it, and it's a real regression from 7.10 even for
other events like fork.  I'm not using --wrapper, so I'm not sure of the
interaction there, but even gdbserver's simple fork+exec can show the
problem.  Basically, on the very first stop we don't set flags yet, so
the first resume from there continues without the right flags.

The sequence I was running into with syscalls goes like this:

- start_inferior calls create_inferior to fork, then calls mywait
  - the forked process calls ptrace(PTRACE_TRACEME), then execs
- linux_low_filter_event sees a raw SIGTRAP for the child after exec
  - (we haven't set PTRACE_O_TRACEEXEC yet, so SIGTRAP is expected)
  - arch setup is needed, so it hits the early return (new since 7.10)
    ... thus child->must_set_ptrace_flags is not dealt with
- start_inferior calls target_arch_setup
- GDB sends QCatchSyscalls:1
- linux_resume_one_lwp_throw calls ptrace(PTRACE_SYSCALL)
  - but we still haven't set any flags, especially PTRACE_O_TRACESYSGOOD
- linux_low_filter_event sees a raw SIGTRAP for the first syscall entry
  - now we finally deal with child->must_set_ptrace_flags
- linux_resume_one_lwp_throw calls ptrace(PTRACE_SYSCALL)
- linux_low_filter_event sees SYSCALL_SIGTRAP for the return
  - entry/return logic is confused now, thinks this is an entry
  - (but if there's any other event, entry/return will get back in sync)


But this problem isn't particular to my syscall patches.  Consider this
simple forking program and use 'catch fork':

  #include <unistd.h>
  int main() { fork(); return 0; }

Compiled normally, with dynamically-linked libc et al, you get:
- SIGTRAP after exec, ignores child->must_set_ptrace_flags.
- SIGTRAP for a swbreak, I guess some gdb setup, then it sets the
necessary flags, especially PTRACE_O_TRACEFORK.
- SIGTRAP for PTRACE_EVENT_FORK, hooray!

But compiled statically:
- SIGTRAP after exec, ignores child->must_set_ptrace_flags.
- CLD_EXITED, flags were never set!
- if I add a breakpoint on main, flags will be set when that's reached,
and then we do get the PTRACE_EVENT_FORK after all.


So, we need some point to get the right flags set before the program
starts running for real.  If you don't like the way I moved the flags
before that arch-setup early return, then when should we do it?

- Perhaps before the ptrace call in linux_resume_one_lwp_throw?  Then if
any state changes while the thread is still stopped, triggering new
must_set_ptrace_flags, we'll deal with it before resuming.  But I don't
know if this would interact well with your wrapper concerns.

- Perhaps at the end of linux_arch_setup?  AIUI this will be after
everything you're worried about wrappers.


>> It's pedantically good, though not crucial, to set PTRACE_O_TRACEEXEC early for
>> that scenario, to get a real PTRACE_EVENT_EXEC event instead of a bare SIGTRAP
>> when the exec wrapper (or in the future, the shell, when we start inferiors
>> with the shell, like gdb does, for arg expansion and globbing) actually execs.
>>
>> If the shell/wrapper forks, enabling fork events while still executing the
>> wrapper/shell breaks startup -- server.c:start_inferior.  The gdb
>> version (fork-child.c:startup_inferior) does handle TARGET_WAITKIND_FORKED,
>> but AFAICS forgets detaching/resuming the child...
>>
>> We _must_ not catch syscall events while running the exec wrapper (or
>> the shell), otherwise server.c:start_inferior would get confused for seeing
>> unexpected syscall stops.  If the backend treats syscall catchpoints, it's OK,
>> since gdb won't insert catchpoints in the process until after vRun returns,
>> indicating the process is stopped at the entry point.  IIRC, gdb actually
>> does NOT handle catchpoint locations per-inferior today, but as long as
>> the backend side thinks of catchpoints per-inferior, we can fix the GDB side.
>>
>> So all in all, I'm not sure this actually buys us anything other than need
>> to fix the wrapper/shell-forks case.
>>
>>>
>>> gdb/gdbserver/ChangeLog:
>>>
>>> 2015-11-25  Josh Stone  <jistone@redhat.com>
>>>
>>> 	* linux-low.c (linux_low_filter_event): Set ptrace options as soon as
>>> 	each thread is stopped, even before arch-specific setup.
>>
>> Thanks,
>> Pedro Alves
>>
>