From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id GTepOfNm8mmGNgQAWB0awg (envelope-from ) for ; Wed, 29 Apr 2026 16:15:47 -0400 Received: by simark.ca (Postfix, from userid 112) id E79C91E0BA; Wed, 29 Apr 2026 16:15:47 -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 1A55F1E093 for ; Wed, 29 Apr 2026 16:15:47 -0400 (EDT) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id A82C94BBCDB5 for ; Wed, 29 Apr 2026 20:15:46 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A82C94BBCDB5 Received: from mail-wm1-f50.google.com (mail-wm1-f50.google.com [209.85.128.50]) by sourceware.org (Postfix) with ESMTPS id 0CF404BB58E0 for ; Wed, 29 Apr 2026 20:15:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0CF404BB58E0 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 0CF404BB58E0 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.128.50 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1777493721; cv=none; b=HvYJszfQFsMas50MuSsIdfYHMTJA8ssAvR75LkqE6xYxdVsqApVEwyJItm+rSOm7lQv+rq4X+QTogrzagf/N8nYvNebI7MtTuVv9tDSPO2pwUrHmGeVIJI+rKY5woUSBnnClSQPEnb3SiLzCmaV9CFvYDZnShQZmGwLPIdZBaRU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1777493721; c=relaxed/simple; bh=Y25GdPQ5/lFsT8CfdWLmAC9AgXprUWCHeGxW6t+o20w=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=QS4z1110/Vq/Wc7EPWDA8BnJaN6QfCwRXghv98QqJjRSZ9Y7y1GVR1wHhvb2GYqYRXp0+x8zBqVk4nj0By5QMpruhjJec6fHhwrP7IdvQb+qkQ5Erd0AQrWYmaMdiVca2nVK740VfFdAIG73U4LwTM4Y7NrNzoXE6qMF7VVEubc= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 0CF404BB58E0 Received: by mail-wm1-f50.google.com with SMTP id 5b1f17b1804b1-48a563e4ef7so1605065e9.0 for ; Wed, 29 Apr 2026 13:15:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1777493719; x=1778098519; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-gg:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=Km1CFxNoDPg28vJPPhj5oMRsmLCuAZMxQdfe0A6AJcE=; b=fy3VoacVdByZBR7WtVpr8OStlGzo+iOfXit8xqaApBMPPej4WhP2IdmbsMI0+YSCXL z5a4KAHheIqFIUIPc06POJ9bnwGdqkRIHc3qckzXiFIRdnPG44sD4quJKpwzWiZHnhsN zl8mR3h0rWlF2QM/NZ2IggMjbbNCH7kTru4egq1sfBnz7ZGUI7PHNEnpzFx61kpxyc7K lB1LzIFpaqzU6eYhLIy96ONQybK6fv34zQkWBdCFZM401lRbqpyR37sAPzHcq4KzKYsP j+MBGIQWAV/WbIxqzLzi8xhUiok7jJAItcAiZ8Fhb8eedyOb5Ef1MqjRd6XT3J30Wkr4 kL2A== X-Gm-Message-State: AOJu0YyEtuHIvC/38HqRlKyxXYFb1DuzYwh2ylbyBzCYZXH4pXIswHJL MGhDWxHK05Tz5Zxj2uKNgjulFm6mmLUStBox1fyrbTN8gJoWGV3pm86y1DqK8w== X-Gm-Gg: AeBDievzSb5LA4LPhecrND8XnZF49va7wElFRO86mtgVer0Hi/5j2EkE+3xhyROUuYv JCoybr52LCePgvZUV8QURPXNONX1HvkYDJ4S2S2GdXnatTb539/3vdp3N2tYBye6A6GwzCEFUX/ EZ0+5ASJOmgmbZCB0LaMqu1n/IRN9V8vezHB+GKuFoDIm+5LuzPVHjI7/QHPYqu8Zn6T8/ckIv1 IlUIbC5kKBQDiHHLXmfJx47QXJl4ipGoX9gowqNIuWuY8pW1V4y+ea6N9Ng/2EDEnMhqP8XtMKx b0MLiWf0TiLDuhiQ/YOt6ELV86qTMJh2ZCcwj+iZyNbSzaGSjfDwtrbmTNWGjpN2thf2yO92iy+ PQhqa+RKFAjkIZ22+Dd3rWUORkBwACQZwWLBKu/kV7MOWGU8C4CQI1VCD3Ue/uws1Rjb5cYBDg3 ZOO/+XtUBCXYNVDp1wGtfzI3FqZ3irzzE= X-Received: by 2002:a05:600c:c049:b0:48a:5301:bb5c with SMTP id 5b1f17b1804b1-48a84468412mr796105e9.16.1777493718923; Wed, 29 Apr 2026 13:15:18 -0700 (PDT) Received: from localhost ([2001:8a0:facb:a800:f22f:591f:e700:e91]) by smtp.gmail.com with UTF8SMTPSA id 5b1f17b1804b1-48a822c832fsm24384835e9.10.2026.04.29.13.15.17 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 29 Apr 2026 13:15:18 -0700 (PDT) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH v3 03/11] Windows GDB: make windows_thread_info be private thread_info data Date: Wed, 29 Apr 2026 21:14:59 +0100 Message-ID: <20260429201507.480870-4-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 With Windows non-stop support, we'll add support for target_thread_events to the Windows target. When that is supported, and the core wants to be notified of thread exit events, the target backend does not delete the thread for which the event is being reported. Instead, infrun takes care of that. That causes one problem on Windows, which is that Windows maintains its own separate Windows threads list, in parallel with the struct thread_info thread list maintained by the core. In the target_thread_events events scenario, when infrun deletes the thread, the corresponding object in the Windows backend thread list is left dangling, causing problems. Fix this by eliminating the parallel thread list from the Windows backend, instead making the windows_thread_info data by registered as the private data associated with thread_info, like other targets do. It also adds a all_windows_threads walker function, and associated range and iterator classes, so that most of the Windows target code can iterate over Windows threads without having to worry about fetching the Windows thread data out of thread_info's private data. Change-Id: I5058969b46e0dd238c89b6c28403c1848f083683 --- gdb/target/waitstatus.h | 4 +- gdb/windows-nat.c | 75 +++++++++++-------------------- gdb/windows-nat.h | 99 ++++++++++++++++++++++++++++++++++++++++- gdb/x86-windows-nat.c | 4 +- 4 files changed, 129 insertions(+), 53 deletions(-) diff --git a/gdb/target/waitstatus.h b/gdb/target/waitstatus.h index cbdaa026218..3a1adcc6256 100644 --- a/gdb/target/waitstatus.h +++ b/gdb/target/waitstatus.h @@ -105,7 +105,9 @@ enum target_waitkind /* The thread was created. */ TARGET_WAITKIND_THREAD_CREATED, - /* The thread has exited. The exit status is in value.integer. */ + /* The thread has exited. The exit status is in value.integer. The + thread should still exist in the thread list when infrun receives + this event. */ TARGET_WAITKIND_THREAD_EXITED, }; diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c index 1dfe2df389d..4029e7dad00 100644 --- a/gdb/windows-nat.c +++ b/gdb/windows-nat.c @@ -272,15 +272,23 @@ windows_nat_target::continue_last_debug_event_main_thread m_continued = !last_call; } +/* Return a pointer to the windows-nat target instance. */ + +static windows_nat_target * +get_windows_nat_target () +{ + return gdb::checked_static_cast (get_native_target ()); +} + /* See nat/windows-nat.h. */ windows_thread_info * windows_per_inferior::find_thread (ptid_t ptid) { - for (auto &th : thread_list) - if (th->tid == ptid.lwp ()) - return th.get (); - return nullptr; + thread_info *thr = get_windows_nat_target ()->find_thread (ptid); + if (thr == nullptr) + return nullptr; + return as_windows_thread_info (thr); } /* See nat/windows-nat.h. */ @@ -297,12 +305,11 @@ windows_thread_info * windows_nat_target::add_thread (ptid_t ptid, HANDLE h, void *tlb, bool main_thread_p) { - windows_thread_info *th; - gdb_assert (ptid.lwp () != 0); - if ((th = windows_process->find_thread (ptid))) - return th; + windows_thread_info *existing = windows_process->find_thread (ptid); + if (existing != nullptr) + return existing; CORE_ADDR base = (CORE_ADDR) (uintptr_t) tlb; #ifdef __x86_64__ @@ -311,18 +318,18 @@ windows_nat_target::add_thread (ptid_t ptid, HANDLE h, void *tlb, if (windows_process->wow64_process) base += 0x2000; #endif - th = new windows_thread_info (windows_process, ptid.lwp (), h, base); - windows_process->thread_list.emplace_back (th); + windows_private_thread_info *th + = new windows_private_thread_info (windows_process, ptid.lwp (), h, base); /* Add this new thread to the list of threads. To be consistent with what's done on other platforms, we add the main thread silently (in reality, this thread is really more of a process to the user than a thread). */ - if (main_thread_p) - add_thread_silent (this, ptid); - else - ::add_thread (this, ptid); + thread_info *gth = (main_thread_p + ? ::add_thread_silent (this, ptid) + : ::add_thread (this, ptid)); + gth->priv.reset (th); /* It's simplest to always set this and update the debug registers. */ @@ -331,15 +338,6 @@ windows_nat_target::add_thread (ptid_t ptid, HANDLE h, void *tlb, return th; } -/* Clear out any old thread list and reinitialize it to a - pristine state. */ -static void -windows_init_thread_list (void) -{ - DEBUG_EVENTS ("called"); - windows_process->thread_list.clear (); -} - /* Delete a thread from the list of threads. PTID is the ptid of the thread to be deleted. @@ -351,12 +349,6 @@ void windows_nat_target::delete_thread (ptid_t ptid, DWORD exit_code, bool main_thread_p) { - DWORD id; - - gdb_assert (ptid.lwp () != 0); - - id = ptid.lwp (); - /* Note that no notification was printed when the main thread was created, and thus, unless in verbose mode, we should be symmetrical, and avoid an exit notification for the main thread here as well. */ @@ -364,16 +356,6 @@ windows_nat_target::delete_thread (ptid_t ptid, DWORD exit_code, bool silent = (main_thread_p && !info_verbose); thread_info *to_del = this->find_thread (ptid); delete_thread_with_exit_code (to_del, exit_code, silent); - - auto iter = std::find_if (windows_process->thread_list.begin (), - windows_process->thread_list.end (), - [=] (std::unique_ptr &th) - { - return th->tid == id; - }); - - if (iter != windows_process->thread_list.end ()) - windows_process->thread_list.erase (iter); } void @@ -712,7 +694,7 @@ BOOL windows_nat_target::windows_continue (DWORD continue_status, int id, windows_continue_flags cont_flags) { - for (auto &th : windows_process->thread_list) + for (auto *th : all_windows_threads ()) { if ((id == -1 || id == (int) th->tid) && !th->suspended @@ -730,9 +712,9 @@ windows_nat_target::windows_continue (DWORD continue_status, int id, } } - for (auto &th : windows_process->thread_list) + for (auto *th : all_windows_threads ()) if (id == -1 || id == (int) th->tid) - continue_one_thread (th.get (), cont_flags); + continue_one_thread (th, cont_flags); continue_last_debug_event_main_thread (_("Failed to resume program execution"), continue_status, @@ -914,7 +896,7 @@ windows_nat_target::get_windows_debug_event /* If there is a relevant pending stop, report it now. See the comment by the definition of "windows_thread_info::pending_status" for details on why this is needed. */ - for (auto &th : windows_process->thread_list) + for (auto *th : all_windows_threads ()) { if (!th->suspended && th->pending_status.kind () != TARGET_WAITKIND_IGNORE) @@ -927,7 +909,7 @@ windows_nat_target::get_windows_debug_event *current_event = th->last_event; ptid_t ptid (windows_process->process_id, thread_id); - windows_process->invalidate_thread_context (th.get ()); + windows_process->invalidate_thread_context (th); return ptid; } } @@ -1226,7 +1208,7 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus, /* All-stop, suspend all threads until they are explicitly resumed. */ - for (auto &thr : windows_process->thread_list) + for (auto *thr : all_windows_threads ()) thr->suspend (); } @@ -1382,7 +1364,6 @@ windows_nat_target::attach (const char *args, int from_tty) warning ("Failed to get SE_DEBUG_NAME privilege\n" "This can cause attach to fail on Windows NT/2K/XP"); - windows_init_thread_list (); windows_process->saw_create = 0; std::optional err; @@ -2142,7 +2123,6 @@ windows_nat_target::create_inferior (const char *exec_file, } } - windows_init_thread_list (); do_synchronously ([&] () { BOOL ok = create_process (nullptr, args, flags, w32_env, @@ -2271,7 +2251,6 @@ windows_nat_target::create_inferior (const char *exec_file, /* Final nil string to terminate new env. */ *temp = 0; - windows_init_thread_list (); do_synchronously ([&] () { BOOL ok = create_process (nullptr, /* image */ diff --git a/gdb/windows-nat.h b/gdb/windows-nat.h index 235844a7c95..d63ef6e7b33 100644 --- a/gdb/windows-nat.h +++ b/gdb/windows-nat.h @@ -77,6 +77,38 @@ enum windows_continue_flag DEF_ENUM_FLAGS_TYPE (windows_continue_flag, windows_continue_flags); +/* We want to register windows_thread_info as struct thread_info + private data. thread_info::priv must point to a class that + inherits from private_thread_info. But we can't make + windows_thread_info inherit private_thread_info, because + windows_thread_info is shared with GDBserver. So we make a new + class that inherits from both private_thread_info, + windows_thread_info, and register that one as thread_info::private. + This multiple inheritance is benign, because private_thread_info is + a java-style interface class with no data. */ +struct windows_private_thread_info : private_thread_info, windows_thread_info +{ + windows_private_thread_info (windows_nat::windows_process_info *proc, + DWORD tid, HANDLE h, CORE_ADDR tlb) + : windows_thread_info (proc, tid, h, tlb) + {} + + ~windows_private_thread_info () override + {} +}; + +/* Get the windows_thread_info object associated with THR. */ + +static inline windows_thread_info * +as_windows_thread_info (thread_info *thr) +{ + /* Cast to windows_private_thread_info, which inherits from + private_thread_info, and is implicitly convertible to + windows_thread_info, the return type. */ + private_thread_info *priv = thr->priv.get (); + return gdb::checked_static_cast (priv); +} + struct windows_per_inferior : public windows_nat::windows_process_info { windows_thread_info *find_thread (ptid_t ptid) override; @@ -91,8 +123,6 @@ struct windows_per_inferior : public windows_nat::windows_process_info int windows_initialization_done = 0; - std::vector> thread_list; - /* Counts of things. */ int saw_create = 0; int open_process_used = 0; @@ -352,4 +382,69 @@ int amd64_windows_segment_register_p (int regnum); extern const int amd64_mappings[]; #endif +/* Creates an iterator that works like all_matching_threads_iterator, + but that returns windows_thread_info pointers instead of + thread_info. This could be replaced with a std::range::transform + when we require C++20. */ +class all_windows_threads_iterator +{ +public: + typedef all_windows_threads_iterator self_type; + typedef windows_thread_info value_type; + typedef windows_thread_info *&reference; + typedef windows_thread_info **pointer; + typedef std::forward_iterator_tag iterator_category; + typedef int difference_type; + + explicit all_windows_threads_iterator (all_non_exited_threads_iterator base_iter) + : m_base_iter (base_iter) + {} + + windows_thread_info * operator* () const { return as_windows_thread_info (&*m_base_iter); } + + all_windows_threads_iterator &operator++ () + { + ++m_base_iter; + return *this; + } + + bool operator== (const all_windows_threads_iterator &other) const + { return m_base_iter == other.m_base_iter; } + + bool operator!= (const all_windows_threads_iterator &other) const + { return !(*this == other); } + +private: + all_non_exited_threads_iterator m_base_iter; +}; + +/* The range for all_windows_threads, below. */ + +class all_windows_threads_range : public all_non_exited_threads_range +{ +public: + all_windows_threads_range (all_non_exited_threads_range base_range) + : m_base_range (base_range) + {} + + all_windows_threads_iterator begin () const + { return all_windows_threads_iterator (m_base_range.begin ()); } + all_windows_threads_iterator end () const + { return all_windows_threads_iterator (m_base_range.end ()); } + +private: + all_non_exited_threads_range m_base_range; +}; + +/* Return a range that can be used to walk over all non-exited Windows + threads of all inferiors, with range-for. */ + +static inline all_windows_threads_range +all_windows_threads () +{ + auto *win_tgt = static_cast (get_native_target ()); + return (all_windows_threads_range + (all_non_exited_threads_range (win_tgt, minus_one_ptid))); +} + #endif /* GDB_WINDOWS_NAT_H */ diff --git a/gdb/x86-windows-nat.c b/gdb/x86-windows-nat.c index 5a4876b7fbe..ccabe80f9b4 100644 --- a/gdb/x86-windows-nat.c +++ b/gdb/x86-windows-nat.c @@ -346,7 +346,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 : x86_windows_process.thread_list) + for (auto *th : all_windows_threads ()) th->debug_registers_changed = true; } @@ -356,7 +356,7 @@ windows_set_dr (int i, CORE_ADDR addr) static void windows_set_dr7 (unsigned long val) { - for (auto &th : x86_windows_process.thread_list) + for (auto *th : all_windows_threads ()) th->debug_registers_changed = true; } -- 2.53.0