From patchwork Mon Sep 4 18:47:37 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 22612 Received: (qmail 33373 invoked by alias); 4 Sep 2017 18:47:47 -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 33198 invoked by uid 89); 4 Sep 2017 18:47:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_STOCKGEN, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 04 Sep 2017 18:47:41 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B427067757 for ; Mon, 4 Sep 2017 18:47:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com B427067757 Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves@redhat.com Received: from cascais.lan (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 19AB46047B for ; Mon, 4 Sep 2017 18:47:39 +0000 (UTC) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH 1/2] Fix "list ambiguous_variable" Date: Mon, 4 Sep 2017 19:47:37 +0100 Message-Id: <1504550858-27936-2-git-send-email-palves@redhat.com> In-Reply-To: <1504550858-27936-1-git-send-email-palves@redhat.com> References: <1504550858-27936-1-git-send-email-palves@redhat.com> The "list" command allows specifying the name of variables as argument, not just functions, so that users can type "list a_global_variable". That support is a broken when it comes to ambiguous locations though. If there's more than one such global variable in the program, e.g., static globals in different compilation units, GDB ends up listing the source of the first variable it finds, only. linespec.c does find both symbol and minsym locations for all the globals. The problem is that it ends up merging all the resulting sals into one, because they all have address, zero. I.e., all sals end up with sal.pc == 0, so maybe_add_address returns false for all but the first. The zero addresses appear because: - in the minsyms case, linespec.c:minsym_found incorrectly treats all minsyms as if they were function/text symbols. In list mode we can end up with data symbols there, and we shouldn't be using find_pc_sect_line on data symbols. - in the debug symbols case, symbol_to_sal misses recording an address (sal.pc) for non-block, non-label symbols. gdb/ChangeLog: yyyy-mm-dd Pedro Alves * linespec.c (minsym_found): Handle non-text minsyms. (symbol_to_sal): Record a sal.pc for non-block, non-label symbols. gdb/testsuite/ChangeLog: yyyy-mm-dd Pedro Alves * gdb.base/list-ambiguous.exp (test_list_ambiguous_function): Rename to ... (test_list_ambiguous_symbol): ... this and add a symbol name parameter. Adjust. (test_list_ambiguous_function): Reimplement on top of test_list_ambiguous_symbol and also test listing ambiguous variables. * gdb.base/list-ambiguous0.c (ambiguous): Rename to ... (ambiguous_fun): ... this. (ambiguous_var): New. * gdb.base/list-ambiguous1.c (ambiguous): Rename to ... (ambiguous_fun): ... this. (ambiguous_var): New. --- gdb/linespec.c | 61 ++++++++++++++++++------------- gdb/testsuite/gdb.base/list-ambiguous.exp | 37 +++++++++++-------- gdb/testsuite/gdb.base/list-ambiguous0.c | 5 ++- gdb/testsuite/gdb.base/list-ambiguous1.c | 5 ++- 4 files changed, 63 insertions(+), 45 deletions(-) diff --git a/gdb/linespec.c b/gdb/linespec.c index 4801808..d72d19d 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -4318,35 +4318,45 @@ minsym_found (struct linespec_state *self, struct objfile *objfile, CORE_ADDR pc; struct symtab_and_line sal; - sal = find_pc_sect_line (MSYMBOL_VALUE_ADDRESS (objfile, msymbol), - (struct obj_section *) 0, 0); - sal.section = MSYMBOL_OBJ_SECTION (objfile, msymbol); + if (msymbol_is_text (msymbol)) + { + sal = find_pc_sect_line (MSYMBOL_VALUE_ADDRESS (objfile, msymbol), + (struct obj_section *) 0, 0); + sal.section = MSYMBOL_OBJ_SECTION (objfile, msymbol); - /* The minimal symbol might point to a function descriptor; - resolve it to the actual code address instead. */ - pc = gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc, ¤t_target); - if (pc != sal.pc) - sal = find_pc_sect_line (pc, NULL, 0); + /* The minimal symbol might point to a function descriptor; + resolve it to the actual code address instead. */ + pc = gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc, ¤t_target); + if (pc != sal.pc) + sal = find_pc_sect_line (pc, NULL, 0); - if (self->funfirstline) - { - if (sal.symtab != NULL - && (COMPUNIT_LOCATIONS_VALID (SYMTAB_COMPUNIT (sal.symtab)) - || SYMTAB_LANGUAGE (sal.symtab) == language_asm)) + if (self->funfirstline) { - /* If gdbarch_convert_from_func_ptr_addr does not apply then - sal.SECTION, sal.LINE&co. will stay correct from above. - If gdbarch_convert_from_func_ptr_addr applies then - sal.SECTION is cleared from above and sal.LINE&co. will - stay correct from the last find_pc_sect_line above. */ - sal.pc = MSYMBOL_VALUE_ADDRESS (objfile, msymbol); - sal.pc = gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc, - ¤t_target); - if (gdbarch_skip_entrypoint_p (gdbarch)) - sal.pc = gdbarch_skip_entrypoint (gdbarch, sal.pc); + if (sal.symtab != NULL + && (COMPUNIT_LOCATIONS_VALID (SYMTAB_COMPUNIT (sal.symtab)) + || SYMTAB_LANGUAGE (sal.symtab) == language_asm)) + { + /* If gdbarch_convert_from_func_ptr_addr does not apply then + sal.SECTION, sal.LINE&co. will stay correct from above. + If gdbarch_convert_from_func_ptr_addr applies then + sal.SECTION is cleared from above and sal.LINE&co. will + stay correct from the last find_pc_sect_line above. */ + sal.pc = MSYMBOL_VALUE_ADDRESS (objfile, msymbol); + sal.pc = gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc, + ¤t_target); + if (gdbarch_skip_entrypoint_p (gdbarch)) + sal.pc = gdbarch_skip_entrypoint (gdbarch, sal.pc); + } + else + skip_prologue_sal (&sal); } - else - skip_prologue_sal (&sal); + } + else + { + sal.objfile = objfile; + sal.pc = MSYMBOL_VALUE_ADDRESS (objfile, msymbol); + sal.pspace = current_program_space; + sal.section = MSYMBOL_OBJ_SECTION (objfile, msymbol); } if (maybe_add_address (self->addr_set, objfile->pspace, sal.pc)) @@ -4633,6 +4643,7 @@ symbol_to_sal (struct symtab_and_line *result, *result = {}; result->symtab = symbol_symtab (sym); result->line = SYMBOL_LINE (sym); + result->pc = SYMBOL_VALUE_ADDRESS (sym); result->pspace = SYMTAB_PSPACE (result->symtab); return 1; } diff --git a/gdb/testsuite/gdb.base/list-ambiguous.exp b/gdb/testsuite/gdb.base/list-ambiguous.exp index db9d028..dd473ca 100644 --- a/gdb/testsuite/gdb.base/list-ambiguous.exp +++ b/gdb/testsuite/gdb.base/list-ambiguous.exp @@ -39,36 +39,41 @@ proc line_range_pattern { range_start range_end } { # Test the "list" command with linespecs that expand to multiple # locations. -proc test_list_ambiguous_function {} { +proc test_list_ambiguous_symbol {symbol_line symbol} { global srcfile srcfile2 - set lineno0 [gdb_get_line_number "ambiguous (void)" $srcfile] - set lineno1 [gdb_get_line_number "ambiguous (void)" $srcfile2] + set lineno0 [gdb_get_line_number $symbol_line $srcfile] + set lineno1 [gdb_get_line_number $symbol_line $srcfile2] set lines0_re [line_range_pattern [expr $lineno0 - 5] [expr $lineno0 + 4]] set lines1_re [line_range_pattern [expr $lineno1 - 5] [expr $lineno1 + 4]] set any "\[^\r\n\]*" set h0_re "file: \"${any}list-ambiguous0.c\", line number: $lineno0" set h1_re "file: \"${any}list-ambiguous1.c\", line number: $lineno1" - gdb_test "list ambiguous" "${h0_re}${lines0_re}\r\n${h1_re}${lines1_re}" - - gdb_test "list main,ambiguous" \ - "Specified last line 'ambiguous' is ambiguous:\r\n${h0_re}\r\n${h1_re}" - gdb_test "list ,ambiguous" \ - "Specified last line 'ambiguous' is ambiguous:\r\n${h0_re}\r\n${h1_re}" - gdb_test "list ambiguous,main" \ - "Specified first line 'ambiguous' is ambiguous:\r\n${h0_re}\r\n${h1_re}" - gdb_test "list ambiguous,ambiguous" \ - "Specified first line 'ambiguous' is ambiguous:\r\n${h0_re}\r\n${h1_re}" - gdb_test "list ambiguous," \ - "Specified first line 'ambiguous' is ambiguous:\r\n${h0_re}\r\n${h1_re}" + gdb_test "list $symbol" "${h0_re}${lines0_re}\r\n${h1_re}${lines1_re}" + + gdb_test "list main,$symbol" \ + "Specified last line '$symbol' is ambiguous:\r\n${h0_re}\r\n${h1_re}" + gdb_test "list ,$symbol" \ + "Specified last line '$symbol' is ambiguous:\r\n${h0_re}\r\n${h1_re}" + gdb_test "list $symbol,main" \ + "Specified first line '$symbol' is ambiguous:\r\n${h0_re}\r\n${h1_re}" + gdb_test "list $symbol,$symbol" \ + "Specified first line '$symbol' is ambiguous:\r\n${h0_re}\r\n${h1_re}" + gdb_test "list $symbol," \ + "Specified first line '$symbol' is ambiguous:\r\n${h0_re}\r\n${h1_re}" # While at it, test the "edit" command as well, since it shares # code with "list". - gdb_test "edit ambiguous" \ + gdb_test "edit $symbol" \ "Specified line is ambiguous:\r\n${h0_re}\r\n${h1_re}" } +proc test_list_ambiguous_function {} { + test_list_ambiguous_symbol "ambiguous_fun (void)" "ambiguous_fun" + test_list_ambiguous_symbol "ambiguous_var" "ambiguous_var" +} + gdb_test_no_output "set listsize 10" test_list_ambiguous_function diff --git a/gdb/testsuite/gdb.base/list-ambiguous0.c b/gdb/testsuite/gdb.base/list-ambiguous0.c index 91f7fe9..afd9457 100644 --- a/gdb/testsuite/gdb.base/list-ambiguous0.c +++ b/gdb/testsuite/gdb.base/list-ambiguous0.c @@ -19,12 +19,13 @@ -/* This function is defined in both +/* These symbols are defined in both list-ambiguous0.c/list-ambiguous1.c files, in order to test "list"'s behavior with ambiguous linespecs. */ -static void __attribute__ ((used)) ambiguous (void) {} +static void __attribute__ ((used)) ambiguous_fun (void) {} +static int ambiguous_var; diff --git a/gdb/testsuite/gdb.base/list-ambiguous1.c b/gdb/testsuite/gdb.base/list-ambiguous1.c index 024299a..69db11e 100644 --- a/gdb/testsuite/gdb.base/list-ambiguous1.c +++ b/gdb/testsuite/gdb.base/list-ambiguous1.c @@ -23,11 +23,12 @@ -/* This function is defined in both +/* These symbols are defined in both list-ambiguous0.c/list-ambiguous1.c files, in order to test "list"'s behavior with ambiguous linespecs. */ -static void __attribute__ ((used)) ambiguous (void) {} +static void __attribute__ ((used)) ambiguous_fun (void) {} +static int ambiguous_var;