From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id I5PEIZr4tGqsMjwAWB0awg (envelope-from ) for ; Thu, 24 Sep 2026 06:16:58 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Ee2KeT7H; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 656BD1E033; Thu, 24 Sep 2026 06:16:58 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-6.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [IPv6:2620:52:6:3111::32]) (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 759691E033 for ; Thu, 24 Sep 2026 06:16:56 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E116B4BB5881 for ; Thu, 24 Sep 2026 10:16:54 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E116B4BB5881 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Ee2KeT7H Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 94BCC4BB3BCE for ; Thu, 24 Sep 2026 10:16:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 94BCC4BB3BCE Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 94BCC4BB3BCE Authentication-Results: sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1790244989; cv=none; b=P75Ha1FsqQ3TFuxa62mcffIAafk/DilNogQIfLgIV2/SnINBNVma35spkCadkJUHT+oDmDY/Qbvd1dz9846/gGWgJqktb77KYdIF0sgoltQ+nekKTQGXaCmnopp5jCf/JTPSxlNY6aY9E9+3hRZOK1C2jVh5qPiSb2Cd6jk+c48= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1790244989; c=relaxed/simple; bh=ysKXGtTAcJ+eQNBheHV1XeLwPLlpviDeKM5F4pBr36c=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=JLJha6Q8+YBXhx7IebJZFvOCGrMF0yjs25p2e6pGQEJ1eqQ1e8KUL0BBdi7CrAT8WZhr05iYUeGHKQnVCZIFeZ3MMM4cEADzw5Bn8YgEB2DR7Gr92k7L7iT/chUMGoUsgyjC6SqsaoK4zwJG2UvbcAIudC764yIajHFKR9Hl9cs= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Ee2KeT7H DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 94BCC4BB3BCE DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1790244989; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=iQGDK5Iup5PC9iyNCo9vJPrbZOnTaFV7x1++B8RXtoM=; b=Ee2KeT7H5sS653e0jUdj0cPEuzh7wBVrQkLqIIbASrlgzDZ4q5ma3w1SDK0SsNZPdHgLLq aNBgijt2Pt8q9ikxqHKpLg16I2mT4ti9rMeWTDn7bl5xUm7Aznyqn8ZqhAn3EIFYMincIy FItcaZvf1aZ7QWHwx4E0roNP9kRrj9U= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-637-5ndgoYzdMwO6P4z1Bw-UWA-1; Thu, 24 Sep 2026 06:16:26 -0400 X-MC-Unique: 5ndgoYzdMwO6P4z1Bw-UWA-1 X-Mimecast-MFC-AGG-ID: 5ndgoYzdMwO6P4z1Bw-UWA_1790244985 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-4994d67d0e3so12914375e9.2 for ; Thu, 24 Sep 2026 03:16:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1790244985; x=1790849785; h=content-type:mime-version:message-id:date:references:in-reply-to :subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to:content-type; bh=iQGDK5Iup5PC9iyNCo9vJPrbZOnTaFV7x1++B8RXtoM=; b=dSnlKQvK4jXqfwrll4BR7CrIZkdiT38FGYQPnnXXl/f5lL5Om92Hu7c1XtEundYw40 OodmDayp5Rf16DeBQedC4oyrKvMkGNYMsO/LG4UY19K41CdUwuToUBzYGYB/2KFBbXoW cbnWeDdMRhxBIx7KaLw/zSL0QXu0oAqOd7u04D6K9D6v1vG5ZqkFbupAbehItyTGX8QB patSHPWqqJOZPv41l0arwSzx1/pICAPA8yR1PLL3Y+NXTuLV3SnjKhv1UO92PYWon9MM gSPXzEX47ywDEkBVvbRhhInqH8mzHrTluw4AgwNFbN6njMESR9peU8a+i54AgWognLH+ iYDQ== X-Forwarded-Encrypted: i=1; AKwUvBx4uLIFwsjM9dG/zXr4udriyaJnqi3kIJeQc+nKF9/LButJ004GqHQTcY86y/lbpE5t1HsRMAHe1OSdhQ==@sourceware.org X-Gm-Message-State: AFuF++khMKi8MkuE9dgtkJfvvpMLIoNDwiBD7dwDXVINVIhkgdMBAZNP rLM0eugmNdPKMjflJLxrbjWBvpbe7JxEh/u+IexBZdIW0ahr9/31ycbhAxQFqVH0BUMuy09PKIU 9fMjGTOZz2RYP55wAGlSSmvzWxqnH0oQPOETE6PwAW+C7onA+MkYIMwMBvCyAN+0IejtmcGI= X-Gm-Gg: AYBFou22qvVHznQGeZwDJgxBPztcz2JCMZB72i5TXfMoaQHMTG+Bb/BF36e19g9gyVi fJp9DTT4DIHETDx0MblUaFfA1E4gOMhHLg4Z+K/BFRljGARTLTNobeo93DO7/9xUz2pkdzk6iYK lXqClb1LILM2xwCT7EZfDKu+/ArLB673UbZgiZy/zRtTUNAbjS+zHKCqAX6OedyqCqIvINGuW1X wQv2NhYUjr6d1P49SARwebQMCLiFMRzOKaITX0rV8m2WEdPaqpuTOcc+CeYFEC5e9Zgxs47pIuv Ou9WtMNE+HXeuilebJpsDMwQCOoEjC67uL/Vil8DtkQ6z9oZH5NkBqSyj3hwZTNmWBEf X-Received: by 2002:a05:600c:4e93:b0:49b:d45:703e with SMTP id 5b1f17b1804b1-49fe66d1097mr34467575e9.8.1790244985377; Thu, 24 Sep 2026 03:16:25 -0700 (PDT) X-Received: by 2002:a05:600c:4e93:b0:49b:d45:703e with SMTP id 5b1f17b1804b1-49fe66d1097mr34467115e9.8.1790244984871; Thu, 24 Sep 2026 03:16:24 -0700 (PDT) Received: from localhost ([213.31.44.29]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-49fe0c5b646sm53511125e9.4.2026.09.24.03.16.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Sep 2026 03:16:23 -0700 (PDT) From: Andrew Burgess To: Simon Marchi , gdb-patches@sourceware.org Cc: Simon Marchi Subject: Re: [PATCH 1/2] gdb: split cooked index finalization into separate steps In-Reply-To: <20260924050002.1539783-2-simon.marchi@efficios.com> References: <20260924050002.1539783-2-simon.marchi@efficios.com> Date: Thu, 24 Sep 2026 11:16:20 +0100 Message-ID: <877bkb2buz.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: 8BMZc-CbYqMUP7PupbAs2HvX-XNNiRP-g4mnZltJ8Z4_1790244985 X-Mimecast-Originator: redhat.com Content-Type: text/plain 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 Simon Marchi writes: > This patch refactors the existing cooked index "finalize" step into > multiple steps. This is a preparatory patch for the following patch, > which will add some more steps. I believe that using this pattern will > help keep the code readable and easy to understand, despite the > increasing amount of work done. > > The following patch introduces a step that needs to run after the > deferred parents have been resolved, but before the name > canonicalization happens, so factor out the deferred parents code into a > step of its own. > > The code that remains in cooked_index_shard::finalize is all about name > canonicalization (if we consider the Ada stuff to be a special case of > that), so rename that step to cooked_index_shard::canonicalize_names(). > > For clarity, move the "write to cache" code out of the "done" callback > to make it clear that it is logically a separate step. > > So as of this patch, we have the steps: > > - resolve deferred parents > - canonicalize names > - write to cache > > The term "finalization" now refers to all these steps. Or, depending on > your definition, it could refer to all but "write to cache", since the > index reaches the FINALIZED state (i.e. becomes fully usable) just > before that one. > > Here are the implementation details: > > - The first step is called by cooked_index::set_contents(), then each > step is responsible for calling the next one (thus forming a chain), > until we are done. > > - Steps that involve per-shard parallel processing use gdb::task_group, > the "done" callback of the task_group starts the next task. > > A special case of that is: if some steps are able to detect ahead of > time that it's unnecessary to run the step for a given shard, they > can omit adding a task for that shard. > > An even more special case of that is: if no tasks are added to a task > group, the "done" callback for that task group is invoked > immediately when the group is started. This allows easily and > cheaply skipping entire steps when they are not needed. > > This is used in this patch for the new "resolve deferred parents" > step. Each shard records whether it has at least one deferred parent > during the initial scan. If a shard has none (which is a common > case), then cooked_index_shard::resolve_deferred_parents would > iterate the index entries for nothing. The step therefore omits > adding a task for that shard in that case. If no shards have any > deferred parents that require resolving, we go directly to the next > step with very little cost. > > During review, Claude pointed out that my change actually fixes a > potential problem. The current code resolves the deferred parent of an > entry in the same loop where it potentially calls "full_name" for that > entry (to create the special Ada entries). "full_name" walks up the > parent chain, and thus requires all grand-parents to be resolved. > Depending on the order of DIEs in the file, it might not be true. > Having the "resolve deferred parents" step run completely before the > name canonicalization step avoids this. > > I built a gdb.dwarf2 test case around the DWARF structure it proposed to > hit the bug. It is perhaps not something we expect from real-world > producers, but it's also not completely unthinkable. It hits this > assert when the patch is not applied: > > gdb/dwarf2/cooked-index-entry.h:217: internal-error: get_parent: Assertion `(flags & IS_PARENT_DEFERRED) == 0' failed. > > ... and passes with it. Simon, Thanks for doing this. My reworked series (unposted) had a patch just like this, but your addition of the new test is a bonus I didn't have, so I think yours is better. I only had one minor suggestion from my take on this change, see below... > + > +void > +cooked_index_shard::canonicalize_names () > { > gdb::unordered_set cooked_index_entry_name_ptr_hash, > @@ -216,13 +236,6 @@ cooked_index_shard::finalize (const parent_map_map *parent_maps) > > for (cooked_index_entry *entry : m_entries) > { > - if ((entry->flags & IS_PARENT_DEFERRED) != 0) > - { > - const cooked_index_entry *new_parent > - = parent_maps->find (entry->get_deferred_parent ()); > - entry->resolve_parent (new_parent); > - } > - I replaced this block with: /* Deferred parents should not reach this point. */ gdb_assert ((entry->flags & IS_PARENT_DEFERRED) == 0); I think the assert is worth keeping. With that: Approved-By: Andrew Burgess Thanks, Andrew