[0/8] Fix missing MI =breakpoint-deleted notifications

Message ID cover.1676901929.git.aburgess@redhat.com
Headers
Series Fix missing MI =breakpoint-deleted notifications |

Message

Andrew Burgess Feb. 20, 2023, 2:13 p.m. UTC
  This series all sprang from a single sentence in this patch review:

  https://sourceware.org/pipermail/gdb-patches/2023-February/196560.html

In this patch (adding inferior specific breakpoints), at one point, I
changed a breakpoints number to zero.  Pedro asks the very good question: 

 Doesn't this mess up MI breakpoint deleted notifications?

Clearly what I wrote was wrong.  But, confession: I copied the code in
question from the code to handle thread-specific breakpoints.  And if
Pedro's point was true for my inferior-speicifc b/p, then it must be
true for thread-specific b/p too, which means we have a bug.

Patch #1 is a trivial drive by clean up.

Patch #2 is an interesting issue I ran into relating to the MI output
while writing tests for this issue.  This patch is worth a review
because I'm proposing a slight change to the MI output.

Patches #3 to #7 are just testsuite cleanup.  There's no GDB changes
in here.  This is all about making it easier for me to write the tests
in the last patch.  You can probably skip these if you're short on
review time.

Patch #8 is the important one.  This adjusts how we handle
thread-specific breakpoints to avoid the issue Pedro pointed out
above.  This fix was mostly trivial, except for a nasty knock-on
problem in the Python FinishBreakpoints code.

---

Andrew Burgess (8):
  gdb: remove an out of date comment about disp_del_at_next_stop
  gdb: don't duplicate 'thread' field in MI breakpoint output
  gdb/testsuite: make more use of mi-support.exp
  gdb/testsuite: extend the use of mi_clean_restart
  gdb/testsuite introduce foreach_mi_ui_mode helper proc
  gdb/testsuite: introduce is_target_non_stop helper proc
  gdb/testsuite: fix failure in gdb.mi/mi-pending.exp with
    extended-remote
  gdb: fix mi breakpoint-deleted notifications for thread-specific b/p

 gdb/NEWS                                      |   8 +
 gdb/breakpoint.c                              |  10 +-
 gdb/python/py-breakpoint.c                    |   3 +
 gdb/python/py-finishbreakpoint.c              |  33 ++-
 gdb/python/python-internal.h                  |   1 +
 gdb/testsuite/gdb.base/access-mem-running.exp |  10 +-
 gdb/testsuite/gdb.base/fork-running-state.exp |  11 +-
 gdb/testsuite/gdb.mi/mi-break.exp             |  17 +-
 gdb/testsuite/gdb.mi/mi-exec-run.exp          |   1 -
 gdb/testsuite/gdb.mi/mi-multi-commands.exp    |   3 +-
 gdb/testsuite/gdb.mi/mi-nsmoribund.exp        |   7 +-
 gdb/testsuite/gdb.mi/mi-pending.exp           |  40 ++-
 gdb/testsuite/gdb.mi/mi-thread-bp-deleted.c   |  88 ++++++
 gdb/testsuite/gdb.mi/mi-thread-bp-deleted.exp | 268 ++++++++++++++++++
 gdb/testsuite/gdb.mi/mi-thread-specific-bp.c  |  44 +++
 .../gdb.mi/mi-thread-specific-bp.exp          |  49 ++++
 gdb/testsuite/gdb.mi/mi-watch.exp             |  17 +-
 gdb/testsuite/gdb.mi/new-ui-mi-sync.exp       |   8 +-
 .../gdb.mi/user-selected-context-sync.exp     |   8 +-
 .../access-mem-running-thread-exit.exp        |  10 +-
 .../gdb.threads/clone-attach-detach.exp       |  11 +-
 .../gdb.threads/detach-step-over.exp          |   8 +-
 gdb/testsuite/gdb.threads/thread-bp-deleted.c |  88 ++++++
 .../gdb.threads/thread-bp-deleted.exp         | 206 ++++++++++++++
 gdb/testsuite/lib/gdb.exp                     |  19 ++
 gdb/testsuite/lib/mi-support.exp              |  89 +++++-
 gdb/thread.c                                  |   2 +
 27 files changed, 935 insertions(+), 124 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-thread-bp-deleted.c
 create mode 100644 gdb/testsuite/gdb.mi/mi-thread-bp-deleted.exp
 create mode 100644 gdb/testsuite/gdb.mi/mi-thread-specific-bp.c
 create mode 100644 gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp
 create mode 100644 gdb/testsuite/gdb.threads/thread-bp-deleted.c
 create mode 100644 gdb/testsuite/gdb.threads/thread-bp-deleted.exp


