From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 38596 invoked by alias); 27 Dec 2019 04:43:56 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 37818 invoked by uid 89); 27 Dec 2019 04:43:55 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.1 required=5.0 tests=AWL,BAYES_00,ENV_AND_HDR_SPF_MATCH,RCVD_IN_DNSWL_NONE,SPF_PASS,USER_IN_DEF_SPF_WL autolearn=ham version=3.3.1 spammy=H*RU:209.85.210.67, HX-Spam-Relays-External:209.85.210.67 X-HELO: mail-ot1-f67.google.com Received: from mail-ot1-f67.google.com (HELO mail-ot1-f67.google.com) (209.85.210.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 27 Dec 2019 04:43:53 +0000 Received: by mail-ot1-f67.google.com with SMTP id a15so34868441otf.1 for ; Thu, 26 Dec 2019 20:43:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=S1lstx2680wthR/CAsYaSyuGvBk7NwcdJGZvoCxYHsA=; b=NdRrUeN2iBYPOSLC/8ItCBl8+WueQFqkNAABtuIXJcppDTQyqts31pCBpf8zTnlmzt nDMPBuTzv1u4tsj6CzzqGW1bM3iYv6yjh+DdcAlHnJ070ZuvxKqRMNfCu7HOnpDlmOBI 2HvYMXnIIcfsmyglhOn0OBp9f1oAmwTBWy9VRzyoOPiQJbA/dtsmqMPQi2ueJsIHnKjS A2tslOxX7vS/K3NkOQtrCXGPEy11AD8DvAxJcvEBvTMtDwUBm16O8zmmLW+J4gomxFPR xWNCMCJtqT4rD7SlFn+bfu5QWUZ0NShgpCwXlutl2zfQCme2Vfig69VyEokikX+NiT6g +6bw== MIME-Version: 1.0 References: <20191226075201.239053-1-cbiesinger@chromium.org> <34ef9777-7067-8669-a9a5-40c8cfe8e8d6@simark.ca> In-Reply-To: <34ef9777-7067-8669-a9a5-40c8cfe8e8d6@simark.ca> From: "Christian Biesinger via gdb-patches" Reply-To: Christian Biesinger Date: Fri, 27 Dec 2019 04:43:00 -0000 Message-ID: Subject: Re: [PATCH] Make symbol_set_names a member function To: Simon Marchi Cc: gdb-patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2019-12/txt/msg01032.txt.bz2 On Fri, Dec 27, 2019 at 4:02 AM Simon Marchi wrote: > > On 2019-12-26 8:09 p.m., Christian Biesinger wrote: > > On Thu, Dec 26, 2019 at 6:50 PM Simon Marchi wrote: > >> > >> On 2019-12-26 2:52 a.m., cbiesinger@chromium.org wrote: > >>> From: Christian Biesinger > >>> > >>> This also renames it to make it clearer that this is not a cheap > >>> functin (to compute_and_set_names). Also renames name to m_name > >> > >> "functin" > > > > Thanks, fixed locally. > > > >>> to make the implementation of the renamed function more readable. > >>> > >>> Most of the places that access sym->m_name directly were also changed > >>> to call linkage_name () instead, to make it clearer which name they > >>> are accessing. > >> > >> I think that all makes sense. > >> > >>> @@ -2110,7 +2109,8 @@ maintenance_check_psymtabs (const char *ignore, int from_tty) > >>> if (!sym) > >>> { > >>> printf_filtered ("Static symbol `"); > >>> - puts_filtered ((*psym)->ginfo.name); > >>> + /* TODO: Should this be print_name ()? */ > >>> + puts_filtered ((*psym)->ginfo.linkage_name ()); > >>> printf_filtered ("' only found in "); > >>> puts_filtered (ps->filename); > >>> printf_filtered (" psymtab\n"); > >>> @@ -2128,7 +2128,8 @@ maintenance_check_psymtabs (const char *ignore, int from_tty) > >>> if (!sym) > >>> { > >>> printf_filtered ("Global symbol `"); > >>> - puts_filtered ((*psym)->ginfo.name); > >>> + /* TODO: Should this be print_name ()? */ > >>> + puts_filtered ((*psym)->ginfo.linkage_name ()); > >>> printf_filtered ("' only found in "); > >>> puts_filtered (ps->filename); > >>> printf_filtered (" psymtab\n"); > >> > >> For this patch, I wouldn't change the behavior (which means using linkage_name), but we could > >> consider a separate patch to change it. > > > > I removed the TODO locally. OK to push with withose two changes? > > Yes. Thanks, pushed: To ssh://sourceware.org/git/binutils-gdb.git b0d674e2b4..4d4eaa3005 HEAD -> master > >> I hacked the code to always enter these ifs and print both the linkage_name > >> and the natural_name. With a C++ test program containing this function: > >> > >> int hello(int); > >> > >> I get: > >> > >> linkage_name: hello > >> natural_name: hello > >> > >> I would have expected linkage_name to be _Z5helloi and the natural_name to be > >> hello(int). Do you know if it's expected for the partial symbol to contain > >> just "hello" for both? > > > > Huh.. > > > > I added a printf in compute_and_set_names and found that there's a > > symbol with the mangled name *and* a symbol with the plain name. I > > guess that's why? But I don't know what that means... > > Ok, I see what happens. The first call to compute_and_set_names > is while parsing minimal symbols, by minimal_symbol_reader::install. > This one has a mangled linkage name (_Z5helloi). > > The second call is while creating partial symbols, by add_psymbol_to_bcache. > This is the one with the linkage name "hello". I suppose that's expected, > I had never really paid attention to this. > > There's also a third call, while creating the full blown symbol, with a > linkage name of "hello(int)". > > So the term "linkage_name" in general_symbol_info is perhaps a bit misleading, > it makes sense for minimal symbols, but not really for partial and full symbols. Hm.. maybe I'll look into this some more to understand this better. It certainly seems surprising that minsyms behave differently from other symbols in this respect? Christian