From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id D5BC93858407 for ; Mon, 3 Oct 2022 17:14:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D5BC93858407 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1664817255; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=BA2G1elWG6HlZ1KsEkQfNJtLr+4tPCA22HPiDvEPvuc=; b=FkwmDrmqqvBwZzNoFAAtyV7v2sGqG2ARsTTVNEYJ5VmK4OO3Y+adAQdHjgoz65DPDoRuaw NQblgwV1KsPOnTK/8nhJCZ8eF6T5Z869WHPCRFdf6nyisIx8PKdxlQOApcCr4qh33v19Y9 PyEJGBcel/aNgkWrZ8eab3ncHkfAgOc= Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-257-MFXRNOgWMjGYY6RX_wc57Q-1; Mon, 03 Oct 2022 13:14:14 -0400 X-MC-Unique: MFXRNOgWMjGYY6RX_wc57Q-1 Received: by mail-qk1-f199.google.com with SMTP id az15-20020a05620a170f00b006cece4cd0beso9655862qkb.22 for ; Mon, 03 Oct 2022 10:14:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date; bh=BA2G1elWG6HlZ1KsEkQfNJtLr+4tPCA22HPiDvEPvuc=; b=RT+00rFNhv5go5A7qw8EJ8iLczGMuB/1Owe2+dH4C49bHOoj6qlP+goZx/0r8T0SRi g2dYqyxKzPBiKhanZlbOaHRCmPE4g13fL9Pyz6CcEDDleGYNMTlDla3TXeDBfyc4pldW 7hy8qt2+gYqzazaz2cEv4AsqgjLJScBIILBYfrASgOTQqEZvTW/+DwKkwbeuqC4Uoezc 5DLGC1HODlZAoyd/vY2HFGGEdu8ARGhtYT+BgkM25xQLN5gKkhGDkxd1dcpnaWmv1tB1 XGOU7Ea2lEG6WuqNbIT4Ib3YUYTHTZTjrfF+YIDmo8zp8CoIKSRmiB31/smZSu/hC7jz pOxw== X-Gm-Message-State: ACrzQf3bGvhOUZI8M0DagSbTBvCjXEGNsEtnJcfnil6gBDti3ayU1hMn K8qjo93Fn2cFWPlMpIP5ltJFSVW+wfalBhSguGafK9JTsTYWv99+6tDiOMsEJngprPk4LfgQiJh 49PHQqdbb0lgn/BpsQGRmmup0zigr5HCfJA== X-Received: by 2002:a05:620a:28c5:b0:6ce:bc17:15b0 with SMTP id l5-20020a05620a28c500b006cebc1715b0mr14214556qkp.245.1664817253870; Mon, 03 Oct 2022 10:14:13 -0700 (PDT) X-Google-Smtp-Source: AMsMyM60BOwuGPoZVg3OQosou+QigGRrRlqdR8/WnLVpwWNlSZw4dCIfnjvKFzp8Tn8mQsRolFKq0AaOvLWOHvQdIvM= X-Received: by 2002:a05:620a:28c5:b0:6ce:bc17:15b0 with SMTP id l5-20020a05620a28c500b006cebc1715b0mr14214548qkp.245.1664817253565; Mon, 03 Oct 2022 10:14:13 -0700 (PDT) MIME-Version: 1.0 References: <1d7b853e-0180-718c-1947-2517272f44e8@gmail.com> <2eb944c4-131e-001d-a3a6-cf1b7aea5614@gmail.com> <36ff419f-ff22-f954-b62b-8d7e62920fb0@gmail.com> In-Reply-To: <36ff419f-ff22-f954-b62b-8d7e62920fb0@gmail.com> From: Jonathan Wakely Date: Mon, 3 Oct 2022 18:14:02 +0100 Message-ID: Subject: Re: [PATCH] Fix gdb printers for std::string To: =?UTF-8?Q?Fran=C3=A7ois_Dumont?= Cc: "libstdc++@gcc.gnu.org" , gcc-patches X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_NONE,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 List-Id: On Mon, 3 Oct 2022 at 17:51, Fran=C3=A7ois Dumont wr= ote: > > On 01/10/22 17:30, Jonathan Wakely wrote: > > On Sat, 1 Oct 2022 at 11:43, Fran=C3=A7ois Dumont wrote: > >> On 01/10/22 12:06, Jonathan Wakely wrote: > >>> On Sat, 1 Oct 2022 at 08:20, Fran=C3=A7ois Dumont via Libstdc++ > >>> wrote: > >>>> I had forgotten to re-run tests after I removed the #define > >>>> _GLIBCXX_USE_CXX11_ABI 0. > >>>> > >>>> The comment was misleading, it could also impact output of std::list= . > >>>> > >>>> I am also restoring the correct std::string alias for > >>>> std::__cxx11::basic_string, even if with my workaround it doesn't re= ally > >>>> matter as the one for std::basic_string will be used. > >>>> > >>>> I also restored the printer for std::__cxx11::string typedef. Is it > >>>> intentional to keep this ? > >>> Yes, I kept that intentionally. There can be programs where some > >>> objects still use that typedef, if those objects were compiled with > >>> GCC 8.x or older. > >>> > >>>> libstdc++: Fix gdb pretty printers when dealing with std::str= ing > >>>> > >>>> Since revision 33b43b0d8cd2de722d177ef823930500948a7487 std::= string > >>>> and other > >>>> similar typedef are ambiguous from a gdb point of view becaus= e it > >>>> matches both > >>>> std::basic_string and std::__cxx11::basic_string > >>>> symbols. For those > >>>> typedef add a workaround to accept the substitution as long a= s the > >>>> same regardless > >>>> of __cxx11 namespace. > >>> Thanks for figuring out what was going wrong here, and how to fix it. > >>> > >>> > >>>> Also avoid to register printers for types in std::__cxx11::__= 8:: > >>>> namespace, there is > >>>> no such symbols. > >>>> > >>>> libstdc++-v3/ChangeLog: > >>>> > >>>> * libstdc++-v3/python/libstdcxx/v6/printers.py > >>>> (Printer.add_version): Do not add > >>>> version namespace for __cxx11 symbols. > >>>> (add_one_template_type_printer): Likewise. > >>>> (add_one_type_printer): Likewise. > >>>> (FilteringTypePrinter._recognizer.recognize): Add a > >>>> workaround for std::string & al > >>>> ambiguous typedef matching both std:: and std::__cxx1= 1:: > >>>> symbols. > >>>> (register_type_printers): Refine type registration to= limit > >>>> false positive in > >>>> FilteringTypePrinter._recognize.recognize requiring t= o look > >>>> for the type in gdb. > >>> I don't really like this part of the change though: > >> I'll check what you are proposing but I don't think it is necessary to > >> fix the problem. > > Most of my patch is an alternative way to make the filter match on > > "basic_string > the ambiguous string typedefs, by using match.split('::')[-1] to get > > the class template name without namespaces and then compare that to > > "basic_string". > > > >> I did this on my path to find out what was going wrong. Once I > >> understood it I consider that it was just a good change to keep. If yo= u > >> think otherwise I can revert this part. > > Yeah it looks like it's just an optimization to fail faster without > > having to do gdb.lookup_type. > > > > Please revert the changes to register_type_printers then, and we can > > consider that part later if we want to revisit it. I'm not opposed to > > making that fail-fast optimization, as long as we keep the property > > that FilteringTypePrinter.match is the class template name. Maybe it > > should be renamed to something other than "match" to make that clear. > > Or change the doc ? For my info, why is it so important to comply to the > current doc ? Is it extracted from some gdb doc ? The 'match' argument is the name of a class template to match. If we want to match a class template with specific template arguments, I would prefer to pass in the name of the class template and the names of the template argument types, not just a string. We can do more with individually named types, rather than just munging it all into a single string and only being able to do string comparisons. > > Now that the problem is fixed it is less important but I never managed > to find any doc about this gdb feature that we are relying on. It's this one: https://sourceware.org/gdb/onlinedocs/gdb/Type-Printing-API.html > > > > > > The rest of the patch is OK for trunk, thanks. > > > >> I also noted that gdb consider the filters as a filo list, not fifo. A= nd > >> I think that the 1st filters registered are the most extensively used.= I > >> can propose to invert all the registration if you think it worth it. > > I've not noticed any performance problems with the printers, but I > > have wondered how many printers is too many. That's an interesting > > observation about the order they're checked. I'll talk to some of the > > GDB devs to find out if they think it's something we should worry > > about. Let's not try make premature optimizations until we know if it > > matters. > > Yes, but with the 1st registration and so the last evaluation being > 'std::string' it sounds more like a premature lowering ;-) Ha, yes, maybe.