[review] [ARM, Thumb] Fix disassembling bug after reloading a symbol file

Luis Machado (Code Review) gerrit@gnutoolchain-gerrit.osci.io
Wed Oct 30 14:11:00 GMT 2019


Luis Machado has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/447
......................................................................


Patch Set 1:

(3 comments)

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/447/1/gdb/arm-tdep.c 
File gdb/arm-tdep.c:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/447/1/gdb/arm-tdep.c@101 
PS1, Line 101: 
 92 | struct arm_per_bfd
 93 | {
 94 |   explicit arm_per_bfd (size_t num_sections)
 95 |   : section_maps (new arm_mapping_symbol_vec[num_sections]),
 96 |     section_maps_sorted (new bool[num_sections] ())
 97 |   {}
 98 | 
 99 |   DISABLE_COPY_AND_ASSIGN (arm_per_bfd);
100 | 
101 |   /* Information about mapping symbols ($a, $d, $t) in the objfile.

> Should this mention of objfile be changed to "bfd" or "bfd object"?

I thought about it, but the symbols themselves are contained in the objfile itself, though we're storing additional information inferred from said symbols in the bfd container.


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/447/1/gdb/testsuite/gdb.arch/pr25124.S 
File gdb/testsuite/gdb.arch/pr25124.S:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/447/1/gdb/testsuite/gdb.arch/pr25124.S@4 
PS1, Line 4: 
1 | /* Test proper disassembling of ARM thumb instructions when reloading a symbol
2 |    file.
3 | 
4 |    Copyright 2012-2019 Free Software Foundation, Inc.

> This year should be 2019 only unless (1) it's a copy of another file that was added in 2012, or (2)  […]

This is in fact a copy of gdb.arch/thumb-bx-pc.S, hence why i left it that way.


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/447/1/gdb/testsuite/gdb.arch/pr25124.exp 
File gdb/testsuite/gdb.arch/pr25124.exp:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/447/1/gdb/testsuite/gdb.arch/pr25124.exp@33 
PS1, Line 33: 
24 | standard_testfile .S
25 | 
26 | if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable debug] != "" } {
27 |     untested "failed to compile"
28 |     return -1
29 | }
30 | 
31 | gdb_test "x /i main+8"
32 | 	"$hex <main\\+8>:\[ \t\]+bx\[ \t\]+lr"
33 | 	"disassemble thumb instruction (1st try)"

> Maybe I'm going crazy, but doesn't like 31 and 32 need to end in a trailing backslash?  The next cal […]

They do. I borked it just before submission, having noticed the overly long line and breaking it into multiple lines. I'll fix it.

As for running it effectively, i haven't due to the lack of access to a box running arm 32-bit. It doesn't seem we have buildbots running those either.



-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I22c3e6ebe9bfedad66d56fe9656994fa1761c485
Gerrit-Change-Number: 447
Gerrit-PatchSet: 1
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Comment-Date: Wed, 30 Oct 2019 14:11:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-MessageType: comment



More information about the Gdb-patches mailing list