From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id UH9XLuzH+GlLCBMAWB0awg (envelope-from ) for ; Mon, 04 May 2026 12:23:08 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1777911788; bh=lI5iyW8SSfZDWp+2laHiAnv8myOCUyJVw8Rq++ftBFI=; h=Date:Subject:To:References:From:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=o4Nwjosz0/EjD183mGdooN6BVBwpkpf8PDiBHkhOcWTZ3idNp4vixq4pr05alNWRL B6hEQUUmD3q3pGkvwkUBPUIfNPwDraIfxXLV7QAu/Zt8wiBsPVJm+RO9eplTDcIR3O Y81AYsClK39oPLNVCRqkkdi6Dk82l3jdiJCEQAC4= Received: by simark.ca (Postfix, from userid 112) id 96D251E067; Mon, 04 May 2026 12:23:08 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED,WEIRD_PORT autolearn=ham autolearn_force=no version=4.0.1 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=BqCiCLM8; dkim-atps=neutral Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id C0FF31E067 for ; Mon, 04 May 2026 12:23:07 -0400 (EDT) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id 4A16C4BABF31 for ; Mon, 4 May 2026 16:23:07 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4A16C4BABF31 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=BqCiCLM8 Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id C90324BABF1C for ; Mon, 4 May 2026 16:22:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C90324BABF1C Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C90324BABF1C Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1777911762; cv=none; b=pwBfFFg8Ia3o+7fhPUcGH4IrBEXr/5xbpdN17HOzzX/8PUy/JRyQCMMHwux7UnlOVhFKeJ00qO9BSqh5TjCKH0vEfTNZbYlYOuAJh4fGh2LWuF/Oy8FPcjYCBXUYhNwTxbAaGGZNA5CxBE1XfRGDa0uE41vhHXnBLjUaxnyIybI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1777911762; c=relaxed/simple; bh=lI5iyW8SSfZDWp+2laHiAnv8myOCUyJVw8Rq++ftBFI=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=P8/wrTB3fmgXlIfEatyyF8FaYTWJiCbJolriJArNQ3p1ZECSCqPYiKeMVnxtfm5ih9yl3xQ8hkxutdswuLPmftjM18DoQd/NNf+uCfjuXGpZSuruBzdA++RocJ+VB2gEj6RIsqpCRAbAZC0/87ksYRz5T68z75pytDTrwRsjKL8= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C90324BABF1C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1777911761; bh=lI5iyW8SSfZDWp+2laHiAnv8myOCUyJVw8Rq++ftBFI=; h=Date:Subject:To:References:From:In-Reply-To:From; b=BqCiCLM82dGtxz9Gc+uF9e0qtGb9F+uxg9f1Xm1w5lTOUShVzrcm4Ad/HaXACn8Nn bD9H3b6K33C+kRZCC85JH9HfSINCv4204ABvby+POK7zX1KJRroWGJJBxSAwDrUCuO lZL7QluHyv+gzoTQ1jqBx83yJcn6s7ZPn7V57GoI= Received: by simark.ca (Postfix) id 4DF571E067; Mon, 04 May 2026 12:22:41 -0400 (EDT) Message-ID: <15ed223f-5dcf-4e9d-927b-cdbfcbe57ae3@simark.ca> Date: Mon, 4 May 2026 12:22:40 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] Fix MI "-break-insert -g i" assertion failure To: Pedro Alves , gdb-patches@sourceware.org References: <20260504145242.1253541-1-pedro@palves.net> Content-Language: fr From: Simon Marchi In-Reply-To: <20260504145242.1253541-1-pedro@palves.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.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 , 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 Maybe it's the lack of space after the colon. The patch LGTM, I noted some minor comments below. Approved-By: Simon Marchi > 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