From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id Q666I+hp6mk+LzcAWB0awg (envelope-from ) for ; Thu, 23 Apr 2026 14:50:16 -0400 Received: by simark.ca (Postfix, from userid 112) id 7968C1E0BA; Thu, 23 Apr 2026 14:50:16 -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.3 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED,RCVD_IN_VALIDITY_RPBL_BLOCKED, RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 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 633121E093 for ; Thu, 23 Apr 2026 14:50:15 -0400 (EDT) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id B60634BAE7F5 for ; Thu, 23 Apr 2026 18:50:14 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B60634BAE7F5 Received: from mail-wm1-f53.google.com (mail-wm1-f53.google.com [209.85.128.53]) by sourceware.org (Postfix) with ESMTPS id 122A44BAE7D1 for ; Thu, 23 Apr 2026 18:49:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 122A44BAE7D1 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 122A44BAE7D1 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.128.53 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1776970189; cv=none; b=kxgGDDl1MMqYLC4HORGnG1R3HKCpJa9fTTYlEMG+UW8zebWiW9/iEtYkUX6F0j2PPjDKk5W8rtddRSv2ahZeJAc1dbBA4USF5IY2aqsJQtai7tBakUy+dyuUlderuQ/SWdc0UhtVQ/muFaQV0vh0xudxPWa/+tTKMbtbII94+Gc= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1776970189; c=relaxed/simple; bh=aAq17vrJkT8Dqlyg8bRbRdhxOsI9fCz1X2oxx1BNl7k=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=oDKnk0xtPQhk9PHokQT6ZvAkSiQHf5GQOuTG9NjaIDv8OfFmQ5OaoA2xnLHqxN0o5UMWjIkYyCv/eFHfeLmC0pRUdw4lq5D7R6F6oB/38EH0UXrhDKzcLOuRquAeZuYIpGS8yrQuA4UVAZ8Epue0AIEQB9ysObyowilCc3xmeIo= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 122A44BAE7D1 Received: by mail-wm1-f53.google.com with SMTP id 5b1f17b1804b1-488af96f6b2so89701585e9.0 for ; Thu, 23 Apr 2026 11:49:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1776970188; x=1777574988; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=yT+ECswySL4VRwHeCcYrbydl8BmdJZBvXRlH4pgH4Ho=; b=GvMOQWI516UmLEmLcc2iEI5sFzuEOiSVrfn3bJC9OQK1NAYxxMDhuzE5nzunnVrW3y kZWeaeMmRSUXoqZ1kZnhWDFFfl/68vDJ4o+dTA1OrMAc/+qoxUEg27CV8QSQXKbfuJfd lbMysj50L475fXRp3yXqt5R3qVCNkNwoKmIBvtpo32smn60FIlPetKiiZcefkUEU4/QU XewIaRwTcyZC7mv1wYxvzUzyWMANQyS1BAuxQo8Rn4q4d903p7meCW6e4Fge0tMQmYia t7vc7+pim4lsPOZmUwSTi4p43D71mOuHoo6X3FH9nBRgFfvFTvk6lSV4/bXNU7tnjtMG zqAw== X-Gm-Message-State: AOJu0YyujpkqgRBcugHmF0k+K+YY5o/Iy90PBNJeZjWzEwAnSfnPnK6f tdSrLck/3UazrplV2tO7wwyXQKZC75c2IaMq5RHkhmMdht0OPq26JHCQHHjdMw== X-Gm-Gg: AeBDiesWMpHDBBS/Y+GDhEbisL4M/K1X/ki0+KQCmyhq5lnZZ33WMdEYj56K7TSQIJ0 rynLB06Q5+H5uBG1MjskrzIlI3LyMcoKnuMZdDaco2D+O78W3FtBInxpP0CK+VGfrKfJ5Amkioh xpkNt1EE6VxlA5N5q7WNiZgu+GQ1O2RdxaTDLqDwnoHqcRL1sTEizUpjov0wOQa/IAmMjiQZ/LE 4s7Ax55+HQBKxWrdogAAczC1uiA0YxGkhH/nhDXeh/tB77fd5+AA7ocEu9Z+UolyrxbT1MQL+IS 38E9qd9/qoiyhhOR9wkqG0eKTWOvN3y1Z7krz6gvOA6SQPid8Cxk9lPCTG+b66yEpUGlC7PH3sJ OKcozZkvaPwpnI5zqHzf/EGe11+TU76tJ+nk4j6sCZJ2mv13ywh1kdGrfpB9sEutxx0vQjHAhY0 rhGT47hLJQExoxczFb5D4DtwoKds3BFPWoQ6iAWJR1jw== X-Received: by 2002:a05:600c:8183:b0:488:b187:d898 with SMTP id 5b1f17b1804b1-488fb771445mr343515545e9.14.1776970187396; Thu, 23 Apr 2026 11:49:47 -0700 (PDT) Received: from localhost ([2001:8a0:facb:a800:81e9:283:29b7:19e0]) by smtp.gmail.com with UTF8SMTPSA id 5b1f17b1804b1-48a5549f582sm72301115e9.33.2026.04.23.11.49.46 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 23 Apr 2026 11:49:47 -0700 (PDT) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH] Don't pretend infcalls don't set the inferior running (PR gdb/34082) Date: Thu, 23 Apr 2026 19:49:46 +0100 Message-ID: <20260423184946.1128623-1-pedro@palves.net> X-Mailer: git-send-email 2.53.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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 Commit 2954dd2b73 ("thread_info::executing+resumed -> thread_info::internal_state"), caused a regression in gdb.threads/hand-call-new-thread.exp: ... (gdb) PASS: gdb.threads/hand-call-new-thread.exp: iter 1: no thread marked running p new_thread () .../src/gdb/infrun.c:3742: internal-error: proceed: Assertion `!thread_is_in_step_over_chain (&tp)' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. ----- Backtrace ----- FAIL: gdb.threads/hand-call-new-thread.exp: iter 2: gdb-command

(GDB internal error) ... This commit fixes it. Let's say we have three threads, 1, 2, and 3. User does: (gdb) continue This makes GDB switch all three threads to THREAD_RUNNING. If some of those threads, now running, spawns a new thread, that thread is also set to state THREAD_RUNNING. We end up with four threads marked THREAD_RUNNING. If e.g., threads 2 and 3 both hit a breakpoint that needs to be stepped over (e.g., condition evals false), and there is only one displaced-stepping slot, then one thread starts a displaced stepping sequence, while the other is put in the step-over queue, waiting for its turn. Now, if meanwhile thread 1 hits a user-visible stop, GDB stops all threads, and transitions all their states to THREAD_STOPPED. Any thread that was still waiting for its turn in the step-over queue is removed from the queue. That happens in the THREAD_RUNNING => THREAD_STOPPED transition, here: thread_state thread_info::set_state (thread_state state, bool suppress_notification) { ... switch (m_state) { case THREAD_STOPPED: if (thread_is_in_step_over_chain (this)) global_thread_step_over_chain_remove (this); The next time the user continues execution, if the breakpoint is still inserted, proceed() sets them stepping the breakpoint again. And again, if there is more than one thread that needs to step-over, and there aren't enough slots, some threads may end up in the step-over queue. Rinse, repeat. All this works well with normal resumption commands, like continue, step, next, etc. The problem exposed by gdb.threads/hand-call-new-thread.exp is if you resume execution with an infcall instead of a normal execution command. In that case, proceed() skips transitioning (pre-existing) threads to THREAD_RUNNING, here: proceed (CORE_ADDR addr, enum gdb_signal siggnal) { ... /* Even if RESUME_PTID is a wildcard, and we end up resuming fewer threads in RESUME_PTID are now running. Unless we're calling an inferior function, as in that case we pretend the inferior doesn't run at all. */ if (!cur_thr->control.in_infcall) set_state (resume_target, resume_ptid, THREAD_RUNNING); So later, when the call finishes for any reason (normal call finish, or some other user-visible stop happens), and GDB transitions all threads to THREAD_STOPPED, we hit the early return in thread_info::set_state: thread_state thread_info::set_state (thread_state state, bool suppress_notification) { thread_state prev_state = m_state; if (prev_state == state) return prev_state; // <== EARLY RETURN m_state = state; switch (m_state) { case THREAD_STOPPED: if (thread_is_in_step_over_chain (this)) global_thread_step_over_chain_remove (this); // NOT REACHED break; ... } } ... and so if any thread had been put in the step-over queue since the last proceed(), it will incorrectly be left still in the step-over queue, with THREAD_STOPPED state. If/when the user re-resumes the program again, we trip the assertion in proceed: (gdb) p new_thread () ../../src/gdb/infrun.c:3742: internal-error: proceed: Assertion `!thread_is_in_step_over_chain (&tp)' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. Before commit 2954dd2b73 ("thread_info::executing+resumed -> thread_info::internal_state"), this didn't happen because set_running_thread(..., running=false) would remove threads from the step-over queue unconditionally, even if they were already marked stopped. I think the right fix is to stop pretending that infcalls don't set the target running. I can't think of a reason we do that. It really does run. Some thoughts: - I added to code to skip set_running (nowadays 'set_state(..., THREAD_RUNNING))' for infcalls back in commit 4d9d9d0423 over 10 years ago, but I honestly don't recall why. My guess is that it must have been to keep backwards compatibility with something, and the code has probably changed sufficiently since then making it no longer necessary. - infcalls are always synchronous, so the intermediate running state can't be observed with commands. - MI still suppresses *running The running state could potentially be observed on frontends, with e.g., MI's *running => *stopped transitions, but if that is a problem, I think it should be handled by the interpreter layer not emiting the notifications, instead of hacking the threads's core state. I.e., make it a presentation detail. I don't think it would be a problem for frontends to see *running during infcalls, but in any case, MI is already suppressing such notifications when they are caused by an infcall. See mi_interp::on_target_resumed. So e.g., if we let the threads transition to THREAD_RUNNING, with MI, we still see no *running/*stopped: (gdb) p malloc(0) &"p malloc(0)\n" ~"$2 = (void *) 0x555555560320\n" ^done (gdb) I don't know if it'd be a problem for DAP, but I assume not too. Note how the code in proceed that skips setting threads to THREAD_RUNNING only applies to already-known threads. Any new thread that appears while the infcall is ongoing will end up marked THREAD_RUNNING, causing frontend notifications. This shows how hiding the running state doesn't really hide it completely. So this is what this commit does. It lets threads transition to THREAD_RUNNING even during infcalls, fixing the problem described above, as now there will be proper THREAD_RUNNING => THREAD_STOPPED transitions when the infcall finishes. It also tweaks the testcase to spawn 10 threads per infcall instead of one. This makes it much more likely to reproduce the problem on my machine. Without it, the test still passes for me, which is why I didn't see the problem before merging 2954dd2b73. Tested on x86-64 Linux, native and gdbserver. Change-Id: I1bdc733ac865102d3f7bf0a4f7f56e6f7d75d457 commit-id:d2abd130 --- gdb/infrun.c | 9 +++------ gdb/testsuite/gdb.threads/hand-call-new-thread.c | 16 +++++++++++----- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index 6401df78e0a..0e359f0ed74 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -3688,11 +3688,8 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal) /* Even if RESUME_PTID is a wildcard, and we end up resuming fewer threads (e.g., we might need to set threads stepping over breakpoints first), from the user/frontend's point of view, all - threads in RESUME_PTID are now running. Unless we're calling an - inferior function, as in that case we pretend the inferior - doesn't run at all. */ - if (!cur_thr->control.in_infcall) - set_state (resume_target, resume_ptid, THREAD_RUNNING); + threads in RESUME_PTID are now running. */ + set_state (resume_target, resume_ptid, THREAD_RUNNING); infrun_debug_printf ("addr=%s, signal=%s, resume_ptid=%s", paddress (gdbarch, addr), @@ -6636,7 +6633,7 @@ restart_threads (struct thread_info *event_thread, inferior *inf) continue; } - if (!(tp.state () == THREAD_RUNNING || tp.control.in_infcall)) + if (tp.state () != THREAD_RUNNING) { infrun_debug_printf ("restart threads: [%s] not meant to be running", tp.ptid.to_string ().c_str ()); diff --git a/gdb/testsuite/gdb.threads/hand-call-new-thread.c b/gdb/testsuite/gdb.threads/hand-call-new-thread.c index 620322fff10..04154576351 100644 --- a/gdb/testsuite/gdb.threads/hand-call-new-thread.c +++ b/gdb/testsuite/gdb.threads/hand-call-new-thread.c @@ -34,14 +34,20 @@ thread_function (void *arg) foo (); } +#define NTHREADS 10 + void new_thread (void) { - pthread_t thread; - int res; - - res = pthread_create (&thread, NULL, thread_function, NULL); - assert (res == 0); + pthread_t thread[NTHREADS]; + int i; + + for (i = 0; i < NTHREADS; i++) + { + int res; + res = pthread_create (&thread[i], NULL, thread_function, NULL); + assert (res == 0); + } } int base-commit: 8dc535c59fdbcf99e28703425e5c1c711e63a1f1 -- 2.53.0