From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id R2i2ElsN1GnUqgoAWB0awg (envelope-from ) for ; Mon, 06 Apr 2026 15:45:31 -0400 Received: by simark.ca (Postfix, from userid 112) id 3E76D1E0BC; Mon, 06 Apr 2026 15:45:31 -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 2EC811E04F for ; Mon, 06 Apr 2026 15:45:30 -0400 (EDT) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id 6E4214BA2E1C for ; Mon, 6 Apr 2026 19:45:29 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6E4214BA2E1C Received: from mail-wr1-f50.google.com (mail-wr1-f50.google.com [209.85.221.50]) by sourceware.org (Postfix) with ESMTPS id 536E74BA2E27 for ; Mon, 6 Apr 2026 19:44:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 536E74BA2E27 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 536E74BA2E27 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.221.50 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1775504668; cv=none; b=eTIQDaihpXbA0i9b9xXHhBBTzms+M85k6VKCf31lL5ifOnWlFUx927yR8LxeItHsg6XI5QJbiyUvGShWnSUMWsMX2EpmNgMlvfwsvAL+W/4fcqx68xrzJvyj3DbtpIARQa8RyS8ZfCluwuB+pMvD81HtSRUprSxUDFq6Pvq9bNc= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1775504668; c=relaxed/simple; bh=CshbP8DjVhbe5yoviwGaqOWOTSS/77rKsA7k05NRYPk=; h=Message-ID:Date:MIME-Version:Subject:From:To; b=A05IrLlhL9RZ2FcW9hSQZLTkB9L9DD3Ag/RrIZjjrP7ivcU5J+0dMXO13YrFJXjw2dcgCDIAw4VKwX89Mgb0QzUGPTCzl21fcPpT73QJOqyOmbjsu8eofoIUKh/t0qJSV9e//uX0tiMasNUaDE6IIR1PiOtr6xRSa67jmmyS6UA= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 536E74BA2E27 Received: by mail-wr1-f50.google.com with SMTP id ffacd0b85a97d-43d02a71526so2442420f8f.3 for ; Mon, 06 Apr 2026 12:44:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1775504667; x=1776109467; h=content-transfer-encoding:in-reply-to:content-language:references :to:from:subject:user-agent:mime-version:date:message-id:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Pccoo7AtFKqAGNf3Sdl2bwdq1HpVaK/X/GBHPYNvAJc=; b=Jxp/DH8/4gacR6Q2yFudaXdt0WP1MFC0VWHWUe/DlW3lJD9hPT4oUeVo8Ro4JRPgBx BZVI3tf+RK/IadXSu1UcRnXVMJ249rAap4ft3dv05I/H/FKCxu8uTF5iyVjCtBlf3j+K PRjmnnn5HaSykLWeaFd93YBlbz3jWfKnFpaVMD3QYGiYyiFlwCl9DXRAqIphUUQYY7kn 81fNtq/xZizwoUGR5YzSrs/5XtvSDfztOh8avcp1XOR7wCo1cWw/36a53CSel7XqY5r/ oXsRuLkfG7UykaR3y4VfMPeaiRMFxB0CYYMt9L1zpOs3Cy6ZxYPwz3GGGJUouiGr3nF9 xdJQ== X-Gm-Message-State: AOJu0YyuGXsWtD0tIKMlHlv6rEmURYIap+uHpK+487vV6k3Usr0oGFu8 bWM4/nuRvCFyqt6aPpiP84hPvRcQ8wtSqD7VZPDdNqUM5zPilqMTQ73JZ/y2pQ== X-Gm-Gg: AeBDies8x31bSUyi3R+UCB+6bova/NmRZElWjQhf9+UaaHB3nLNEIfQzNx+lB3dVVeQ 9V8ep06NNXMRWvZvMQZFliobr7MiRx9JjcPlo/5S1nq409T9sMg1Wt40M7xR2gHqTC0ny7hZTSS brn5DEPBjg41iSrqpJfsZGnSjIOYLQ6K6UgavwpD1R8RH/wKJH8rEn/xfLvPcE/UjFY3SsnUHaW BuZmnvWjreNJ6jO94nWR/VA196+WYIwnHRnDuOuG+nxyOiMzBPTApD5l7nsgy2F0jCvzc6+3mBI Qh5r0/bvYw1sxk57zaFwesU2klklDd0S2L4ciFjryYKqPl+zzv0MWT2XrK9clEabZI4etuyM+Rg F+bRkLU6xB0++Zhpe7rusdLYMPDK4UHjX54QniauBwZsD+2ri2MqYU5FnkexdZAlWBb8trAaz8a 4yE6DaHYQRXp3jJ1XOcyuOK9+YHj8VvgGgGUhIBypoD/k149404LgJl8Dv9+EhtrHD0A== X-Received: by 2002:a05:6000:25c6:b0:43c:f719:a7f0 with SMTP id ffacd0b85a97d-43d2929467bmr20756068f8f.12.1775504666703; Mon, 06 Apr 2026 12:44:26 -0700 (PDT) Received: from ?IPV6:2001:8a0:fac8:3000:7350:2ce3:8979:9fcb? ([2001:8a0:fac8:3000:7350:2ce3:8979:9fcb]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-43d1e4d27a8sm43930726f8f.17.2026.04.06.12.44.26 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 06 Apr 2026 12:44:26 -0700 (PDT) Message-ID: <11ce3c82-66ea-49cc-9307-144c908e31e6@palves.net> Date: Mon, 6 Apr 2026 20:44:25 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 07/47] Windows gdb: Eliminate global current_process.dr[8] global From: Pedro Alves To: gdb-patches@sourceware.org References: <20250519132308.3553663-1-pedro@palves.net> <20250519132308.3553663-8-pedro@palves.net> Content-Language: en-US In-Reply-To: <20250519132308.3553663-8-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 2025-05-19 14:22, Pedro Alves wrote: > current_process.dr needs to be per-thread for non-stop. Actually, it > doesn't even need to exist at all. I've now rebased this patch, which required moving code around since the x86 Windows watchpoints code meanwhile moved to new files, but all mostly mechanical, and merged it. > I don't understand why would windows_nat_target::resume want to call > SetThreadContext itself. That duplicates things as it is currently > worrying about setting the debug registers as well. windows_continue > also does that, and windows_nat_target::resume always calls it. So > this patch simplifies windows_nat_target::resume too. I've dropped this whole paragraph from the commit log, as meanwhile Hannes merged a patch doing the same (commit f0f1ae77fc, "Remove duplicate code from windows_nat_target::resume"). Retested with: $ make check TESTS="gdb.*/*watch*.exp" ... on Cygwin. No regressions. Here's what I merged. >From 8de9be5a80d1ff5920e5c515571668b7cfb78ea3 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Tue, 2 May 2023 20:42:35 +0100 Subject: [PATCH] Windows gdb: Eliminate global current_process.dr[8] global current_process.dr needs to be per-thread for non-stop. Actually, it doesn't even need to exist at all. We have x86_debug_reg_state recording intent, and then the cygwin_get_dr/cygwin_get_dr6/cygwin_get_dr7 functions are registered as x86_dr_low_type vector functions, so they should return the current value in the inferior's registers. See this comment in x86-dregs.c: ~~~ /* In non-stop/async, threads can be running while we change the global dr_mirror (and friends). Say, we set a watchpoint, and let threads resume. Now, say you delete the watchpoint, or add/remove watchpoints such that dr_mirror changes while threads are running. On targets that support non-stop, inserting/deleting watchpoints updates the global dr_mirror only. It does not update the real thread's debug registers; that's only done prior to resume. Instead, 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. Now, say, a thread hit a watchpoint before having been updated with the new dr_mirror contents, and we haven't yet handled the corresponding SIGTRAP. If we trusted dr_mirror below, we'd mistake the real trapped address (from the last time we had updated debug registers in the thread) with whatever was currently in dr_mirror. So to fix this, dr_mirror always represents intention, what we _want_ threads to have in debug registers. To get at the address and cause of the trap, we need to read the state the thread still has in its debug registers. In sum, always get the current debug register values the current thread has, instead of trusting the global mirror. If the thread was running when we last changed watchpoints, the mirror no longer represents what was set in this thread's debug registers. */ ~~~ This patch makes the Windows native target follow that model as well. Tromey pointed out that gdb/2388 mentioned in the code being removed was moved to https://sourceware.org/bugzilla/show_bug.cgi?id=9493 in the bugzilla migration. I tried the reproducer mentioned there, and it still works correctly. Approved-By: Tom Tromey Change-Id: Id762d0faa7d5e788402f2ff5adad5352447a7526 commit-id:8a975ed0 --- gdb/nat/windows-nat.h | 1 + gdb/windows-nat.c | 1 + gdb/x86-windows-nat.c | 73 ++++++++++++++++++++++-------------------- gdbserver/win32-low.cc | 1 + 4 files changed, 42 insertions(+), 34 deletions(-) diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h index 5411c73ff97..f4b3669df1b 100644 --- a/gdb/nat/windows-nat.h +++ b/gdb/nat/windows-nat.h @@ -150,6 +150,7 @@ struct windows_process_info { /* The process handle */ HANDLE handle = 0; + DWORD process_id = 0; DWORD main_thread_id = 0; enum gdb_signal last_sig = GDB_SIGNAL_0; diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c index 507a25199c8..e25ae81f054 100644 --- a/gdb/windows-nat.c +++ b/gdb/windows-nat.c @@ -1235,6 +1235,7 @@ windows_nat_target::do_initial_windows_stuff (DWORD pid, bool attaching) windows_process->cygwin_load_start = 0; windows_process->cygwin_load_end = 0; #endif + windows_process->process_id = pid; memset (&windows_process->current_event, 0, sizeof (windows_process->current_event)); inf = current_inferior (); diff --git a/gdb/x86-windows-nat.c b/gdb/x86-windows-nat.c index f8830809694..c7be192a894 100644 --- a/gdb/x86-windows-nat.c +++ b/gdb/x86-windows-nat.c @@ -44,8 +44,6 @@ enum struct x86_windows_per_inferior : public windows_per_inferior { - uintptr_t dr[8] {}; - /* The function to use in order to determine whether a register is a segment register or not. */ segment_register_p_ftype *segment_register_p = nullptr; @@ -77,8 +75,6 @@ static x86_windows_per_inferior x86_windows_process; void x86_windows_nat_target::initialize_windows_arch (bool attaching) { - memset (x86_windows_process.dr, 0, sizeof (x86_windows_process.dr)); - #ifdef __x86_64__ x86_windows_process.ignore_first_breakpoint = !attaching && x86_windows_process.wow64_process; @@ -113,19 +109,6 @@ x86_windows_nat_target::fill_thread_context (windows_thread_info *th) { context->ContextFlags = WindowsContext::all; CHECK (get_thread_context (th->h, context)); - - /* Copy dr values from that thread. - But only if there were not modified since last stop. - PR gdb/2388 */ - if (!th->debug_registers_changed) - { - x86_windows_process.dr[0] = context->Dr0; - x86_windows_process.dr[1] = context->Dr1; - x86_windows_process.dr[2] = context->Dr2; - x86_windows_process.dr[3] = context->Dr3; - x86_windows_process.dr[6] = context->Dr6; - x86_windows_process.dr[7] = context->Dr7; - } }); } @@ -137,15 +120,18 @@ x86_windows_nat_target::thread_context_continue (windows_thread_info *th, { x86_windows_process.with_context (th, [&] (auto *context) { + struct x86_debug_reg_state *state + = x86_debug_reg_state (windows_process->process_id); + if (th->debug_registers_changed) { context->ContextFlags |= WindowsContext::debug; - context->Dr0 = x86_windows_process.dr[0]; - context->Dr1 = x86_windows_process.dr[1]; - context->Dr2 = x86_windows_process.dr[2]; - context->Dr3 = x86_windows_process.dr[3]; + context->Dr0 = state->dr_mirror[0]; + context->Dr1 = state->dr_mirror[1]; + context->Dr2 = state->dr_mirror[2]; + context->Dr3 = state->dr_mirror[3]; context->Dr6 = DR6_CLEAR_VALUE; - context->Dr7 = x86_windows_process.dr[7]; + context->Dr7 = state->dr_control_mirror; th->debug_registers_changed = false; } @@ -301,7 +287,6 @@ cygwin_set_dr (int i, CORE_ADDR addr) { if (i < 0 || i > 3) internal_error (_("Invalid register %d in cygwin_set_dr.\n"), i); - x86_windows_process.dr[i] = addr; for (auto &th : x86_windows_process.thread_list) th->debug_registers_changed = true; @@ -313,8 +298,6 @@ cygwin_set_dr (int i, CORE_ADDR addr) static void cygwin_set_dr7 (unsigned long val) { - x86_windows_process.dr[7] = (CORE_ADDR) val; - for (auto &th : x86_windows_process.thread_list) th->debug_registers_changed = true; } @@ -324,26 +307,48 @@ cygwin_set_dr7 (unsigned long val) static CORE_ADDR cygwin_get_dr (int i) { - return x86_windows_process.dr[i]; + windows_thread_info *th + = windows_process->thread_rec (inferior_ptid, DONT_INVALIDATE_CONTEXT); + + return windows_process->with_context (th, [&] (auto *context) -> CORE_ADDR + { + gdb_assert (context->ContextFlags != 0); + switch (i) + { + case 0: + return context->Dr0; + case 1: + return context->Dr1; + case 2: + return context->Dr2; + case 3: + return context->Dr3; + case 6: + return context->Dr6; + case 7: + return context->Dr7; + }; + + gdb_assert_not_reached ("invalid x86 dr register number: %d", i); + }); } -/* Get the value of the DR6 debug status register from the inferior. - Here we just return the value stored in dr[6] - by the last call to thread_rec for current_event.dwThreadId id. */ +/* Get the value of the DR6 debug status register from the + inferior. */ + static unsigned long cygwin_get_dr6 (void) { - return (unsigned long) x86_windows_process.dr[6]; + return cygwin_get_dr (6); } -/* Get the value of the DR7 debug status register from the inferior. - Here we just return the value stored in dr[7] by the last call to - thread_rec for current_event.dwThreadId id. */ +/* Get the value of the DR7 debug status register from the + inferior. */ static unsigned long cygwin_get_dr7 (void) { - return (unsigned long) x86_windows_process.dr[7]; + return cygwin_get_dr (7); } static int diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc index b05f6af6b56..ddc5e5475ec 100644 --- a/gdbserver/win32-low.cc +++ b/gdbserver/win32-low.cc @@ -291,6 +291,7 @@ do_initial_child_stuff (HANDLE proch, DWORD pid, int attached) windows_process.last_sig = GDB_SIGNAL_0; windows_process.handle = proch; + windows_process.process_id = pid; windows_process.main_thread_id = 0; windows_process.soft_interrupt_requested = 0; base-commit: fb1546987b21e3a63e43d4587d320fcbddf83025 -- 2.53.0