Prevent internal-error when computing $pc in ARM assembly code

  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.
  

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 Brobecker <brobecker@adacore.com> writes:

> 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.

I agree with Joel that we need more details about the problem you are
trying to fix.
  

> > 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.
> 
> I agree with Joel that we need more details about the problem you are
> trying to fix.

I think the problem comes from improper unwinding of a function
having an unusual frame, causing us to use a bogus return address
which could be anywhere, including in an unreadable memory area.