[gdb/symtab] Prefer var def over decl
Commit Message
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?
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(-)
Comments
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.
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>"
>
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>"
>>
@@ -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;
@@ -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>"