From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id WHGSCK4zK2hiOCoAWB0awg (envelope-from ) for ; Mon, 19 May 2025 09:35:42 -0400 Received: by simark.ca (Postfix, from userid 112) id 208621E11C; Mon, 19 May 2025 09:35:42 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-9.0 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED, RCVD_IN_VALIDITY_RPBL,RCVD_IN_VALIDITY_SAFE autolearn=ham autolearn_force=no version=4.0.1 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (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 3CB731E102 for ; Mon, 19 May 2025 09:35:41 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id EEE0C3858C78 for ; Mon, 19 May 2025 13:35:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EEE0C3858C78 Received: from mail-wm1-f45.google.com (mail-wm1-f45.google.com [209.85.128.45]) by sourceware.org (Postfix) with ESMTPS id DE2A53858C31 for ; Mon, 19 May 2025 13:24:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DE2A53858C31 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 DE2A53858C31 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.128.45 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1747661100; cv=none; b=VYnJJl6mMJZrpxHiGqeFlTc8X3ij4qJEH+uxslGNh1mrERzzbONR1EksYyvA5qTG6gGcEt8vs1yoFIdpcfMxv/g1laX+l1zQaUkTRd1S+Q/osoWkNmUpg5A9fGeP8J8w4zXpVg1PpWFGVjNsTZbm6jz5BZZbU5PhAn86OaoUFL0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1747661100; c=relaxed/simple; bh=3QKH0+1kkC7Yz3V8/tHcJ/u9OZZH43QN6sfzCAzq/8E=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=M3cYmuv5II6zsEm017haOtstFgMTGrkI1W/EXdDtGtOSeEsnFLLOWs4EYPr1WiOu7jeWqg3GH4goXZ1z5i93NVxfe3DGEjJBYJ3m+9cP8Q4weX4koy4D+ombyQAGN0kKXUka3qxp2y2rQ48TtXdlgWEIR17VVUqt8EfVy43xeb0= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DE2A53858C31 Received: by mail-wm1-f45.google.com with SMTP id 5b1f17b1804b1-43cf848528aso38621415e9.2 for ; Mon, 19 May 2025 06:24:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1747661098; x=1748265898; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=PivHDA4InpNwYj5tibAqDOFgUnIgonoG+/viZjjiNsI=; b=fnDfA6crYlFhmeEVKIu2cDNryW4glf1wC9emx0yb+4V/qmvX7m9dE9GCetCjhV51aZ fSOw+8NKyowBExND4hcR0C9idgT5aCq9DGWIL8+0JdmJksYLR3qag8otTSiytoNUPTSd o6o8vDPRI2pqIgazAxNuzy6EYPbudmht9NfF468dC5H2xEX3L9jq8cijIV0p4Caf165g D3B4dd1jEPsAHWYVQDlBPYBE4Vvr0Zi1WQDztW9xssIzY33uv93mL5hajBU+YpEsh6ax zi6xrzm4lbf90zlNQrx+YvNAHMPEp9SErQ+kccE7UUZc+4dmNT5/OX+JDIDHQPTUa2xt KK4Q== X-Gm-Message-State: AOJu0YxyRNHkZ7xjkgulfeHlv2QzdsB78RA1LC8ozqMh3FdO2cCSBE6x EU5StQ9Ul5y/XWoFH7lcb+NIjJELclAlX1x3IcFjKAnmeEMF78FkXXOoMRgwY1bD X-Gm-Gg: ASbGnctUyKbrf0H48PAKEdBaN5fQ5qqjX22mJPJAwEJY6MzwFiwaBljgl40DfTcd0Mv WSlK4F1ajVPWad/IHAhyUdbzX1MQtbN24V9GWcP6lUshHp3CbyciNzLbcjPRIkVAXAK2A/g2eak 3CUe1uKN3TRb4IIAY65F876afVharfioYJ52h+knh+50a6AXmmDX19Xy1wsrsCRdmOcS11jc/Vc fR8CEpUJNjFSfjlsuCx/IYS4D5YFkxoRSU+vTKpGQNx2KkDcM08L0JzbGz2k3crX4kpuCsvVZ+Q smasQ/KBd0EigZFPdnqmkrCXIDCFT/mK5laHqIx52ZtSD5xGOs8= X-Google-Smtp-Source: AGHT+IFDyev2OXagGDj7c5LWMnS+ECYK/gbm5wQCc1YfkChSinv2aws2Ymm2xTKgrobaxm5W57xb4Q== X-Received: by 2002:a05:600c:1d81:b0:43c:fc04:6d35 with SMTP id 5b1f17b1804b1-442fd606b8emr116500175e9.4.1747661098204; Mon, 19 May 2025 06:24:58 -0700 (PDT) Received: from localhost ([2001:8a0:4fe9:b400:8d90:6f0d:36bf:32df]) by smtp.gmail.com with UTF8SMTPSA id 5b1f17b1804b1-442f3951854sm208124205e9.24.2025.05.19.06.24.57 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 19 May 2025 06:24:57 -0700 (PDT) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH v2 34/47] Windows gdb: Avoid writing debug registers if watchpoint hit pending Date: Mon, 19 May 2025 14:22:55 +0100 Message-ID: <20250519132308.3553663-35-pedro@palves.net> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250519132308.3553663-1-pedro@palves.net> References: <20250519132308.3553663-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 Several watchpoint-related testcases, such as gdb.threads/watchthreads.exp for example, when tested with the backend in non-stop mode, exposed an interesting detail of the Windows debug API that wasn't considered before. The symptom observed is spurious SIGTRAPs, like: Thread 1 "watchthreads" received signal SIGTRAP, Trace/breakpoint trap. 0x00000001004010b1 in main () at .../src/gdb/testsuite/gdb.threads/watchthreads.c:48 48 args[i] = 1; usleep (1); /* Init value. */ After a good amount of staring at logs and headscratching, I realized the problem: #0 - It all starts with the fact that multiple threads can hit an event at the same time. Say, a watchpoint for thread A, and a breakpoint for thread B. #1 - Say, WaitForDebugEvent reports the breakpoint hit for thread B first, then GDB for some reason decides to update debug registers, and continue. Updating debug registers means writing the debug registers to _all_ threads, with SetThreadContext. #2 - WaitForDebugEvent reports the watchpoint hit for thread A. Watchpoint hits are reported as EXCEPTION_SINGLE_STEP. #3 - windows-nat checks the Dr6 debug register to check if the step was a watchpoint or hardware breakpoint stop, and finds that Dr6 is completely cleared. So windows-nat reports a plain SIGTRAP (given EXCEPTION_SINGLE_STEP) to the core. #4 - Thread A was not supposed to be stepping, so infrun reports the SIGTRAP to the user as a random signal. The strange part is #3 above. Why was Dr6 cleared? Turns out that (at least in Windows 10 & 11), writing to _any_ debug register has the side effect of clearing Dr6, even if you write the same values the registers already had, back to the registers. I confirmed it clearly by adding this hack to GDB: if (th->context.ContextFlags == 0) { th->context.ContextFlags = CONTEXT_DEBUGGER_DR; /* Get current values of debug registers. */ CHECK (GetThreadContext (th->h, &th->context)); DEBUG_EVENTS ("For 0x%x (once), Dr6=0x%llx", th->tid, th->context.Dr6); /* Write debug registers back to thread, same values, and re-read them. */ CHECK (SetThreadContext (th->h, &th->context)); CHECK (GetThreadContext (th->h, &th->context)); DEBUG_EVENTS ("For 0x%x (twice), Dr6=0x%llx", th->tid, th->context.Dr6); } Which showed Dr6=0 after the write + re-read: [windows events] fill_thread_context: For 0x6a0 (once), Dr6=0xffff0ff1 [windows events] fill_thread_context: For 0x6a0 (twice), Dr6=0x0 This commit fixes the issue by detecting that a thread has a pending watchpoint hit to report (Dr6 has interesting bits set), and if so, avoid mofiying any debug register. Instead, let the pending watchpoint hit be reported by WaitForDebugEvent. If infrun did want to modify watchpoints, it will still be done when the thread is eventually re-resumed after the pending watchpoint hit is reported. (infrun knows how to gracefully handle the case of a watchpoint hit for a watchpoint that has since been deleted.) Change-Id: I21a3daa9e34eecfa054f0fea706e5ab40aabe70a --- gdb/nat/windows-nat.h | 7 ++++ gdb/windows-nat.c | 80 ++++++++++++++++++++++++++++++++++-------- gdbserver/win32-low.cc | 8 +++++ gdbserver/win32-low.h | 2 ++ 4 files changed, 82 insertions(+), 15 deletions(-) diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h index 4ec2f4575be..795d6c485fe 100644 --- a/gdb/nat/windows-nat.h +++ b/gdb/nat/windows-nat.h @@ -211,6 +211,13 @@ struct windows_process_info virtual bool handle_access_violation (const EXCEPTION_RECORD *rec) = 0; + /* Fill in the thread's CONTEXT/WOW64_CONTEXT, if it wasn't filled + in yet. + + This function must be supplied by the embedding application. */ + + virtual void fill_thread_context (windows_thread_info *th) = 0; + handle_exception_result handle_exception (DEBUG_EVENT ¤t_event, struct target_waitstatus *ourstatus, bool debug_exceptions); diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c index 2c76848df58..d336889a251 100644 --- a/gdb/windows-nat.c +++ b/gdb/windows-nat.c @@ -117,6 +117,7 @@ struct windows_per_inferior : public windows_process_info bool handle_access_violation (const EXCEPTION_RECORD *rec) override; void invalidate_context (windows_thread_info *th); + void fill_thread_context (windows_thread_info *th) override; void continue_one_thread (windows_thread_info *th, windows_continue_flags cont_flags); @@ -740,15 +741,8 @@ windows_fetch_one_register (struct regcache *regcache, } void -windows_nat_target::fetch_registers (struct regcache *regcache, int r) +windows_per_inferior::fill_thread_context (windows_thread_info *th) { - windows_thread_info *th = windows_process.find_thread (regcache->ptid ()); - - /* Check if TH exists. Windows sometimes uses a non-existent - thread id in its events. */ - if (th == NULL) - return; - windows_process.with_context (th, [&] (auto *context) { if (context->ContextFlags == 0) @@ -757,6 +751,19 @@ windows_nat_target::fetch_registers (struct regcache *regcache, int r) CHECK (get_thread_context (th->h, context)); } }); +} + +void +windows_nat_target::fetch_registers (struct regcache *regcache, int r) +{ + windows_thread_info *th = windows_process.find_thread (regcache->ptid ()); + + /* Check if TH exists. Windows sometimes uses a non-existent + thread id in its events. */ + if (th == nullptr) + return; + + windows_process.fill_thread_context (th); if (r < 0) for (r = 0; r < gdbarch_num_regs (regcache->arch()); r++) @@ -1231,13 +1238,56 @@ windows_per_inferior::continue_one_thread (windows_thread_info *th, { if (th->debug_registers_changed) { - context->ContextFlags |= WindowsContext::debug; - 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 = state->dr_control_mirror; + windows_process.fill_thread_context (th); + + gdb_assert ((context->ContextFlags & CONTEXT_DEBUG_REGISTERS) != 0); + + /* Check whether the thread has Dr6 set indicating a + watchpoint hit, and we haven't seen the watchpoint event + yet (reported as + EXCEPTION_SINGLE_STEP/STATUS_WX86_SINGLE_STEP). In that + case, don't change the debug registers. Changing debug + registers, even if to the same values, makes the kernel + clear Dr6. The result would be we would lose the + unreported watchpoint hit. */ + if ((context->Dr6 & ~DR6_CLEAR_VALUE) != 0) + { + if (th->last_event.dwDebugEventCode == EXCEPTION_DEBUG_EVENT + && (th->last_event.u.Exception.ExceptionRecord.ExceptionCode + == EXCEPTION_SINGLE_STEP + || (th->last_event.u.Exception.ExceptionRecord.ExceptionCode + == STATUS_WX86_SINGLE_STEP))) + { + DEBUG_EVENTS ("0x%x already reported watchpoint", th->tid); + } + else + { + DEBUG_EVENTS ("0x%x last reported something else (0x%x)", + th->tid, + th->last_event.dwDebugEventCode); + + /* Don't touch debug registers. Let the pending + watchpoint event be reported instead. We will + update the debug registers later when the thread + is re-resumed by the core after the watchpoint + event. */ + context->ContextFlags &= ~CONTEXT_DEBUG_REGISTERS; + } + } + else + DEBUG_EVENTS ("0x%x has no dr6 set", th->tid); + + if ((context->ContextFlags & CONTEXT_DEBUG_REGISTERS) != 0) + { + DEBUG_EVENTS ("0x%x changing dregs", th->tid); + 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 = state->dr_control_mirror; + } + th->debug_registers_changed = false; } if (context->ContextFlags) diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc index 4aaadf0d671..1a80269678e 100644 --- a/gdbserver/win32-low.cc +++ b/gdbserver/win32-low.cc @@ -123,6 +123,14 @@ win32_require_context (windows_thread_info *th) /* See nat/windows-nat.h. */ +void +gdbserver_windows_process::fill_thread_context (windows_thread_info *th) +{ + win32_require_context (th); +} + +/* See nat/windows-nat.h. */ + windows_thread_info * gdbserver_windows_process::find_thread (ptid_t ptid) { diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h index 1891f82b046..123fbc76f57 100644 --- a/gdbserver/win32-low.h +++ b/gdbserver/win32-low.h @@ -188,6 +188,8 @@ struct gdbserver_windows_process : public windows_nat::windows_process_info void handle_unload_dll (const DEBUG_EVENT ¤t_event) override; bool handle_access_violation (const EXCEPTION_RECORD *rec) override; + void fill_thread_context (windows_nat::windows_thread_info *th) override; + int attaching = 0; /* A status that hasn't been reported to the core yet, and so -- 2.49.0