Re: [PATCH v3 00/31] Step over thread clone and thread exit

[Pedro Alves <pedro@palves.net>]

(Slightly fixing subject, I had forgotten to include "v3" there when I posted this.)

I had planned to wait until the branch was cut before considering putting this in, and meanwhile
it's been a few weeks since that has happened.  So I am considering doing that (putting this in).
I'll wait a bit more in case someone wants to chime in.

Pedro Alves

On 2022-12-12 8:30 p.m., Pedro Alves wrote:
> Here's v3 of the series I previously posted here:
> 
>   https://inbox.sourceware.org/gdb-patches/20220713222433.374898-1-pedro@palves.net/
> 
> New in v3:
> 
>   - Addressed Simon's comments throughout.
> 
>   - Addressed Tom de Vries' comments: now builds with
>     --enable-targets=all, and the testcases should work with no glibc
>     debug info.
> 
>   - There are a few new patches:
> 
>       - [PATCH 07/31] enum_flags to_string
> 
>       Already approved by Simon.  The patches that introduce the new
>       GDB_THREAD_OPTION_XXX option flags make use of this now to
>       pretty-print the flags.
> 
>       - [PATCH 16/31] Move deleting thread on TARGET_WAITKIND_THREAD_EXITED to core
> 
>       This was half done in another patch previously.  In v3, it's
>       been moved to a separate preparatory patch, and in all-stop, we
>       now delete threads when pending-exits just before we report a
>       stop to the user.
> 
>       - [PATCH 31/31] Cancel execution command on thread exit, when stepping, nexting, etc.
> 
>       The testcase was augmented to better cover all scenarios.
> 
>   - This series now applies on top of this other series I posted last
>     week:
> 
>     [PATCH 0/6] Eliminate infrun_thread_thread_exit observer
>     https://inbox.sourceware.org/gdb-patches/20221203211338.2264994-1-pedro@palves.net/T/#t
> 
> For your convenience, I've pushed this to the
> users/palves/step-over-thread-exit-v3 branch.
> 
> Here's the series description, updated for v3:
> 
> This is a new series that replaces two different series from last
> year.
> 
> The first is this series Simon and I wrote, here:
> 
>   [PATCH 00/10] Step over thread exit (PR gdb/27338)
>   https://sourceware.org/pipermail/gdb-patches/2021-July/180567.html
> 
> The other is a series that coincidentally, back then, Andrew posted at
> about the same time, and that addressed problems in kind of the mirror
> scenario.  His patch series was about stepping over clone (creating
> new threads), instead of stepping over thread exit:
> 
>   [PATCH 0/3] Stepping over clone syscall
>   https://sourceware.org/pipermail/gdb-patches/2021-June/180517.html
> 
> My & Simon's solution back then involved adding a new contract between
> GDB and GDBserver -- if a thread is single stepping, and it exits, the
> server was supposed to report back the thread's exit to GDB.  One of
> the motivations for this approach was to be able to control the
> enablement of thread exit events per thread, to avoid creating
> thread-exit event traffic unnecessarily, as done by
> target_thread_events()/QThreadEvents.
> 
> Andrew's solution involves using the QThreadEvents mechanism, which
> tells the server to report thread create and thread exit events for
> all threads.  This would conflict with the desire to avoid unnecessary
> traffic in the step over thread exit series.
> 
> The step over clone fixes back then also weren't yet fully complete,
> as Andrew's series only addressed inline step overs.  Fixing displaced
> stepping over clone syscall would still remain broken.
> 
> This new series fixes all of stepping over thread exit and clone, for
> both of displaced stepping and inline step overs.  It:
> 
> - Merges both Andrew's and my/Simon's series, and then reworks both
>   parts in different ways.
> 
> - Introduces clone events at the GDB core and remote protocol level.
> 
> - Gets rid of the idea of "reporting thread exit if thread is
>   single-stepping", replaces it by a new mechanism GDB can use to
>   explicitly enable thread clone and/or thread exit events, and other
>   events in the future.  The old mechanism also only worked when the
>   remote server supported hardware single-stepping.  This new approach
>   has an advantage of also working on software single-step targets.
> 
> - Uses the new clone events to fix displaced stepping over clone
>   syscalls too.
> 
> - Addresses an issue that Andrew alluded to in his series, and that
>   coincidentally, we/AMD also ran into with AMDGPU debugging --
>   currently, with "set scheduler-locking on", if you step over a
>   function that spawns a thread, that thread runs free, for a bit at
>   least, and then may stop or not, basically in an unspecified manner.
> 
> - Addresses Simon's review comments on the original "Step over thread
>   exit" series referenced above.
> 
> - Centralizes "[Thread ...exited]" notifications in core code.
> 
> - Cancels next/step/until/etc. commands on thread exit event, like so:
> 
>      (gdb) n
>      [Thread 0x7ffff7d89700 (LWP 3961883) exited]
>      Command aborted, thread exited.
>      (gdb)
> 
> There are documentation changes in the following patches:
> 
>    [PATCH 23/31] Don't resume new threads if scheduler-locking is in effect
>    [PATCH 28/31] Document remote clone events, and QThreadOptions packet
> 
> ... and they have both already been approved by Eli, in v2.  (They
> haven't changed in v3).
> 
> Tested on x86-64 Ubuntu 20.04, native and gdbserver.
> 
> Andrew Burgess (1):
>   Add test for stepping over clone syscall
> 
> Pedro Alves (29):
>   displaced step: pass down target_waitstatus instead of gdb_signal
>   linux-nat: introduce pending_status_str
>   gdb/linux: Delete all other LWPs immediately on ptrace exec event
>   Step over clone syscall w/ breakpoint, TARGET_WAITKIND_THREAD_CLONED
>   Support clone events in the remote protocol
>   Avoid duplicate QThreadEvents packets
>   enum_flags to_string
>   Thread options & clone events (core + remote)
>   Thread options & clone events (native Linux)
>   Thread options & clone events (Linux GDBserver)
>   gdbserver: Hide and don't detach pending clone children
>   Remove gdb/19675 kfails (displaced stepping + clone)
>   all-stop/synchronous RSP support thread-exit events
>   gdbserver/linux-low.cc: Ignore event_ptid if TARGET_WAITKIND_IGNORE
>   Move deleting thread on TARGET_WAITKIND_THREAD_EXITED to core
>   Introduce GDB_THREAD_OPTION_EXIT thread option, fix
>     step-over-thread-exit
>   Implement GDB_THREAD_OPTION_EXIT support for Linux GDBserver
>   Implement GDB_THREAD_OPTION_EXIT support for native Linux
>   gdb: clear step over information on thread exit (PR gdb/27338)
>   stop_all_threads: (re-)enable async before waiting for stops
>   gdbserver: Queue no-resumed event after thread exit
>   Don't resume new threads if scheduler-locking is in effect
>   Report thread exit event for leader if reporting thread exit events
>   Ignore failure to read PC when resuming
>   gdb/testsuite/lib/my-syscalls.S: Refactor new SYSCALL macro
>   Document remote clone events, and QThreadOptions packet
>   inferior::clear_thread_list always silent
>   Centralize "[Thread ...exited]" notifications
>   Cancel execution command on thread exit, when stepping, nexting, etc.
> 
> Simon Marchi (1):
>   Testcases for stepping over thread exit syscall (PR gdb/27338)
> 
>  gdb/NEWS                                      |  27 +
>  gdb/annotate.c                                |   4 +-
>  gdb/breakpoint.c                              |   4 +-
>  gdb/displaced-stepping.c                      |  18 +-
>  gdb/displaced-stepping.h                      |   2 +-
>  gdb/doc/gdb.texinfo                           | 131 +++-
>  gdb/fbsd-nat.c                                |   3 -
>  gdb/gdbarch-components.py                     |   6 +-
>  gdb/gdbarch-gen.h                             |  10 +-
>  gdb/gdbarch.c                                 |   4 +-
>  gdb/gdbthread.h                               |  38 +-
>  gdb/inferior.c                                |  12 +-
>  gdb/inferior.h                                |   7 +-
>  gdb/infrun.c                                  | 615 +++++++++++++++---
>  gdb/linux-nat.c                               | 362 ++++++-----
>  gdb/linux-nat.h                               |   4 +
>  gdb/linux-tdep.c                              |   5 +-
>  gdb/linux-tdep.h                              |   2 +-
>  gdb/mi/mi-interp.c                            |   8 +-
>  gdb/netbsd-nat.c                              |   5 -
>  gdb/observable.h                              |  11 +-
>  gdb/procfs.c                                  |   6 -
>  gdb/python/py-inferior.c                      |   4 +-
>  gdb/remote.c                                  | 264 +++++++-
>  gdb/rs6000-tdep.c                             |   4 +-
>  gdb/target-debug.h                            |   2 +
>  gdb/target-delegates.c                        |  52 ++
>  gdb/target.c                                  |  16 +
>  gdb/target.h                                  |  10 +
>  gdb/target/target.c                           |  12 +
>  gdb/target/target.h                           |  20 +
>  gdb/target/waitstatus.c                       |   1 +
>  gdb/target/waitstatus.h                       |  31 +-
>  gdb/testsuite/gdb.base/step-over-syscall.exp  |  44 +-
>  .../gdb.threads/schedlock-new-thread.c        |  54 ++
>  .../gdb.threads/schedlock-new-thread.exp      |  67 ++
>  gdb/testsuite/gdb.threads/step-over-exec.exp  |   6 +
>  ...-over-thread-exit-while-stop-all-threads.c |  77 +++
>  ...ver-thread-exit-while-stop-all-threads.exp |  69 ++
>  .../gdb.threads/step-over-thread-exit.c       |  52 ++
>  .../gdb.threads/step-over-thread-exit.exp     | 151 +++++
>  gdb/testsuite/gdb.threads/stepi-over-clone.c  |  90 +++
>  .../gdb.threads/stepi-over-clone.exp          | 392 +++++++++++
>  gdb/testsuite/lib/my-syscalls.S               |  54 +-
>  gdb/testsuite/lib/my-syscalls.h               |   5 +
>  gdb/thread.c                                  |  71 +-
>  gdb/unittests/enum-flags-selftests.c          |  69 +-
>  gdb/windows-nat.c                             |  16 +-
>  gdbserver/gdbthread.h                         |   3 +
>  gdbserver/linux-low.cc                        | 399 +++++++-----
>  gdbserver/linux-low.h                         |  56 +-
>  gdbserver/remote-utils.cc                     |  26 +-
>  gdbserver/server.cc                           | 158 ++++-
>  gdbserver/target.cc                           |  15 +-
>  gdbserver/target.h                            |  27 +-
>  gdbsupport/enum-flags.h                       |  66 ++
>  56 files changed, 3043 insertions(+), 624 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.threads/schedlock-new-thread.c
>  create mode 100644 gdb/testsuite/gdb.threads/schedlock-new-thread.exp
>  create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit-while-stop-all-threads.c
>  create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit-while-stop-all-threads.exp
>  create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit.c
>  create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit.exp
>  create mode 100644 gdb/testsuite/gdb.threads/stepi-over-clone.c
>  create mode 100644 gdb/testsuite/gdb.threads/stepi-over-clone.exp
> 
> 
> base-commit: fb699bafb5f23c2fd43d7f20495171b16903b20f
> prerequisite-patch-id: 75b36824a7ee067b57a8d8db6016cb57e4d7f620
> prerequisite-patch-id: d5bf8b612deed2abf58a48c553178e080347f34d
> prerequisite-patch-id: 3f00d1a451bed27d1b1c27174e15eb604d109a44
> prerequisite-patch-id: 5c458d627f349446e40114cb5424b42af08c0824
> prerequisite-patch-id: d81a20c336a23c79dafd4da3d780033052b3ac28
> prerequisite-patch-id: 8cf0c661a601ff3eacfcd26f11e26bcd11cc867d
>