From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id AKBINaYyK2g2NyoAWB0awg (envelope-from ) for ; Mon, 19 May 2025 09:31:18 -0400 Received: by simark.ca (Postfix, from userid 112) id D5BB81E11C; Mon, 19 May 2025 09:31:18 -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 14F3D1E102 for ; Mon, 19 May 2025 09:31:18 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id BF6B5385802C for ; Mon, 19 May 2025 13:31:17 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BF6B5385802C Received: from mail-wm1-f48.google.com (mail-wm1-f48.google.com [209.85.128.48]) by sourceware.org (Postfix) with ESMTPS id 950933858C41 for ; Mon, 19 May 2025 13:24:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 950933858C41 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 950933858C41 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.128.48 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1747661062; cv=none; b=Rij+9T8RdbE255OLtHuYcP4GJOJymM44ssXSMRNVsEMu4mMFXh9n6N20iRLK21Tk0Rg2ediWENQanwOhMNWC8cbANs7sE52FVhUe5BCqiBT4J3242tKLNUY+o/Q8GaZTJA2GGPjd/pHU3L8XgcoLqiB8BI6xxtcUbKpnd6h8Dec= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1747661062; c=relaxed/simple; bh=qMj9vdpFZeIBun8CSqC5uIXRKmZy1C8bHauJ+VJddmQ=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=oQZl7VCWPrFVhYlx8pJ/EIwCxj5n546MwNeGywo1To6I4zVTh3umzpzy8IVnftwFXnC0ZOJc79S3z6dxN9Is9hoSWGQYxYJ975ttxEqBwVH15VsNXvVNJUt52VH8eGowNB0IN7kcxlsBiDuGUgPwxU4fC9Ezt5m3pnzHjRe5ZqI= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 950933858C41 Received: by mail-wm1-f48.google.com with SMTP id 5b1f17b1804b1-43cf257158fso31802325e9.2 for ; Mon, 19 May 2025 06:24:22 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1747661061; x=1748265861; 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=WL/tfszyniCVHsNFemmwGF2vbaOF9RejVRZGOhmRGZU=; b=TX+WvSoLYnzso5Qj9RYlZuzDfEGadsDLwLvZ2bPuh8TdyVB+wcbh4UufiLnb4B2SJW hxu2thFkzQOIha3m29X7O0AnBj6QrQCZ692jDL5F8XDu1K1DyibIfN83llnKccEMvzYY mGhLgEhF/j9xd6snouOd56pg+xaWth7XFUv12lwzbHT0FFVMYcvXCgJPOQJavktDsu++ A0c4cj9b+pgA81YqsdUExkTfdJ19FtPgMu31b2kD6C10lvCC+JeBGlHmmPeCqieAdyn2 R7T5uwoCkQ+MIDwWA5xKMA8VIvqeZRGUjqdxzqEtXn9zh+zMFR244t/yfekiWWLYHCPI QT9g== X-Gm-Message-State: AOJu0YzvhTV4AEniC5iOdM7mdF2bl+jJxTGLj1EGIHbIHzODpweHfkH5 jhjM63PWX1R0OZxo5ine2MXJHaQA9Z1JFmpjz7AsQ9bTafqHXTN2QfCeVsLzL0Cz X-Gm-Gg: ASbGncuwHf5XH20k/qYN9u8J5SHO+vRT7V7SSt9M/hXgfmqWgFR5hdtUJLy4ewsQUiA hfI7qiqkZmEJxtjGD70JmpqYTDIbZTWO75Mf6k5adqfkMf38+bxmP+U8zUaq3e6tCafv6rbeGUz CBOLqfTKFgL3LEQOnnG3tANW9MCI7j5/mDXUNpD73yZXOKDHispo799wT177Rv3gw0J5AAmU3Ro 6KEIhR6kwAXpj5E4YVCIF/TQ02RJRhVi0OXP8LvCBgVehhGRasRgPvtauNhbaRmsbS606eDLGtB tG2b14t9/VBx/9W0tGMXEsdc6fB6rG8c2DQfXfSgdBoGuzOh8p6HUpJY9p3bfw== X-Google-Smtp-Source: AGHT+IGd5lqqfzrbppaXFZwN9R9y79W19gxRYSL7esZVS9wXUCXhOjaTKHZCiMe9MBbU+8s21nFM0A== X-Received: by 2002:a05:600c:3588:b0:43d:a90:9f1 with SMTP id 5b1f17b1804b1-442fefdad6bmr97370755e9.6.1747661060842; Mon, 19 May 2025 06:24:20 -0700 (PDT) Received: from localhost ([2001:8a0:4fe9:b400:8d90:6f0d:36bf:32df]) by smtp.gmail.com with UTF8SMTPSA id ffacd0b85a97d-3a35ca6254bsm13084267f8f.52.2025.05.19.06.24.20 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 19 May 2025 06:24:20 -0700 (PDT) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH v2 23/47] Windows gdbserver: Fix scheduler-locking Date: Mon, 19 May 2025 14:22:44 +0100 Message-ID: <20250519132308.3553663-24-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 This rewrites win32_process_target::resume such that scheduler-locking is implemented properly. It also uses the new get_last_debug_event_ptid function to avoid considering passing a signal to the wrong thread, like done for the native side in a previous patch. Note this code/comment being removed: - /* Yes, we're ignoring resume_info[0].thread. It'd be tricky to make - the Windows resume code do the right thing for thread switching. */ - tid = windows_process.current_event.dwThreadId; This meant that scheduler-locking currently is broken badly unless you stay in the thread that last reported an event. If you switch to a different thread from the one that last reported an event and step, you get a spurious SIGTRAP in the thread that last reported a stop, not the one that you tried to step: (gdb) t 1 [Switching to thread 1 (Thread 3908)] #0 0x00007fffc768d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll (gdb) set scheduler-locking on (gdb) set disassemble-next-line on (gdb) frame #0 0x00007fffc768d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll => 0x00007fffc768d6e4 : c3 ret (gdb) si Thread 3 received signal SIGTRAP, Trace/breakpoint trap. [Switching to Thread 2660] 0x00007fffc4e4e92e in KERNELBASE!EncodeRemotePointer () from target:C:/Windows/System32/KernelBase.dll => 0x00007fffc4e4e92e : eb 78 jmp 0x7fffc4e4e9a8 (gdb) Note how we switched to thread 1, stepped, and GDBserver still stepped thread 3... This is fixed by this patch. We now get: (gdb) info threads Id Target Id Frame 1 Thread 920 0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll 2 Thread 8528 0x00007ffe03730ad4 in ntdll!ZwWaitForWorkViaWorkerFactory () from target:C:/Windows/SYSTEM32/ntdll.dll 3 Thread 3128 0x00007ffe03730ad4 in ntdll!ZwWaitForWorkViaWorkerFactory () from target:C:/Windows/SYSTEM32/ntdll.dll * 4 Thread 7164 0x00007ffe0102e929 in KERNELBASE!EncodeRemotePointer () from target:C:/Windows/System32/KernelBase.dll 5 Thread 8348 0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll 6 Thread 2064 0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll (gdb) t 1 [Switching to thread 1 (Thread 920)] #0 0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll (gdb) set scheduler-locking on (gdb) si 0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll (gdb) si 0x00007ffe00f9b44e in SleepEx () from target:C:/Windows/System32/KernelBase.dll (gdb) si 0x00007ffe00f9b453 in SleepEx () from target:C:/Windows/System32/KernelBase.dll I.e., we kept stepping the right thread, thread 1. Note we stopped again at 0x00007ffe0372d6e4 the first time (same PC the thread already was at before the first stepi) because the thread had been stopped at a syscall, so that's normal: (gdb) disassemble Dump of assembler code for function ntdll!ZwDelayExecution: 0x00007ffe0372d6d0 <+0>: mov %rcx,%r10 0x00007ffe0372d6d3 <+3>: mov $0x34,%eax 0x00007ffe0372d6d8 <+8>: testb $0x1,0x7ffe0308 0x00007ffe0372d6e0 <+16>: jne 0x7ffe0372d6e5 0x00007ffe0372d6e2 <+18>: syscall => 0x00007ffe0372d6e4 <+20>: ret Change-Id: I44f4fe4cb98592517569c6716b9d189f42db25a0 --- gdbserver/win32-low.cc | 158 ++++++++++++++++++++++------------------- 1 file changed, 85 insertions(+), 73 deletions(-) diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc index 32464024722..2e3e4442012 100644 --- a/gdbserver/win32-low.cc +++ b/gdbserver/win32-low.cc @@ -405,19 +405,13 @@ continue_one_thread (thread_info *thread, int thread_id) } } +/* Helper for win32_process_target::kill. Resume all suspended + threads that match THREAD_ID, and continue the last debug event + with CONTINUE_STATUS. */ + static BOOL -child_continue (DWORD continue_status, int thread_id) +child_continue_for_kill (DWORD continue_status, int thread_id) { - process_info *proc = find_process_pid (windows_process.process_id); - for (thread_info &thread : proc->thread_list ()) - { - auto *th = static_cast (thread.target_data ()); - if ((thread_id == -1 || thread_id == (int) th->tid) - && !th->suspended - && th->pending_stop.status.kind () != TARGET_WAITKIND_IGNORE) - return TRUE; - } - /* The inferior will only continue after the ContinueDebugEvent call. */ for_each_thread ([&] (thread_info *thread) @@ -676,7 +670,7 @@ win32_process_target::kill (process_info *process) TerminateProcess (windows_process.handle, 0); for (;;) { - if (!child_continue (DBG_CONTINUE, -1)) + if (!child_continue_for_kill (DBG_CONTINUE, -1)) break; if (!wait_for_debug_event (&windows_process.current_event, INFINITE)) break; @@ -743,90 +737,108 @@ win32_process_target::thread_alive (ptid_t ptid) return find_thread_ptid (ptid) != NULL; } -/* Resume the inferior process. RESUME_INFO describes how we want - to resume. */ -void -win32_process_target::resume (thread_resume *resume_info, size_t n) -{ - DWORD tid; - enum gdb_signal sig; - int step; - windows_thread_info *th; - DWORD continue_status = DBG_CONTINUE; - ptid_t ptid; - - /* This handles the very limited set of resume packets that GDB can - currently produce. */ - - if (n == 1 && resume_info[0].thread == minus_one_ptid) - tid = -1; - else if (n > 1) - tid = -1; - else - /* Yes, we're ignoring resume_info[0].thread. It'd be tricky to make - the Windows resume code do the right thing for thread switching. */ - tid = windows_process.current_event.dwThreadId; +/* Helper for win32_process_target::resume. Resume THREAD, with + signal SIG, and stepping if STEP. If resuming the thread that last + reported an event, sets CONTINUE_STATUS appropriately to the status + that should be passed to ContinueDebugEvent. */ - if (resume_info[0].thread != minus_one_ptid) - { - sig = gdb_signal_from_host (resume_info[0].sig); - step = resume_info[0].kind == resume_step; - } - else - { - sig = GDB_SIGNAL_0; - step = 0; - } +static void +resume_one_thread (thread_info *thread, bool step, gdb_signal sig, + DWORD *continue_status) +{ + auto *th = static_cast (thread->target_data ()); if (sig != GDB_SIGNAL_0) { - if (windows_process.current_event.dwDebugEventCode - != EXCEPTION_DEBUG_EVENT) + /* Allow continuing with the same signal that interrupted us. + Otherwise complain. */ + if (thread->id != get_last_debug_event_ptid ()) + { + /* ContinueDebugEvent will be for a different thread. */ + OUTMSG (("Cannot continue with signal %d here. " + "Not last-event thread", sig)); + } + else if (windows_process.current_event.dwDebugEventCode + != EXCEPTION_DEBUG_EVENT) { - OUTMSG (("Cannot continue with signal %s here.\n", + OUTMSG (("Cannot continue with signal %s here. " + "Not stopped for EXCEPTION_DEBUG_EVENT.\n", gdb_signal_to_string (sig))); } else if (sig == windows_process.last_sig) - continue_status = DBG_EXCEPTION_NOT_HANDLED; + *continue_status = DBG_EXCEPTION_NOT_HANDLED; else OUTMSG (("Can only continue with received signal %s.\n", gdb_signal_to_string (windows_process.last_sig))); } - windows_process.last_sig = GDB_SIGNAL_0; + win32_prepare_to_resume (th); - /* Get context for the currently selected thread. */ - ptid = debug_event_ptid (&windows_process.current_event); - th = windows_process.find_thread (ptid); - if (th) + DWORD *context_flags = windows_process.context_flags_ptr (th); + if (*context_flags) { - win32_prepare_to_resume (th); + /* Move register values from the inferior into the thread + context structure. */ + regcache_invalidate (); - DWORD *context_flags = windows_process.context_flags_ptr (th); - if (*context_flags) + if (step) { - /* Move register values from the inferior into the thread - context structure. */ - regcache_invalidate (); + if (the_low_target.single_step != NULL) + (*the_low_target.single_step) (th); + else + error ("Single stepping is not supported " + "in this configuration.\n"); + } + + win32_set_thread_context (th); + *context_flags = 0; + } + + continue_one_thread (thread, th->tid); +} + +/* Resume the inferior process. RESUME_INFO describes how we want + to resume. */ +void +win32_process_target::resume (thread_resume *resume_info, size_t n) +{ + DWORD continue_status = DBG_CONTINUE; + bool any_pending = false; + + for_each_thread ([&] (thread_info *thread) + { + auto *th = static_cast (thread->target_data ()); - if (step) + for (int ndx = 0; ndx < n; ndx++) + { + thread_resume &r = resume_info[ndx]; + ptid_t ptid = r.thread; + gdb_signal sig = gdb_signal_from_host (r.sig); + bool step = r.kind == resume_step; + + if (ptid == minus_one_ptid || ptid == thread->id + /* Handle both 'pPID' and 'pPID.-1' as meaning 'all threads + of PID'. */ + || (ptid.pid () == thread->id.pid () + && (ptid.is_pid () || ptid.lwp () == -1))) { - if (the_low_target.single_step != NULL) - (*the_low_target.single_step) (th); - else - error ("Single stepping is not supported " - "in this configuration.\n"); - } + /* Ignore (wildcard) resume requests for already-resumed + threads. */ + if (!th->suspended) + continue; - win32_set_thread_context (th); - *context_flags = 0; + resume_one_thread (thread, step, sig, &continue_status); + break; + } } - } - /* Allow continuing with the same signal that interrupted us. - Otherwise complain. */ + if (!th->suspended + && th->pending_stop.status.kind () != TARGET_WAITKIND_IGNORE) + any_pending = true; + }); - child_continue (continue_status, tid); + if (!any_pending) + continue_last_debug_event (continue_status, debug_threads); } /* See nat/windows-nat.h. */ -- 2.49.0