From patchwork Tue Jan 10 18:33:35 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 62900 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 E260638493D1 for ; Tue, 10 Jan 2023 18:34:15 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E260638493D1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1673375655; bh=KeV1bWAdoQUzhrdQ+PUXRn0w9106CLPMKm+N+JYjUN0=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=uHA+I2wCpcTMDD2KGHGnytj+YX05Up1aDdocHSJNSSe2MCxc+/K9Gqk6rDgo3YLe9 hUHN0jWMyP/ziu1NSiz7iW1uFXzVJ8bj3qRwLeYXOa36A7KewQMsGF2tTnT+YTvXEX QyYI9GQd2Emz6A7RS5phllDv2BdU/UzZyHNmjQ2c= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-io1-xd32.google.com (mail-io1-xd32.google.com [IPv6:2607:f8b0:4864:20::d32]) by sourceware.org (Postfix) with ESMTPS id D331B3858CDA for ; Tue, 10 Jan 2023 18:33:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D331B3858CDA Received: by mail-io1-xd32.google.com with SMTP id z71so302033iof.11 for ; Tue, 10 Jan 2023 10:33:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=KeV1bWAdoQUzhrdQ+PUXRn0w9106CLPMKm+N+JYjUN0=; b=lF3VXQqXBSiq2Vh/lIJZjAVqJnaO4SYI6ZF5PLo/T9IhUIf3StWumXO3QXqk3M7sL3 0RqM7sKzmRPSojnd38YQNtCg9aN+s6SiWCWFidjX7vGfaXjvwMxalav7MdJQSGxt17vV 9XeHdYAOsaPIr6RMMDo+a+f/iAH8Nx0VDYXo3mXqLnDNKBkvT6yvi4jT6/uDaUC1KwDs rwEd1fhHC5mxlTgXbKnGrnUXXSSNJcEeCSoBAOuDfTp5seI2wLUL9nqvFJC5S1NcBZES p1KPIoeMVr2tj43zp4NrUy/Rfmwj9qkJ0C8xdcPIXIuC2QhrumiaeqsXM/L7JHNvsbVL djuA== X-Gm-Message-State: AFqh2krPt6gI1fOn6kWFYk8ca+HJXmOTQVZIdsyC7DmMxqbd/EoTxHdl J/Oc+q+uT6TVUhmNwJ3h2bdpXELKnUlxWYHs X-Google-Smtp-Source: AMrXdXs9ZEyrtzITTU1T7ZS5v5plvc5bEMSjp4ahH9DckzMzzBFt336UjYj/wsSZLYk/GxHWxLfSow== X-Received: by 2002:a05:6602:124b:b0:6e2:d3f9:56d with SMTP id o11-20020a056602124b00b006e2d3f9056dmr54077745iou.0.1673375630112; Tue, 10 Jan 2023 10:33:50 -0800 (PST) Received: from localhost.localdomain (97-122-76-186.hlrn.qwest.net. [97.122.76.186]) by smtp.gmail.com with ESMTPSA id w20-20020a05663800d400b00375783003fcsm3739933jao.136.2023.01.10.10.33.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Jan 2023 10:33:49 -0800 (PST) To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH v2 1/4] Avoid submitting empty tasks in parallel_for_each Date: Tue, 10 Jan 2023 11:33:35 -0700 Message-Id: <20230110183338.2623088-2-tromey@adacore.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20230110183338.2623088-1-tromey@adacore.com> References: <20230110183338.2623088-1-tromey@adacore.com> MIME-Version: 1.0 X-Spam-Status: No, score=-11.3 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 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-Patchwork-Original-From: Tom Tromey via Gdb-patches From: Tom Tromey Reply-To: Tom Tromey Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" I found that parallel_for_each would submit empty tasks to the thread pool. For example, this can happen if the number of tasks is smaller than the number of available threads. In the DWARF reader, this resulted in the cooked index containing empty sub-indices. This patch arranges to instead shrink the result vector and process the trailing entries in the calling thread. --- gdb/unittests/parallel-for-selftests.c | 39 ++++++++++++++++++++++++++ gdbsupport/parallel-for.h | 30 ++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/gdb/unittests/parallel-for-selftests.c b/gdb/unittests/parallel-for-selftests.c index 3162db18df1..15a095ae62b 100644 --- a/gdb/unittests/parallel-for-selftests.c +++ b/gdb/unittests/parallel-for-selftests.c @@ -149,6 +149,45 @@ TEST (int n_threads) SELF_CHECK (counter == NUMBER); #undef NUMBER + + /* Check that if there are fewer tasks than threads, then we won't + end up with a null result. */ + std::vector> intresults; + std::atomic any_empty_tasks (false); + + FOR_EACH (1, 0, 1, + [&] (int start, int end) + { + if (start == end) + any_empty_tasks = true; + return std::unique_ptr (new int (end - start)); + }); + SELF_CHECK (!any_empty_tasks); + SELF_CHECK (std::all_of (intresults.begin (), + intresults.end (), + [] (const std::unique_ptr &entry) + { + return entry != nullptr; + })); + + /* The same but using the task size parameter. */ + intresults.clear (); + any_empty_tasks = false; + FOR_EACH (1, 0, 1, + [&] (int start, int end) + { + if (start == end) + any_empty_tasks = true; + return std::unique_ptr (new int (end - start)); + }, + task_size_one); + SELF_CHECK (!any_empty_tasks); + SELF_CHECK (std::all_of (intresults.begin (), + intresults.end (), + [] (const std::unique_ptr &entry) + { + return entry != nullptr; + })); } #endif /* FOR_EACH */ diff --git a/gdbsupport/parallel-for.h b/gdbsupport/parallel-for.h index b565676a0d0..de9ebb15746 100644 --- a/gdbsupport/parallel-for.h +++ b/gdbsupport/parallel-for.h @@ -70,6 +70,12 @@ struct par_for_accumulator return result; } + /* Resize the results to N. */ + void resize (size_t n) + { + m_futures.resize (n); + } + private: /* A vector of futures coming from the tasks run in the @@ -108,6 +114,12 @@ struct par_for_accumulator } } + /* Resize the results to N. */ + void resize (size_t n) + { + m_futures.resize (n); + } + private: std::vector> m_futures; @@ -232,6 +244,24 @@ parallel_for_each (unsigned n, RandomIt first, RandomIt last, end = j; remaining_size -= chunk_size; } + + /* This case means we don't have enough elements to really + distribute them. Rather than ever submit a task that does + nothing, we short-circuit here. */ + if (first == end) + end = last; + + if (end == last) + { + /* We're about to dispatch the last batch of elements, which + we normally process in the main thread. So just truncate + the result list here. This avoids submitting empty tasks + to the thread pool. */ + count = i; + results.resize (count); + break; + } + if (parallel_for_each_debug) { debug_printf (_("Parallel for: elements on worker thread %i\t: %zu"), From patchwork Tue Jan 10 18:33:36 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 62901 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 9B9AA38460AD for ; Tue, 10 Jan 2023 18:34:18 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9B9AA38460AD DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1673375658; bh=yInp+UCMToB4+H3Td8N36YfyuEfUu7Xh8g2JSEKErFQ=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=Ozd6Nd1dcqg7bC34BEjwvOib0SobKPDOVLo0zfeQRGORGanCHsw55eJWJk9eWvc7M +J6QqZrfk1D9yqV7cJOh6R7slqdEGvuuXUD9CeeO89N7APbz2QOxHS/PqaMsiyX6nY +cOj4pHKUgd/jMs3Ay0BXbNvlD/xSCxUp6iBf2Gc= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-il1-x12e.google.com (mail-il1-x12e.google.com [IPv6:2607:f8b0:4864:20::12e]) by sourceware.org (Postfix) with ESMTPS id 9C6B43858C52 for ; Tue, 10 Jan 2023 18:33:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9C6B43858C52 Received: by mail-il1-x12e.google.com with SMTP id i1so3357801ilu.8 for ; Tue, 10 Jan 2023 10:33:51 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=yInp+UCMToB4+H3Td8N36YfyuEfUu7Xh8g2JSEKErFQ=; b=QZIn2NREZbK4cjMEfUzLMn0wznJ/JZSNzPU4I/HzPVZEM6dBjVpQ8TW4MBhiCUlbh5 IU9MO7w62V/RyCxXSgHf9s/8aO1o7vITkNJhEyIrlOppbb8eQFXDoz3aS4zdx6S6K939 kOGxjzuPFC0AT58nKf2+1EiXb22bNJ33D3vbBv7W9Mmc94O+J/E0Ua5GczvXYPXJtIQk VWY7pkmYsSu+VVeCQL/joIlW2OPDv2Kv4Xc9BYPmAGpfUb++Gbgb8OEr9NkdKjXY/C8e lJdig1GEvMJEJybda/AAkxMCZP9EpGUOwXZpqaU9swwaeohwgco8siZ1bQL7tg67Iqu/ uVpw== X-Gm-Message-State: AFqh2kqLHY67sReq+1R/RVAKuaxCTkPExenRWv9jKLS6vA1rzXLSGB5u TeGUVPPRQUiSyZLuOEcZmJ97YIG+Y6EyMwRi X-Google-Smtp-Source: AMrXdXs6KQhG9WbenRh3bXjvXOwANZ84zsSOi+Aer+VOsDqhG4UPZ2UD5kdMIkvMPFT91HsCiojY7A== X-Received: by 2002:a92:d80a:0:b0:30d:859b:156a with SMTP id y10-20020a92d80a000000b0030d859b156amr14125728ilm.16.1673375630863; Tue, 10 Jan 2023 10:33:50 -0800 (PST) Received: from localhost.localdomain (97-122-76-186.hlrn.qwest.net. [97.122.76.186]) by smtp.gmail.com with ESMTPSA id w20-20020a05663800d400b00375783003fcsm3739933jao.136.2023.01.10.10.33.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Jan 2023 10:33:50 -0800 (PST) To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH v2 2/4] Don't erase empty indices in DWARF reader Date: Tue, 10 Jan 2023 11:33:36 -0700 Message-Id: <20230110183338.2623088-3-tromey@adacore.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20230110183338.2623088-1-tromey@adacore.com> References: <20230110183338.2623088-1-tromey@adacore.com> MIME-Version: 1.0 X-Spam-Status: No, score=-11.3 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 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-Patchwork-Original-From: Tom Tromey via Gdb-patches From: Tom Tromey Reply-To: Tom Tromey Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" The DWARF reader has some code to remove empty indices. However, I think this code has been obsolete since some earlier changes to parallel_for_each. This patch removes this code. --- gdb/dwarf2/read.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 851852404eb..c3f246fedf7 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -7170,16 +7170,6 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile) print_tu_stats (per_objfile); indexes.push_back (index_storage.release ()); - /* Remove any NULL entries. This might happen if parallel-for - decides to throttle the number of threads that were used. */ - indexes.erase - (std::remove_if (indexes.begin (), - indexes.end (), - [] (const std::unique_ptr &entry) - { - return entry == nullptr; - }), - indexes.end ()); indexes.shrink_to_fit (); cooked_index_vector *vec = new cooked_index_vector (std::move (indexes)); From patchwork Tue Jan 10 18:33:37 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 62902 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 8165F38493F2 for ; Tue, 10 Jan 2023 18:34:20 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8165F38493F2 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1673375660; bh=yDoG+hoZdmsgsavRXKiT0DPykIzUxIM/OrYARLnfAxs=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=YYHb2DHAodh3kuKKh2hTrvMS7Xd7t8RCBqz2Ho12heBFx5f3uDNB/rJlLsVEtATmA PEITwGmdxGaolln2+/p94iSBiZ6s22TyBM8i0vNaOl+D0Rubd9QwyifskzOcmbUxJ8 i/AJclSdWnF7iYyf7Izah5lENyceu+J66gGgKDUU= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-il1-x12c.google.com (mail-il1-x12c.google.com [IPv6:2607:f8b0:4864:20::12c]) by sourceware.org (Postfix) with ESMTPS id 304843858C78 for ; Tue, 10 Jan 2023 18:33:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 304843858C78 Received: by mail-il1-x12c.google.com with SMTP id o13so6826797ilc.7 for ; Tue, 10 Jan 2023 10:33:53 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=yDoG+hoZdmsgsavRXKiT0DPykIzUxIM/OrYARLnfAxs=; b=FxfoxinFtwU5y9clMoBkrAjqZicC8s0SlIirKrsTUuy8dOAwxMSuSrSQ3n9ebEQcTN 6mREm2RgGzYJBq6ntyA9yZ4eQH+ZBJIUBec96GaknBtSkpqTrfd6h4UIwB7A5MD/MgSb 1TYCUOftS13Ct407EFdT4nSqHo6/Tu/Tu295uI8aT9gLZZ+e77XGGD7OhgL1zpAk7HL+ dQ0EiVZ2PaiAfR3eHG1gYQujTOWjUnOQlhlhX++OdCLGB3RuT0vPv9dHtMlpFrDsBA2n wD7aT9Xkd64GHgpiaH7wXc1bOVYU7f2u5VzaMFwcu8rtj6ywGUQ9bG1IsuPdfbRyj6O/ 5o/Q== X-Gm-Message-State: AFqh2kqCGoNBVZKONLZu6RRQz0WEG2uiMD9wdHS29QYCZSvldbiXeZ/K +aY3l5A2ImCjlRXkisQr8moKrDuQl75wDpW0 X-Google-Smtp-Source: AMrXdXtz8jvGjW+U6A8iw8cERQgZlABkODkzIqBQlLo9Ba9W4wsbmgmmznVUqtNe0p0wXbi05dyHaw== X-Received: by 2002:a92:6e04:0:b0:302:b56c:63f6 with SMTP id j4-20020a926e04000000b00302b56c63f6mr48095982ilc.15.1673375631564; Tue, 10 Jan 2023 10:33:51 -0800 (PST) Received: from localhost.localdomain (97-122-76-186.hlrn.qwest.net. [97.122.76.186]) by smtp.gmail.com with ESMTPSA id w20-20020a05663800d400b00375783003fcsm3739933jao.136.2023.01.10.10.33.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Jan 2023 10:33:51 -0800 (PST) To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH v2 3/4] Move hash_entry and eq_entry into cooked_index::do_finalize Date: Tue, 10 Jan 2023 11:33:37 -0700 Message-Id: <20230110183338.2623088-4-tromey@adacore.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20230110183338.2623088-1-tromey@adacore.com> References: <20230110183338.2623088-1-tromey@adacore.com> MIME-Version: 1.0 X-Spam-Status: No, score=-11.3 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 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-Patchwork-Original-From: Tom Tromey via Gdb-patches From: Tom Tromey Reply-To: Tom Tromey Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" I was briefly confused by the hash_entry and eq_entry functions in the cooked index. They are only needed in a single method, and that method already has a couple of local lambdas for a different hash table. So, it seemed cleaner to move these there as well. --- gdb/dwarf2/cooked-index.c | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c index 93ffd923c76..c711e3b9b2a 100644 --- a/gdb/dwarf2/cooked-index.c +++ b/gdb/dwarf2/cooked-index.c @@ -26,26 +26,6 @@ #include "split-name.h" #include -/* Hash function for cooked_index_entry. */ - -static hashval_t -hash_entry (const void *e) -{ - const cooked_index_entry *entry = (const cooked_index_entry *) e; - return dwarf5_djb_hash (entry->canonical); -} - -/* Equality function for cooked_index_entry. */ - -static int -eq_entry (const void *a, const void *b) -{ - const cooked_index_entry *ae = (const cooked_index_entry *) a; - const gdb::string_view *sv = (const gdb::string_view *) b; - return (strlen (ae->canonical) == sv->length () - && strncasecmp (ae->canonical, sv->data (), sv->length ()) == 0); -} - /* See cooked-index.h. */ const char * @@ -191,6 +171,20 @@ cooked_index::do_finalize () htab_up seen_names (htab_create_alloc (10, hash_name_ptr, eq_name_ptr, nullptr, xcalloc, xfree)); + auto hash_entry = [] (const void *e) + { + const cooked_index_entry *entry = (const cooked_index_entry *) e; + return dwarf5_djb_hash (entry->canonical); + }; + + auto eq_entry = [] (const void *a, const void *b) -> int + { + const cooked_index_entry *ae = (const cooked_index_entry *) a; + const gdb::string_view *sv = (const gdb::string_view *) b; + return (strlen (ae->canonical) == sv->length () + && strncasecmp (ae->canonical, sv->data (), sv->length ()) == 0); + }; + htab_up gnat_entries (htab_create_alloc (10, hash_entry, eq_entry, nullptr, xcalloc, xfree)); From patchwork Tue Jan 10 18:33:38 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 62903 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 A11B5385B539 for ; Tue, 10 Jan 2023 18:34:44 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A11B5385B539 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1673375684; bh=/ZDsjsdodwnfpMHHPmRwmmiOxjGJdagJZZqkwhzOnbM=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=s244+IXPnReSb1b//W7F+mrcRoYg+pjaDaAz7wS06ANxLNxDuTjR3q4aXKQPcbmhe s4I7NKRLV3Le+U7GEfMAFXgrIfdWNB5upMIzs6yUtEhvc59w6dWZG+juFoAFr4YTQK 6h6k5p/vy77AUCzL5U9wsLW70EDqMz1v3tjrHR6A= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-il1-x136.google.com (mail-il1-x136.google.com [IPv6:2607:f8b0:4864:20::136]) by sourceware.org (Postfix) with ESMTPS id 369483858C2D for ; Tue, 10 Jan 2023 18:33:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 369483858C2D Received: by mail-il1-x136.google.com with SMTP id a3so892171ilp.6 for ; Tue, 10 Jan 2023 10:33:53 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=/ZDsjsdodwnfpMHHPmRwmmiOxjGJdagJZZqkwhzOnbM=; b=iO+3XVSvjXbGS1Kz+s5eMWButCyReOdaOCGUb00n0VtSt3+HkB1D4RIOcnn0RM0VcZ 5ZpXwWnDJ4KDnVTsEseDwIDJ2q0TL31tiyigk+9FBw4AFH8i+xbi4uQhev0w2ilj2lKZ KyxDhbcBzThgLxLXa6liYBJ/tZQYVXVAzNxpqZ0gwz5cUCNHRFYmNMo2VU/hEJr8InRM psY/EugMMtu8roFM/+vpA6zm1IY3VADePXYipx8jGtk6JE0HkxK9ckLZOc0VwHoIWwm/ JTmy/alvYRH9BYNdTB0h3eKrwJftdMX8gUEI/2Dh0KZn7Lt+aNpnbsTPaKQZt7hefOsC CxrQ== X-Gm-Message-State: AFqh2kquam5VfezgShb94L6rzL9N2bT7bMgt+da7vCOucKwaY8naNM6P z9EcYeqCbNzH2EWswnNojSs1I3YJzrYySGut X-Google-Smtp-Source: AMrXdXuFhxwaZdahontfr2D/uLcP154b3WliCeobLWgt14fBAYQhAUz9pFyTvYWfPFr7Hh23ODTUqQ== X-Received: by 2002:a05:6e02:214e:b0:302:db15:3981 with SMTP id d14-20020a056e02214e00b00302db153981mr62542424ilv.27.1673375632296; Tue, 10 Jan 2023 10:33:52 -0800 (PST) Received: from localhost.localdomain (97-122-76-186.hlrn.qwest.net. [97.122.76.186]) by smtp.gmail.com with ESMTPSA id w20-20020a05663800d400b00375783003fcsm3739933jao.136.2023.01.10.10.33.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Jan 2023 10:33:51 -0800 (PST) To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH v2 4/4] Fix parameter-less template regression in new DWARF reader Date: Tue, 10 Jan 2023 11:33:38 -0700 Message-Id: <20230110183338.2623088-5-tromey@adacore.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20230110183338.2623088-1-tromey@adacore.com> References: <20230110183338.2623088-1-tromey@adacore.com> MIME-Version: 1.0 X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP 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-Patchwork-Original-From: Tom Tromey via Gdb-patches From: Tom Tromey Reply-To: Tom Tromey Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" PR c++/29896 points out a regression in the new DWARF reader. It does not properly handle a case like "break fn", where "fn" is a template function. This happens because the new index uses strncasecmp to compare. However, to make this work correctly, we need a custom function that ignores template parameters. This patch adds a custom comparison function and fixes the bug. A new test case is included. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29896 --- gdb/dwarf2/cooked-index.c | 143 +++++++++++++++++++++++++---- gdb/dwarf2/cooked-index.h | 15 ++- gdb/dwarf2/read.c | 3 +- gdb/testsuite/gdb.cp/paramless.cc | 46 ++++++++++ gdb/testsuite/gdb.cp/paramless.exp | 41 +++++++++ 5 files changed, 226 insertions(+), 22 deletions(-) create mode 100644 gdb/testsuite/gdb.cp/paramless.cc create mode 100644 gdb/testsuite/gdb.cp/paramless.exp diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c index c711e3b9b2a..09b3fd70b26 100644 --- a/gdb/dwarf2/cooked-index.c +++ b/gdb/dwarf2/cooked-index.c @@ -25,6 +25,114 @@ #include "ada-lang.h" #include "split-name.h" #include +#include "safe-ctype.h" +#include "gdbsupport/selftest.h" + +/* See cooked-index.h. */ + +bool +cooked_index_entry::compare (const char *stra, const char *strb, + bool completing) +{ + /* If we've ever matched "<" in both strings, then we disable the + special template parameter handling. */ + bool seen_lt = false; + + while (*stra != '\0' + && *strb != '\0' + && (TOLOWER ((unsigned char) *stra) + == TOLOWER ((unsigned char ) *strb))) + { + if (*stra == '<') + seen_lt = true; + ++stra; + ++strb; + } + + unsigned c1 = TOLOWER ((unsigned char) *stra); + unsigned c2 = TOLOWER ((unsigned char) *strb); + + if (completing) + { + /* When completing, if one string ends earlier than the other, + consider them as equal. Also, completion mode ignores the + special '<' handling. */ + if (c1 == '\0' || c2 == '\0') + return false; + /* Fall through to the generic case. */ + } + else if (seen_lt) + { + /* Fall through to the generic case. */ + } + else if (c1 == '\0' || c1 == '<') + { + /* Maybe they both end at the same spot. */ + if (c2 == '\0' || c2 == '<') + return false; + /* First string ended earlier. */ + return true; + } + else if (c2 == '\0' || c2 == '<') + { + /* Second string ended earlier. */ + return false; + } + + return c1 < c2; +} + +#if GDB_SELF_TEST + +namespace { + +void +test_compare () +{ + SELF_CHECK (!cooked_index_entry::compare ("abcd", "abcd", false)); + SELF_CHECK (!cooked_index_entry::compare ("abcd", "abcd", false)); + SELF_CHECK (!cooked_index_entry::compare ("abcd", "abcd", true)); + SELF_CHECK (!cooked_index_entry::compare ("abcd", "abcd", true)); + + SELF_CHECK (cooked_index_entry::compare ("abcd", "ABCDE", false)); + SELF_CHECK (!cooked_index_entry::compare ("ABCDE", "abcd", false)); + SELF_CHECK (!cooked_index_entry::compare ("abcd", "ABCDE", true)); + SELF_CHECK (!cooked_index_entry::compare ("ABCDE", "abcd", true)); + + SELF_CHECK (!cooked_index_entry::compare ("name", "name<>", false)); + SELF_CHECK (!cooked_index_entry::compare ("name<>", "name", false)); + SELF_CHECK (!cooked_index_entry::compare ("name", "name<>", true)); + SELF_CHECK (!cooked_index_entry::compare ("name<>", "name", true)); + + SELF_CHECK (!cooked_index_entry::compare ("name", "name", false)); + SELF_CHECK (!cooked_index_entry::compare ("name", "name", false)); + SELF_CHECK (!cooked_index_entry::compare ("name", "name", true)); + SELF_CHECK (!cooked_index_entry::compare ("name", "name", true)); + + SELF_CHECK (!cooked_index_entry::compare ("name>", + "name>", false)); + + SELF_CHECK (!cooked_index_entry::compare ("name", "name>", false)); + SELF_CHECK (!cooked_index_entry::compare ("name>", "name", false)); + SELF_CHECK (cooked_index_entry::compare ("name>", + false)); + SELF_CHECK (!cooked_index_entry::compare ("name>", + true)); + SELF_CHECK (!cooked_index_entry::compare ("name>", "name>", "namecanonical, n.data (), n.length ()); - if (cmp != 0 || completing) - return cmp < 0; - return strlen (entry->canonical) < n.length (); + return cooked_index_entry::compare (entry->canonical, n.c_str (), + completing); }); - auto upper = std::upper_bound (m_entries.begin (), m_entries.end (), - name, - [=] (const gdb::string_view &n, + auto upper = std::upper_bound (m_entries.begin (), m_entries.end (), name, + [=] (const std::string &n, const cooked_index_entry *entry) { - int cmp = strncasecmp (n.data (), entry->canonical, n.length ()); - if (cmp != 0 || completing) - return cmp < 0; - return n.length () < strlen (entry->canonical); + return cooked_index_entry::compare (n.c_str (), entry->canonical, + completing); }); return range (lower, upper); @@ -311,7 +413,7 @@ cooked_index_vector::get_addrmaps () /* See cooked-index.h. */ cooked_index_vector::range -cooked_index_vector::find (gdb::string_view name, bool completing) +cooked_index_vector::find (const std::string &name, bool completing) { std::vector result_range; result_range.reserve (m_vector.size ()); @@ -339,3 +441,12 @@ cooked_index_vector::get_main () const return result; } + +void _initialize_cooked_index (); +void +_initialize_cooked_index () +{ +#if GDB_SELF_TEST + selftests::register_test ("cooked_index_entry::compare", test_compare); +#endif +} diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h index 607e07745f9..2b34a6534e8 100644 --- a/gdb/dwarf2/cooked-index.h +++ b/gdb/dwarf2/cooked-index.h @@ -143,11 +143,16 @@ struct cooked_index_entry : public allocate_on_obstack STORAGE. */ const char *full_name (struct obstack *storage) const; - /* Entries must be sorted case-insensitively; this compares two - entries. */ + /* Compare two strings, case-insensitively. Return true if STRA is + less than STRB. If one string has template parameters, but the + other does not, then they are considered to be equal; so for + example "t" == "t", "t" < "t", but "t" == "t". */ + static bool compare (const char *stra, const char *strb, bool completing); + + /* Compare two entries by canonical name. */ bool operator< (const cooked_index_entry &other) const { - return strcasecmp (canonical, other.canonical) < 0; + return compare (canonical, other.canonical, false); } /* The name as it appears in DWARF. This always points into one of @@ -232,7 +237,7 @@ class cooked_index /* Look up an entry by name. Returns a range of all matching results. If COMPLETING is true, then a larger range, suitable for completion, will be returned. */ - range find (gdb::string_view name, bool completing); + range find (const std::string &name, bool completing); private: @@ -335,7 +340,7 @@ class cooked_index_vector : public dwarf_scanner_base /* Look up an entry by name. Returns a range of all matching results. If COMPLETING is true, then a larger range, suitable for completion, will be returned. */ - range find (gdb::string_view name, bool completing); + range find (const std::string &name, bool completing); /* Return a range of all the entries. */ range all_entries () diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index c3f246fedf7..44b54f77de9 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -18741,8 +18741,9 @@ cooked_index_functions::expand_symtabs_matching { std::vector name_vec = lookup_name_without_params.split_name (lang); + std::string last_name = gdb::to_string (name_vec.back ()); - for (const cooked_index_entry *entry : table->find (name_vec.back (), + for (const cooked_index_entry *entry : table->find (last_name, completing)) { QUIT; diff --git a/gdb/testsuite/gdb.cp/paramless.cc b/gdb/testsuite/gdb.cp/paramless.cc new file mode 100644 index 00000000000..392b15f4256 --- /dev/null +++ b/gdb/testsuite/gdb.cp/paramless.cc @@ -0,0 +1,46 @@ +/* Test case for template breakpoint test. + + Copyright 2023 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +template +struct outer +{ + template + int fn (int x) + { + return x + 23; + } +}; + +template +int toplev (int y) +{ + return y; +} + +outer outer1; +outer outer2; + +int main () +{ + int x1 = outer1.fn (0); + int x2 = outer2.fn (-46); + int x3 = toplev (0); + int x4 = toplev (0); + return x1 + x2; +} diff --git a/gdb/testsuite/gdb.cp/paramless.exp b/gdb/testsuite/gdb.cp/paramless.exp new file mode 100644 index 00000000000..4fc8fd9e015 --- /dev/null +++ b/gdb/testsuite/gdb.cp/paramless.exp @@ -0,0 +1,41 @@ +# Copyright 2023 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# This file is part of the gdb testsuite. + +# Test template breakpoints without parameters. + +if { [skip_cplus_tests] } { continue } + +standard_testfile .cc + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} { + return -1 +} + +gdb_breakpoint "outer::fn" message +delete_breakpoints + +gdb_breakpoint "outer::fn" message +delete_breakpoints + +gdb_test "break outer::fn" "Breakpoint $decimal at .*2 locations." +delete_breakpoints + +gdb_test "break toplev" "Breakpoint $decimal at .*2 locations." +delete_breakpoints + +gdb_breakpoint "toplev" message +delete_breakpoints