gdb/mi: check thread exists when creating thread-specific b/p

Message ID b227bccbcc3f1692426b843d85d6a0857e7bdc08.1678136261.git.aburgess@redhat.com
State New
Headers
Series gdb/mi: check thread exists when creating thread-specific b/p |

Commit Message

Andrew Burgess March 6, 2023, 8:58 p.m. UTC
  I noticed the following behaviour:

  $ gdb -q -i=mi /tmp/hello.x
  =thread-group-added,id="i1"
  =cmd-param-changed,param="print pretty",value="on"
  ~"Reading symbols from /tmp/hello.x...\n"
  (gdb)
  -break-insert -p 99 main
  ^done,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x0000000000401198",func="main",file="/tmp/hello.c",fullname="/tmp/hello.c",line="18",thread-groups=["i1"],thread="99",times="0",original-location="main"}
  (gdb)
  info breakpoints
  &"info breakpoints\n"
  ~"Num     Type           Disp Enb Address            What\n"
  ~"1       breakpoint     keep y   0x0000000000401198 in main at /tmp/hello.c:18\n"
  &"../../src/gdb/thread.c:1434: internal-error: print_thread_id: Assertion `thr != nullptr' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable."
  &"\n"
  &"----- Backtrace -----\n"
  &"Backtrace unavailable\n"
  &"---------------------\n"
  &"\nThis is a bug, please report it."
  &"  For instructions, see:\n"
  &"<https://www.gnu.org/software/gdb/bugs/>.\n\n"
  Aborted (core dumped)

What we see here is that when using the MI a user can create
thread-specific breakpoints for non-existent threads.  Then if we try
to use the CLI 'info breakpoints' command GDB throws an assertion.
The assert is a result of the print_thread_id call when trying to
build the 'stop only in thread xx.yy' line; print_thread_id requires a
valid thread_info pointer, which we can't have for a non-existent
thread.

In contrast, when using the CLI we see this behaviour:

  $ gdb -q /tmp/hello.x
  Reading symbols from /tmp/hello.x...
  (gdb) break main thread 99
  Unknown thread 99.
  (gdb)

The CLI doesn't allow a breakpoint to be created for a non-existent
thread.  So the 'info breakpoints' command is always fine.

Interestingly, the MI -break-info command doesn't crash, this is
because the MI uses global thread-ids, and so never calls
print_thread_id.  However, GDB does support using CLI and MI in
parallel, so we need to solve this problem.

One option would be to change the CLI behaviour to allow printing
breakpoints for non-existent threads.  This would preserve the current
MI behaviour.

The other option is to pull the MI into line with the CLI and prevent
breakpoints being created for non-existent threads.  This is good for
consistency, but is a breaking change for the MI.

In the end I figured that it was probably better to retain the
consistent CLI behaviour, and just made the MI reject requests to
place a breakpoint on a non-existent thread.  The only test we had
that depended on the old behaviour was
gdb.mi/mi-thread-specific-bp.exp, which was added by me in commit:

  commit 2fd9a436c8d24eb0af85ccb3a2fbdf9a9c679a6c
  Date:   Fri Feb 17 10:48:06 2023 +0000

      gdb: don't duplicate 'thread' field in MI breakpoint output

I certainly didn't intend for this test to rely on this feature of the
MI, so I propose to update this test to only create breakpoints for
threads that exist.

Actually, I've added a new test that checks the MI rejects creating a
breakpoint for a non-existent thread, and I've also extended the test
to run with the separate MI/CLI UIs, and then tested 'info
breakpoints' to ensure this command doesn't crash.

I've extended the documentation of the `-p` flag to explain the
constraints better.

I have also added a NEWS entry just in case someone runs into this
issue, at least then they'll know this change in behaviour was
intentional.

One thing that I did wonder about while writing this patch, is whether
we should treat requests like this, on both the MI and CLI, as another
form of pending breakpoint, something like:

  (gdb) break foo thread 9
  Thread 9 does not exist.
  Make breakpoint pending on future thread creation? (y or [n]) y
  Breakpoint 1 (foo thread 9) pending.
  (gdb) info breakpoints
  Num     Type           Disp Enb Address    What
  1       breakpoint     keep y   <PENDING>  foo thread 9

