[PATCH] Fix MI "-break-insert -g i<UNKNOWN>" assertion failure

[Pedro Alves <pedro@palves.net>]

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(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index ab0216ff477..435265bd74f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -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.
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;
+}
+
 struct inferior *
 find_inferior_pid (process_stratum_target *targ, int pid)
 {
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 9c031035a23..a483e60a7ef 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -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 *
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 1771e5208af..2231e90c192 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -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;
diff --git a/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp b/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp
index 9f250912b2c..53a0c7878d0 100644
--- a/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp
+++ b/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp
@@ -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
+	}
     }
 }

base-commit: 8c0ac471835ec86a67c5b42713d9f138f31e4014
-- 
2.53.0