From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 123678 invoked by alias); 1 Nov 2019 13:13:15 -0000 Mailing-List: contact gdb-cvs-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: , Sender: gdb-cvs-owner@sourceware.org List-Subscribe: Sender: gdb-cvs-owner@sourceware.org Received: (qmail 123533 invoked by uid 9722); 1 Nov 2019 13:13:14 -0000 Date: Fri, 01 Nov 2019 13:13:00 -0000 Message-ID: <20191101131314.123532.qmail@sourceware.org> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Luis Machado To: gdb-cvs@sourceware.org Subject: [binutils-gdb] [ARM, thumb] Fix disassembling bug after reloading a symbol file X-Act-Checkin: binutils-gdb X-Git-Author: Luis Machado X-Git-Refname: refs/heads/master X-Git-Oldrev: d0d6922c6855c7473d0d2dda45674443ba503cb5 X-Git-Newrev: bd5766ec689140aa80e5d4fd5c0bf9c990f5a38b X-SW-Source: 2019-11/txt/msg00000.txt.bz2 https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=bd5766ec689140aa80e5d4fd5c0bf9c990f5a38b commit bd5766ec689140aa80e5d4fd5c0bf9c990f5a38b Author: Luis Machado Date: Fri Nov 1 10:11:17 2019 -0300 [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 Diff: --- gdb/ChangeLog | 12 ++++++++++ gdb/arm-tdep.c | 20 ++++++++-------- gdb/testsuite/ChangeLog | 7 ++++++ gdb/testsuite/gdb.arch/pr25124.S | 35 +++++++++++++++++++++++++++ gdb/testsuite/gdb.arch/pr25124.exp | 49 ++++++++++++++++++++++++++++++++++++++ 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 @@ struct arm_mapping_symbol 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 @@ struct arm_per_objfile 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 @@ arm_find_mapping_symbol (CORE_ADDR memaddr, CORE_ADDR *start) 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 @@ arm_record_special_symbol (struct gdbarch *gdbarch, struct objfile *objfile, 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)"