Don't know if folk think that would be a useful idea or not?  Either
way, I think that would be a separate patch from this one.
---
 gdb/NEWS                                      |  6 ++
 gdb/doc/gdb.texinfo                           |  5 +-
 gdb/mi/mi-cmd-break.c                         |  2 +
 .../gdb.mi/mi-thread-specific-bp.exp          | 68 +++++++++++++++----
 4 files changed, 67 insertions(+), 14 deletions(-)


base-commit: 6a208145d24c47912c8beb4f1f4b9abeb8d51134
  

Comments

Andrew Burgess April 3, 2023, 3:52 p.m. UTC | #1
Ping!

Thanks,
Andrew

Andrew Burgess <aburgess@redhat.com> writes:

> I noticed the following behaviour:
>
>   $ gdb -q -i=mi /tmp/hello.x
>   =thread-group-added,id="i1"
>   =cmd-param-changed,param="print pretty",value="on"
>   ~"Reading symbols from /tmp/hello.x...\n"
>   (gdb)
>   -break-insert -p 99 main
>   ^done,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x0000000000401198",func="main",file="/tmp/hello.c",fullname="/tmp/hello.c",line="18",thread-groups=["i1"],thread="99",times="0",original-location="main"}
>   (gdb)
>   info breakpoints
>   &"info breakpoints\n"
>   ~"Num     Type           Disp Enb Address            What\n"
>   ~"1       breakpoint     keep y   0x0000000000401198 in main at /tmp/hello.c:18\n"
>   &"../../src/gdb/thread.c:1434: internal-error: print_thread_id: Assertion `thr != nullptr' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable."
>   &"\n"
>   &"----- Backtrace -----\n"
>   &"Backtrace unavailable\n"
>   &"---------------------\n"
>   &"\nThis is a bug, please report it."
>   &"  For instructions, see:\n"
>   &"<https://www.gnu.org/software/gdb/bugs/>.\n\n"
>   Aborted (core dumped)
>
> What we see here is that when using the MI a user can create
> thread-specific breakpoints for non-existent threads.  Then if we try
> to use the CLI 'info breakpoints' command GDB throws an assertion.
> The assert is a result of the print_thread_id call when trying to
> build the 'stop only in thread xx.yy' line; print_thread_id requires a
> valid thread_info pointer, which we can't have for a non-existent
> thread.
>
> In contrast, when using the CLI we see this behaviour:
>
>   $ gdb -q /tmp/hello.x
>   Reading symbols from /tmp/hello.x...
>   (gdb) break main thread 99
>   Unknown thread 99.
>   (gdb)
>
> The CLI doesn't allow a breakpoint to be created for a non-existent
> thread.  So the 'info breakpoints' command is always fine.
>
> Interestingly, the MI -break-info command doesn't crash, this is
> because the MI uses global thread-ids, and so never calls
> print_thread_id.  However, GDB does support using CLI and MI in
> parallel, so we need to solve this problem.
>
> One option would be to change the CLI behaviour to allow printing
> breakpoints for non-existent threads.  This would preserve the current
> MI behaviour.
>
> The other option is to pull the MI into line with the CLI and prevent
> breakpoints being created for non-existent threads.  This is good for
> consistency, but is a breaking change for the MI.
>
> In the end I figured that it was probably better to retain the
> consistent CLI behaviour, and just made the MI reject requests to
> place a breakpoint on a non-existent thread.  The only test we had
> that depended on the old behaviour was
> gdb.mi/mi-thread-specific-bp.exp, which was added by me in commit:
>
>   commit 2fd9a436c8d24eb0af85ccb3a2fbdf9a9c679a6c
>   Date:   Fri Feb 17 10:48:06 2023 +0000
>
>       gdb: don't duplicate 'thread' field in MI breakpoint output
>
> I certainly didn't intend for this test to rely on this feature of the
> MI, so I propose to update this test to only create breakpoints for
> threads that exist.
>
> Actually, I've added a new test that checks the MI rejects creating a
> breakpoint for a non-existent thread, and I've also extended the test
> to run with the separate MI/CLI UIs, and then tested 'info
> breakpoints' to ensure this command doesn't crash.
>
> I've extended the documentation of the `-p` flag to explain the
> constraints better.
>
> I have also added a NEWS entry just in case someone runs into this
> issue, at least then they'll know this change in behaviour was
> intentional.
>
> One thing that I did wonder about while writing this patch, is whether
> we should treat requests like this, on both the MI and CLI, as another
> form of pending breakpoint, something like:
>
>   (gdb) break foo thread 9
>   Thread 9 does not exist.
>   Make breakpoint pending on future thread creation? (y or [n]) y
>   Breakpoint 1 (foo thread 9) pending.
>   (gdb) info breakpoints
>   Num     Type           Disp Enb Address    What
>   1       breakpoint     keep y   <PENDING>  foo thread 9
>
> Don't know if folk think that would be a useful idea or not?  Either
> way, I think that would be a separate patch from this one.
> ---
>  gdb/NEWS                                      |  6 ++
>  gdb/doc/gdb.texinfo                           |  5 +-
>  gdb/mi/mi-cmd-break.c                         |  2 +
>  .../gdb.mi/mi-thread-specific-bp.exp          | 68 +++++++++++++++----
>  4 files changed, 67 insertions(+), 14 deletions(-)
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index c32ff92c98a..e4a78dca2df 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -96,6 +96,12 @@ show always-read-ctf
>     without a thread restriction.  The same is also true for the 'task'
>     field of an Ada task-specific breakpoint.
>  
> +** It is no longer possible to create a thread-specific breakpoint for
> +   a thread that doesn't exist using '-break-insert -p ID'.  Creating
> +   breakpoints for non-existent threads is not allowed when using the
> +   CLI, that the MI allowed it was a long standing bug, which has now
> +   been fixed.
> +
>  *** Changes in GDB 13
>  
>  * MI version 1 is deprecated, and will be removed in GDB 14.
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index bfda7edc4f7..4a81580b061 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -32161,7 +32161,10 @@
>  Initialize the @var{ignore-count}.
>  @item -p @var{thread-id}
>  Restrict the breakpoint to the thread with the specified global
> -@var{thread-id}.
> +@var{thread-id}.  @var{thread-id} must be a valid thread-id at the
> +time the breakpoint is requested.  Breakpoints created with a
> +@var{thread-id} will automatically be deleted when the corresponding
> +thread exits.
>  @item --qualified
>  This option makes @value{GDBN} interpret a function name specified as
>  a complete fully-qualified name.
> diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
> index 75957b75bad..e5432d58990 100644
> --- a/gdb/mi/mi-cmd-break.c
> +++ b/gdb/mi/mi-cmd-break.c
> @@ -243,6 +243,8 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
>  	  break;
>  	case THREAD_OPT:
>  	  thread = atol (oarg);
> +	  if (!valid_global_thread_id (thread))
> +	    error (_("Unknown thread %d."), thread);
>  	  break;
>  	case PENDING_OPT:
>  	  pending = 1;
> diff --git a/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp b/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp
> index 4586fa44bbc..9c04d6f8c2a 100644
> --- a/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp
> +++ b/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp
> @@ -29,21 +29,63 @@ if [build_executable ${testfile}.exp ${binfile} ${srcfile}] {
>      return -1
>  }
>  
> -if {[mi_clean_restart $binfile]} {
> -    return -1
> -}
> -
> -mi_create_breakpoint "-p 1 bar" "thread-specific b/p on bar" \
> -    -thread "1"
> -
>  proc make_loc {num} {
>      return [mi_make_breakpoint_loc -thread "1" -number "$::decimal\\.$num"]
>  }
>  
> -set loc1 [make_loc 1]
> -set loc2 [make_loc 2]
> -set loc3 [make_loc 3]
> +foreach_mi_ui_mode mode {
> +
> +    if {$mode == "separate"} {
> +	set start_ops "separate-mi-tty"
> +    } else {
> +	set start_ops ""
> +    }
> +
> +    if {[mi_clean_restart $binfile $start_ops]} {
> +	return -1
> +    }
> +
> +    # Ensure we get an error when placing a b/p for thread 1 at a point
> +    # where thread 1 doesn't exist.
> +    mi_gdb_test "-break-insert -p 1 bar" \
> +	"\\^error,msg=\"Unknown thread 1\\.\""
>  
> -mi_create_breakpoint_multi "-p 1 foo" "thread-specific b/p on foo" \
> -    -thread "1" \
> -    -locations "\\\[$loc1,$loc2,$loc3\\\]"
> +    # If we have a separate CLI UI then run the 'info breakpoints'
> +    # command.  There was a time when the previous breakpoint request
> +    # would succeed, and then 'info breakpoint' on the CLI would
> +    # trigger an assertion.
> +    if {$mode eq "separate"} {
> +	with_spawn_id $gdb_main_spawn_id {
> +	    gdb_test "info breakpoints" "No breakpoints or watchpoints\\." \
> +		"check CLI 'info breakpoints' when there are no breakpoints"
> +	}
> +    }
> +
> +    if {[mi_runto_main] == -1} {
> +	return -1
> +    }
> +
> +    # Ensure we get an error when placing a b/p for a thread that doesn't
> +    # exist (when other threads do exist).
> +    mi_gdb_test "-break-insert -p 999 bar" \
> +	"\\^error,msg=\"Unknown thread 999\\.\""
> +
> +    mi_create_breakpoint "-p 1 bar" "thread-specific b/p on bar" \
> +	-thread "1"
> +
> +    set loc1 [make_loc 1]
> +    set loc2 [make_loc 2]
> +    set loc3 [make_loc 3]
> +
> +    mi_create_breakpoint_multi "-p 1 foo" "thread-specific b/p on foo" \
> +	-thread "1" \
> +	-locations "\\\[$loc1,$loc2,$loc3\\\]"
> +
> +    # Check that 'info breakpoints' on the CLI succeeds.
> +    if {$mode eq "separate"} {
> +	with_spawn_id $gdb_main_spawn_id {
> +	    gdb_test "info breakpoints" ".*" \
> +		"check CLI 'info breakpoints' when there are some breakpoints"
> +	}
> +    }
> +}
>
> base-commit: 6a208145d24c47912c8beb4f1f4b9abeb8d51134
> -- 
> 2.25.4
  
