From patchwork Fri Dec 6 17:53:07 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Simon Marchi (Code Review)" X-Patchwork-Id: 36573 Received: (qmail 121715 invoked by alias); 6 Dec 2019 17:53:21 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 121607 invoked by uid 89); 6 Dec 2019 17:53:20 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_STOCKGEN autolearn=ham version=3.3.1 spammy= X-HELO: mx1.osci.io Received: from polly.osci.io (HELO mx1.osci.io) (8.43.85.229) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 06 Dec 2019 17:53:18 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id 890E120362; Fri, 6 Dec 2019 12:53:15 -0500 (EST) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [IPv6:2620:52:3:1:5054:ff:fe06:16ca]) by mx1.osci.io (Postfix) with ESMTP id A17382030C; Fri, 6 Dec 2019 12:53:07 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id 7E75428174; Fri, 6 Dec 2019 12:53:07 -0500 (EST) X-Gerrit-PatchSet: 5 Date: Fri, 6 Dec 2019 12:53:07 -0500 From: "Sourceware to Gerrit sync (Code Review)" To: Tom de Vries , gdb-patches@sourceware.org Cc: Tom Tromey , Joel Brobecker , Luis Machado , Andrew Burgess Auto-Submitted: auto-generated X-Gerrit-MessageType: merged Subject: [pushed] [gdb/symtab] Prefer var def over decl X-Gerrit-Change-Id: Id92326cb8ef9903b121ef9e320658eb565d0f5a9 X-Gerrit-Change-Number: 29 X-Gerrit-ChangeURL: X-Gerrit-Commit: 93e55f0a031b0e677d22aaba00857de902ebe685 In-Reply-To: References: Reply-To: noreply@gnutoolchain-gerrit.osci.io, tdevries@suse.de, tromey@sourceware.org, andrew.burgess@embecosm.com, luis.machado@linaro.org, brobecker@adacore.com, gdb-patches@sourceware.org MIME-Version: 1.0 Content-Disposition: inline User-Agent: Gerrit/3.0.3-79-g83ff7f88f1 Message-Id: <20191206175307.7E75428174@gnutoolchain-gerrit.osci.io> Sourceware to Gerrit sync has submitted this change. Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/29 ...................................................................... [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 --- M gdb/ChangeLog M gdb/block.c M gdb/testsuite/ChangeLog M gdb/testsuite/gdb.dwarf2/varval.exp 4 files changed, 123 insertions(+), 10 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 2d60712..55ba844 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 a4592e3..b49c548 100644 --- a/gdb/block.c +++ b/gdb/block.c @@ -657,6 +657,43 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 f7447dc..c9f66ad 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 5945910..8f5e16f 100644 --- a/gdb/testsuite/gdb.dwarf2/varval.exp +++ b/gdb/testsuite/gdb.dwarf2/varval.exp @@ -55,7 +55,8 @@ 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 @@ {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 @@ 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 @@ 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"