From: Joel Brobecker <brobecker@adacore.com>
To: Martin Simmons <martin@lispworks.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH] Prevent internal-error when computing $pc in ARM assembly code
Date: Tue, 09 Jun 2015 18:44:00 -0000 [thread overview]
Message-ID: <20150609184408.GG2855@adacore.com> (raw)
In-Reply-To: <201505200949.t4K9nlqt015737@heapu.cam.lispworks.com>
On Wed, May 20, 2015 at 10:49:47AM +0100, Martin Simmons wrote:
> This patch prevents a gdb internal-error when computing $pc for ARM assembly
> code that doesn't use the normal stack frame convention.
>
> It might also help with gdb/12223.
>
> I'm not subscribed to the list so please include me on any replies.
>
>
> gdb/ChangeLog:
>
> * arm-tdep.c (arm_analyze_prologue): Read memory without throwing an
> exception, to allow debugging of assembly code.
>
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 8181f25..d47b4af 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -1669,8 +1669,11 @@ arm_analyze_prologue (struct gdbarch *gdbarch,
> current_pc < prologue_end;
> current_pc += 4)
> {
> - unsigned int insn
> - = read_memory_unsigned_integer (current_pc, 4, byte_order_for_code);
> + unsigned int insn;
> + gdb_byte buf[4];
> + if (target_read_memory (current_pc, buf, 4))
> + break;
> + insn = extract_unsigned_integer (buf, 4, byte_order_for_code);
It would be good to have a few more details about what causes us
to be in a situation where we'd be failing to read memory at
an address. Perhaps just showing the disassembled code and
a quick explanation of what happens might be sufficient.
Also, for our piece of mind, we normally ask that the change be
validated by running the testsuite. Did you do that? If yes,
on which platform?
Lastly - a small nitpick, due to GDB's Coding style, which requires
that an empty line be inserted between local variable declarations
and the first statement after that. So, would you mind adding an empty
line after "buf"'s declaration?
Other than that, I think the patch looks reasonable. I'm just a little
unsure about how you can be trying to read an instruction at an address
that's unreadable. Looking at the code, and in particular, looking at
the callers, I think I might have some idea, but I'd prefer if you
showed us the issue that you actually hit.
Thank you,
--
Joel
next prev parent reply other threads:[~2015-06-09 18:44 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-20 9:49 Martin Simmons
2015-06-09 18:44 ` Joel Brobecker [this message]
2015-06-11 17:25 ` Martin Simmons
2015-06-12 13:21 ` Joel Brobecker
2015-06-12 15:09 ` Martin Simmons
2015-06-12 17:58 ` Joel Brobecker
2015-06-15 10:02 ` Yao Qi
2015-06-15 13:36 ` Joel Brobecker
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=20150609184408.GG2855@adacore.com \
--to=brobecker@adacore.com \
--cc=gdb-patches@sourceware.org \
--cc=martin@lispworks.com \
/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