This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[PING^2][PATCH][gdb/symtab] Prefer var def over decl
- From: Tom de Vries <tdevries at suse dot de>
- To: gdb-patches at sourceware dot org
- Date: Tue, 8 Oct 2019 08:55:52 +0200
- Subject: [PING^2][PATCH][gdb/symtab] Prefer var def over decl
- References: <20190910154310.GA20844@delia> <9cd61565-d50e-a0d3-78bc-957f18e109ab@suse.de>
On 24-09-19 17:28, Tom de Vries wrote:
> On 10-09-19 17:43, Tom de Vries wrote:
>> Hi,
>>
>> Consider the DWARF as generated by gcc with the tentative patch to fix gcc
>> PR91507 - "wrong debug for completed array with previous incomplete
>> declaration":
>> ...
>> <1><f4>: Abbrev Number: 2 (DW_TAG_array_type)
>> <f5> DW_AT_type : <0xff>
>> <f9> DW_AT_sibling : <0xff>
>> <2><fd>: Abbrev Number: 3 (DW_TAG_subrange_type)
>> <2><fe>: Abbrev Number: 0
>> <1><ff>: Abbrev Number: 4 (DW_TAG_pointer_type)
>> <100> DW_AT_byte_size : 8
>> <101> DW_AT_type : <0x105>
>> <1><105>: Abbrev Number: 5 (DW_TAG_base_type)
>> <106> DW_AT_byte_size : 1
>> <107> DW_AT_encoding : 6 (signed char)
>> <108> DW_AT_name : (indirect string, offset: 0x19f): char
>> <1><10c>: Abbrev Number: 6 (DW_TAG_variable)
>> <10d> DW_AT_name : zzz
>> <111> DW_AT_decl_file : 1
>> <112> DW_AT_decl_line : 1
>> <113> DW_AT_decl_column : 14
>> <114> DW_AT_type : <0xf4>
>> <118> DW_AT_external : 1
>> <118> DW_AT_declaration : 1
>> <1><118>: Abbrev Number: 2 (DW_TAG_array_type)
>> <119> DW_AT_type : <0xff>
>> <11d> DW_AT_sibling : <0x128>
>> <1><12f>: Abbrev Number: 8 (DW_TAG_variable)
>> <130> DW_AT_specification: <0x10c>
>> <134> DW_AT_decl_line : 2
>> <135> DW_AT_decl_column : 7
>> <136> DW_AT_type : <0x118>
>> <13a> DW_AT_location : 9 byte block: 3 30 10 60 0 0 0 0 0 (DW_OP_addr: 601030)
>> ...
>>
>> The DWARF will result in two entries in the symbol table, a decl with type
>> char *[] and a def with type char*[2].
>>
>> When trying to print the value of zzz:
>> ...
>> $ gdb a.spec.out -batch -ex "p zzz"
>> ...
>> the decl (rather than the def) will be found in the symbol table, which is
>> missing the location information, and consequently we get:
>> ...
>> $1 = 0x601030 <zzz>
>> ...
>>
>> [ There is a fallback mechanism that finds the address of the variable in the
>> minimal symbol table, but that's not used here, because the type of the decl
>> does not specify a size. We could use the symbol size here to get the size
>> of the type, but that's currently not done: PR exp/24989. Still, fixing that
>> PR would not fix the generic case, where minimal symbol info is not
>> available. ]
>>
>> Fix this by preferring defs over decls when searching in the symbol table.
>>
>> Build and reg-tested on x86_64-linux.
>>
>> [ The test-case is a bit simpler than the DWARF example listed above, because
>> the new variable varval3 that is used is not listed in the minimal symbols, so
>> there's no need to work around the fallback mechanism to trigger the problem. ]
>>
>> OK for trunk?
>
Ping^2.
Thanks,
- Tom
>> [gdb/symtab] Prefer var def over decl
>>
>> gdb/ChangeLog:
>>
>> 2019-09-10 Tom de Vries <tdevries@suse.de>
>>
>> PR symtab/24971
>> * block.c (best_symbol, better_symbol): New function.
>> (block_lookup_symbol_primary): Prefer def over decl.
>>
>> gdb/testsuite/ChangeLog:
>>
>> 2019-09-10 Tom de Vries <tdevries@suse.de>
>>
>> * gdb.dwarf2/varval.exp: Add decl before def test.
>>
>> ---
>> gdb/block.c | 36 ++++++++++++++++++++++++++++++++++--
>> gdb/testsuite/gdb.dwarf2/varval.exp | 18 +++++++++++++++++-
>> 2 files changed, 51 insertions(+), 3 deletions(-)
>>
>> diff --git a/gdb/block.c b/gdb/block.c
>> index ca4dc22cf3..1c0d206a7a 100644
>> --- a/gdb/block.c
>> +++ b/gdb/block.c
>> @@ -725,6 +725,38 @@ block_lookup_symbol (const struct block *block, const char *name,
>> }
>> }
>>
>> +static bool
>> +best_symbol (struct symbol *a, const domain_enum domain)
>> +{
>> + return (SYMBOL_DOMAIN (a) == domain
>> + && SYMBOL_CLASS (a) != LOC_UNRESOLVED);
>> +}
>> +
>> +static struct symbol *
>> +better_symbol (struct symbol *a, struct symbol *b, const domain_enum domain)
>> +{
>> + if (a == NULL)
>> + return b;
>> + if (b == NULL)
>> + return a;
>> +
>> + if (SYMBOL_DOMAIN (a) == domain
>> + && SYMBOL_DOMAIN (b) != domain)
>> + return a;
>> + if (SYMBOL_DOMAIN (b) == domain
>> + && SYMBOL_DOMAIN (a) != domain)
>> + return b;
>> +
>> + if (SYMBOL_CLASS (a) != LOC_UNRESOLVED
>> + && SYMBOL_CLASS (b) == LOC_UNRESOLVED)
>> + return a;
>> + if (SYMBOL_CLASS (b) != LOC_UNRESOLVED
>> + && SYMBOL_CLASS (a) == LOC_UNRESOLVED)
>> + return b;
>> +
>> + return a;
>> +}
>> +
>> /* See block.h. */
>>
>> struct symbol *
>> @@ -746,7 +778,7 @@ block_lookup_symbol_primary (const struct block *block, const char *name,
>> sym != NULL;
>> sym = mdict_iter_match_next (lookup_name, &mdict_iter))
>> {
>> - if (SYMBOL_DOMAIN (sym) == domain)
>> + if (best_symbol (sym, domain))
>> return sym;
>>
>> /* This is a bit of a hack, but symbol_matches_domain might ignore
>> @@ -755,7 +787,7 @@ block_lookup_symbol_primary (const struct block *block, const char *name,
>> exactly the same domain. PR 16253. */
>> if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
>> SYMBOL_DOMAIN (sym), domain))
>> - other = sym;
>> + other = better_symbol (other, sym, domain);
>> }
>>
>> return other;
>> diff --git a/gdb/testsuite/gdb.dwarf2/varval.exp b/gdb/testsuite/gdb.dwarf2/varval.exp
>> index 594591025f..0f3707bd5a 100644
>> --- a/gdb/testsuite/gdb.dwarf2/varval.exp
>> +++ b/gdb/testsuite/gdb.dwarf2/varval.exp
>> @@ -55,7 +55,7 @@ proc setup_exec { arg_bad } {
>> var_b_label var_c_label var_p_label var_bad_label \
>> varval_label var_s_label var_untyped_label \
>> var_a_abstract_label var_a_concrete_label \
>> - varval2_label
>> + varval2_label varval3_def_label varval3_decl_label
>>
>> set int_size [get_sizeof "int" -1]
>>
>> @@ -171,6 +171,20 @@ proc setup_exec { arg_bad } {
>> {DW_AT_location {DW_OP_addr [gdb_target_symbol "var_b"]} SPECIAL_expr}
>> }
>>
>> + varval3_decl_label: DW_TAG_variable {
>> + {DW_AT_name "varval3"}
>> + {DW_AT_type :${int_label}}
>> + {DW_AT_external 1 DW_FORM_flag}
>> + {DW_AT_declaration 1 DW_FORM_flag}
>> + }
>> + varval3_def_label: DW_TAG_variable {
>> + {DW_AT_name "varval3"}
>> + {DW_AT_external 1 DW_FORM_flag}
>> + {DW_AT_type :${int_label}}
>> + {DW_AT_location {DW_OP_addr [gdb_target_symbol "var_a"]} SPECIAL_expr}
>> + }
>> +
>> +
>> DW_TAG_subprogram {
>> {MACRO_AT_func { "main" "${srcdir}/${subdir}/${srcfile}" }}
>> {DW_AT_type :${int_label}}
>> @@ -192,6 +206,7 @@ proc setup_exec { arg_bad } {
>> DW_OP_stack_value
>> } SPECIAL_expr}
>> }
>> +
>> var_a_concrete_label: DW_TAG_variable {
>> {DW_AT_abstract_origin :${var_a_abstract_label}}
>> {DW_AT_location {DW_OP_addr [gdb_target_symbol "var_a"]} SPECIAL_expr}
>> @@ -290,6 +305,7 @@ if { [setup_exec 0] == -1 } {
>>
>> gdb_test "print varval" "= 8"
>> gdb_test "print varval2" "= 8"
>> +gdb_test "print varval3" "= 8"
>> gdb_test "print constval" "= 53"
>> gdb_test "print mixedval" "= 42"
>> gdb_test "print pointerval" "= \\(int \\*\\) $hex <var_b>"
>>