Eli Zaretskii April 3, 2023, 4:11 p.m. UTC | #2
> Date: Mon, 03 Apr 2023 16:52:26 +0100
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> 
> 
> Ping!

Sorry for the delay.  OK for the documentation parts.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
  
Andrew Burgess April 28, 2023, 11:18 p.m. UTC | #3
Andrew Burgess <aburgess@redhat.com> writes:

> I noticed the following behaviour:
>
>   $ gdb -q -i=mi /tmp/hello.x
>   =thread-group-added,id="i1"
>   =cmd-param-changed,param="print pretty",value="on"
>   ~"Reading symbols from /tmp/hello.x...\n"
>   (gdb)
>   -break-insert -p 99 main
>   ^done,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x0000000000401198",func="main",file="/tmp/hello.c",fullname="/tmp/hello.c",line="18",thread-groups=["i1"],thread="99",times="0",original-location="main"}
>   (gdb)
>   info breakpoints
>   &"info breakpoints\n"
>   ~"Num     Type           Disp Enb Address            What\n"
>   ~"1       breakpoint     keep y   0x0000000000401198 in main at /tmp/hello.c:18\n"
>   &"../../src/gdb/thread.c:1434: internal-error: print_thread_id: Assertion `thr != nullptr' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable."
>   &"\n"
>   &"----- Backtrace -----\n"
>   &"Backtrace unavailable\n"
>   &"---------------------\n"
>   &"\nThis is a bug, please report it."
>   &"  For instructions, see:\n"
>   &"<https://www.gnu.org/software/gdb/bugs/>.\n\n"
>   Aborted (core dumped)
>
> What we see here is that when using the MI a user can create
> thread-specific breakpoints for non-existent threads.  Then if we try
> to use the CLI 'info breakpoints' command GDB throws an assertion.
> The assert is a result of the print_thread_id call when trying to
> build the 'stop only in thread xx.yy' line; print_thread_id requires a
> valid thread_info pointer, which we can't have for a non-existent
> thread.
>
> In contrast, when using the CLI we see this behaviour:
>
>   $ gdb -q /tmp/hello.x
>   Reading symbols from /tmp/hello.x...
>   (gdb) break main thread 99
>   Unknown thread 99.
>   (gdb)
>
> The CLI doesn't allow a breakpoint to be created for a non-existent
> thread.  So the 'info breakpoints' command is always fine.
>
> Interestingly, the MI -break-info command doesn't crash, this is
> because the MI uses global thread-ids, and so never calls
> print_thread_id.  However, GDB does support using CLI and MI in
> parallel, so we need to solve this problem.
>
> One option would be to change the CLI behaviour to allow printing
> breakpoints for non-existent threads.  This would preserve the current
> MI behaviour.
>
> The other option is to pull the MI into line with the CLI and prevent
> breakpoints being created for non-existent threads.  This is good for
> consistency, but is a breaking change for the MI.
>
> In the end I figured that it was probably better to retain the
> consistent CLI behaviour, and just made the MI reject requests to
> place a breakpoint on a non-existent thread.  The only test we had
> that depended on the old behaviour was
> gdb.mi/mi-thread-specific-bp.exp, which was added by me in commit:
>
>   commit 2fd9a436c8d24eb0af85ccb3a2fbdf9a9c679a6c
>   Date:   Fri Feb 17 10:48:06 2023 +0000
>
>       gdb: don't duplicate 'thread' field in MI breakpoint output
>
> I certainly didn't intend for this test to rely on this feature of the
> MI, so I propose to update this test to only create breakpoints for
> threads that exist.
>
> Actually, I've added a new test that checks the MI rejects creating a
> breakpoint for a non-existent thread, and I've also extended the test
> to run with the separate MI/CLI UIs, and then tested 'info
> breakpoints' to ensure this command doesn't crash.
>
> I've extended the documentation of the `-p` flag to explain the
> constraints better.
>
> I have also added a NEWS entry just in case someone runs into this
> issue, at least then they'll know this change in behaviour was
> intentional.
>
> One thing that I did wonder about while writing this patch, is whether
> we should treat requests like this, on both the MI and CLI, as another
> form of pending breakpoint, something like:
>
>   (gdb) break foo thread 9
>   Thread 9 does not exist.
>   Make breakpoint pending on future thread creation? (y or [n]) y
>   Breakpoint 1 (foo thread 9) pending.
>   (gdb) info breakpoints
>   Num     Type           Disp Enb Address    What
>   1       breakpoint     keep y   <PENDING>  foo thread 9
>
> Don't know if folk think that would be a useful idea or not?  Either
> way, I think that would be a separate patch from this one.
> ---

