From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from omta38.uswest2.a.cloudfilter.net (omta38.uswest2.a.cloudfilter.net [35.89.44.37]) by sourceware.org (Postfix) with ESMTPS id 2E1893858284 for ; Sun, 10 Dec 2023 21:41:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2E1893858284 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=tromey.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tromey.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 2E1893858284 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=35.89.44.37 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702244486; cv=none; b=vOqmg6UPJDEzRRgzcPQwf2lr/oscPiAMHbUbjB2UxKs/nxmLA5pbof/Is9PEY+MyjQfqVJxAjj3Fo4AZXlg07cjqv3pL1KzWWpyGA/s9EL/5Q5ryMrZiUhw1RhF7uFQ5g+ogO9T6l9/e0Ao7xe88WHRO0Ht0K+yUrZ5P+A8WE/E= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702244486; c=relaxed/simple; bh=iWx8f+hJFTGhQZ+UEn+yUKV4NsMloMCXgL8PaIMR6ZA=; h=DKIM-Signature:From:Date:Subject:MIME-Version:Message-Id:To; b=CwSrNCxHHJhKeMNsxAVzxobW3s8JkpyDcKyRPwzpXKMRQXWm1b12mxyyq6VgvFAUAuNJjhrUqO4SPgiNP8AekiKoo5xYMyCziCd7SSZ//qihGRWn8p61aS2HugL+ltjJqlXHAOJyeC7l+ngWGGb9etR3E7wAGfZQWYXTsQq//BQ= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from eig-obgw-6010a.ext.cloudfilter.net ([10.0.30.248]) by cmsmtp with ESMTPS id CDnIrPetYVly7CRY9rWz9X; Sun, 10 Dec 2023 21:41:21 +0000 Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with ESMTPS id CRY8rlVgO8vT0CRY8rz0nn; Sun, 10 Dec 2023 21:41:20 +0000 X-Authority-Analysis: v=2.4 cv=ffi+dmcF c=1 sm=1 tr=0 ts=65763080 a=ApxJNpeYhEAb1aAlGBBbmA==:117 a=ApxJNpeYhEAb1aAlGBBbmA==:17 a=OWjo9vPv0XrRhIrVQ50Ab3nP57M=:19 a=dLZJa+xiwSxG16/P+YVxDGlgEgI=:19 a=IkcTkHD0fZMA:10 a=e2cXIFwxEfEA:10 a=Qbun_eYptAEA:10 a=5X6PLcaWoE8leiRlH68A:9 a=QEXdDO2ut3YA:10 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=To:In-Reply-To:References:Message-Id:Content-Transfer-Encoding: Content-Type:MIME-Version:Subject:Date:From:Sender:Reply-To:Cc:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=/PgI8iPMTaw39btbAVh6IWDdJCeqk8VJN+LSEP3agVs=; b=K7LzY/gxZYB6A7xaqxxVTBPWZy KFT7WyTgRBijsaFC1Axsf9w/kFk5ClXuuyjLPvwBwaLT7ux1D92cc6hZi4U+hxdYSVULpG/MzUemF zkeOBDgEMWfih3jcn2klhYoeV; Received: from [198.59.47.65] (port=52450 helo=[192.168.131.83]) by box5379.bluehost.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96.2) (envelope-from ) id 1rCRY8-000SHj-0g for gdb-patches@sourceware.org; Sun, 10 Dec 2023 14:41:20 -0700 From: Tom Tromey Date: Sun, 10 Dec 2023 14:41:13 -0700 Subject: [PATCH v4 05/19] Refactor complaint thread-safety approach MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20231210-t-bg-dwarf-reading-v4-5-b978c32fd12f@tromey.com> References: <20231210-t-bg-dwarf-reading-v4-0-b978c32fd12f@tromey.com> In-Reply-To: <20231210-t-bg-dwarf-reading-v4-0-b978c32fd12f@tromey.com> To: gdb-patches@sourceware.org X-Mailer: b4 0.12.4 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - box5379.bluehost.com X-AntiAbuse: Original Domain - sourceware.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - tromey.com X-BWhitelist: no X-Source-IP: 198.59.47.65 X-Source-L: No X-Exim-ID: 1rCRY8-000SHj-0g X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: ([192.168.131.83]) [198.59.47.65]:52450 X-Source-Auth: tom+tromey.com X-Email-Count: 7 X-Org: HG=bhshared;ORG=bluehost; X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes X-CMAE-Envelope: MS4xfAe528s4yeGaq/VBzRKeO23lxx1jA+vGHLm34ac9FZ8WaxyYy2fwow4cpklX3EvibD+6ltSYJs7mMnUq1Q4qbY6R4DmE8ILn9yISY+84chwrB1uqpBPe ubUmVJmWfVgtwdF3xFznIOtISBV50WMGWNt/k2gNxTS7qiKOmxwcQp+a7u/wcWktEU0H/GRpxLfzit9M77chFM/Vh8cfZHNvQRI= X-Spam-Status: No, score=-3024.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,GIT_PATCH_0,JMQ_SPF_NEUTRAL,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: This patch changes the way complaint works in a background thread. The new approach requires installing a complaint interceptor in each worker, and then the resulting complaints are treated as one of the results of the computation. This change is needed for a subsequent patch, where installing a complaint interceptor around a parallel-for is no longer a viable approach. --- gdb/complaints.c | 24 +++++++++++------------- gdb/complaints.h | 37 +++++++++++++++++++++++++++++-------- gdb/defs.h | 2 +- gdb/dwarf2/read.c | 31 ++++++++++++++++++------------- gdb/top.c | 2 +- 5 files changed, 60 insertions(+), 36 deletions(-) diff --git a/gdb/complaints.c b/gdb/complaints.c index 302f107b519..eb648c655ed 100644 --- a/gdb/complaints.c +++ b/gdb/complaints.c @@ -21,6 +21,7 @@ #include "complaints.h" #include "command.h" #include "gdbcmd.h" +#include "run-on-main-thread.h" #include "gdbsupport/selftest.h" #include #include @@ -78,17 +79,14 @@ clear_complaints () /* See complaints.h. */ -complaint_interceptor *complaint_interceptor::g_complaint_interceptor; +thread_local complaint_interceptor *complaint_interceptor::g_complaint_interceptor; /* See complaints.h. */ complaint_interceptor::complaint_interceptor () - : m_saved_warning_hook (deprecated_warning_hook) + : m_saved_warning_hook (&deprecated_warning_hook, issue_complaint), + m_saved_complaint_interceptor (&g_complaint_interceptor, this) { - /* These cannot be stacked. */ - gdb_assert (g_complaint_interceptor == nullptr); - g_complaint_interceptor = this; - deprecated_warning_hook = issue_complaint; } /* A helper that wraps a warning hook. */ @@ -104,19 +102,19 @@ wrap_warning_hook (void (*hook) (const char *, va_list), ...) /* See complaints.h. */ -complaint_interceptor::~complaint_interceptor () +void +re_emit_complaints (const complaint_collection &complaints) { - for (const std::string &str : m_complaints) + gdb_assert (is_main_thread ()); + + for (const std::string &str : complaints) { - if (m_saved_warning_hook) - wrap_warning_hook (m_saved_warning_hook, str.c_str ()); + if (deprecated_warning_hook) + wrap_warning_hook (deprecated_warning_hook, str.c_str ()); else gdb_printf (gdb_stderr, _("During symbol reading: %s\n"), str.c_str ()); } - - g_complaint_interceptor = nullptr; - deprecated_warning_hook = m_saved_warning_hook; } /* See complaints.h. */ diff --git a/gdb/complaints.h b/gdb/complaints.h index e9e846684ac..1626f200685 100644 --- a/gdb/complaints.h +++ b/gdb/complaints.h @@ -57,29 +57,45 @@ have_complaint () extern void clear_complaints (); +/* Type of collected complaints. */ + +typedef std::unordered_set complaint_collection; + /* A class that can handle calls to complaint from multiple threads. When this is instantiated, it hooks into the complaint mechanism, - so the 'complaint' macro can continue to be used. When it is - destroyed, it issues all the complaints that have been stored. It - should only be instantiated in the main thread. */ + so the 'complaint' macro can continue to be used. It collects all + the complaints (and warnings) emitted while it is installed, and + these can be retrieved before the object is destroyed. This is + intended for use from worker threads (though it will also work on + the main thread). Messages can be re-emitted on the main thread + using re_emit_complaints, see below. */ class complaint_interceptor { public: complaint_interceptor (); - ~complaint_interceptor (); + ~complaint_interceptor () = default; DISABLE_COPY_AND_ASSIGN (complaint_interceptor); + complaint_collection &&release () + { + return std::move (m_complaints); + } + private: /* The issued complaints. */ - std::unordered_set m_complaints; + complaint_collection m_complaints; + + typedef void (*saved_warning_hook_ftype) (const char *, va_list); /* The saved value of deprecated_warning_hook. */ - void (*m_saved_warning_hook) (const char *, va_list) - ATTRIBUTE_FPTR_PRINTF (1,0); + scoped_restore_tmpl m_saved_warning_hook; + + /* The saved value of g_complaint_interceptor. */ + scoped_restore_tmpl m_saved_complaint_interceptor; /* A helper function that is used by the 'complaint' implementation to issue a complaint. */ @@ -87,7 +103,12 @@ class complaint_interceptor ATTRIBUTE_PRINTF (1, 0); /* This object. Used by the static callback function. */ - static complaint_interceptor *g_complaint_interceptor; + static thread_local complaint_interceptor *g_complaint_interceptor; }; +/* Re-emit complaints that were collected by complaint_interceptor. + This can only be called on the main thread. */ + +extern void re_emit_complaints (const complaint_collection &); + #endif /* !defined (COMPLAINTS_H) */ diff --git a/gdb/defs.h b/gdb/defs.h index bcce4f4c3e4..e73a82402c9 100644 --- a/gdb/defs.h +++ b/gdb/defs.h @@ -539,7 +539,7 @@ extern void (*deprecated_print_frame_info_listing_hook) (struct symtab * s, int noerror); extern int (*deprecated_query_hook) (const char *, va_list) ATTRIBUTE_FPTR_PRINTF(1,0); -extern void (*deprecated_warning_hook) (const char *, va_list) +extern thread_local void (*deprecated_warning_hook) (const char *, va_list) ATTRIBUTE_FPTR_PRINTF(1,0); extern void (*deprecated_readline_begin_hook) (const char *, ...) ATTRIBUTE_FPTR_PRINTF_1; diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 78acbd39ef1..bd45c2c9170 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -4930,9 +4930,6 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile) index_storage.get_addrmap ()); { - /* Ensure that complaints are handled correctly. */ - complaint_interceptor complaint_handler; - using iter_type = decltype (per_bfd->all_units.begin ()); auto task_size_ = [] (iter_type iter) @@ -4942,18 +4939,23 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile) }; auto task_size = gdb::make_function_view (task_size_); - /* Each thread returns a pair holding a cooked index, and a vector - of errors that should be printed. The latter is done because - GDB's I/O system is not thread-safe. run_on_main_thread could be - used, but that would mean the messages are printed after the - prompt, which looks weird. */ - using result_type = std::pair, - std::vector>; + /* Each thread returns a tuple holding a cooked index, any + collected complaints, and a vector of errors that should be + printed. The latter is done because GDB's I/O system is not + thread-safe. run_on_main_thread could be used, but that would + mean the messages are printed after the prompt, which looks + weird. */ + using result_type = std::tuple, + complaint_collection, + std::vector>; std::vector results = gdb::parallel_for_each (1, per_bfd->all_units.begin (), per_bfd->all_units.end (), [=] (iter_type iter, iter_type end) { + /* Ensure that complaints are handled correctly. */ + complaint_interceptor complaint_handler; + std::vector errors; cooked_index_storage thread_storage; for (; iter != end; ++iter) @@ -4969,15 +4971,18 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile) errors.push_back (std::move (except)); } } - return result_type (thread_storage.release (), std::move (errors)); + return result_type (thread_storage.release (), + complaint_handler.release (), + std::move (errors)); }, task_size); /* Only show a given exception a single time. */ std::unordered_set seen_exceptions; for (auto &one_result : results) { - indexes.push_back (std::move (one_result.first)); - for (auto &one_exc : one_result.second) + indexes.push_back (std::move (std::get<0> (one_result))); + re_emit_complaints (std::get<1> (one_result)); + for (auto &one_exc : std::get<2> (one_result)) if (seen_exceptions.insert (one_exc).second) exception_print (gdb_stderr, one_exc); } diff --git a/gdb/top.c b/gdb/top.c index d211f1b08be..009bf2b0c1c 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -221,7 +221,7 @@ int (*deprecated_query_hook) (const char *, va_list); /* Replaces most of warning. */ -void (*deprecated_warning_hook) (const char *, va_list); +thread_local void (*deprecated_warning_hook) (const char *, va_list); /* These three functions support getting lines of text from the user. They are used in sequence. First deprecated_readline_begin_hook is -- 2.43.0