From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id Af04ATJn8mlHNwQAWB0awg (envelope-from ) for ; Wed, 29 Apr 2026 16:16:50 -0400 Received: by simark.ca (Postfix, from userid 112) id 0116D1E0BA; Wed, 29 Apr 2026 16:16:49 -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_MSPIKE_H2, 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 B5D361E093 for ; Wed, 29 Apr 2026 16:16:48 -0400 (EDT) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id 5006E4BBC0B5 for ; Wed, 29 Apr 2026 20:16:48 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5006E4BBC0B5 Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) by sourceware.org (Postfix) with ESMTPS id 05B614BBCDC2 for ; Wed, 29 Apr 2026 20:15:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 05B614BBCDC2 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 05B614BBCDC2 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.128.54 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1777493732; cv=none; b=vVDu1OH7lIuxOQx3kwEqufm79ManLWZAp9yzdNAsaeueBXD3qr5WA9GPLJVZ5Nqzdcw8c+coyOWADrq8lQ/+FwU1LFKpDHhCDGA+O/0xxo0iCTcYWBn64SWhKW7GobccI1f1HSwo2SXXA9daLT+SVTFGHvYdDj0ugWGbPEEyqL0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1777493732; c=relaxed/simple; bh=8h5YFbyKqOIBZ2odYdzZB19oKC61y/bE4+smeOu+sbw=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=gXtsicYLuv98GLgYDNQn9NWTHRU0JPW5FBLiK5pkkSK5REZs2MY9NLHQu+9smR6AwqA956a+z36/OL63gA8au1EyG0buvM3spVNyrc6hH2hJX+M4UOPA0HO5Jyl9JueHtx4WaO80ROKHW8q88KqqetyRbw/CubVpDq6hESJX/MA= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 05B614BBCDC2 Received: by mail-wm1-f54.google.com with SMTP id 5b1f17b1804b1-488af96f6b2so2142825e9.0 for ; Wed, 29 Apr 2026 13:15:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1777493731; x=1778098531; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=A1bphYfgZpmJ0gdHJ4CR2yuFQ+Gpf6+fspXFyOwgwX4=; b=ffE7Q7nZD3lvQAkxRfOAlNkX73e3//vguBdAIop62Tn24pjgJMAkCtV7OsTW/Af62m EEez9lbQdB2rAIUNFd4N0q2fFvtpYkUkbGzUx/NXk2n/WViH4sLGerGmYdU8blvh690o dzJh6cgJH4/nNpCK4ktabAbbdu7NPSOqEy5jLEk9UHE1fxhGT6HcTmJ5xbH8xfkCwNx9 js9EW6YltuUV277YXwHt9FRfshA631RHSbR3sQ+f/FkGFL6KVN8fxIiRYsCTaY0TZlCg 1v26bKtjgMzWhJaccvMT4fBYsQdgFW9I0DDnD2U7VL60/2FTQmI9+Bg7f0E/9qr0nwf0 EdyA== X-Gm-Message-State: AOJu0Yzd5hZUOTFyXBOq0iD3tbr0Fz6ItBfrNLkMq3rugvz3s80Ytsys irI4uOp4WdUG9ePrwN7dfDLyn3rbvqf1gEl0uzStHn4d3H1C22+9ckhEUNRTTQ== X-Gm-Gg: AeBDievnVnrnausmYfBaD6BU04HZqOL23HOgmcqMWh6UcFjjPHcSfH0gGngmPPIcU2a H+BnxwIvmM/cRJBXWp6UZlwh8SGuxHNFPKfjkfX7rQkngo4+ZDIEF7zTvIV5Q2hZOM7vJ3q5Gzx /KGnLFLiRTqn//8MCIt489mGOcjpaiRj6GUzBFgcL9PpR38yk/GNClk7BxxxY3DE2h+E1VKw85K 9/6ESHYQWhaosq5Y4MLg4aLBEQ/rOMrAWD9a1JDDs/ch09gSRkbdh13gxst+8OgXXNRr5e+Axyh X9ZaAOxyqagIh/e3kFEVfWHhMrVW+fzqERyUfcScBAEzArL749llgXmM5Rs5egJCQy2GedT9cGX 6BbPJ7vjtTc3DVwqDm89s0Z5Axrm8twiIokZI2tnAsQ63vnQOPtpKx7xwb86g3+xCpauaRqfg0k VW044ZGGMUKDZ79ZyywOZE5MyWLgl8UiM= X-Received: by 2002:a05:600c:6285:b0:483:709e:f238 with SMTP id 5b1f17b1804b1-48a84486f35mr1111245e9.29.1777493730360; Wed, 29 Apr 2026 13:15:30 -0700 (PDT) Received: from localhost ([2001:8a0:facb:a800:f22f:591f:e700:e91]) by smtp.gmail.com with UTF8SMTPSA id 5b1f17b1804b1-48a822d4b57sm35983135e9.15.2026.04.29.13.15.29 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 29 Apr 2026 13:15:29 -0700 (PDT) From: Pedro Alves To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH v3 06/11] Windows gdb: Watchpoints while running (internal vs external stops) Date: Wed, 29 Apr 2026 21:15:02 +0100 Message-ID: <20260429201507.480870-7-pedro@palves.net> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20260429201507.480870-1-pedro@palves.net> References: <20260429201507.480870-1-pedro@palves.net> 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 Teach the Windows target to temporarily pause all threads when we change the debug registers for a watchpoint. Implements the same logic as Linux uses: ~~~ /* (...) if threads are running when the mirror changes, a temporary and transparent stop on all threads is forced so they can get their copy of the debug registers updated on re-resume. (...) */ ~~~ On Linux, we send each thread a SIGSTOP to step them. On Windows, SuspendThread itself doesn't cause any asynchronous debug event to be reported. However, we've implemented windows_nat_target::stop such that it uses SuspendThread, and then queues a pending GDB_SIGNAL_0 stop on the thread. That results in a user-visible stop, while here we want a non-user-visible stop. So what we do is re-use that windows_nat_target::stop stopping mechanism, but add an external vs internal stopping kind distinction. An internal stop results in windows_nat_target::wait immediately re-resuming the thread. Note we don't make the debug registers poking code SuspendThread -> write debug registers -> ContinueThread itself, because SuspendThread is actually asynchronous and may take a bit to stop the thread (a following GetThreadContext blocks until the thread is actually suspended), and, there will be several debug register writes when a watchpoint is set, because we have to set all of DR0, DR1, DR2, DR3, and DR7. Defering the actual writes to ::wait avoids a bunch of SuspendThread/ResumeThread sequences, so in principle should be faster. Reviewed-By: Tom Tromey Change-Id: I39c2492c7aac06d23ef8f287f4afe3747b7bc53f commit-id:22d7a7e0 --- gdb/aarch64-windows-nat.c | 3 +- gdb/nat/windows-nat.h | 27 ++++++++-- gdb/windows-nat.c | 102 ++++++++++++++++++++++++++++++++------ gdb/windows-nat.h | 11 +++- gdb/x86-windows-nat.c | 6 +-- 5 files changed, 123 insertions(+), 26 deletions(-) diff --git a/gdb/aarch64-windows-nat.c b/gdb/aarch64-windows-nat.c index e6b596a38cb..29a6c95d485 100644 --- a/gdb/aarch64-windows-nat.c +++ b/gdb/aarch64-windows-nat.c @@ -317,8 +317,7 @@ aarch64_notify_debug_reg_change (ptid_t ptid, = aarch64_get_debug_reg_state (inferior_ptid.pid ()); aarch64_windows_process.dr_state = *state; - for (auto &th : aarch64_windows_process.thread_list) - th->debug_registers_changed = true; + windows_debug_registers_changed_all_threads (); } INIT_GDB_FILE (aarch64_windows_nat) diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h index f61eab766ac..52378765438 100644 --- a/gdb/nat/windows-nat.h +++ b/gdb/nat/windows-nat.h @@ -40,6 +40,24 @@ namespace windows_nat struct windows_process_info; +/* The reason for explicitly stopping a thread. Note the enumerators + are ordered such that when comparing two stopping_kind's numerical + value, the highest should prevail. */ +enum stopping_kind + { + /* Not really stopping the thread. */ + SK_NOT_STOPPING = 0, + + /* We're stopping the thread for internal reasons, the stop should + not be reported as an event to the core. */ + SK_INTERNAL = 1, + + /* We're stopping the thread for external reasons, meaning, the + core/user asked us to stop the thread, so we must report a stop + event to the core. */ + SK_EXTERNAL = 2, + }; + /* Thread information structure used to track extra information about each thread. */ struct windows_thread_info @@ -123,9 +141,10 @@ struct windows_thread_info int suspended = 0; /* This flag indicates whether we are explicitly stopping this - thread in response to a target_stop request. This allows - distinguishing between threads that are explicitly stopped by the - debugger and threads that are stopped due to other reasons. + thread in response to a target_stop request or for + backend-internal reasons. This allows distinguishing between + threads that are explicitly stopped by the debugger and threads + that are stopped due to other reasons. Typically, when we want to stop a thread, we suspend it, enqueue a pending GDB_SIGNAL_0 stop status on the thread, and then set @@ -134,7 +153,7 @@ struct windows_thread_info already has an event to report. In such case, we simply set the 'stopping' flag without suspending the thread or enqueueing a pending stop. See stop_one_thread. */ - bool stopping = false; + stopping_kind stopping = SK_NOT_STOPPING; /* Info about a potential pending stop. diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c index d56510c23f6..6f3803c887b 100644 --- a/gdb/windows-nat.c +++ b/gdb/windows-nat.c @@ -855,7 +855,7 @@ windows_per_inferior::handle_output_debug_string a pending event. It will be picked up by windows_nat_target::wait. */ th->suspend (); - th->stopping = true; + th->stopping = SK_EXTERNAL; th->last_event = {}; th->pending_status.set_stopped (gotasig); @@ -923,7 +923,7 @@ windows_nat_target::continue_one_thread (windows_thread_info *th, thread_context_continue (th, killed); th->resume (); - th->stopping = false; + th->stopping = SK_NOT_STOPPING; th->last_sig = GDB_SIGNAL_0; } @@ -997,8 +997,19 @@ windows_nat_target::windows_continue (DWORD continue_status, int id, #endif } - if (!target_is_non_stop_p () - || (cont_flags & WCONT_CONTINUE_DEBUG_EVENT) != 0) + /* WCONT_DONT_CONTINUE_DEBUG_EVENT and WCONT_CONTINUE_DEBUG_EVENT + can't both be enabled at the same time. */ + gdb_assert ((cont_flags & WCONT_DONT_CONTINUE_DEBUG_EVENT) == 0 + || (cont_flags & WCONT_CONTINUE_DEBUG_EVENT) == 0); + + bool continue_debug_event; + if ((cont_flags & WCONT_CONTINUE_DEBUG_EVENT) != 0) + continue_debug_event = true; + else if ((cont_flags & WCONT_DONT_CONTINUE_DEBUG_EVENT) != 0) + continue_debug_event = false; + else + continue_debug_event = !target_is_non_stop_p (); + if (continue_debug_event) { DEBUG_EVENTS ("windows_continue -> continue_last_debug_event"); continue_last_debug_event_main_thread @@ -1201,11 +1212,13 @@ windows_nat_target::interrupt () "Press Ctrl-c in the program console.")); } -/* Stop thread TH. This leaves a GDB_SIGNAL_0 pending in the thread, - which is later consumed by windows_nat_target::wait. */ +/* Stop thread TH, for STOPPING_KIND reason. This leaves a + GDB_SIGNAL_0 pending in the thread, which is later consumed by + windows_nat_target::wait. */ void -windows_nat_target::stop_one_thread (windows_thread_info *th) +windows_nat_target::stop_one_thread (windows_thread_info *th, + enum stopping_kind stopping_kind) { ptid_t thr_ptid (windows_process->process_id, th->tid); @@ -1221,12 +1234,18 @@ windows_nat_target::stop_one_thread (windows_thread_info *th) #ifdef __CYGWIN__ else if (th->suspended && th->signaled_thread != nullptr - && th->pending_status.kind () == TARGET_WAITKIND_IGNORE) + && th->pending_status.kind () == TARGET_WAITKIND_IGNORE + /* If doing an internal stop to update debug registers, + then just leave the "sig" thread suspended. Otherwise + windows_nat_target::wait would incorrectly break the + signaled_thread lock when it later processes the pending + stop and calls windows_continue on this thread. */ + && stopping_kind == SK_EXTERNAL) { DEBUG_EVENTS ("explict stop for \"sig\" thread %s held for signal", thr_ptid.to_string ().c_str ()); - th->stopping = true; + th->stopping = stopping_kind; th->pending_status.set_stopped (GDB_SIGNAL_0); th->last_event = {}; serial_event_set (m_wait_event); @@ -1240,7 +1259,9 @@ windows_nat_target::stop_one_thread (windows_thread_info *th) thr_ptid.to_string ().c_str (), th->suspended, th->stopping); - th->stopping = true; + /* Upgrade stopping. */ + if (stopping_kind > th->stopping) + th->stopping = stopping_kind; } else { @@ -1255,14 +1276,20 @@ windows_nat_target::stop_one_thread (windows_thread_info *th) { DEBUG_EVENTS ("suspension of %s failed, expect thread exit event", thr_ptid.to_string ().c_str ()); + if (stopping_kind > th->stopping) + th->stopping = stopping_kind; return; } gdb_assert (th->suspended == 1); - th->stopping = true; - th->pending_status.set_stopped (GDB_SIGNAL_0); - th->last_event = {}; + if (stopping_kind > th->stopping) + { + th->stopping = stopping_kind; + th->pending_status.set_stopped (GDB_SIGNAL_0); + th->last_event = {}; + } + serial_event_set (m_wait_event); } } @@ -1275,7 +1302,7 @@ windows_nat_target::stop (ptid_t ptid) for (thread_info &thr : all_non_exited_threads (this)) { if (thr.ptid.matches (ptid)) - stop_one_thread (as_windows_thread_info (&thr)); + stop_one_thread (as_windows_thread_info (&thr), SK_EXTERNAL); } } @@ -1777,6 +1804,17 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus, { windows_thread_info *th = windows_process->find_thread (result); + /* If this thread was temporarily stopped just so we + could update its debug registers on the next + resumption, do it now. */ + if (th->stopping == SK_INTERNAL) + { + gdb_assert (fake); + windows_continue (DBG_CONTINUE, th->tid, + WCONT_DONT_CONTINUE_DEBUG_EVENT); + continue; + } + th->stopped_at_software_breakpoint = false; if (current_event.dwDebugEventCode == EXCEPTION_DEBUG_EVENT @@ -1821,7 +1859,7 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus, for (auto *thr : all_windows_threads ()) thr->suspend (); - th->stopping = false; + th->stopping = SK_NOT_STOPPING; } /* If something came out, assume there may be more. This is @@ -3404,6 +3442,40 @@ Use \"%ps\" or \"%ps\" command to load executable/libraries directly."), } } +/* For each thread, set the debug_registers_changed flag, and + temporarily stop it so we can update its debug registers. */ + +void +windows_nat_target::debug_registers_changed_all_threads () +{ + for (auto *th : all_windows_threads ()) + { + th->debug_registers_changed = true; + + /* Note we don't SuspendThread => update debug regs => + ResumeThread, because SuspendThread is actually asynchronous + (and GetThreadContext blocks until the thread really + suspends), and doing that for all threads may take a bit. + Also, the core does one call per DR register update, so that + would result in a lot of suspend-resumes. So instead, we + suspend the thread if it wasn't already suspended, and queue + a pending stop to be handled by windows_nat_target::wait. + This means we only stop each thread once, and, we don't block + waiting for each individual thread stop. */ + stop_one_thread (th, SK_INTERNAL); + } +} + +/* Trampoline helper to get at the + windows_nat_target::debug_registers_changed_all_threads method in + the native target. */ + +void +windows_debug_registers_changed_all_threads () +{ + get_windows_nat_target ()->debug_registers_changed_all_threads (); +} + /* Determine if the thread referenced by "ptid" is alive by "polling" it. If WaitForSingleObject returns WAIT_OBJECT_0 it means that the thread has died. Otherwise it is assumed to be alive. */ diff --git a/gdb/windows-nat.h b/gdb/windows-nat.h index 7ce71690e20..5ef68dff274 100644 --- a/gdb/windows-nat.h +++ b/gdb/windows-nat.h @@ -78,6 +78,10 @@ enum windows_continue_flag all-stop mode. This flag indicates that windows_continue should call ContinueDebugEvent even in non-stop mode. */ WCONT_CONTINUE_DEBUG_EVENT = 4, + + /* Skip calling ContinueDebugEvent even in all-stop mode. This is + the default in non-stop mode. */ + WCONT_DONT_CONTINUE_DEBUG_EVENT = 8, }; DEF_ENUM_FLAGS_TYPE (windows_continue_flag, windows_continue_flags); @@ -258,6 +262,8 @@ struct windows_nat_target : public inf_child_target return serial_event_fd (m_wait_event); } + void debug_registers_changed_all_threads (); + protected: /* Initialize arch-specific data for a new inferior (debug registers, @@ -303,7 +309,8 @@ struct windows_nat_target : public inf_child_target void delete_thread (ptid_t ptid, DWORD exit_code, bool main_thread_p); DWORD fake_create_process (const DEBUG_EVENT ¤t_event); - void stop_one_thread (windows_thread_info *th); + void stop_one_thread (windows_thread_info *th, + enum windows_nat::stopping_kind stopping_kind); DWORD continue_status_for_event_detaching (const DEBUG_EVENT &event, size_t *reply_later_events_left = nullptr); @@ -470,4 +477,6 @@ all_windows_threads () (all_non_exited_threads_range (win_tgt, minus_one_ptid))); } +extern void windows_debug_registers_changed_all_threads (); + #endif /* GDB_WINDOWS_NAT_H */ diff --git a/gdb/x86-windows-nat.c b/gdb/x86-windows-nat.c index 8ad9d3bedea..61764da8deb 100644 --- a/gdb/x86-windows-nat.c +++ b/gdb/x86-windows-nat.c @@ -350,8 +350,7 @@ windows_set_dr (int i, CORE_ADDR addr) if (i < 0 || i > 3) internal_error (_("Invalid register %d in windows_set_dr.\n"), i); - for (auto *th : all_windows_threads ()) - th->debug_registers_changed = true; + windows_debug_registers_changed_all_threads (); } /* Pass the value VAL to the inferior in the DR7 debug control @@ -360,8 +359,7 @@ windows_set_dr (int i, CORE_ADDR addr) static void windows_set_dr7 (unsigned long val) { - for (auto *th : all_windows_threads ()) - th->debug_registers_changed = true; + windows_debug_registers_changed_all_threads (); } /* Get the value of debug register I from the inferior. */ -- 2.53.0