From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id YIa2HCo0K2hiOCoAWB0awg (envelope-from ) for ; Mon, 19 May 2025 09:37:46 -0400 Received: by simark.ca (Postfix, from userid 112) id 71A571E11C; Mon, 19 May 2025 09:37:46 -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 A5F381E102 for ; Mon, 19 May 2025 09:37:45 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5DDE4385841F for ; Mon, 19 May 2025 13:37:45 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5DDE4385841F Received: from mail-wm1-f46.google.com (mail-wm1-f46.google.com [209.85.128.46]) by sourceware.org (Postfix) with ESMTPS id 74D793858429 for ; Mon, 19 May 2025 13:25:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 74D793858429 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 74D793858429 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.128.46 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1747661106; cv=none; b=ljprmNairh+bwiIKUzwg0bDPTxfyNx230ULwXgOtPaOB6GymfNqNrdkCO+O25VKCzQIff+P/n0xdMeR8x5sqZkjXEWvS6qIyVjWabS3TsjzY2t0CwzvMcWxJmIAW4VvcSFd2Se+3K8DDL/OIoneynZtBFhhi6/nkhYxOcXm/Vy8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1747661106; c=relaxed/simple; bh=ns1ihF1zGLtaXM0qpqI/pVSGWw1GvyTCw6xLX8DQwvQ=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=sDd4/uIM1D9Vt7qcmWtqHcGnmRgz3SwnssxFppxKIZwnQOy4CmzSoAYkJizm1ih+Y5wJP3UC3DCu8xbULsaiVDwennRRjHqzv7fTCiGIgzp7HjWWyARDjy5ZqQhUOIh/LtuSVUiYmuTk46bykDVkocMYaSdBteR0jTXt8z0s3Zs= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 74D793858429 Received: by mail-wm1-f46.google.com with SMTP id 5b1f17b1804b1-440685d6afcso47245015e9.0 for ; Mon, 19 May 2025 06:25:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1747661105; x=1748265905; 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=3PfncT0jxdemDlFfZe+4YgRO3mnN9IVBwAqnJoI8sHA=; b=rdgwVUZNiPr0bekCLdxeQ/ilIa1fpYGHcTdlGk3XkQHkQNMup0urgzincQdX81Hjap BXnFe9d7OKRg62yZ5zwwb/MlqGebE7to69uWznO8ml64LBv99qm/t3UUAggtuRM9EnlZ i0cmrIYrqay1dnnIAR3n2s/gQEnJZLZlwkzTp5nRM3OoMGuv1NCCf1mWPsCCPFM/DjQf J+v7bNnr0z3nSnqrYWrM5c/arCvdRAHHzHj9gJkXJMsTnnY8/VifEHEY8ibLfVEi13CV wlpPjcJPrF76ZZsUmHvLm/YxbOQI7EU+lTYwHBmhygAON5VxrJBUVQtjZUN1AyLagHVj bARA== X-Gm-Message-State: AOJu0YyAqZw9ebVIfvAwE9juPuBFBoaxXT/RG1ostErXRnHrzNsSqdpu fq2vtDl9yc2HCC0PNmIyQS9jvFEpjoII5MFEcHX11BoeiyCnxJYJeGnu10S0SjLo X-Gm-Gg: ASbGncvdpwk6WWJ9S6Pc99+gurJDRyTdDD2JCUKizE712azSATW0DyN+by6rKEyp78y TO4hmQYbkTiFb5XZzPqEANLB1Z3g7ERAwMuzbpNylwTNJ/OsWi2fkzLb0/b1md1WYq3yXXmSnaK xpiWv9T03ZwsiorWvQwJhe/9LvPbUbgfJOF8q2quF30ewgpjwU1fx3sFfORkh3vY7+l4A65idxm ygFtOFgUhArVPx5YRFOL71rEhJSFc1PuWskIkNUqEGfgYP0Ok2OUQPFyXPJLSRIKzi6bpmg39Rf lY76uc+VmNUDARHl3pcXQ+05M0ypPBzazA8xlcIK2kG8TC1rCvY= X-Google-Smtp-Source: AGHT+IFR9m4eWT/UjjNTV8FHctM8UvMJ7wxRoYqp6gMBhe5VodHeslvPNAQMABNzI3Le+kzdyKrQCw== X-Received: by 2002:a05:600c:1d8c:b0:442:e109:3032 with SMTP id 5b1f17b1804b1-445917d2a0amr39579535e9.24.1747661104798; Mon, 19 May 2025 06:25:04 -0700 (PDT) Received: from localhost ([2001:8a0:4fe9:b400:8d90:6f0d:36bf:32df]) by smtp.gmail.com with UTF8SMTPSA id 5b1f17b1804b1-442f3369293sm215947485e9.6.2025.05.19.06.25.04 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 19 May 2025 06:25:04 -0700 (PDT) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH v2 37/47] Windows GDB: make windows_thread_info be private thread_info data Date: Mon, 19 May 2025 14:22:58 +0100 Message-ID: <20250519132308.3553663-38-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 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/windows-nat.c | 170 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 118 insertions(+), 52 deletions(-) diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c index d336889a251..492d2e3f55b 100644 --- a/gdb/windows-nat.c +++ b/gdb/windows-nat.c @@ -107,6 +107,26 @@ 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_process_info *proc, + DWORD tid, HANDLE h, CORE_ADDR tlb) + : windows_thread_info (proc, tid, h, tlb) + {} + + ~windows_private_thread_info () override + {} +}; + struct windows_per_inferior : public windows_process_info { windows_thread_info *find_thread (ptid_t ptid) override; @@ -124,8 +144,6 @@ struct windows_per_inferior : public windows_process_info int windows_initialization_done = 0; - std::vector> thread_list; - /* Counts of things. */ int saw_create = 0; int open_process_used = 0; @@ -415,6 +433,82 @@ struct windows_nat_target final : public x86_nat_target bool m_continued = false; }; +/* Get the windows_thread_info object associated with THR. */ + +static 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. */ + return static_cast (thr->priv.get ()); +} + +/* 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. */ + +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))); +} + static void check (BOOL ok, const char *file, int line) { @@ -562,10 +656,11 @@ windows_nat_target::continue_last_debug_event_main_thread 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; + auto *win_tgt = static_cast (get_native_target ()); + thread_info *thr = win_tgt->find_thread (ptid); + if (thr == nullptr) + return nullptr; + return as_windows_thread_info (thr); } /* Invalidate TH's context. */ @@ -593,12 +688,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__ @@ -607,18 +701,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. */ @@ -627,15 +721,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. @@ -647,12 +732,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. */ @@ -660,16 +739,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); } /* Fetches register number R from the given windows_thread_info, @@ -1318,7 +1387,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 @@ -1336,9 +1405,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) - windows_process.continue_one_thread (th.get (), cont_flags); + windows_process.continue_one_thread (th, cont_flags); continue_last_debug_event_main_thread (_("Failed to resume program execution"), continue_status, @@ -1516,7 +1585,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) @@ -1529,7 +1598,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_context (th.get ()); + windows_process.invalidate_context (th); return ptid; } } @@ -1833,7 +1902,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 (); } @@ -2003,7 +2072,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; @@ -2756,7 +2824,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, @@ -2885,7 +2952,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 */ @@ -3254,7 +3320,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 : windows_process.thread_list) + for (auto *th : all_windows_threads ()) th->debug_registers_changed = true; } @@ -3264,7 +3330,7 @@ windows_set_dr (int i, CORE_ADDR addr) static void windows_set_dr7 (unsigned long val) { - for (auto &th : windows_process.thread_list) + for (auto *th : all_windows_threads ()) th->debug_registers_changed = true; } -- 2.49.0