From patchwork Sun Nov 12 20:25:42 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 79671 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 792723858020 for ; Sun, 12 Nov 2023 20:27:14 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from omta36.uswest2.a.cloudfilter.net (omta36.uswest2.a.cloudfilter.net [35.89.44.35]) by sourceware.org (Postfix) with ESMTPS id 08F753858C5F for ; Sun, 12 Nov 2023 20:25:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 08F753858C5F 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 08F753858C5F Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=35.89.44.35 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699820740; cv=none; b=ff5dKU9A6rfvBgmS+7LA6QAVQ8JNhSRq9aELkLa1yQcKNBaQbarrXM8qkzQVt+F7G/xDxf/hUSy3e42b1rRx+pOA5+xk/vbEbYQ4s4QJ11jcmclyMcsquU1KpwSrEUtO63x+/ZmZxNhfk3pBZBtypRGh2pUEVFMyIZSzfQIvBg4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699820740; c=relaxed/simple; bh=6YNorzUPApC14XmXmFO+mMLdNDHZD2H0gigBGNMGRHk=; h=DKIM-Signature:From:Date:Subject:MIME-Version:Message-Id:To; b=LtmkByno5g1Z/EU4afubJ1SfswNFS4lSjflMlkMNNneB7LE3bodqRgdw7E69W1E2v/iFTQ9dvMUJSzNVGUm4RNHmXbAIplBsLuxs9lsFBxFSFTf3dCnMuYuxsZnnLqZZIjxPZskhaMvpzw1zPDMNzv/dcNamW4+PtIiY5RL1+cI= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from eig-obgw-5006a.ext.cloudfilter.net ([10.0.29.179]) by cmsmtp with ESMTPS id 1mBfrylbMhqFd2H1RrSTXO; Sun, 12 Nov 2023 20:25:33 +0000 Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with ESMTPS id 2H1QrfEyT1J282H1QrZzs4; Sun, 12 Nov 2023 20:25:33 +0000 X-Authority-Analysis: v=2.4 cv=Tqz1ORbh c=1 sm=1 tr=0 ts=655134bd a=ApxJNpeYhEAb1aAlGBBbmA==:117 a=ApxJNpeYhEAb1aAlGBBbmA==:17 a=OWjo9vPv0XrRhIrVQ50Ab3nP57M=:19 a=dLZJa+xiwSxG16/P+YVxDGlgEgI=:19 a=IkcTkHD0fZMA:10 a=BNY50KLci1gA: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=ds1rKtR8OgzanSYHVLaQEeAy03vSDqCB6AHeWXX/LPE=; b=OQS6nP/tBRnj0ixPpNpfaM7DIh WlgcOlI+Ygmm/p2SmCvuVUNSE2fVkSMdDB1TS9G7Z6QKGL9yyfc7T2sT6bdiMNm+9p6eiOSguJMiA g6qIFZxEc0y8t1y6Im8yptGAp; Received: from 97-122-77-73.hlrn.qwest.net ([97.122.77.73]:34540 helo=[192.168.0.21]) by box5379.bluehost.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96.2) (envelope-from ) id 1r2H1Q-000cUU-0z for gdb-patches@sourceware.org; Sun, 12 Nov 2023 13:25:32 -0700 From: Tom Tromey Date: Sun, 12 Nov 2023 13:25:42 -0700 Subject: [PATCH v2 05/18] Refactor complaint thread-safety approach MIME-Version: 1.0 Message-Id: <20231112-t-bg-dwarf-reading-v2-5-70fb170012ba@tromey.com> References: <20231112-t-bg-dwarf-reading-v2-0-70fb170012ba@tromey.com> In-Reply-To: <20231112-t-bg-dwarf-reading-v2-0-70fb170012ba@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: 97.122.77.73 X-Source-L: No X-Exim-ID: 1r2H1Q-000cUU-0z X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: 97-122-77-73.hlrn.qwest.net ([192.168.0.21]) [97.122.77.73]:34540 X-Source-Auth: tom+tromey.com X-Email-Count: 6 X-Org: HG=bhshared;ORG=bluehost; X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes X-CMAE-Envelope: MS4xfLMTWnGWZImWk8Z3GTzX3lgHZAr/+vA9fBoimztMqTgtOobKT7v4pseO3tE7opHYUkl7Nprn8yagwrg65Ds0ZU4LPJ0eKzmyaTiTw6R9BYQmA5YqJEC2 Ag6NP3mcLm/HpnXIBYr8n8krpMiX/WwNwMpfZOINo/I1Kdj57IGRHhSEpckAxNSf1L6/yf8yguQRWE05ENqx/fUfhkoVf6QwI2I= X-Spam-Status: No, score=-3024.4 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 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+patchwork=sourceware.org@sourceware.org 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 b8612e1ac6d..a539faa00f0 100644 --- a/gdb/defs.h +++ b/gdb/defs.h @@ -545,7 +545,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 86fd42c3579..e1f1db4dcb3 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -4946,9 +4946,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) @@ -4958,18 +4955,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) @@ -4985,15 +4987,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 5028440b671..93c30c06ef3 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