From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id TCG1OOWiHWpqwy0AWB0awg (envelope-from ) for ; Mon, 01 Jun 2026 11:19:01 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1780327141; bh=yROd2p0QKePYG6G8CHguhGOnlU2oIZoGC3yt/8zo1Bg=; h=Date:Subject:To:Cc:References:From:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=L0VR+FDH1mmClbZeaICEYK9IN6uWWCHBbKhYzPqlVHS6KJjKXb7R1w8v6Lj3UfjAw 9yoDcjsN1J2GSLqR3CUt7WuVJFzHvtvY1SSHSn6D6ZZOAW/74gssw2XE7lnENML7D5 Ar75tSAzLrVzN1de1PsEKp85ctehSad+VfLgg28Q= Received: by simark.ca (Postfix, from userid 112) id C7A691E0A3; Mon, 01 Jun 2026 11:19:01 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=GA4wIb1M; dkim-atps=neutral Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.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 785821E062 for ; Mon, 01 Jun 2026 11:19:00 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 311D24BA2E29 for ; Mon, 1 Jun 2026 15:18:59 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 311D24BA2E29 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=GA4wIb1M Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id EBA7C4BA2E1B for ; Mon, 1 Jun 2026 15:18:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EBA7C4BA2E1B Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org EBA7C4BA2E1B Authentication-Results: sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1780327112; cv=none; b=Qso0Zc1CJNpCiPe2A1hsU7DnoVgrMmR/EthlJqps6Oh5vcvopge3acc2kjAH/a7VKUsuJJdw1hSKPKbOY/kx2tq3YmKwJyjRK1W/EIAly6wKEn1fWvXx65gfZbP1pXmm6fnLVko/oC9DveJP/PpqWkqW160SZTazEsBOFms3ho0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1780327112; c=relaxed/simple; bh=yROd2p0QKePYG6G8CHguhGOnlU2oIZoGC3yt/8zo1Bg=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=mc7eP5oNlhLgYbLJRBL7p6BrVnSH9Fxfm5VxeBSQGOyO9WHa6DFKhM5Met9x+UuWqwzrVs/tWeTjpS+SwdgiOJcniNI1fxUFwPo/53Wdl0eE83NqU+D3oRSKvkUgDn1jmIm9vAtNYhl/a2w4KzZkr59cYUdhER/EsDCJ0kwjVKM= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=GA4wIb1M DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EBA7C4BA2E1B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1780327110; bh=yROd2p0QKePYG6G8CHguhGOnlU2oIZoGC3yt/8zo1Bg=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=GA4wIb1MYBaMvbH7AFsNPVP4hjEmS4X9wk9OpMyYhW3zxxID7KOhtlqE1SFgHfix0 43EeGkbvpnuZH/E20zUkm7SebZ6OXZfOC9KVYpW2Ku+4EALD5rcZZotqPVx1Qd3ngJ 51jwxtjUl5GOdWUY8ttK2kOv/zlF2OFZrZB9OD9k= Received: by simark.ca (Postfix) id B7F531E062; Mon, 01 Jun 2026 11:18:29 -0400 (EDT) Message-ID: Date: Mon, 1 Jun 2026 11:18:29 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] [gdb/symtab] Add assert in free_cached_comp_units constructor To: Tom de Vries , Tom Tromey Cc: gdb-patches@sourceware.org References: <20260527095715.2481440-1-tdevries@suse.de> <87pl2f5vsy.fsf@tromey.com> <83c3a4ea-9f74-4afe-8963-ce0ec9495a22@suse.de> <7168b9e1-3102-4484-928e-53b5e55ffdf4@suse.de> Content-Language: fr From: Simon Marchi In-Reply-To: <7168b9e1-3102-4484-928e-53b5e55ffdf4@suse.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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 On 6/1/26 4:02 AM, Tom de Vries wrote: > On 5/30/26 2:52 PM, Tom de Vries wrote: >> On 5/28/26 6:43 PM, Tom Tromey wrote: >>>>>>>> "Tom" == Tom de Vries writes: >>> >>> Tom> Detect this situation using an assert in the free_cached_comp_units >>> Tom> constructor. >>> >>> Seems fine to me. >> >> Thanks for the review. >> >> I committed this, but afterwards ran into trouble on x86_64-linux with test-cases gdb.ada/uninitialized-variable-record.exp and gdb.ada/ uninitialized_vars.exp on x86_64-linux, so I've reverted this. >> > > I've investigated this, and found that this is due to calls to load_cu in places other than dw2_do_instantiate_symtab. > > For test-case gdb.ada/uninitialized_vars.exp, it's dwarf2_fetch_die_loc_cu_off. > > I do wonder if ~free_cached_comp_units is a bit overeager, and should refrain from deleting cached comp units that were present at construction time. > > Anyway, the assert detected the use-after-free I created, but just doesn't hold in general. Let's try to understand why things are the way they are currently. Why do we want to delete the just created dwarf2_cus in dw2_instantiate_symtab? I presume it's because the chances of them being useful again are slim. We created the GDB types and symbols, we don't need to keep the DIE structure loaded in memory, so it's better to free up the memory. The cases where the dwarf2_cus are needed again later appear to be when evaluating a some DWARF operator that refers to other DIEs directly, such as DW_OP_call*, DW_OP_implicit_pointer, DW_OP_GNU_variable_value, etc. - dwarf2_fetch_die_loc_sect_off - dwarf2_fetch_die_loc_cu_off - dwarf2_fetch_constant_bytes - dwarf2_fetch_die_type_sect_off In those cases we re-load the right dwarf2_cu in memory to be able to look up the DIE and get what we want from it. In those cases, we don't free up the just-loaded dwarf2_cu right away, because it presumably has good chances of being needed again in the near future, for other operators. We instead use the "age_comp_units" mechanism, which frees the dwarf2_cus once they have been sitting there unused for a while. I am unable to reproduce the gdb.ada failures, but the case that you looked at appears to be one where a dwarf2_cu was loaded by one of those "dwarf2_fetch_*" functions, while evaluating a DWARF expression, and then dw2_instantiate_symtab was called to expand a compunit into full symbols. So free_cached_comp_units deletes the dwarf2_cu previously cached by the "dwarf2_fetch_*" functions. I don't think this is wrong (as in a correctness bug), so I don't think the assert was right, it might just be ineffcient cache usage. I don't know if it's problematic enough to be worth fixing, but if you can think of a simple solution for dw2_instantiate_symtab not to delete the pre-existing dwarf2_cus, we could consider it. It would perhaps be good to investigate whether this "age comp units" machinery is still working as initially intended. It's possible that will all the refactors we've done over the years, it's not working as intended. And because it's just a cache, it would not break any tests, but it could cause peformance problems. While searching the code, I looked where `cu->last_used` is reset, to prevent the CU from being freed by age_comp_units. It is reset in maybe_queue_comp_unit, which is itself called in: - follow_die_sig_1 - process_imported_unit_die - follow_die_offset Note that those are all used in cross-CU reference cases, when a CU needs something from another CU. `age_comp_units()` is called in: - dw2_do_instantiate_symtab, when done expanding the symtab(s) - dwarf2_fetch_die_loc_sect_off, when done fetching the location info Some fishy things I spotted: - The age_comp_units() call in dw2_do_instantiate_symtab suggests that the comp unit aging system was also meant to be used when expanding symtabs. The fact that all the places that reset `cu->last_used` are some that handle cross-CU references also suggests this. The idea might have been that if a CU refers to another CU (via DW_AT_import for instance), then there is a good chance that subsequent CUs will also refer to that second CUs. So when you're done expanding the first CU, better keep that second CU in cache for when you'll be expanding more CUs. However, if we free up all cached CUs in dw2_instantiate_symtab via free_cached_comp_units, doesn't it defeat the purpose? Why call age_comp_units() in dw2_do_instantiate_symtab if we're going to free them all up in the caller anyway? - Calling dwarf2_fetch_die_loc_sect_off ages the comp units, but shouldn't it also reset the `cu->last_used` field of the CU from which we found the info? Otherwise, repeated calls to dwarf2_fetch_die_loc_sect_off targetting the same CU will cause that CU to be freed, even if we just used it and will keep needing it (causing it to be re-loaded from scratch the next time). Simon