From 93e55f0a031b0e677d22aaba00857de902ebe685 Mon Sep 17 00:00:00 2001 From: Tom de Vries Date: Fri, 6 Dec 2019 18:51:49 +0100 Subject: [PATCH] [gdb/symtab] Prefer var def over decl 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>: Abbrev Number: 2 (DW_TAG_array_type) DW_AT_type : <0xff> DW_AT_sibling : <0xff> <2>: Abbrev Number: 3 (DW_TAG_subrange_type) <2>: Abbrev Number: 0 <1>: 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 ... [ 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. gdb/ChangeLog: 2019-12-06 Tom de Vries PR symtab/24971 * block.c (best_symbol, better_symbol): New function. (block_lookup_symbol_primary, block_lookup_symbol): Prefer def over decl. gdb/testsuite/ChangeLog: 2019-12-06 Tom de Vries * gdb.dwarf2/varval.exp: Add decl before def test. Change-Id: Id92326cb8ef9903b121ef9e320658eb565d0f5a9 --- gdb/ChangeLog | 7 +++ gdb/block.c | 74 +++++++++++++++++++++++++++-- gdb/testsuite/ChangeLog | 4 ++ gdb/testsuite/gdb.dwarf2/varval.exp | 48 ++++++++++++++++--- 4 files changed, 123 insertions(+), 10 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 2d6071271ac..55ba8443cdf 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2019-12-06 Tom de Vries + + PR symtab/24971 + * block.c (best_symbol, better_symbol): New function. + (block_lookup_symbol_primary, block_lookup_symbol): Prefer def over + decl. + 2019-12-06 Tankut Baris Aktemur * gdbtypes.h: Define the REFERENCE_SEE_THROUGH_BADNESS value. diff --git a/gdb/block.c b/gdb/block.c index a4592e36e65..b49c548adc8 100644 --- a/gdb/block.c +++ b/gdb/block.c @@ -657,6 +657,43 @@ block_iter_match_next (const lookup_name_info &name, return block_iter_match_step (iterator, name, 0); } +/* Return true if symbol A is the best match possible for DOMAIN. */ + +static bool +best_symbol (struct symbol *a, const domain_enum domain) +{ + return (SYMBOL_DOMAIN (a) == domain + && SYMBOL_CLASS (a) != LOC_UNRESOLVED); +} + +/* Return symbol B if it is a better match than symbol A for DOMAIN. + Otherwise return A. */ + +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. Note that if NAME is the demangled form of a C++ symbol, we will fail @@ -684,7 +721,9 @@ block_lookup_symbol (const struct block *block, const char *name, ALL_BLOCK_SYMBOLS_WITH_NAME (block, lookup_name, iter, sym) { - if (SYMBOL_DOMAIN (sym) == domain) + /* See comment related to PR gcc/debug/91507 in + block_lookup_symbol_primary. */ + if (best_symbol (sym, domain)) return sym; /* This is a bit of a hack, but symbol_matches_domain might ignore STRUCT vs VAR domain symbols. So if a matching symbol is found, @@ -692,7 +731,7 @@ block_lookup_symbol (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; } @@ -746,7 +785,34 @@ 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) + /* With the fix for PR gcc/debug/91507, we get for: + ... + extern char *zzz[]; + char *zzz[ ] = { + "abc", + "cde" + }; + ... + DWARF which will result in two entries in the symbol table, a decl + with type char *[] and a def with type char *[2]. + + If we return the decl here, we don't get the value of zzz: + ... + $ gdb a.spec.out -batch -ex "p zzz" + $1 = 0x601030 + ... + because we're returning the symbol without location information, and + because the fallback that uses the address from the minimal symbols + doesn't work either because the type of the decl does not specify a + size. + + To fix this, we prefer def over decl in best_symbol and + better_symbol. + + In absence of the gcc fix, both def and decl have type char *[], so + the only option to make this work is improve the fallback to use the + size of the minimal symbol. Filed as PR exp/24989. */ + if (best_symbol (sym, domain)) return sym; /* This is a bit of a hack, but symbol_matches_domain might ignore @@ -755,7 +821,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/ChangeLog b/gdb/testsuite/ChangeLog index f7447dcb2b9..c9f66ad40aa 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2019-12-06 Tom de Vries + + * gdb.dwarf2/varval.exp: Add decl before def test. + 2019-12-06 Tankut Baris Aktemur * gdb.cp/rvalue-ref-overload.exp: Minor cleanup. diff --git a/gdb/testsuite/gdb.dwarf2/varval.exp b/gdb/testsuite/gdb.dwarf2/varval.exp index 594591025f5..8f5e16fd361 100644 --- a/gdb/testsuite/gdb.dwarf2/varval.exp +++ b/gdb/testsuite/gdb.dwarf2/varval.exp @@ -55,7 +55,8 @@ 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 \ + int_array_label int_array_of_1_label set int_size [get_sizeof "int" -1] @@ -171,6 +172,32 @@ proc setup_exec { arg_bad } { {DW_AT_location {DW_OP_addr [gdb_target_symbol "var_b"]} SPECIAL_expr} } + int_array_label: DW_TAG_array_type { + {DW_AT_type :${int_label}} + } { + DW_TAG_subrange_type {} + } + varval3_decl_label: DW_TAG_variable { + {DW_AT_name "varval3"} + {DW_AT_type :${int_array_label}} + {DW_AT_external 1 DW_FORM_flag} + {DW_AT_declaration 1 DW_FORM_flag} + } + int_array_of_1_label: DW_TAG_array_type { + {DW_AT_type :${int_label}} + } { + DW_TAG_subrange_type { + {DW_AT_type :$int_label} + {DW_AT_upper_bound 0 DW_FORM_data1} + } + } + varval3_def_label: DW_TAG_variable { + {DW_AT_name "varval3"} + {DW_AT_external 1 DW_FORM_flag} + {DW_AT_type :${int_array_of_1_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}} @@ -277,19 +304,24 @@ proc setup_exec { arg_bad } { if [prepare_for_testing "failed to prepare" ${executable} [list ${asm_file} ${srcfile}] {}] { return -1 } - - # DW_OP_GNU_variable_value implementation requires a valid frame. - if ![runto_main] { - return -1 - } } if { [setup_exec 0] == -1 } { return -1 } +with_test_prefix "pre-main" { + gdb_test "print varval3" "= \\{8\\}" "" +} + +# DW_OP_GNU_variable_value implementation requires a valid frame. +if ![runto_main] { + return -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 " @@ -311,6 +343,10 @@ if { [setup_exec 1] == -1 } { return -1 } +# DW_OP_GNU_variable_value implementation requires a valid frame. +if ![runto_main] { + return -1 +} gdb_test "print badval" "value has been optimized out" gdb_test "print bad_die_val1" \ "invalid dwarf2 offset 0xabcdef11"