I've gone ahead and pushed this change.  This was blocking another
series I'd like to post to the list.

Let me know if there are any problems.

Thanks,
Andrew



>  gdb/NEWS                                      |  6 ++
>  gdb/doc/gdb.texinfo                           |  5 +-
>  gdb/mi/mi-cmd-break.c                         |  2 +
>  .../gdb.mi/mi-thread-specific-bp.exp          | 68 +++++++++++++++----
>  4 files changed, 67 insertions(+), 14 deletions(-)
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index c32ff92c98a..e4a78dca2df 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -96,6 +96,12 @@ show always-read-ctf
>     without a thread restriction.  The same is also true for the 'task'
>     field of an Ada task-specific breakpoint.
>  
> +** It is no longer possible to create a thread-specific breakpoint for
> +   a thread that doesn't exist using '-break-insert -p ID'.  Creating
> +   breakpoints for non-existent threads is not allowed when using the
> +   CLI, that the MI allowed it was a long standing bug, which has now
> +   been fixed.
> +
>  *** Changes in GDB 13
>  
>  * MI version 1 is deprecated, and will be removed in GDB 14.
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index bfda7edc4f7..4a81580b061 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -32161,7 +32161,10 @@
>  Initialize the @var{ignore-count}.
>  @item -p @var{thread-id}
>  Restrict the breakpoint to the thread with the specified global
> -@var{thread-id}.
> +@var{thread-id}.  @var{thread-id} must be a valid thread-id at the
> +time the breakpoint is requested.  Breakpoints created with a
> +@var{thread-id} will automatically be deleted when the corresponding
> +thread exits.
>  @item --qualified
>  This option makes @value{GDBN} interpret a function name specified as
>  a complete fully-qualified name.
> diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
> index 75957b75bad..e5432d58990 100644
> --- a/gdb/mi/mi-cmd-break.c
> +++ b/gdb/mi/mi-cmd-break.c
> @@ -243,6 +243,8 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
>  	  break;
>  	case THREAD_OPT:
>  	  thread = atol (oarg);
> +	  if (!valid_global_thread_id (thread))
> +	    error (_("Unknown thread %d."), thread);
>  	  break;
>  	case PENDING_OPT:
>  	  pending = 1;
> diff --git a/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp b/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp
> index 4586fa44bbc..9c04d6f8c2a 100644
> --- a/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp
> +++ b/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp
> @@ -29,21 +29,63 @@ if [build_executable ${testfile}.exp ${binfile} ${srcfile}] {
>      return -1
>  }
>  
> -if {[mi_clean_restart $binfile]} {
> -    return -1
> -}
> -
> -mi_create_breakpoint "-p 1 bar" "thread-specific b/p on bar" \
> -    -thread "1"
> -
>  proc make_loc {num} {
>      return [mi_make_breakpoint_loc -thread "1" -number "$::decimal\\.$num"]
>  }
>  
> -set loc1 [make_loc 1]
> -set loc2 [make_loc 2]
> -set loc3 [make_loc 3]
> +foreach_mi_ui_mode mode {
> +
> +    if {$mode == "separate"} {
> +	set start_ops "separate-mi-tty"
> +    } else {
> +	set start_ops ""
> +    }
> +
> +    if {[mi_clean_restart $binfile $start_ops]} {
> +	return -1
> +    }
> +
> +    # Ensure we get an error when placing a b/p for thread 1 at a point
> +    # where thread 1 doesn't exist.
> +    mi_gdb_test "-break-insert -p 1 bar" \
> +	"\\^error,msg=\"Unknown thread 1\\.\""
>  
> -mi_create_breakpoint_multi "-p 1 foo" "thread-specific b/p on foo" \
> -    -thread "1" \
> -    -locations "\\\[$loc1,$loc2,$loc3\\\]"
> +    # If we have a separate CLI UI then run the 'info breakpoints'
> +    # command.  There was a time when the previous breakpoint request
> +    # would succeed, and then 'info breakpoint' on the CLI would
> +    # trigger an assertion.
> +    if {$mode eq "separate"} {
> +	with_spawn_id $gdb_main_spawn_id {
> +	    gdb_test "info breakpoints" "No breakpoints or watchpoints\\." \
> +		"check CLI 'info breakpoints' when there are no breakpoints"
> +	}
> +    }
> +
> +    if {[mi_runto_main] == -1} {
> +	return -1
> +    }
> +
> +    # Ensure we get an error when placing a b/p for a thread that doesn't
> +    # exist (when other threads do exist).
> +    mi_gdb_test "-break-insert -p 999 bar" \
> +	"\\^error,msg=\"Unknown thread 999\\.\""
> +
> +    mi_create_breakpoint "-p 1 bar" "thread-specific b/p on bar" \
> +	-thread "1"
> +
> +    set loc1 [make_loc 1]
> +    set loc2 [make_loc 2]
> +    set loc3 [make_loc 3]
> +
> +    mi_create_breakpoint_multi "-p 1 foo" "thread-specific b/p on foo" \
> +	-thread "1" \
> +	-locations "\\\[$loc1,$loc2,$loc3\\\]"
> +
> +    # Check that 'info breakpoints' on the CLI succeeds.
> +    if {$mode eq "separate"} {
> +	with_spawn_id $gdb_main_spawn_id {
> +	    gdb_test "info breakpoints" ".*" \
> +		"check CLI 'info breakpoints' when there are some breakpoints"
> +	}
> +    }
> +}
>
> base-commit: 6a208145d24c47912c8beb4f1f4b9abeb8d51134
> -- 
> 2.25.4
  

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index c32ff92c98a..e4a78dca2df 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -96,6 +96,12 @@  show always-read-ctf
    without a thread restriction.  The same is also true for the 'task'
    field of an Ada task-specific breakpoint.
 
