Fix MI "-break-insert -g i<UNKNOWN>" assertion failure
Commit Message
Passing a non-existing inferior to -break-insert's -g option trips an
assertion:
(gdb) interpreter-exec mi "222-break-insert -g i100 foo"
&"../../src/gdb/breakpoint.c:9165: internal-error: find_program_space_for_breakpoint: Assertion `inf != nullptr' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable."
&"\n"
...
From here:
(top-gdb) bt
#0 internal_error_loc (file=0x555556187e56 "../../src/gdb/breakpoint.c", line=9165, fmt=0x555556187bc8 "%s: Assertion `%s' failed.") at ../../src/gdbsupport/errors.cc:53
#1 0x0000555555791a55 in find_program_space_for_breakpoint (thread=-1, inferior=100) at ../../src/gdb/breakpoint.c:9165
#2 0x0000555555791f96 in create_breakpoint (gdbarch=0x5555568db900, locspec=0x5555567e7100, cond_string=0x0, thread=-1, inferior=100, extra_string=0x0, force_condition=false, parse_extra=0, tempflag=0, type_wanted=bp_breakpoint, ignore_count=0, pending_break_support=AUTO_BOOLEAN_FALSE, ops=0x555556642aa0 <code_breakpoint_ops>, from_tty=0, enabled=1, internal=0, flags=0) at ../../src/gdb/breakpoint.c:9275
#3 0x0000555555bb9d15 in mi_cmd_break_insert_1 (dprintf=0, command=0x5555567e6fd0 "break-insert", argv=0x5555567e7070, argc=3) at ../../src/gdb/mi/mi-cmd-break.c:366
#4 0x0000555555bb9e15 in mi_cmd_break_insert (command=0x5555567e6fd0 "break-insert", argv=0x5555567e7070, argc=3) at ../../src/gdb/mi/mi-cmd-break.c:383
...
This commit fixes it by adding an input validation check to
mi_cmd_break_insert_1, similar to how we validate global thread
numbers for "-p THREAD", just a few lines above.
gdb.mi/mi-thread-specific-bp.exp already exercises the similar case
for thread-specific breakpoints. Extended it to test
inferior-specific breakpoints too.
In the GDB manual, describe that the inferior passed to `-g` must be
valid, exactly like commit 00cdd79a5d ("gdb/mi: check thread exists
when creating thread-specific b/p") did for `-p THREAD`
Change-Id: Ibde0d4d098bf0b5d7b057e818a77a63c84806a3c
commit-id:b791b7ee
---
I propose renaming the testcase to
gdb.mi/mi-thread-inferior-specific-bp.exp, but I'm not doing it in
this patch, as by default git diff isn't able to detect that this
would be a rename + modifications, instead presenting the diff as a
delete + new file.
Change-Id: I468e62bfb02e85d97bd43c54ea8c2e4532972434
commit-id:15849578
---
gdb/doc/gdb.texinfo | 3 +-
gdb/inferior.c | 9 ++
gdb/inferior.h | 3 +
gdb/mi/mi-cmd-break.c | 3 +
.../gdb.mi/mi-thread-specific-bp.exp | 88 +++++++++++++------
5 files changed, 76 insertions(+), 30 deletions(-)
base-commit: 8c0ac471835ec86a67c5b42713d9f138f31e4014
Comments
> From: Pedro Alves <pedro@palves.net>
> Date: Mon, 4 May 2026 15:52:42 +0100
>
> gdb/doc/gdb.texinfo | 3 +-
> gdb/inferior.c | 9 ++
> gdb/inferior.h | 3 +
> gdb/mi/mi-cmd-break.c | 3 +
> .../gdb.mi/mi-thread-specific-bp.exp | 88 +++++++++++++------
> 5 files changed, 76 insertions(+), 30 deletions(-)
Thanks, the gdb.texinfo part is approved.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
On 5/4/26 10:52 AM, Pedro Alves wrote:
> Passing a non-existing inferior to -break-insert's -g option trips an
> assertion:
>
> (gdb) interpreter-exec mi "222-break-insert -g i100 foo"
> &"../../src/gdb/breakpoint.c:9165: internal-error: find_program_space_for_breakpoint: Assertion `inf != nullptr' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable."
> &"\n"
> ...
>
> From here:
>
> (top-gdb) bt
> #0 internal_error_loc (file=0x555556187e56 "../../src/gdb/breakpoint.c", line=9165, fmt=0x555556187bc8 "%s: Assertion `%s' failed.") at ../../src/gdbsupport/errors.cc:53
> #1 0x0000555555791a55 in find_program_space_for_breakpoint (thread=-1, inferior=100) at ../../src/gdb/breakpoint.c:9165
> #2 0x0000555555791f96 in create_breakpoint (gdbarch=0x5555568db900, locspec=0x5555567e7100, cond_string=0x0, thread=-1, inferior=100, extra_string=0x0, force_condition=false, parse_extra=0, tempflag=0, type_wanted=bp_breakpoint, ignore_count=0, pending_break_support=AUTO_BOOLEAN_FALSE, ops=0x555556642aa0 <code_breakpoint_ops>, from_tty=0, enabled=1, internal=0, flags=0) at ../../src/gdb/breakpoint.c:9275
> #3 0x0000555555bb9d15 in mi_cmd_break_insert_1 (dprintf=0, command=0x5555567e6fd0 "break-insert", argv=0x5555567e7070, argc=3) at ../../src/gdb/mi/mi-cmd-break.c:366
> #4 0x0000555555bb9e15 in mi_cmd_break_insert (command=0x5555567e6fd0 "break-insert", argv=0x5555567e7070, argc=3) at ../../src/gdb/mi/mi-cmd-break.c:383
> ...
>
> This commit fixes it by adding an input validation check to
> mi_cmd_break_insert_1, similar to how we validate global thread
> numbers for "-p THREAD", just a few lines above.
>
> gdb.mi/mi-thread-specific-bp.exp already exercises the similar case
> for thread-specific breakpoints. Extended it to test
> inferior-specific breakpoints too.
>
> In the GDB manual, describe that the inferior passed to `-g` must be
> valid, exactly like commit 00cdd79a5d ("gdb/mi: check thread exists
> when creating thread-specific b/p") did for `-p THREAD`
>
> Change-Id: Ibde0d4d098bf0b5d7b057e818a77a63c84806a3c
> commit-id:b791b7ee
Is this "commit-id" trailer on purpose?
It confused "b4 shazam", because when I applied the patch locally it
converted it to:
commit-id:b791b7ee
Change-Id: Ibde0d4d098bf0b5d7b057e818a77a63c84806a3c
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Maybe it's the lack of space after the colon.
The patch LGTM, I noted some minor comments below.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
> diff --git a/gdb/inferior.c b/gdb/inferior.c
> index 1481f46cdd1..931115f46c1 100644
> --- a/gdb/inferior.c
> +++ b/gdb/inferior.c
> @@ -389,6 +389,15 @@ find_inferior_id (int num)
> return NULL;
> }
>
> +/* See inferior.h. */
> +
> +bool
> +valid_inferior_id (int num)
> +{
> + inferior *inf = find_inferior_id (num);
> + return inf != nullptr;
> +}
IMO you can get rid of the inf variable (but it's fine if it was a
conscious choice, sometimes intermediate variables make debugging
easier because that gives you something to print).
> +proc do_test { mode specificity_kind } {
> +
> + if { $specificity_kind == "thread" } {
> + # Ensure we get an error when placing a b/p for thread 1 at a
> + # point where thread 1 doesn't exist. This test doesn't make
> + # sense for inferior-specific breakpoints.
> + mi_gdb_test "-break-insert -p 1 bar" \
> + "\\^error,msg=\"Unknown thread 1\\.\""
> + }
The comment above is not clear to me. Does this mean to test adding a
thread specific breakpoints when _no_ threads exist at all? Because
below we have another similar test, but when threads exist. If so, the
comment could say "at a point where threads don't exist" instead of
"where thread 1 doesn't exist". And then I would understand why it
doesn't make sense for inferiors: because there is always at least one
inferior.
> @@ -90,17 +117,20 @@ foreach_mi_ui_mode mode {
> set start_ops ""
> }
>
> - if {[mi_clean_restart $::testfile $start_ops]} {
> - break
> - }
> + foreach_with_prefix specificity_kind {"inferior" "thread" } {
>
> - set res [do_test $mode]
> + if {[mi_clean_restart $::testfile $start_ops]} {
> + break
> + }
> +
> + set res [do_test $mode $specificity_kind]
>
> - # mi_clean_restart and gdb_finish call gdb_exit, which doesn't work for
> - # separate-mi-tty. Use mi_gdb_exit instead.
> - mi_gdb_exit
> + # mi_clean_restart and gdb_finish call gdb_exit, which doesn't
> + # work for separate-mi-tty. Use mi_gdb_exit instead.
> + mi_gdb_exit
>
> - if { $res == -1 } {
> - break
> + if { $res == -1 } {
> + break
Not a big deal but: I guess those "break"s were meant to exist the test
case completely when something goes wrong? Now, with the nested for
loops, it won't do that. We typically don't do that, unless we know for
a fact that letting the test run after some failure will cause lengthy,
cascading failures.
Simon
@@ -33515,7 +33515,8 @@ time the breakpoint is requested. Breakpoints created with a
thread exits.
@item -g @var{thread-group-id}
Restrict the breakpoint to the thread group with the specified
-@var{thread-group-id}.
+@var{thread-group-id}. @var{thread-group-id} must be a valid
+thread-group-id at the time the breakpoint is requested.
@item --qualified
This option makes @value{GDBN} interpret a function name specified as
a complete fully-qualified name.
@@ -389,6 +389,15 @@ find_inferior_id (int num)
return NULL;
}
+/* See inferior.h. */
+
+bool
+valid_inferior_id (int num)
+{
+ inferior *inf = find_inferior_id (num);
+ return inf != nullptr;
+}
+
struct inferior *
find_inferior_pid (process_stratum_target *targ, int pid)
{
@@ -754,6 +754,9 @@ extern struct inferior *find_inferior_ptid (process_stratum_target *targ,
/* Search function to lookup an inferior by GDB 'num'. */
extern struct inferior *find_inferior_id (int num);
+/* Return true if inferior NUM exists, false otherwise. */
+extern bool valid_inferior_id (int num);
+
/* Find an inferior bound to PSPACE, giving preference to the current
inferior. */
extern struct inferior *
@@ -29,6 +29,7 @@
#include "location.h"
#include "linespec.h"
#include "tracepoint.h"
+#include "inferior.h"
enum
{
@@ -248,6 +249,8 @@ mi_cmd_break_insert_1 (int dprintf, const char *command,
break;
case THREAD_GROUP_OPT:
thread_group = mi_parse_thread_group_id (oarg);
+ if (!valid_inferior_id (thread_group))
+ error (_("Unknown thread-group %d."), thread_group);
break;
case PENDING_OPT:
pending = 1;
@@ -13,8 +13,8 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# This test is for creating thread-specific breakpoint using the MI,
-# and checking the results from GDB.
+# Create thread- and inferior-specific breakpoints using the MI, and
+# check the results from GDB.
load_lib mi-support.exp
set MIFLAGS "-i=mi"
@@ -29,16 +29,41 @@ if {[build_executable ${testfile}.exp ${binfile} ${srcfile}]} {
return -1
}
-proc make_loc {num} {
- return [mi_make_breakpoint_loc -thread "1" -number "$::decimal\\.$num"]
+# SPECIFICITY_KIND can be "inferior" or "thread".
+proc make_loc {specificity_kind num} {
+ return [mi_make_breakpoint_loc -$specificity_kind "1" -number "$::decimal\\.$num"]
}
-proc do_test { mode } {
+# Return the -break-insert option for inserting a breakpoint that is
+# specific to NUM, of SPECIFICITY_KIND kind.
+proc make_opt {specificity_kind num} {
+ if { $specificity_kind == "thread" } {
+ return "-p $num"
+ } elseif { $specificity_kind == "inferior" } {
+ return "-g i$num"
+ } else {
+ error "unknown specificity kind $specificity_kind"
+ }
+}
+
+# Convert SPECIFICITY_KIND to the corresponding MI attribute.
+proc as_mi_kind {specificity_kind} {
+ if { $specificity_kind == "inferior" } {
+ return "thread-group"
+ } else {
+ return $specificity_kind
+ }
+}
- # 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\\.\""
+proc do_test { mode specificity_kind } {
+
+ if { $specificity_kind == "thread" } {
+ # Ensure we get an error when placing a b/p for thread 1 at a
+ # point where thread 1 doesn't exist. This test doesn't make
+ # sense for inferior-specific breakpoints.
+ mi_gdb_test "-break-insert -p 1 bar" \
+ "\\^error,msg=\"Unknown thread 1\\.\""
+ }
# If we have a separate CLI UI then run the 'info breakpoints'
# command. There was a time when the previous breakpoint request
@@ -55,20 +80,22 @@ proc do_test { mode } {
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\\.\""
+ # Ensure we get an error when placing a b/p for a thread/inferior
+ # that doesn't exist.
+ mi_gdb_test "-break-insert [make_opt $specificity_kind 999] bar" \
+ "\\^error,msg=\"Unknown [as_mi_kind $specificity_kind] 999\\.\""
- mi_create_breakpoint "-p 1 bar" "thread-specific b/p on bar" \
- -thread "1"
+ mi_create_breakpoint "[make_opt $specificity_kind 1] bar" \
+ "inferior-specific b/p on bar" \
+ -$specificity_kind "1"
- set loc1 [make_loc 1]
- set loc2 [make_loc 2]
- set loc3 [make_loc 3]
+ set loc1 [make_loc $specificity_kind 1]
+ set loc2 [make_loc $specificity_kind 2]
+ set loc3 [make_loc $specificity_kind 3]
- mi_create_breakpoint_multi "-p 1 foo" "thread-specific b/p on foo" \
- -thread "1" \
+ mi_create_breakpoint_multi "[make_opt $specificity_kind 1] foo" \
+ "$specificity_kind-specific b/p on foo" \
+ -$specificity_kind "1" \
-locations "\\\[$loc1,$loc2,$loc3\\\]"
# Check that 'info breakpoints' on the CLI succeeds.
@@ -90,17 +117,20 @@ foreach_mi_ui_mode mode {
set start_ops ""
}
- if {[mi_clean_restart $::testfile $start_ops]} {
- break
- }
+ foreach_with_prefix specificity_kind {"inferior" "thread" } {
- set res [do_test $mode]
+ if {[mi_clean_restart $::testfile $start_ops]} {
+ break
+ }
+
+ set res [do_test $mode $specificity_kind]
- # mi_clean_restart and gdb_finish call gdb_exit, which doesn't work for
- # separate-mi-tty. Use mi_gdb_exit instead.
- mi_gdb_exit
+ # mi_clean_restart and gdb_finish call gdb_exit, which doesn't
+ # work for separate-mi-tty. Use mi_gdb_exit instead.
+ mi_gdb_exit
- if { $res == -1 } {
- break
+ if { $res == -1 } {
+ break
+ }
}
}