[00/31] Step over thread clone and thread exit

Message ID 20221212203101.1034916-1-pedro@palves.net
Headers
Series Step over thread clone and thread exit |

Message

Pedro Alves Dec. 12, 2022, 8:30 p.m. UTC
  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
  

Comments

Pedro Alves Jan. 24, 2023, 7:47 p.m. UTC | #1
(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
>