+** It is no longer possible to create a thread-specific breakpoint for
+   a thread that doesn't exist using '-break-insert -p ID'.  Creating
+   breakpoints for non-existent threads is not allowed when using the
+   CLI, that the MI allowed it was a long standing bug, which has now
+   been fixed.
+
 *** Changes in GDB 13
 
 * MI version 1 is deprecated, and will be removed in GDB 14.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index bfda7edc4f7..4a81580b061 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -32161,7 +32161,10 @@ 
 Initialize the @var{ignore-count}.
 @item -p @var{thread-id}
 Restrict the breakpoint to the thread with the specified global
-@var{thread-id}.
+@var{thread-id}.  @var{thread-id} must be a valid thread-id at the
+time the breakpoint is requested.  Breakpoints created with a
+@var{thread-id} will automatically be deleted when the corresponding
+thread exits.
 @item --qualified
 This option makes @value{GDBN} interpret a function name specified as
 a complete fully-qualified name.
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 75957b75bad..e5432d58990 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -243,6 +243,8 @@  mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
 	  break;
 	case THREAD_OPT:
 	  thread = atol (oarg);
+	  if (!valid_global_thread_id (thread))
+	    error (_("Unknown thread %d."), thread);
 	  break;
 	case PENDING_OPT:
 	  pending = 1;
