From patchwork Fri Nov 1 13:14: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: 35566 Received: (qmail 14198 invoked by alias); 1 Nov 2019 13:14: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 14031 invoked by uid 89); 1 Nov 2019 13:14:20 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-21.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT 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, 01 Nov 2019 13:14:17 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id 336CF203A6; Fri, 1 Nov 2019 09:14:16 -0400 (EDT) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [8.43.85.239]) by mx1.osci.io (Postfix) with ESMTP id 6131320339; Fri, 1 Nov 2019 09:14:07 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id 32D7C218A2; Fri, 1 Nov 2019 09:14:07 -0400 (EDT) X-Gerrit-PatchSet: 6 Date: Fri, 1 Nov 2019 09:14:07 -0400 From: "Sourceware to Gerrit sync (Code Review)" To: Luis Machado , gdb-patches@sourceware.org Cc: Andrew Burgess , Tom Tromey Auto-Submitted: auto-generated X-Gerrit-MessageType: merged Subject: [pushed] [ARM, thumb] Fix disassembling bug after reloading a symbol file X-Gerrit-Change-Id: I22c3e6ebe9bfedad66d56fe9656994fa1761c485 X-Gerrit-Change-Number: 447 X-Gerrit-ChangeURL: X-Gerrit-Commit: bd5766ec689140aa80e5d4fd5c0bf9c990f5a38b In-Reply-To: References: Reply-To: noreply@gnutoolchain-gerrit.osci.io, tromey@sourceware.org, andrew.burgess@embecosm.com, luis.machado@linaro.org, gdb-patches@sourceware.org MIME-Version: 1.0 Content-Disposition: inline User-Agent: Gerrit/3.0.3-75-g9005159e5d Message-Id: <20191101131407.32D7C218A2@gnutoolchain-gerrit.osci.io> Sourceware to Gerrit sync has submitted this change. Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/447 ...................................................................... [ARM, thumb] Fix disassembling bug after reloading a symbol file The speed optimization from commit 5f6cac4085c95c5339b9549dc06d4f9184184fa6 made GDB skip reloading all symbols when the same symbol file is reloaded. As a result, ARM targets only read the mapping symbols the first time we load a symbol file. When reloaded, the speed optimization above will cause an early return and gdbarch_record_special_symbol won't be called to save mapping symbol data, which in turn affects disassembling of thumb instructions. First load and correct disassemble output: Dump of assembler code for function main: 0x0000821c <+0>: bx pc 0x0000821e <+2>: nop 0x00008220 <+4>: mov r0, #0 0x00008224 <+8>: bx lr Second load and incorrect disassemble output: Dump of assembler code for function main: 0x0000821c <+0>: bx pc 0x0000821e <+2>: nop 0x00008220 <+4>: movs r0, r0 0x00008222 <+6>: b.n 0x8966 0x00008224 <+8>: vrhadd.u16 d14, d14, d31 This happens because the mapping symbol data is stored in an objfile_key-based container, and that data isn't preserved across the two symbol loading operations. The following patch fixes this by storing the mapping symbol data in a bfd_key-based container, which doesn't change as long as the bfd is the same. I've also added a new test to verify the correct disassemble output. gdb/ChangeLog: 2019-11-01 Luis Machado PR gdb/25124 * arm-tdep.c (arm_per_objfile): Rename to ... (arm_per_bfd): ... this. (arm_objfile_data_key): Rename to ... (arm_bfd_data_key): ... this. (arm_find_mapping_symbol): Adjust access to new bfd_key-based data. (arm_record_special_symbol): Likewise. gdb/testsuite/ChangeLog: 2019-11-01 Luis Machado PR gdb/25124 * gdb.arch/pr25124.S: New file. * gdb.arch/pr25124.exp: New file. Change-Id: I22c3e6ebe9bfedad66d56fe9656994fa1761c485 --- M gdb/ChangeLog M gdb/arm-tdep.c M gdb/testsuite/ChangeLog A gdb/testsuite/gdb.arch/pr25124.S A gdb/testsuite/gdb.arch/pr25124.exp 5 files changed, 113 insertions(+), 10 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 381147b..e2bdd5f 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,15 @@ +2019-11-01 Luis Machado + + PR gdb/25124 + + * arm-tdep.c (arm_per_objfile): Rename to ... + (arm_per_bfd): ... this. + (arm_objfile_data_key): Rename to ... + (arm_bfd_data_key): ... this. + (arm_find_mapping_symbol): Adjust access to new bfd_key-based + data. + (arm_record_special_symbol): Likewise. + 2019-10-31 Andrew Burgess * ada-typeprint.c (ada_print_typedef): Don't print newline at the diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index 48772d7..3cf3abb 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -89,14 +89,14 @@ typedef std::vector arm_mapping_symbol_vec; -struct arm_per_objfile +struct arm_per_bfd { - explicit arm_per_objfile (size_t num_sections) + explicit arm_per_bfd (size_t num_sections) : section_maps (new arm_mapping_symbol_vec[num_sections]), section_maps_sorted (new bool[num_sections] ()) {} - DISABLE_COPY_AND_ASSIGN (arm_per_objfile); + DISABLE_COPY_AND_ASSIGN (arm_per_bfd); /* Information about mapping symbols ($a, $d, $t) in the objfile. @@ -113,8 +113,8 @@ std::unique_ptr section_maps_sorted; }; -/* Per-objfile data used for mapping symbols. */ -static objfile_key arm_objfile_data_key; +/* Per-bfd data used for mapping symbols. */ +static bfd_key arm_bfd_data_key; /* The list of available "set arm ..." and "show arm ..." commands. */ static struct cmd_list_element *setarmcmdlist = NULL; @@ -350,7 +350,7 @@ sec = find_pc_section (memaddr); if (sec != NULL) { - arm_per_objfile *data = arm_objfile_data_key.get (sec->objfile); + arm_per_bfd *data = arm_bfd_data_key.get (sec->objfile->obfd); if (data != NULL) { unsigned int section_idx = sec->the_bfd_section->index; @@ -8561,17 +8561,17 @@ asymbol *sym) { const char *name = bfd_asymbol_name (sym); - struct arm_per_objfile *data; + struct arm_per_bfd *data; struct arm_mapping_symbol new_map_sym; gdb_assert (name[0] == '$'); if (name[1] != 'a' && name[1] != 't' && name[1] != 'd') return; - data = arm_objfile_data_key.get (objfile); + data = arm_bfd_data_key.get (objfile->obfd); if (data == NULL) - data = arm_objfile_data_key.emplace (objfile, - objfile->obfd->section_count); + data = arm_bfd_data_key.emplace (objfile->obfd, + objfile->obfd->section_count); arm_mapping_symbol_vec &map = data->section_maps[bfd_asymbol_section (sym)->index]; diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 4d1eecd..f6464e9 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2019-11-01 Luis Machado + + PR gdb/25124 + + * gdb.arch/pr25124.S: New file. + * gdb.arch/pr25124.exp: New file. + 2019-10-31 Andrew Burgess * gdb.fortran/info-modules.exp: Update expected results, and add diff --git a/gdb/testsuite/gdb.arch/pr25124.S b/gdb/testsuite/gdb.arch/pr25124.S new file mode 100644 index 0000000..79f82c7 --- /dev/null +++ b/gdb/testsuite/gdb.arch/pr25124.S @@ -0,0 +1,35 @@ +/* Test proper disassembling of ARM thumb instructions when reloading a symbol + file. + + Copyright 2012-2019 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + + .syntax unified + .thumb + .text + .p2align 2 + .global main + .thumb + .thumb_func + .type main, %function +main: + bx pc + nop +.code 32 + mov r0, #0 + bx lr + .size main, .-main diff --git a/gdb/testsuite/gdb.arch/pr25124.exp b/gdb/testsuite/gdb.arch/pr25124.exp new file mode 100644 index 0000000..656079c --- /dev/null +++ b/gdb/testsuite/gdb.arch/pr25124.exp @@ -0,0 +1,49 @@ +# Copyright 2019 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Test proper disassembling of ARM thumb instructions when reloading a symbol +# file. + +if {![is_aarch32_target]} then { + verbose "Skipping ARM tests." + return +} + +standard_testfile .S + +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable debug] != "" } { + untested "failed to compile" + return -1 +} + +gdb_exit +gdb_start +gdb_reinitialize_dir $srcdir/$subdir + +# Load the symbol file the first time. +gdb_load ${binfile} + +# Check if the disassemble ouput is correct. +gdb_test "x /i main+8" \ + "$hex :\[ \t\]+bx\[ \t\]+lr" \ + "disassemble thumb instruction (1st try)" + +# Reload the symbol file to trigger the bug. +gdb_load ${binfile} + +# Check if the disassemble output is the same as above. +gdb_test "x /i main+8" \ + "$hex :\[ \t\]+bx\[ \t\]+lr" \ + "disassemble thumb instruction (2nd try)"