[PATCHv11,0/5] Infcalls from B/P conditions in multi-threaded inferiors

Message ID cover.1709653159.git.aburgess@redhat.com
Headers
Series Infcalls from B/P conditions in multi-threaded inferiors |

Message

Andrew Burgess March 5, 2024, 3:40 p.m. UTC
  In v11:

  - Rebased onto current upstream/master, only merge conflict was in
    NEWS file.  Retested with no regressions.

In v10:

  - Rebased onto current upstream/master, I've updated the copyright
    dates from 2023 to 2024.

  - No merge conflicts (other than NEWS) and no build problems.

In v9:

  - Dropped previous patch #4 (gdb/remote: avoid SIGINT after calling
    remote_target::stop) after feedback from Pedro.  This patch wasn't
    essential for this series, instead I've just updated some of the
    tests to expect different output from remote targets.

  - Updated to more recent master, and applied a small fix due to the
    recent C++17 changes, nothing huge.

  - Retested on unix, native-gdbserver, and native-extended-gdbserver
    boards.

In v8:

  - Rebased onto current upstream/master and retested.

  - Addressed feedback from Baris on patches 1, 2, 3, & 4.  This was
    mostly minor stuff, comment typos and some improvements to the
    testsuite.  There was one real (but minor) GDB change in patch 4.

In v7:

  - Rebased onto current upstream/master, fixed use of
    find_thread_ptid which is now a process_stratum_target method,

  - Retested, with no regressions seen.

In v6:

  - Pushed the 5 initial patches.  These were the smaller fixes, and,
    I felt were pretty obvious fixes.  I'm sure folk will raise
    objections if they disagree.

  - Of the remaining patches, #1 to #5 really needs review before they
    can be merged.  Patch #6 is an obvious cleanup once the first five
    have been merged.

  - I've rebased onto current HEAD of master, there's no significant
    changes.

  - All other documentation changes have been reviewed and approved.

In v5:

  - Rebased to current HEAD of master, minor merge conflict resolved.
    No other code or documentation changes.

  - First patch, which was pure documentation, has now been merged.

  - All other documentation changes have been reviewed and approved.

In v4:

  - I believe all the docs changes have been reviewed and approved by Eli,

  - Rebased onto current master,

  - Dropped patch #2 from the V3 series,

  - I have addressed all the issues Baris pointed out, including the
    fixes for the patch #9 ('gdb: add timeouts for inferior function
    calls'), which I forgot to do in V3.

In v3:

  - Updates for review feedback, biggest changes in #10 and #11, but
    minor changes to most patches.

In V2:

  - Rebased onto something closer to HEAD of master,

  - Patches #1, #2, #12, and #13 are new in this series,

  - Patches #3 to #9, and #11 are unchanged since their V1 iteration,

  - Patches #10 has changed slightly in implementation since v1, and
    the docs have been significantly updated.

---

Andrew Burgess (5):
  Revert "gdb: remove unnecessary parameter wait_ptid from
    do_target_wait"
  gdb: fix b/p conditions with infcalls in multi-threaded inferiors
  gdb: add timeouts for inferior function calls
  gdb: introduce unwind-on-timeout setting
  gdb: rename unwindonsignal to unwind-on-signal

 gdb/NEWS                                      |  36 +++
 gdb/breakpoint.c                              |   2 +
 gdb/doc/gdb.texinfo                           | 111 ++++++-
 gdb/gdbthread.h                               |   3 +
 gdb/infcall.c                                 | 301 +++++++++++++++++-
 gdb/infrun.c                                  |  70 +++-
 gdb/infrun.h                                  |   3 +-
 gdb/testsuite/gdb.base/callfuncs.exp          |   4 +-
 gdb/testsuite/gdb.base/help.exp               |   2 +-
 gdb/testsuite/gdb.base/infcall-failure.exp    |   4 +-
 gdb/testsuite/gdb.base/infcall-timeout.c      |  36 +++
 gdb/testsuite/gdb.base/infcall-timeout.exp    | 115 +++++++
 gdb/testsuite/gdb.base/unwindonsignal.exp     |  36 ++-
 gdb/testsuite/gdb.compile/compile-cplus.exp   |   6 +-
 gdb/testsuite/gdb.compile/compile.exp         |   6 +-
 gdb/testsuite/gdb.cp/gdb2495.exp              |  16 +-
 gdb/testsuite/gdb.fortran/function-calls.exp  |   2 +-
 gdb/testsuite/gdb.mi/mi-condbreak-fail.exp    |   6 +-
 gdb/testsuite/gdb.mi/mi-condbreak-throw.exp   |   2 +-
 gdb/testsuite/gdb.mi/mi-syn-frame.exp         |   2 +-
 .../infcall-from-bp-cond-other-thread-event.c | 135 ++++++++
 ...nfcall-from-bp-cond-other-thread-event.exp | 174 ++++++++++
 .../gdb.threads/infcall-from-bp-cond-simple.c |  89 ++++++
 .../infcall-from-bp-cond-simple.exp           | 235 ++++++++++++++
 .../gdb.threads/infcall-from-bp-cond-single.c | 139 ++++++++
 .../infcall-from-bp-cond-single.exp           | 117 +++++++
 .../infcall-from-bp-cond-timeout.c            | 169 ++++++++++
 .../infcall-from-bp-cond-timeout.exp          | 191 +++++++++++
 .../gdb.threads/thread-unwindonsignal.exp     |   8 +-
 29 files changed, 1943 insertions(+), 77 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/infcall-timeout.c
 create mode 100644 gdb/testsuite/gdb.base/infcall-timeout.exp
 create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-other-thread-event.c
 create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-other-thread-event.exp
 create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-simple.c
 create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-simple.exp
 create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-single.c
 create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-single.exp
 create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.c
 create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp


base-commit: a7ea089b0bccb6379e079e2ab764c2012d94b472
  

Comments

Keith Seitz March 14, 2024, 4:08 p.m. UTC | #1
On 3/5/24 07:40, Andrew Burgess wrote:
> In v11:
> 
>    - Rebased onto current upstream/master, only merge conflict was in
>      NEWS file.  Retested with no regressions.
> 

I have lightly perused this series and run tests on an internal infra,
covering all supported Fedora 38 architectures. Having found no issues
or regressions, I think it would be great to see this series released 
with GDB 15.

Keith
  
Luis Machado March 15, 2024, 1:26 p.m. UTC | #2
On 3/5/24 15:40, Andrew Burgess wrote:
> In v11:
> 
>   - Rebased onto current upstream/master, only merge conflict was in
>     NEWS file.  Retested with no regressions.
> 
> In v10:
> 
>   - Rebased onto current upstream/master, I've updated the copyright
>     dates from 2023 to 2024.
> 
>   - No merge conflicts (other than NEWS) and no build problems.
> 
> In v9:
> 
>   - Dropped previous patch #4 (gdb/remote: avoid SIGINT after calling
>     remote_target::stop) after feedback from Pedro.  This patch wasn't
>     essential for this series, instead I've just updated some of the
>     tests to expect different output from remote targets.
> 
>   - Updated to more recent master, and applied a small fix due to the
>     recent C++17 changes, nothing huge.
> 
>   - Retested on unix, native-gdbserver, and native-extended-gdbserver
>     boards.
> 
> In v8:
> 
>   - Rebased onto current upstream/master and retested.
> 
>   - Addressed feedback from Baris on patches 1, 2, 3, & 4.  This was
>     mostly minor stuff, comment typos and some improvements to the
>     testsuite.  There was one real (but minor) GDB change in patch 4.
> 
> In v7:
> 
>   - Rebased onto current upstream/master, fixed use of
>     find_thread_ptid which is now a process_stratum_target method,
> 
>   - Retested, with no regressions seen.
> 
> In v6:
> 
>   - Pushed the 5 initial patches.  These were the smaller fixes, and,
>     I felt were pretty obvious fixes.  I'm sure folk will raise
>     objections if they disagree.
> 
>   - Of the remaining patches, #1 to #5 really needs review before they
>     can be merged.  Patch #6 is an obvious cleanup once the first five
>     have been merged.
> 
>   - I've rebased onto current HEAD of master, there's no significant
>     changes.
> 
>   - All other documentation changes have been reviewed and approved.
> 
> In v5:
> 
>   - Rebased to current HEAD of master, minor merge conflict resolved.
>     No other code or documentation changes.
> 
>   - First patch, which was pure documentation, has now been merged.
> 
>   - All other documentation changes have been reviewed and approved.
> 
> In v4:
> 
>   - I believe all the docs changes have been reviewed and approved by Eli,
> 
>   - Rebased onto current master,
> 
>   - Dropped patch #2 from the V3 series,
> 
>   - I have addressed all the issues Baris pointed out, including the
>     fixes for the patch #9 ('gdb: add timeouts for inferior function
>     calls'), which I forgot to do in V3.
> 
> In v3:
> 
>   - Updates for review feedback, biggest changes in #10 and #11, but
>     minor changes to most patches.
> 
> In V2:
> 
>   - Rebased onto something closer to HEAD of master,
> 
>   - Patches #1, #2, #12, and #13 are new in this series,
> 
>   - Patches #3 to #9, and #11 are unchanged since their V1 iteration,
> 
>   - Patches #10 has changed slightly in implementation since v1, and
>     the docs have been significantly updated.
> 
> ---
> 
> Andrew Burgess (5):
>   Revert "gdb: remove unnecessary parameter wait_ptid from
>     do_target_wait"
>   gdb: fix b/p conditions with infcalls in multi-threaded inferiors
>   gdb: add timeouts for inferior function calls
>   gdb: introduce unwind-on-timeout setting
>   gdb: rename unwindonsignal to unwind-on-signal
> 
>  gdb/NEWS                                      |  36 +++
>  gdb/breakpoint.c                              |   2 +
>  gdb/doc/gdb.texinfo                           | 111 ++++++-
>  gdb/gdbthread.h                               |   3 +
>  gdb/infcall.c                                 | 301 +++++++++++++++++-
>  gdb/infrun.c                                  |  70 +++-
>  gdb/infrun.h                                  |   3 +-
>  gdb/testsuite/gdb.base/callfuncs.exp          |   4 +-
>  gdb/testsuite/gdb.base/help.exp               |   2 +-
>  gdb/testsuite/gdb.base/infcall-failure.exp    |   4 +-
>  gdb/testsuite/gdb.base/infcall-timeout.c      |  36 +++
>  gdb/testsuite/gdb.base/infcall-timeout.exp    | 115 +++++++
>  gdb/testsuite/gdb.base/unwindonsignal.exp     |  36 ++-
>  gdb/testsuite/gdb.compile/compile-cplus.exp   |   6 +-
>  gdb/testsuite/gdb.compile/compile.exp         |   6 +-
>  gdb/testsuite/gdb.cp/gdb2495.exp              |  16 +-
>  gdb/testsuite/gdb.fortran/function-calls.exp  |   2 +-
>  gdb/testsuite/gdb.mi/mi-condbreak-fail.exp    |   6 +-
>  gdb/testsuite/gdb.mi/mi-condbreak-throw.exp   |   2 +-
>  gdb/testsuite/gdb.mi/mi-syn-frame.exp         |   2 +-
>  .../infcall-from-bp-cond-other-thread-event.c | 135 ++++++++
>  ...nfcall-from-bp-cond-other-thread-event.exp | 174 ++++++++++
>  .../gdb.threads/infcall-from-bp-cond-simple.c |  89 ++++++
>  .../infcall-from-bp-cond-simple.exp           | 235 ++++++++++++++
>  .../gdb.threads/infcall-from-bp-cond-single.c | 139 ++++++++
>  .../infcall-from-bp-cond-single.exp           | 117 +++++++
>  .../infcall-from-bp-cond-timeout.c            | 169 ++++++++++
>  .../infcall-from-bp-cond-timeout.exp          | 191 +++++++++++
>  .../gdb.threads/thread-unwindonsignal.exp     |   8 +-
>  29 files changed, 1943 insertions(+), 77 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/infcall-timeout.c
>  create mode 100644 gdb/testsuite/gdb.base/infcall-timeout.exp
>  create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-other-thread-event.c
>  create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-other-thread-event.exp
>  create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-simple.c
>  create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-simple.exp
>  create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-single.c
>  create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-single.exp
>  create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.c
>  create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp
> 
> 
> base-commit: a7ea089b0bccb6379e079e2ab764c2012d94b472

Just a heads-up I tested this on aarch64-linux and observed no regressions.

Tested-By: Luis Machado <luis.machado@arm.com>
  
Andrew Burgess March 25, 2024, 5:47 p.m. UTC | #3
Andrew Burgess <aburgess@redhat.com> writes:

> In v11:
>
>   - Rebased onto current upstream/master, only merge conflict was in
>     NEWS file.  Retested with no regressions.

After the independent testing from Keith and Luis, I've gone ahead and
pushed this commit.

If there is any feedback post commit, just let me know, I'm happy to do
any additional work needed.

Thanks,
Andrew