diff --git a/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp b/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp
index 4586fa44bbc..9c04d6f8c2a 100644
--- a/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp
+++ b/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp
@@ -29,21 +29,63 @@  if [build_executable ${testfile}.exp ${binfile} ${srcfile}] {
     return -1
 }
 
-if {[mi_clean_restart $binfile]} {
-    return -1
-}
-
-mi_create_breakpoint "-p 1 bar" "thread-specific b/p on bar" \
-    -thread "1"
-
 proc make_loc {num} {
     return [mi_make_breakpoint_loc -thread "1" -number "$::decimal\\.$num"]
 }
 
-set loc1 [make_loc 1]
-set loc2 [make_loc 2]
-set loc3 [make_loc 3]
+foreach_mi_ui_mode mode {
+
+    if {$mode == "separate"} {
+	set start_ops "separate-mi-tty"
+    } else {
+	set start_ops ""
+    }
+
+    if {[mi_clean_restart $binfile $start_ops]} {
+	return -1
+    }
+
+    # Ensure we get an error when placing a b/p for thread 1 at a point
+    # where thread 1 doesn't exist.
+    mi_gdb_test "-break-insert -p 1 bar" \
+	"\\^error,msg=\"Unknown thread 1\\.\""
 
-mi_create_breakpoint_multi "-p 1 foo" "thread-specific b/p on foo" \
-    -thread "1" \
-    -locations "\\\[$loc1,$loc2,$loc3\\\]"
+    # If we have a separate CLI UI then run the 'info breakpoints'
+    # command.  There was a time when the previous breakpoint request
+    # would succeed, and then 'info breakpoint' on the CLI would
+    # trigger an assertion.
+    if {$mode eq "separate"} {
+	with_spawn_id $gdb_main_spawn_id {
+	    gdb_test "info breakpoints" "No breakpoints or watchpoints\\." \
+		"check CLI 'info breakpoints' when there are no breakpoints"
+	}
+    }
+
+    if {[mi_runto_main] == -1} {
+	return -1
+    }
+
+    # Ensure we get an error when placing a b/p for a thread that doesn't
+    # exist (when other threads do exist).
+    mi_gdb_test "-break-insert -p 999 bar" \
+	"\\^error,msg=\"Unknown thread 999\\.\""
+
+    mi_create_breakpoint "-p 1 bar" "thread-specific b/p on bar" \
+	-thread "1"
+
+    set loc1 [make_loc 1]
+    set loc2 [make_loc 2]
+    set loc3 [make_loc 3]
+
+    mi_create_breakpoint_multi "-p 1 foo" "thread-specific b/p on foo" \
+	-thread "1" \
+	-locations "\\\[$loc1,$loc2,$loc3\\\]"
+
+    # Check that 'info breakpoints' on the CLI succeeds.
+    if {$mode eq "separate"} {
+	with_spawn_id $gdb_main_spawn_id {
+	    gdb_test "info breakpoints" ".*" \
+		"check CLI 'info breakpoints' when there are some breakpoints"
+	}
+    }
+}