From bd5766ec689140aa80e5d4fd5c0bf9c990f5a38b Mon Sep 17 00:00:00 2001 From: Luis Machado Date: Fri, 1 Nov 2019 10:11:17 -0300 Subject: [PATCH] [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 --- 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(-) create mode 100644 gdb/testsuite/gdb.arch/pr25124.S create mode 100644 gdb/testsuite/gdb.arch/pr25124.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 381147b3217..e2bdd5f3e9c 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 48772d7b380..3cf3abb9fa6 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 4d1eecdb5c0..f6464e9246c 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 00000000000..79f82c7522f --- /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 00000000000..656079c4009 --- /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)" -- 2.30.2