From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-il1-x12b.google.com (mail-il1-x12b.google.com [IPv6:2607:f8b0:4864:20::12b]) by sourceware.org (Postfix) with ESMTPS id 06F553858C39 for ; Wed, 6 Oct 2021 20:43:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 06F553858C39 Received: by mail-il1-x12b.google.com with SMTP id k13so4156009ilo.7 for ; Wed, 06 Oct 2021 13:43:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=7Dao5ytbwPDwiAB3fAE5hozjYZj6Zbj2kpiZINndiYw=; b=y2/832+3F9OjNIj1jDaoOi2z/lI66FIFj1OzRwARTh7bdKex9cF9UscXvESN+46JaK XigEfUXAd2psLQNP4lYVItReotpZ1Jhm8FpaFko8qeMgb9iGm/8NODX6I32glwwN27lw +DkE5teRv8DMwPkqsCRQKV65pZ+4eqOZaCBhyAyx1Ti8TSXsKCEdeW1f6zp85XaF+v1w PeAwHJB15+TfosGdkosnohIFMlXwZew9Z0Dx22p9A9bbs2aRhZP0NOwc9igyIeOvNZFz IzXgpgLD+44keXs5hR90tzbW2qZ27qDXOzXcLPl1+LUElfnH6skWo3aRFjkbENq+2fh4 um9Q== X-Gm-Message-State: AOAM532RmZuFJ4GdgJIr1e5jSg1SKNt3KuapWCuwB8hE7z8jJdBqyfkF 6xh1mPF+rv4B0b3nvADuHBmmIr80oba5Hw== X-Google-Smtp-Source: ABdhPJwmWqw0Q8pWr8bo8MRTBH0HFfAsTe5cK4aM3788ek2/3JcXT0eIhEQrwjXal/zYWLbHAK7irg== X-Received: by 2002:a05:6e02:1808:: with SMTP id a8mr207679ilv.119.1633553005354; Wed, 06 Oct 2021 13:43:25 -0700 (PDT) Received: from murgatroyd.Home (174-16-0-219.hlrn.qwest.net. [174.16.0.219]) by smtp.gmail.com with ESMTPSA id d9sm1981446ilu.16.2021.10.06.13.43.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Oct 2021 13:43:24 -0700 (PDT) From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH] Fix watchpoints with multiple threads on Windows Date: Wed, 6 Oct 2021 14:43:22 -0600 Message-Id: <20211006204322.2175432-1-tromey@adacore.com> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Oct 2021 20:43:28 -0000 A recent internal change pointed out that watchpoints were not working on Windows when the inferior was multi-threaded. This happened because the debug registers were only updated for certain threads -- in particular, those that were being resumed and that were not marked as suspended. In the case of single-stepping, the need to update the debug registers in other threads could also be "forgotten". This patch changes windows-nat.c to mark all threads needing a debug register update. This brings the code closer to what gdbserver does (though, unfortunately, it still seems more complicated than needed). --- gdb/windows-nat.c | 71 +++++++++++++---------------------------------- 1 file changed, 20 insertions(+), 51 deletions(-) diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c index 736b794fc82..231f21d4df7 100644 --- a/gdb/windows-nat.c +++ b/gdb/windows-nat.c @@ -123,8 +123,6 @@ enum | CONTEXT_EXTENDED_REGISTERS static uintptr_t dr[8]; -static int debug_registers_changed; -static int debug_registers_used; static int windows_initialization_done; #define DR6_CLEAR_VALUE 0xffff0ff0 @@ -386,40 +384,10 @@ windows_add_thread (ptid_t ptid, HANDLE h, void *tlb, bool main_thread_p) else add_thread (&the_windows_nat_target, ptid); - /* Set the debug registers for the new thread if they are used. */ - if (debug_registers_used) - { -#ifdef __x86_64__ - if (wow64_process) - { - /* Only change the value of the debug registers. */ - th->wow64_context.ContextFlags = CONTEXT_DEBUG_REGISTERS; - CHECK (Wow64GetThreadContext (th->h, &th->wow64_context)); - th->wow64_context.Dr0 = dr[0]; - th->wow64_context.Dr1 = dr[1]; - th->wow64_context.Dr2 = dr[2]; - th->wow64_context.Dr3 = dr[3]; - th->wow64_context.Dr6 = DR6_CLEAR_VALUE; - th->wow64_context.Dr7 = dr[7]; - CHECK (Wow64SetThreadContext (th->h, &th->wow64_context)); - th->wow64_context.ContextFlags = 0; - } - else -#endif - { - /* Only change the value of the debug registers. */ - th->context.ContextFlags = CONTEXT_DEBUG_REGISTERS; - CHECK (GetThreadContext (th->h, &th->context)); - th->context.Dr0 = dr[0]; - th->context.Dr1 = dr[1]; - th->context.Dr2 = dr[2]; - th->context.Dr3 = dr[3]; - th->context.Dr6 = DR6_CLEAR_VALUE; - th->context.Dr7 = dr[7]; - CHECK (SetThreadContext (th->h, &th->context)); - th->context.ContextFlags = 0; - } - } + /* It's simplest to always set this and update the debug + registers. */ + th->debug_registers_changed = true; + return th; } @@ -593,7 +561,7 @@ windows_nat_target::fetch_registers (struct regcache *regcache, int r) /* Copy dr values from that thread. But only if there were not modified since last stop. PR gdb/2388 */ - if (!debug_registers_changed) + if (!th->debug_registers_changed) { dr[0] = th->wow64_context.Dr0; dr[1] = th->wow64_context.Dr1; @@ -611,7 +579,7 @@ windows_nat_target::fetch_registers (struct regcache *regcache, int r) /* Copy dr values from that thread. But only if there were not modified since last stop. PR gdb/2388 */ - if (!debug_registers_changed) + if (!th->debug_registers_changed) { dr[0] = th->context.Dr0; dr[1] = th->context.Dr1; @@ -1194,12 +1162,10 @@ windows_continue (DWORD continue_status, int id, int killed) for (windows_thread_info *th : thread_list) if (id == -1 || id == (int) th->tid) { - if (!th->suspended) - continue; #ifdef __x86_64__ if (wow64_process) { - if (debug_registers_changed) + if (th->debug_registers_changed) { th->wow64_context.ContextFlags |= CONTEXT_DEBUG_REGISTERS; th->wow64_context.Dr0 = dr[0]; @@ -1208,6 +1174,7 @@ windows_continue (DWORD continue_status, int id, int killed) th->wow64_context.Dr3 = dr[3]; th->wow64_context.Dr6 = DR6_CLEAR_VALUE; th->wow64_context.Dr7 = dr[7]; + th->debug_registers_changed = false; } if (th->wow64_context.ContextFlags) { @@ -1228,7 +1195,7 @@ windows_continue (DWORD continue_status, int id, int killed) else #endif { - if (debug_registers_changed) + if (th->debug_registers_changed) { th->context.ContextFlags |= CONTEXT_DEBUG_REGISTERS; th->context.Dr0 = dr[0]; @@ -1237,6 +1204,7 @@ windows_continue (DWORD continue_status, int id, int killed) th->context.Dr3 = dr[3]; th->context.Dr6 = DR6_CLEAR_VALUE; th->context.Dr7 = dr[7]; + th->debug_registers_changed = false; } if (th->context.ContextFlags) { @@ -1269,7 +1237,6 @@ windows_continue (DWORD continue_status, int id, int killed) " (ContinueDebugEvent failed, error %u)"), (unsigned int) GetLastError ()); - debug_registers_changed = 0; return res; } @@ -1366,7 +1333,7 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig) if (th->wow64_context.ContextFlags) { - if (debug_registers_changed) + if (th->debug_registers_changed) { th->wow64_context.Dr0 = dr[0]; th->wow64_context.Dr1 = dr[1]; @@ -1374,6 +1341,7 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig) th->wow64_context.Dr3 = dr[3]; th->wow64_context.Dr6 = DR6_CLEAR_VALUE; th->wow64_context.Dr7 = dr[7]; + th->debug_registers_changed = false; } CHECK (Wow64SetThreadContext (th->h, &th->wow64_context)); th->wow64_context.ContextFlags = 0; @@ -1393,7 +1361,7 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig) if (th->context.ContextFlags) { - if (debug_registers_changed) + if (th->debug_registers_changed) { th->context.Dr0 = dr[0]; th->context.Dr1 = dr[1]; @@ -1401,6 +1369,7 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig) th->context.Dr3 = dr[3]; th->context.Dr6 = DR6_CLEAR_VALUE; th->context.Dr7 = dr[7]; + th->debug_registers_changed = false; } CHECK (SetThreadContext (th->h, &th->context)); th->context.ContextFlags = 0; @@ -1806,8 +1775,6 @@ windows_nat_target::do_initial_windows_stuff (DWORD pid, bool attaching) last_sig = GDB_SIGNAL_0; open_process_used = 0; - debug_registers_changed = 0; - debug_registers_used = 0; for (i = 0; i < sizeof (dr) / sizeof (dr[0]); i++) dr[i] = 0; #ifdef __CYGWIN__ @@ -3211,8 +3178,9 @@ cygwin_set_dr (int i, CORE_ADDR addr) internal_error (__FILE__, __LINE__, _("Invalid register %d in cygwin_set_dr.\n"), i); dr[i] = addr; - debug_registers_changed = 1; - debug_registers_used = 1; + + for (windows_thread_info *th : thread_list) + th->debug_registers_changed = true; } /* Pass the value VAL to the inferior in the DR7 debug control @@ -3222,8 +3190,9 @@ static void cygwin_set_dr7 (unsigned long val) { dr[7] = (CORE_ADDR) val; - debug_registers_changed = 1; - debug_registers_used = 1; + + for (windows_thread_info *th : thread_list) + th->debug_registers_changed = true; } /* Get the value of debug register I from the inferior. */ -- 2.31.1