From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id xXIxNZdmv2PM+RQAWB0awg (envelope-from ) for ; Wed, 11 Jan 2023 20:47:03 -0500 Received: by simark.ca (Postfix, from userid 112) id CDE2F1E128; Wed, 11 Jan 2023 20:47:03 -0500 (EST) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=KAazByD/; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-8.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,NICE_REPLY_A, RCVD_IN_DNSWL_HI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 29A391E0D3 for ; Wed, 11 Jan 2023 20:47:03 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 95148385B53C for ; Thu, 12 Jan 2023 01:47:01 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 95148385B53C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1673488021; bh=xf0QpUaYyfM3/OFs5Wb6nso+CXD0bOiLXMjMQiWtFWc=; h=Date:Subject:To:Cc:References:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=KAazByD/X3+EMdUiNGePJgdvZFojzSKphOHDEUgmVAV0LrktLtszOsDXs4s6rcB1T xZbZvxWYSegSy2OlrLy6mDuIWbXSMA3/PKzf+GHtPN1xZg5LIyni23UzWUrkQU7EIO /HABnKwZuRdSaKvV+CeWNNB7tlhixSO03p60/oKA= Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id B78ED385840D for ; Thu, 12 Jan 2023 01:46:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B78ED385840D Received: from [10.0.0.11] (unknown [217.28.27.60]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 812D51E0D3; Wed, 11 Jan 2023 20:46:32 -0500 (EST) Message-ID: <525f9315-27f1-935a-4e5e-4a043b24eecf@simark.ca> Date: Wed, 11 Jan 2023 20:46:32 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: Decl/def matching with templates without template parameters in the DW_AT_name Content-Language: en-US To: David Blaikie Cc: gdb References: In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: gdb@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Simon Marchi via Gdb Reply-To: Simon Marchi Errors-To: gdb-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb" On 1/11/23 18:50, David Blaikie wrote: > On Wed, Jan 11, 2023 at 10:24 AM Simon Marchi wrote: >> >> >> >> On 1/6/23 12:37, David Blaikie via Gdb wrote: >>> I've been working on reducing debug info size, especially for >>> expression-template heavy code like in Tensorflow and Eigen. >>> >>> To that end, I've implemented the flag -gsimple-template-names in >>> Clang that simplifies the DW_AT_name for templates by omitting >>> template parameters (eg: "vector" instead of "vector>> std::allocator>") when the DW_TAG_template_*_parameters contain >>> sufficient information to reconstruct the original name. >> >> This reduces the length of strings, and more importantly it allows more >> sharing, is that it? > > Yep. Especially for complex expression templates - the fully expanded > template names can be massive (some examples I had involve 50k > character names... ) - they don't have the benefits of mangled names > that include redundancy elimination/deduplication at least within a > single name (admittedly the mangled names for these things are massive > too - and at some point I might start some discussions about > alternative mangling/symbol naming schemes, for those who can diverge > from the Itanium ABI) > >> It doesn't change the layout of the DIE tree? > > Correct... > > well, in one case it does change the DIE tree - template declarations. > > GCC and Clang produce template declarations without template parameter DIEs, eg: > > $ cat > test.cpp > template > struct t1 { }; > t1 *v; > $ clang++-tot test.cpp -g -c && llvm-dwarfdump-tot test.o > ... > 0x0000002e: DW_TAG_structure_type > DW_AT_name ("t1") > DW_AT_declaration (true) > > 0x00000030: NULL > > So if we dropped the `` from that name, there'd be no hope to > connect the declaration with the right definition. > > So when using -gsimple-template-names, we enable template parameter > DIEs on declarations. (Sony implemented this previously, presumably > because their debugger needed it for something like this - but it > wasn't enabled by defaulte because GDB and LLDB didn't seem to need > these extra DIEs so it wasn't worth the extra size - but maybe it's > not so much to be a real problem/maybe we should switch that on by > default at some point if we're going further in this direction). > >>> This generally seems to work in GDB - that looks intentional (perhaps >>> because someone else implemented this feature elsewhere, or just for >>> canonicalization reasons (the full string with template parameters >>> might have different whitespace, parentheses, other formatting)), it >>> seems unlikely that GDB would accidentally be able to connect two >>> "vector" declarations up to the right "vector" definitions based on >>> the template parameters without intentional code to support this >>> scenario. >> >> Probably this code: >> >> https://gitlab.com/gnutools/binutils-gdb/-/blob/1b9af5b949bff0c750ededb459400c1857fec416/gdb/dwarf2/read.c#L9002 > > Something like that. Though the comment here is interesting: > > https://gitlab.com/gnutools/binutils-gdb/-/blob/1b9af5b949bff0c750ededb459400c1857fec416/gdb/dwarf2/read.c#L8944 > > /* Template parameters may be specified in the DIE's DW_AT_name, or > as children with DW_TAG_template_type_param or > DW_TAG_value_type_param. If the latter, add them to the name > here. If the name already has template parameters, then > skip this step; some versions of GCC emit both, and > it is more efficient to use the pre-computed name. > > At least usually GCC and Clang produce both (DW_AT_name with > parameters, and DIEs for the parameters) for definitions, and > name-only, no DIEs for declarations. > > I wonder which compiler has previously produced simplified names - > it'd be nice as a proof-of-concept/existence proof/prior art for the > new Clang feature. Digging in the history leads me to: https://inbox.sourceware.org/gdb-patches/201007302017.41074.pedro@codesourcery.com/ So RVCT, the RealView compiler. I don't have access to that, unfortunately. It seems obsolete, also. > It's also interesting that it says to use the name if it's already got > them - that suggests no canonicalization, but I'm pretty sure GDB does > do some canonicalization to allow fuzzy matching of decls/defs between > Clang and GCC (& whatever other compilers) which might have minor > differences between how they print template parameters. I don't know, unfortunately. > > But yeah, that code that specifically says "if there's no '<' in the > name, go and rebuild the name" is very explicit support for the > -gsimple-template-names naming... someone must've done this before. > > >>> But one place I found a problem was in pretty printers. I have >>> situations where a type declaration isn't resolved to a definition >>> when working within a pretty printer (resulting in the pretty printer >>> being unable to find member variables in the object/of that type) - >>> but if I print out the variable/member it works correctly - or if I >>> "list" the source of the file where the definition of the type is, >>> then the pretty printer works correctly. >>> >>> Does this ring a bell for anyone/sound like something sufficient to >>> file a bug, or would I need to dig in further to create an isolated >>> reproduction to help communicate the issue I'm seeing? >> >> I have no idea about what happens without digging into the code. I just >> remember some bug where GDB would generate the templated name with let's >> say "<5u>" for an integer value template parameter, so if you entered >> "<5>", it didn't match textually. Maybe something like that is >> happening. >> >> I also found: >> >> https://sourceware.org/bugzilla/show_bug.cgi?id=17762 > > Yeah, I certainly hit a few cases like that when testing > -gsimple-template-names canonicalization/roundtripping to make sure it > wasn't lossy. In order to avoid any bug like this, for you tests, maybe you can use the -readnow switch to bypass the partial symbol steps. > >> There's the possibility that the "partial symbol" and "expanded symbol" >> don't have the same name. So you could see a difference in behavior >> depending on whether the CU has been expanded by GDB or not (I'm just >> speculating). >> >> Is it valid DWARF (5) for DW_AT_name of a templated struct instantiation >> to omit the template parameters? I don't see DWARF mandating one or the >> other, so I assume that both including them or not are valid. > > Yeah, this is a case where DWARF is like "here are some tools you > could use to express some language features, have at!" and doesn't say > "to describe this particular language feature you must use DWARF in > this particular way" Typical "DWARF is a permissive standard, not a prescriptive one" thing. > >> If so, I >> think we can consider it a GDB bug if it doesn't process the "without" >> version correctly. Feel free to file a bug with the relevant background >> information. It would be useful to include some binaries, one compiled >> with -gsimple-tempalte-names and one without, so it's easy to compare >> the two. > > Yeah, for now I'm only seeing the problem with pretty printers, so to > provide repro steps will be a bit more involved. (I've also got a > crash that seems to happen when using the feature without > -gsplit-dwarf (it's probably reproducable with -gsplit-dwarf too, but > we enable -Wl,-gdb-index in that case and so GDB doesn't parse as much > DWARF on startup - but if we tickled it into loading more DWARF it'd > probably hit the same crash, I'd wager)) > > I'll see what I can do. If you end up merging this, it will be interesting to run the full GDB testsuite against clang with that flag, it would cover a lot of things. Simon