[v2] Fix MI "-break-insert -g i<UNKNOWN>" assertion failure
Commit Message
Hi!
On 2026-05-04 17:22, Simon Marchi wrote:
> 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?
Yes, it's for "git spr".
>
> 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.
Could well be. git spr used to be picky until very recently and use (and require) the
non-standard format with no space. That was fixed very recently, I'll look into rebasing my
fork to pick the fix.
>
> 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).
I don't mind either way. I'll change it.
>
>> +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?
Yes, I think so.
> 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.
Done.
>
>> @@ -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.
>
I think we should just remove the early break. We always clean-restart for
each iteration, so it's not a case of cascading failures.
Here's a v2 with those changes.
From 7942a0a716a9c05e69f24cde39c32b20d2dfad24 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Wed, 9 Apr 2025 14:29:53 +0100
Subject: [PATCH v2] Fix MI "-break-insert -g i<UNKNOWN>" assertion failure
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 | 8 ++
gdb/inferior.h | 3 +
gdb/mi/mi-cmd-break.c | 3 +
.../gdb.mi/mi-thread-specific-bp.exp | 85 ++++++++++++-------
5 files changed, 72 insertions(+), 30 deletions(-)
base-commit: 8c0ac471835ec86a67c5b42713d9f138f31e4014
Comments
On 5/4/26 1:49 PM, Pedro Alves wrote:
> Hi!
>
> On 2026-05-04 17:22, Simon Marchi wrote:
>> 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?
>
> Yes, it's for "git spr".
>
>>
>> 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.
>
> Could well be. git spr used to be picky until very recently and use (and require) the
> non-standard format with no space. That was fixed very recently, I'll look into rebasing my
> fork to pick the fix.
>
>>
>> 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).
>
> I don't mind either way. I'll change it.
>
>>
>>> +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?
>
> Yes, I think so.
>
>> 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.
>
> Done.
>
>>
>>> @@ -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.
>>
> I think we should just remove the early break. We always clean-restart for
> each iteration, so it's not a case of cascading failures.
>
> Here's a v2 with those changes.
We are not using Gerrit and I can't easily diff between versions, so I
trust that you did the corrections correctly :).
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,14 @@ find_inferior_id (int num)
return NULL;
}
+/* See inferior.h. */
+
+bool
+valid_inferior_id (int num)
+{
+ return find_inferior_id (num) != 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,42 @@ 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"
+ }
+}
- # 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\\.\""
+# 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
+ }
+}
+
+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 no threads exist yet. This test doesn't make
+ # sense for inferior-specific breakpoints, as there is always
+ # at least one inferior.
+ 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 +81,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 +118,16 @@ 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
+ }
- # 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
+ do_test $mode $specificity_kind
- if { $res == -1 } {
- break
+ # 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
}
}