base-commit: c9802aca6d152c4a47252f69ad81556dbc24e312
  

Comments

Pedro Alves Feb. 22, 2023, 3:23 p.m. UTC | #1
On 2023-02-20 2:13 p.m., Andrew Burgess via Gdb-patches wrote:
> This series all sprang from a single sentence in this patch review:
> 
>   https://sourceware.org/pipermail/gdb-patches/2023-February/196560.html
> 
> In this patch (adding inferior specific breakpoints), at one point, I
> changed a breakpoints number to zero.  Pedro asks the very good question: 
> 
>  Doesn't this mess up MI breakpoint deleted notifications?
> 
> Clearly what I wrote was wrong.  But, confession: I copied the code in
> question from the code to handle thread-specific breakpoints.  And if
> Pedro's point was true for my inferior-speicifc b/p, then it must be
> true for thread-specific b/p too, which means we have a bug.
> 
> Patch #1 is a trivial drive by clean up.
> 
> Patch #2 is an interesting issue I ran into relating to the MI output
> while writing tests for this issue.  This patch is worth a review
> because I'm proposing a slight change to the MI output.
> 
> Patches #3 to #7 are just testsuite cleanup.  There's no GDB changes
> in here.  This is all about making it easier for me to write the tests
> in the last patch.  You can probably skip these if you're short on
> review time.
> 
> Patch #8 is the important one.  This adjusts how we handle
> thread-specific breakpoints to avoid the issue Pedro pointed out
> above.  This fix was mostly trivial, except for a nasty knock-on
> problem in the Python FinishBreakpoints code.

I read the whole series and replied to a number of patches where I
had something to say (though nothing major).  Those I didn't reply to LGTM as is.

Thanks for doing this.  Appreciated.
  
Andrew Burgess Feb. 28, 2023, 11:09 a.m. UTC | #2
Pedro Alves <pedro@palves.net> writes:

> On 2023-02-20 2:13 p.m., Andrew Burgess via Gdb-patches wrote:
>> This series all sprang from a single sentence in this patch review:
>> 
>>   https://sourceware.org/pipermail/gdb-patches/2023-February/196560.html
>> 
>> In this patch (adding inferior specific breakpoints), at one point, I
>> changed a breakpoints number to zero.  Pedro asks the very good question: 
>> 
>>  Doesn't this mess up MI breakpoint deleted notifications?
>> 
>> Clearly what I wrote was wrong.  But, confession: I copied the code in
>> question from the code to handle thread-specific breakpoints.  And if
>> Pedro's point was true for my inferior-speicifc b/p, then it must be
>> true for thread-specific b/p too, which means we have a bug.
>> 
>> Patch #1 is a trivial drive by clean up.
>> 
>> Patch #2 is an interesting issue I ran into relating to the MI output
>> while writing tests for this issue.  This patch is worth a review
>> because I'm proposing a slight change to the MI output.
>> 
>> Patches #3 to #7 are just testsuite cleanup.  There's no GDB changes
>> in here.  This is all about making it easier for me to write the tests
>> in the last patch.  You can probably skip these if you're short on
>> review time.
>> 
>> Patch #8 is the important one.  This adjusts how we handle
>> thread-specific breakpoints to avoid the issue Pedro pointed out
>> above.  This fix was mostly trivial, except for a nasty knock-on
>> problem in the Python FinishBreakpoints code.
>
> I read the whole series and replied to a number of patches where I
> had something to say (though nothing major).  Those I didn't reply to LGTM as is.
>
> Thanks for doing this.  Appreciated.

I made the changes you suggested and pushed this series.

Thanks,
Andrew