This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Andrew Burgess has posted comments on this change.

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


Patch Set 1:

(3 comments)

Basically fine, few minor issues I think need addressing before it can be merged.

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"?


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 patch was originally submitted in 2012.


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 call to gdb_test does, and then the third below again, no backslash.  I don't have an ARM target to hand, but does this test run correctly like this?



-- 
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-CC: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Comment-Date: Wed, 30 Oct 2019 14:00:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]