From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 56BDC3893671 for ; Fri, 24 Apr 2020 16:06:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 56BDC3893671 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tdevries@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 73DE3AD11; Fri, 24 Apr 2020 16:06:39 +0000 (UTC) Subject: Re: [PATCH 08/10] Use the linkage name if it exists To: Tom Tromey , gdb-patches@sourceware.org References: <20200325200715.12947-1-tom@tromey.com> <20200325200715.12947-9-tom@tromey.com> From: Tom de Vries Autocrypt: addr=tdevries@suse.de; keydata= xsBNBF0ltCcBCADDhsUnMMdEXiHFfqJdXeRvgqSEUxLCy/pHek88ALuFnPTICTwkf4g7uSR7 HvOFUoUyu8oP5mNb4VZHy3Xy8KRZGaQuaOHNhZAT1xaVo6kxjswUi3vYgGJhFMiLuIHdApoc u5f7UbV+egYVxmkvVLSqsVD4pUgHeSoAcIlm3blZ1sDKviJCwaHxDQkVmSsGXImaAU+ViJ5l CwkvyiiIifWD2SoOuFexZyZ7RUddLosgsO0npVUYbl6dEMq2a5ijGF6/rBs1m3nAoIgpXk6P TCKlSWVW6OCneTaKM5C387972qREtiArTakRQIpvDJuiR2soGfdeJ6igGA1FZjU+IsM5ABEB AAHNH1RvbSBkZSBWcmllcyA8dGRldnJpZXNAc3VzZS5kZT7CwKsEEwEIAD4WIQSsnSe5hKbL MK1mGmjuhV2rbOJEoAUCXSW0JwIbAwUJA8JnAAULCQgHAgYVCgkICwIEFgIDAQIeAQIXgAAh CRDuhV2rbOJEoBYhBKydJ7mEpsswrWYaaO6FXats4kSgc48H/Ra2lq5p3dHsrlQLqM7N68Fo eRDf3PMevXyMlrCYDGLVncQwMw3O/AkousktXKQ42DPJh65zoXB22yUt8m0g12xkLax98KFJ 5NyUloa6HflLl+wQL/uZjIdNUQaHQLw3HKwRMVi4l0/Jh/TygYG1Dtm8I4o708JS4y8GQxoQ UL0z1OM9hyM3gI2WVTTyprsBHy2EjMOu/2Xpod95pF8f90zBLajy6qXEnxlcsqreMaqmkzKn 3KTZpWRxNAS/IH3FbGQ+3RpWkNGSJpwfEMVCeyK5a1n7yt1podd1ajY5mA1jcaUmGppqx827 8TqyteNe1B/pbiUt2L/WhnTgW1NC1QDOwE0EXSW0JwEIAM99H34Bu4MKM7HDJVt864MXbx7B 1M93wVlpJ7Uq+XDFD0A0hIal028j+h6jA6bhzWto4RUfDl/9mn1StngNVFovvwtfzbamp6+W pKHZm9X5YvlIwCx131kTxCNDcF+/adRW4n8CU3pZWYmNVqhMUiPLxElA6QhXTtVBh1RkjCZQ Kmbd1szvcOfaD8s+tJABJzNZsmO2hVuFwkDrRN8Jgrh92a+yHQPd9+RybW2l7sJv26nkUH5Z 5s84P6894ebgimcprJdAkjJTgprl1nhgvptU5M9Uv85Pferoh2groQEAtRPlCGrZ2/2qVNe9 XJfSYbiyedvApWcJs5DOByTaKkcAEQEAAcLAkwQYAQgAJhYhBKydJ7mEpsswrWYaaO6FXats 4kSgBQJdJbQnAhsMBQkDwmcAACEJEO6FXats4kSgFiEErJ0nuYSmyzCtZhpo7oVdq2ziRKD3 twf7BAQBZ8TqR812zKAD7biOnWIJ0McV72PFBxmLIHp24UVe0ZogtYMxSWKLg3csh0yLVwc7 H3vldzJ9AoK3Qxp0Q6K/rDOeUy3HMqewQGcqrsRRh0NXDIQk5CgSrZslPe47qIbe3O7ik/MC q31FNIAQJPmKXX25B115MMzkSKlv4udfx7KdyxHrTSkwWZArLQiEZj5KG4cCKhIoMygPTA3U yGaIvI/BGOtHZ7bEBVUCFDFfOWJ26IOCoPnSVUvKPEOH9dv+sNy7jyBsP5QxeTqwxC/1ZtNS DUCSFQjqA6bEGwM22dP8OUY6SC94x1G81A9/xbtm9LQxKm0EiDH8KBMLfQ== Message-ID: Date: Fri, 24 Apr 2020 18:06:38 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: <20200325200715.12947-9-tom@tromey.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-27.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_DMARC_STATUS, KAM_SHORT, KAM_STOCKGEN, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 Apr 2020 16:06:45 -0000 On 25-03-2020 21:07, Tom Tromey wrote: > The DWARF reader has had some odd code since the "physname" patches landed. > > In particular, these patches caused PR symtab/12707; namely, they made > it so "set print demangle off" no longer works. > > This patch attempts to fix the problem. It arranges to store the > linkage name on the symbol if it exists, and it changes the DWARF > reader so that the demangled name is no longer (usually) stored in the > symbol's "linkage name" field. > > c-linkage-name.exp needed a tweak, because it started working > correctly. This conforms to what I think ought to happen, so this > seems like an improvement here. > > compile-object-load.c needed a small change to use > symbol_matches_search_name rather than directly examining the linkage > name. Looking directly at the name does the wrong thing for C++. > > There is still some name-related confusion in the DWARF reader: > > * "physname" often refers to the logical name and not what I would > consider to be the "physical" name; > > * dwarf2_full_name, dwarf2_name, and dwarf2_physname all exist and > return different strings -- but this seems like at least one name > too many. For example, Fortran requires dwarf2_full_name, but other > languages do not. > > * To my surprise, dwarf2_physname prefers the form emitted by the > demangler over the one that it computes. This seems backward to me, > given that the partial symbol reader prefers the opposite, and it > seems to me that this choice may perform better as well. > > I didn't attempt to clean up these things. It would be good to do, > but whenever I contemplate it I get caught up in dreams of truly > rewriting the DWARF reader instead. > Hi, As you mentioned on IRC, there's a regression with gdb.dwarf2/dw2-bad-mips-linkage-name.exp and target board cc-with-debug-names. I tracked that down to this patch in the series. Using readelf -w, I can read the .debug_names section, and see without this patch: ... [ 6] #0002b60b f: <3> DW_TAG_subprogram DW_IDX_compile_unit=3 DW_IDX_GNU_internal=1 [ 8] #0002b60c g: <3> DW_TAG_subprogram DW_IDX_compile_unit=3 DW_IDX_GNU_internal=1 ... and with this patch: ... [ 6] #0ef9dc4b _Z1fv: <3> DW_TAG_subprogram DW_IDX_compile_unit=3 DW_IDX_GNU_internal=1 [ 8] #0002b60c g: <3> DW_TAG_subprogram DW_IDX_compile_unit=3 DW_IDX_GNU_internal=1 ... So we end up with the mangled name for f in the index. That looks significant, but I'm not sure yet if that is the reason why we're not able to print the type of f. Thanks, - Tom > gdb/ChangeLog > 2020-03-25 Tom Tromey > > PR symtab/12707: > * dwarf2/read.c (add_partial_symbol): Use the linkage name if it > exists. > (new_symbol): Likewise. > * compile/compile-object-load.c (get_out_value_type): Use > symbol_matches_search_name. > > gdb/testsuite/ChangeLog > 2020-03-25 Tom Tromey > > PR symtab/12707: > * gdb.python/py-symbol.exp: Update expected results for > linkage_name test. > * gdb.cp/print-demangle.exp: New file. > * gdb.base/c-linkage-name.exp: Fix test. > * gdb.guile/scm-symbol.exp: Update expected results for > linkage_name test. > --- > gdb/ChangeLog | 9 +++++++ > gdb/compile/compile-object-load.c | 7 ++--- > gdb/dwarf2/read.c | 29 ++++++++++++++------ > gdb/testsuite/ChangeLog | 10 +++++++ > gdb/testsuite/gdb.base/c-linkage-name.exp | 2 +- > gdb/testsuite/gdb.cp/print-demangle.exp | 32 +++++++++++++++++++++++ > gdb/testsuite/gdb.guile/scm-symbol.exp | 4 +-- > gdb/testsuite/gdb.python/py-symbol.exp | 2 +- > 8 files changed, 79 insertions(+), 16 deletions(-) > create mode 100644 gdb/testsuite/gdb.cp/print-demangle.exp > > diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c > index 3fe95183e32..be4fa767142 100644 > --- a/gdb/compile/compile-object-load.c > +++ b/gdb/compile/compile-object-load.c > @@ -402,6 +402,9 @@ get_out_value_type (struct symbol *func_sym, struct objfile *objfile, > int nblocks = 0; > int block_loop = 0; > > + lookup_name_info func_matcher (GCC_FE_WRAPPER_FUNCTION, > + symbol_name_match_type::SEARCH_NAME); > + > bv = SYMTAB_BLOCKVECTOR (func_sym->owner.symtab); > nblocks = BLOCKVECTOR_NBLOCKS (bv); > > @@ -433,9 +436,7 @@ get_out_value_type (struct symbol *func_sym, struct objfile *objfile, > if (function != NULL > && (BLOCK_SUPERBLOCK (function_block) > == BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK)) > - && (strcmp_iw (function->linkage_name (), > - GCC_FE_WRAPPER_FUNCTION) > - == 0)) > + && symbol_matches_search_name (function, func_matcher)) > break; > } > if (block_loop == nblocks) > diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c > index 2bc7b521f2e..1e824ca399d 100644 > --- a/gdb/dwarf2/read.c > +++ b/gdb/dwarf2/read.c > @@ -8370,7 +8370,14 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu) > { > if (built_actual_name != nullptr) > actual_name = objfile->intern (actual_name); > - psymbol.ginfo.set_linkage_name (actual_name); > + if (pdi->linkage_name == nullptr || cu->language == language_ada) > + psymbol.ginfo.set_linkage_name (actual_name); > + else > + { > + psymbol.ginfo.set_demangled_name (actual_name, > + &objfile->objfile_obstack); > + psymbol.ginfo.set_linkage_name (pdi->linkage_name); > + } > add_psymbol_to_list (psymbol, *where, objfile); > } > } > @@ -20550,7 +20557,6 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu, > name = dwarf2_name (die, cu); > if (name) > { > - const char *linkagename; > int suppress_add = 0; > > if (space) > @@ -20561,14 +20567,21 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu, > > /* Cache this symbol's name and the name's demangled form (if any). */ > sym->set_language (cu->language, &objfile->objfile_obstack); > - linkagename = dwarf2_physname (name, die, cu); > - sym->compute_and_set_names (linkagename, false, objfile->per_bfd); > - > /* Fortran does not have mangling standard and the mangling does differ > between gfortran, iFort etc. */ > - if (cu->language == language_fortran > - && symbol_get_demangled_name (sym) == NULL) > - sym->set_demangled_name (dwarf2_full_name (name, die, cu), NULL); > + const char *physname > + = (cu->language == language_fortran > + ? dwarf2_full_name (name, die, cu) > + : dwarf2_physname (name, die, cu)); > + const char *linkagename = dw2_linkage_name (die, cu); > + > + if (linkagename == nullptr || cu->language == language_ada) > + sym->set_linkage_name (physname); > + else > + { > + sym->set_demangled_name (physname, &objfile->objfile_obstack); > + sym->set_linkage_name (linkagename); > + } > > /* Default assumptions. > Use the passed type or decode it from the die. */ > diff --git a/gdb/testsuite/gdb.base/c-linkage-name.exp b/gdb/testsuite/gdb.base/c-linkage-name.exp > index 4551c8dcac2..cf3e3780bb6 100644 > --- a/gdb/testsuite/gdb.base/c-linkage-name.exp > +++ b/gdb/testsuite/gdb.base/c-linkage-name.exp > @@ -48,7 +48,7 @@ verify_psymtab_expanded c-linkage-name-2.c no > # Try to print MUNDANE, but using its linkage name. > > gdb_test "print symada__cS" \ > - "'symada__cS' has unknown type; cast it to its declared type" \ > + " = {a = 100829103}" \ > "print symada__cS before partial symtab expansion" > > # Force the symbols to be expanded for the unit that contains > diff --git a/gdb/testsuite/gdb.cp/print-demangle.exp b/gdb/testsuite/gdb.cp/print-demangle.exp > new file mode 100644 > index 00000000000..9e0f706febf > --- /dev/null > +++ b/gdb/testsuite/gdb.cp/print-demangle.exp > @@ -0,0 +1,32 @@ > +# Copyright (C) 2013, 2020 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +if { [skip_cplus_tests] } { continue } > + > +standard_testfile bool.cc > + > +if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}]} { > + return -1 > +} > + > +runto_main > + > +gdb_breakpoint "return_true" > + > +gdb_continue_to_breakpoint "return_true" > + > +gdb_test_no_output "set print demangle off" > + > +gdb_test "frame" " _Z11return_truev .*" > diff --git a/gdb/testsuite/gdb.guile/scm-symbol.exp b/gdb/testsuite/gdb.guile/scm-symbol.exp > index 4addc0d10d0..486fc8fcfdb 100644 > --- a/gdb/testsuite/gdb.guile/scm-symbol.exp > +++ b/gdb/testsuite/gdb.guile/scm-symbol.exp > @@ -169,10 +169,8 @@ gdb_test "guile (print (symbol-name cplusfunc))" \ > "= SimpleClass::valueofi().*" "test method.name" > gdb_test "guile (print (symbol-print-name cplusfunc))" \ > "= SimpleClass::valueofi().*" "test method.print_name" > -# FIXME: GDB is broken here and we're verifying broken behaviour. > -# (linkage-name should be the mangled name) > gdb_test "guile (print (symbol-linkage-name cplusfunc))" \ > - "SimpleClass::valueofi().*" "test method.linkage_name" > + "_ZN11SimpleClass8valueofiEv.*" "test method.linkage_name" > gdb_test "guile (print (= (symbol-addr-class cplusfunc) SYMBOL_LOC_BLOCK))" "= #t" > > # Test is_valid when the objfile is unloaded. This must be the last > diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp > index c4bae9f07f2..caa7ddc8007 100644 > --- a/gdb/testsuite/gdb.python/py-symbol.exp > +++ b/gdb/testsuite/gdb.python/py-symbol.exp > @@ -211,7 +211,7 @@ gdb_test "python print (cplusfunc.is_function)" \ > > gdb_test "python print (cplusfunc.name)" "SimpleClass::valueofi().*" "test method.name" > gdb_test "python print (cplusfunc.print_name)" "SimpleClass::valueofi().*" "test method.print_name" > -gdb_test "python print (cplusfunc.linkage_name)" "SimpleClass::valueofi().*" "test method.linkage_name" > +gdb_test "python print (cplusfunc.linkage_name)" "_ZN11SimpleClass8valueofiEv.*" "test method.linkage_name" > gdb_test "python print (cplusfunc.addr_class == gdb.SYMBOL_LOC_BLOCK)" "True" "test method.addr_class" > > # Test is_valid when the objfile is unloaded. This must be the last >