From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 80576 invoked by alias); 8 Oct 2019 06:55:57 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 80567 invoked by uid 89); 8 Oct 2019 06:55:57 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.7 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_STOCKGEN,SPF_PASS autolearn=ham version=3.3.1 spammy=sk:DW_AT_a, sk:dw_at_a, UD:varval.exp, varval.exp X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 08 Oct 2019 06:55:55 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 6B9A3AC18 for ; Tue, 8 Oct 2019 06:55:53 +0000 (UTC) Subject: [PING^2][PATCH][gdb/symtab] Prefer var def over decl From: Tom de Vries To: gdb-patches@sourceware.org References: <20190910154310.GA20844@delia> <9cd61565-d50e-a0d3-78bc-957f18e109ab@suse.de> Openpgp: preference=signencrypt Message-ID: Date: Tue, 08 Oct 2019 06:55:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <9cd61565-d50e-a0d3-78bc-957f18e109ab@suse.de> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2019-10/txt/msg00218.txt.bz2 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>: 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. >> >> [ 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 >> >> 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 >> >> * 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 " >>