[PING][PATCH][gdb/symtab] Fix check-psymtab failure for inline function
Tom de Vries
tdevries@suse.de
Thu Apr 2 08:46:29 GMT 2020
On 18-03-2020 16:58, Tom de Vries wrote:
> On 11-03-2020 01:08, Tom Tromey wrote:
>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> 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?
>
Ping.
Thanks,- Tom
> 0001-gdb-symtab-Fix-check-psymtab-failure-for-inline-function.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 <tdevries@suse.de>
>
> * 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 <http://www.gnu.org/licenses/>. */
> +
> +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 <http://www.gnu.org/licenses/>. */
> +
> +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"
>
More information about the Gdb-patches
mailing list