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 767C63877027 for ; Wed, 18 Mar 2020 15:58:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 767C63877027 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 E85E8AB8F; Wed, 18 Mar 2020 15:58:22 +0000 (UTC) Subject: Re: [PATCH][gdb/symtab] Fix check-psymtab failure for inline function To: Tom Tromey Cc: gdb-patches@sourceware.org References: <20200309145237.GA20334@delia> <874kuv7nxk.fsf@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: <6d39b3ec-c8f2-ae28-5fa1-caa04c2d75ff@suse.de> Date: Wed, 18 Mar 2020 16:58:22 +0100 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: <874kuv7nxk.fsf@tromey.com> Content-Type: multipart/mixed; boundary="------------5970080C6FEF0AC3DF327AC3" Content-Language: en-US X-Spam-Status: No, score=-25.0 required=5.0 tests=GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS 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: Wed, 18 Mar 2020 15:58:26 -0000 This is a multi-part message in MIME format. --------------5970080C6FEF0AC3DF327AC3 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 11-03-2020 01:08, Tom Tromey wrote: >>>>>> "Tom" == Tom de Vries writes: > > Tom> When loading the executable in gdb, we create a partial symbol for foo, but > Tom> after expansion into a full symbol table no actual symbol is created, > Tom> resulting in a maint check-psymtab failure: > Tom> ... > Tom> (gdb) maint check-psymtab > Tom> Static symbol `foo' only found in inline.c psymtab > Tom> ... > > Tom> Fix this by preventing the creation of this type of partial symbol. > > With this patch, if there is a function which is only inlined (there is > no "out-line" copy), will "break function" still work? > > It seems to me that it wouldn't always, because there wouldn't be a > partial symbol to match. Maybe it might work accidentally if the inline > function appears in a CU that is already expanded, like main's. So the > test would have to be an always-inline function that isn't used in > main's CU. > > If it does work, then I wonder how, since my mental model is that all > the paths to expansion require a matching partial symbol. > > If it doesn't work, then I think a different fix is needed. Indeed, a different fix is needed. This patch fixes things in the check itself instead. OK for trunk? Thanks, - Tom --------------5970080C6FEF0AC3DF327AC3 Content-Type: text/x-patch; charset=UTF-8; name="0001-gdb-symtab-Fix-check-psymtab-failure-for-inline-function.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename*0="0001-gdb-symtab-Fix-check-psymtab-failure-for-inline-functio"; filename*1="n.patch" [gdb/symtab] Fix check-psymtab failure for inline function Consider test-case inline.c, containing an inline function foo: ... static inline int foo (void) { return 0; } int main (void) { return foo (); } ... And the test-case compiled with -O2 and debug info: ... $ gcc -g inline.c -O2 ... This results in a DWARF entry for foo without pc info: ... <1><114>: Abbrev Number: 4 (DW_TAG_subprogram) <115> DW_AT_name : foo <119> DW_AT_decl_file : 1 <11a> DW_AT_decl_line : 2 <11b> DW_AT_prototyped : 1 <11b> DW_AT_type : <0x10d> <11f> DW_AT_inline : 3 (declared as inline and inlined) ... When loading the executable in gdb, we create a partial symbol for foo, but after expansion into a full symbol table no actual symbol is created, resulting in a maint check-psymtab failure: ... (gdb) maint check-psymtab Static symbol `foo' only found in inline.c psymtab ... Fix this by skipping this type of partial symbol during the check. Note that we're not fixing this by not creating the partial symbol, because this breaks setting a breakpoint on an inlined inline function in a CU for which the partial symtab has not been expanded (test-case gdb.dwarf2/break-inline-psymtab.exp). Tested on x86_64-linux. gdb/ChangeLog: 2020-03-18 Tom de Vries * psymtab.c (maintenance_check_psymtabs): Skip static LOC_BLOCK symbols without address. --- gdb/psymtab.c | 16 +++++++++------- gdb/testsuite/gdb.base/check-psymtab.c | 28 ++++++++++++++++++++++++++++ gdb/testsuite/gdb.base/check-psymtab.exp | 27 +++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 7 deletions(-) diff --git a/gdb/psymtab.c b/gdb/psymtab.c index f77f6d5108..b65795d062 100644 --- a/gdb/psymtab.c +++ b/gdb/psymtab.c @@ -2104,7 +2104,7 @@ maintenance_check_psymtabs (const char *ignore, int from_tty) struct compunit_symtab *cust = NULL; const struct blockvector *bv; const struct block *b; - int length; + int i; for (objfile *objfile : current_program_space->objfiles ()) for (partial_symtab *ps : require_partial_symbols (objfile, true)) @@ -2138,9 +2138,14 @@ maintenance_check_psymtabs (const char *ignore, int from_tty) b = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK); partial_symbol **psym = &objfile->partial_symtabs->static_psymbols[ps->statics_offset]; - length = ps->n_static_syms; - while (length--) + for (i = 0; i < ps->n_static_syms; psym++, i++) { + /* Skip symbols for inlined functions without address. These may + or may not have a match in the full symtab. */ + if ((*psym)->aclass == LOC_BLOCK + && (*psym)->ginfo.value.address == 0) + continue; + sym = block_lookup_symbol (b, (*psym)->ginfo.search_name (), symbol_name_match_type::SEARCH_NAME, (*psym)->domain); @@ -2152,12 +2157,10 @@ maintenance_check_psymtabs (const char *ignore, int from_tty) puts_filtered (ps->filename); printf_filtered (" psymtab\n"); } - psym++; } b = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK); psym = &objfile->partial_symtabs->global_psymbols[ps->globals_offset]; - length = ps->n_global_syms; - while (length--) + for (i = 0; i < ps->n_global_syms; psym++, i++) { sym = block_lookup_symbol (b, (*psym)->ginfo.search_name (), symbol_name_match_type::SEARCH_NAME, @@ -2170,7 +2173,6 @@ maintenance_check_psymtabs (const char *ignore, int from_tty) puts_filtered (ps->filename); printf_filtered (" psymtab\n"); } - psym++; } if (ps->raw_text_high () != 0 && (ps->text_low (objfile) < BLOCK_START (b) diff --git a/gdb/testsuite/gdb.base/check-psymtab.c b/gdb/testsuite/gdb.base/check-psymtab.c new file mode 100644 index 0000000000..2c1e69955e --- /dev/null +++ b/gdb/testsuite/gdb.base/check-psymtab.c @@ -0,0 +1,28 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 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 . */ + +static inline int +foo (void) +{ + return 0; +} + +int +main (void) +{ + return foo (); +} diff --git a/gdb/testsuite/gdb.base/check-psymtab.exp b/gdb/testsuite/gdb.base/check-psymtab.exp new file mode 100644 index 0000000000..06438fc7c0 --- /dev/null +++ b/gdb/testsuite/gdb.base/check-psymtab.exp @@ -0,0 +1,27 @@ +# Copyright 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 . */ + +standard_testfile + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile \ + {debug optimize=-O2}]} { + return -1 +} + +gdb_test_no_output "maint expand-symtabs" + +# Check that we don't get: +# Static symbol `foo' only found in check-psymtab.c psymtab +gdb_test_no_output "maint check-psymtab" --------------5970080C6FEF0AC3DF327AC3--