From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd35.google.com (mail-io1-xd35.google.com [IPv6:2607:f8b0:4864:20::d35]) by sourceware.org (Postfix) with ESMTPS id 9E7B7394800C for ; Tue, 7 Jun 2022 17:10:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9E7B7394800C Received: by mail-io1-xd35.google.com with SMTP id 134so9878610iou.12 for ; Tue, 07 Jun 2022 10:10:11 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=gyEZ+78PN4BB9yg/wNqq7p5AvzSh/FUWZcK9/Kak7K4=; b=uRIoRblgbI5b/f184NJzmza1Qg5lkDxQGwHIqojeUMSs1vu0U9pMfmx/XzXnfK+g5b eBeBQH+ChGO5mbs8Dhefqeatw191AbrZ1ZKtAEYlnxuPMS2BEiAZ0+HDvc/VqzjDhSeR H3JwYtuhp1+goctpQcLtBgyrcG+XCkdKPrOGCLWmkaxehC0xVkhWbdIIZKS8QAzQTxWF DJvySVk+kS6eRp1jwY39537tm6H63o8RfBnoKeVrhhENAYnlMcYRvAT8G8VEIrUef/u9 +gJOapge1zDqduQjT/Hkiy3LP+NyELu1Pd87lEQJTLgM+uu0MI7Ph5SSbg4aiw4jT3zd ipeg== X-Gm-Message-State: AOAM530TiSekTdRfPuWJqcJTt6uqEvvh4LBbXuBTYB1F2CglfsHh67e1 jnnj3i3F8a9eZiZCjiYwHT8IWI2nZSPKww== X-Google-Smtp-Source: ABdhPJztyCABoMv3CXjcIgA2EeZsd15RK3qJsThjxsYsAZTMa+opy78WD7Uh/10uazwfBhmkFDBE/g== X-Received: by 2002:a05:6638:1c07:b0:331:a800:1062 with SMTP id ca7-20020a0566381c0700b00331a8001062mr6526780jab.4.1654621810769; Tue, 07 Jun 2022 10:10:10 -0700 (PDT) Received: from murgatroyd.Home (71-211-171-143.hlrn.qwest.net. [71.211.171.143]) by smtp.gmail.com with ESMTPSA id r2-20020a92d982000000b002d11397f4f9sm7526636iln.74.2022.06.07.10.10.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 07 Jun 2022 10:10:09 -0700 (PDT) From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH v2 2/2] Write the DWARF index in the background Date: Tue, 7 Jun 2022 11:10:02 -0600 Message-Id: <20220607171002.196132-3-tromey@adacore.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20220607171002.196132-1-tromey@adacore.com> References: <20220607171002.196132-1-tromey@adacore.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, 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.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Jun 2022 17:10:13 -0000 The new DWARF cooked indexer interacts poorly with the DWARF index cache. In particular, the cache will require gdb to wait for the cooked index to be finalized. As this happens in the foreground, it means that users with this setting enabled will see a slowdown. This patch changes gdb to write the cache entry a worker thread. (As usual, in the absence of threads, this work is simply done immediately in the main thread.) Some care is taken to ensure that this can't crash, and that gdb will not exit before the task is complete. To avoid use-after-free problems, the DWARF per-BFD object explicitly waits for the index cache task to complete. To avoid gdb exiting early, an exit observer is used to wait for all such pending tasks. In normal use, neither of these waits will be very visible. For users using "-batch" to pre-generate the index, though, it would be. However I don't think there is much to be done about this, as it was the status quo ante. --- gdb/dwarf2/cooked-index.c | 74 +++++++++++++++++++++++++++++++++++++-- gdb/dwarf2/cooked-index.h | 31 +++++++++------- gdb/dwarf2/mapped-index.h | 9 +++++ gdb/dwarf2/read.c | 11 +++--- 4 files changed, 107 insertions(+), 18 deletions(-) diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c index 3e34ad0e501..a0ad950282c 100644 --- a/gdb/dwarf2/cooked-index.c +++ b/gdb/dwarf2/cooked-index.c @@ -20,10 +20,19 @@ #include "defs.h" #include "dwarf2/cooked-index.h" #include "dwarf2/read.h" +#include "dwarf2/index-cache.h" #include "cp-support.h" #include "ada-lang.h" #include "split-name.h" +#include "observable.h" +#include "run-on-main-thread.h" #include +#include + +/* We don't want gdb to exit while it is in the process of writing to + the index cache. So, all live cooked index vectors are stored + here, and then these are all waited for before exit proceeds. */ +static std::unordered_set active_vectors; /* Hash function for cooked_index_entry. */ @@ -281,11 +290,44 @@ cooked_index::find (gdb::string_view name, bool completing) return range (lower, upper); } -cooked_index_vector::cooked_index_vector (vec_type &&vec) - : m_vector (std::move (vec)) +cooked_index_vector::cooked_index_vector (vec_type &&vec, + dwarf2_per_bfd *per_bfd) + : m_vector (std::move (vec)), + m_write_future (gdb::thread_pool::g_thread_pool->post_task + ([this, per_bfd] () + { + maybe_write_index (per_bfd); + })) { for (auto &idx : m_vector) idx->finalize (); + + /* ACTIVE_VECTORS is not locked, and this assert ensures that this + will be caught if ever moved to the background. */ + gdb_assert (is_main_thread ()); + active_vectors.insert (this); +} + +cooked_index_vector::~cooked_index_vector () +{ + /* The 'finalize' method may be run in a different thread. If + this object is destroyed before this completes, then the method + will end up writing to freed memory. Waiting for this to + complete avoids this problem; and the cost seems ignorable + because creating and immediately destroying the debug info is a + relatively rare thing to do. */ + for (auto &idx : m_vector) + idx->wait (); + + /* Likewise for the index-creating future, though this one must also + waited for by the per-BFD object to ensure the required data + remains live. */ + m_write_future.wait (); + + /* Remove our entry from the global list. See the assert in the + constructor to understand this. */ + gdb_assert (is_main_thread ()); + active_vectors.erase (this); } /* See cooked-index.h. */ @@ -344,3 +386,31 @@ cooked_index_vector::get_main () const return result; } + +void +cooked_index_vector::maybe_write_index (dwarf2_per_bfd *per_bfd) +{ + /* Wait for finalization. */ + for (auto &idx : m_vector) + idx->wait (); + + /* (maybe) store an index in the cache. */ + global_index_cache.store (per_bfd); +} + +/* Wait for all the index cache entries to be written before gdb + exits. */ +static void +wait_for_index_cache (int) +{ + gdb_assert (is_main_thread ()); + for (cooked_index_vector *item : active_vectors) + item->wait (); +} + +void _initialize_cooked_index (); +void +_initialize_cooked_index () +{ + gdb::observers::gdb_exiting.attach (wait_for_index_cache, "cooked-index"); +} diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h index 4e65e9d0c61..40f1bfdbf99 100644 --- a/gdb/dwarf2/cooked-index.h +++ b/gdb/dwarf2/cooked-index.h @@ -36,6 +36,7 @@ #include "gdbsupport/range-chain.h" struct dwarf2_per_cu_data; +struct dwarf2_per_bfd; /* Flags that describe an entry in the index. */ enum cooked_index_flag_enum : unsigned char @@ -292,20 +293,10 @@ class cooked_index_vector : public dwarf_scanner_base object. */ typedef std::vector> vec_type; - explicit cooked_index_vector (vec_type &&vec); + explicit cooked_index_vector (vec_type &&vec, dwarf2_per_bfd *per_bfd); DISABLE_COPY_AND_ASSIGN (cooked_index_vector); - ~cooked_index_vector () - { - /* The 'finalize' methods may be run in a different thread. If - this object is destroyed before these complete, then one will - end up writing to freed memory. Waiting for finalization to - complete avoids this problem; and the cost seems ignorable - because creating and immediately destroying the debug info is a - relatively rare thing to do. */ - for (auto &item : m_vector) - item->wait (); - } + ~cooked_index_vector (); /* A range over a vector of subranges. */ typedef range_chain range; @@ -345,11 +336,27 @@ class cooked_index_vector : public dwarf_scanner_base quick_symbol_functions_up make_quick_functions () const override; + /* Wait for the index to be completely finished. For ordinary uses, + the index code ensures this itself -- e.g., 'all_entries' will + wait on the 'finalize' future. However, on destruction, if an + index is being written, it's also necessary to wait for that to + complete. */ + void wait () override + { + m_write_future.wait (); + } + private: + /* Maybe write the index to the index cache. */ + void maybe_write_index (dwarf2_per_bfd *per_bfd); + /* The vector of cooked_index objects. This is stored because the entries are stored on the obstacks in those objects. */ vec_type m_vector; + + /* A future that tracks when the 'index_write' method is done. */ + std::future m_write_future; }; #endif /* GDB_DWARF2_COOKED_INDEX_H */ diff --git a/gdb/dwarf2/mapped-index.h b/gdb/dwarf2/mapped-index.h index 7d71347f9f4..de2ddb2c8d5 100644 --- a/gdb/dwarf2/mapped-index.h +++ b/gdb/dwarf2/mapped-index.h @@ -73,6 +73,15 @@ struct dwarf_scanner_base will return 'this' as a cooked index. For other forms, it will throw an exception with an appropriate error message. */ virtual cooked_index_vector *index_for_writing () = 0; + + /* Wait for reading of the debuginfo to be completely finished. + This normally has a trivial implementation, but if a subclass + does any background reading, it's needed to ensure that the + reading is completed before destroying the containing per-BFD + object. */ + virtual void wait () + { + } }; /* Base class containing bits shared by both .gdb_index and diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 766eb650710..e604a0f0f08 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -1460,6 +1460,11 @@ dwarf2_per_bfd::dwarf2_per_bfd (bfd *obfd, const dwarf2_debug_sections *names, dwarf2_per_bfd::~dwarf2_per_bfd () { + /* Data from the per-BFD may be needed when finalizing the cooked + index table, so wait here while this happens. */ + if (index_table != nullptr) + index_table->wait (); + for (auto &per_cu : all_comp_units) { per_cu->imported_symtabs_free (); @@ -5375,9 +5380,6 @@ dwarf2_build_psymtabs (struct objfile *objfile) try { dwarf2_build_psymtabs_hard (per_objfile); - - /* (maybe) store an index in the cache. */ - global_index_cache.store (per_objfile->per_bfd); } catch (const gdb_exception_error &except) { @@ -7147,7 +7149,8 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile) indexes.end ()); indexes.shrink_to_fit (); - cooked_index_vector *vec = new cooked_index_vector (std::move (indexes)); + cooked_index_vector *vec = new cooked_index_vector (std::move (indexes), + per_bfd); per_bfd->index_table.reset (vec); const cooked_index_entry *main_entry = vec->get_main (); -- 2.34.1