From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 58014 invoked by alias); 9 Jan 2019 04:36:50 -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 57993 invoked by uid 89); 9 Jan 2019 04:36:48 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=Keith X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 09 Jan 2019 04:36:46 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id x094ae9p004354 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 8 Jan 2019 23:36:45 -0500 Received: by simark.ca (Postfix, from userid 112) id 4DA481E7B7; Tue, 8 Jan 2019 23:36:40 -0500 (EST) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 34A511E4A3; Tue, 8 Jan 2019 23:36:39 -0500 (EST) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Wed, 09 Jan 2019 04:36:00 -0000 From: Simon Marchi To: Keith Seitz Cc: Simon Marchi , gdb-patches@sourceware.org Subject: Re: [PATCH] gdb: Remove support for old mangling schemes In-Reply-To: References: <20190108232208.17487-1-simon.marchi@ericsson.com> Message-ID: <6bbbc56c6c8e7db98242bda10af0db02@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.6 X-IsSubscribed: yes X-SW-Source: 2019-01/txt/msg00175.txt.bz2 On 2019-01-08 19:24, Keith Seitz wrote: > On 1/8/19 3:22 PM, Simon Marchi wrote: >> An upcoming sync with gcc's libiberty [1] will remove support for old >> mangling schemes (GNU v2, Lucid, ARM, HP and EDG). It will remove the >> cplus_demangle_opname function, so we need to get rid of its usages in >> GDB (it's a GNU v2 specific function). > > Thank you so much for doing this. I was working on my own version of > this > before other work got in the way. Thanks for looking at mine then! > Your patch looks just about identical to mine, and I had a few > additional > questions about my own patch that I needed to answer. > > I would think this should have/deserve doc and NEWS changes. I agree. > The manual talks about several demangling-related features which > raised questions for me about the general support for gnu-v2 code. If > libiberty is removing support for v2 demangling, how useful will GDB be > without the ability to demangle v2 symbols? Should we deprecate or > remove > that, too? /me opens can of worms Yes, probably. The goal of this patch is to unblock the sync'ing with gcc's libiberty (remove usages of cplus_demangle_opname), but we can certainly do some cleanup later. > WRT docs specifically, "set cp-abi" and "set demangling-style" still > carry > either direct or indirect mentions of the different ABIs/mangling > schemes in > the documentation that are now no longer valid, e.g., "lucid" and > "arm". > Some comments in demangle.c do, too. Well, this patch has the effect of removing those demangling styles, so it should certainly update the doc for "set demangle-style". But it doesn't touch the cp-abi, so I won't change that part in this patch (although as you noted, gnu-v2 will probably be useless if binaries with this ABI also use the "gnu" demangling style). > The help system references that exist today/right now, I presume, will > disappear > when the libiberty patch goes back in (when libiberty_demanglers is > updated), so > I *think* there's nothing to do there. Indeed, "set demangle-style" and "set demande-style" are both automatically updated. > Now the bonus. :-) > >> diff --git a/gdb/testsuite/gdb.cp/demangle.exp >> b/gdb/testsuite/gdb.cp/demangle.exp >> index 698231b82b4..9231b54f787 100644 >> --- a/gdb/testsuite/gdb.cp/demangle.exp >> +++ b/gdb/testsuite/gdb.cp/demangle.exp >> - >> - >> proc catch_demangling_errors {command} { >> if {[catch $command result]} { >> puts "ERROR: demangle.exp: while running $command: $result" >> @@ -1593,10 +183,7 @@ proc do_tests {} { >> # Using catch_demangling_errors this way ensures that, if one of >> # the functions raises a Tcl error, then it'll get reported, and >> # the rest of the functions will still run. >> - catch_demangling_errors test_lucid_style_demangling >> - catch_demangling_errors test_gnu_style_demangling >> - catch_demangling_errors test_arm_style_demangling >> - catch_demangling_errors test_hp_style_demangling >> + catch_demangling_errors test_gnuv3_style_demangling >> > > Please consider getting rid of catch_demangling_errors and removing the > "error" calls in set_demangling_style. They are not needed, and in > fact, > they cause the test suite to hang if that error is ever called. I ran > across this while prepping my patch, and planned to remove it. > > Of course, this (nit) is sorta orthogonal, so please feel free to ask > me to do it! > ;-) Right, I noticed the hang too. I was thinking of doing a pass of cleanup/modernization on that file after, as I spotted some other things (such as unused "global"). I won't ask you to do it nor prevent you :P. Simon