From: "Andrew Burgess (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>
To: Luis Machado <luis.machado@linaro.org>, gdb-patches@sourceware.org
Subject: [review] [ARM, Thumb] Fix disassembling bug after reloading a symbol file
Date: Wed, 30 Oct 2019 14:01:00 -0000 [thread overview]
Message-ID: <20191030140042.3F7AD20AF6@gnutoolchain-gerrit.osci.io> (raw)
In-Reply-To: <gerrit.1572442722000.I22c3e6ebe9bfedad66d56fe9656994fa1761c485@gnutoolchain-gerrit.osci.io>
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
next prev parent reply other threads:[~2019-10-30 14:01 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-30 13:38 Luis Machado (Code Review)
2019-10-30 14:01 ` Andrew Burgess (Code Review) [this message]
2019-10-30 14:11 ` Luis Machado (Code Review)
2019-10-30 14:22 ` [review v2] " Luis Machado (Code Review)
2019-10-30 15:15 ` Tom Tromey (Code Review)
2019-10-30 15:22 ` Luis Machado (Code Review)
2019-10-30 15:59 ` [review v3] " Luis Machado (Code Review)
2019-10-31 16:44 ` [review v5] " Luis Machado (Code Review)
2019-10-31 16:45 ` Luis Machado (Code Review)
2019-11-01 1:09 ` Andrew Burgess (Code Review)
2019-11-01 13:14 ` [pushed] [ARM, thumb] " Sourceware to Gerrit sync (Code Review)
2019-11-01 13:14 ` Sourceware to Gerrit sync (Code Review)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191030140042.3F7AD20AF6@gnutoolchain-gerrit.osci.io \
--to=gerrit@gnutoolchain-gerrit.osci.io \
--cc=gdb-patches@sourceware.org \
--cc=gnutoolchain-gerrit@osci.io \
--cc=luis.machado@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).