This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: RFC: GDB crash in Thumb prologue when executing register movs to SP
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Cc: Meador Inge <minge at codesourcery dot com>
- Date: Fri, 3 Jun 2011 11:58:48 +0100
- Subject: Re: RFC: GDB crash in Thumb prologue when executing register movs to SP
- References: <4DE7F66D.4080601@codesourcery.com>
On Thursday 02 June 2011 21:45:33, Meador Inge wrote:
> One way to fix the problem would be to exit the scanning loop when code writes
> the SP:
>
> @@ -766,6 +766,14 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
> {
> int dst_reg = (insn & 0x7) + ((insn & 0x80) >> 4);
> int src_reg = (insn & 0x78) >> 3;
> +
> + /* If the SP is directly written to, then we can't be sure where the
> + stack has gone. Therefore it is better to just bail out. This
> + type of case happens in code that is switching stacks around
> + (e.g. OSes, threading libraries, etc...). */
> + if (dst_reg == ARM_SP_REGNUM)
> + break;
> +
> regs[dst_reg] = regs[src_reg];
> }
> else if ((insn & 0xf800) == 0x9000) /* str rd, [sp, #off] */
>
>
> However, this raises the question of whether there are other cases that can end
> up in that final 'else' branch. I am still analyzing whether or not that can
> happen.
Another question is whether your change breaks something out there.
E.g., if you had some prologue that started with the
(contrived, but still...) sequence:
" mov r$foo, sp\n"
" mov sp, r$foo\n"
... <setup regular frame here> ...
wouldn't the prologue-value.c machinery end up working
out through frame setup? It wouldn't with your change, of course.
>
> With that in mind, though: is there any particular reason why the frame
> register is set to -1? It seems like the code could be written to just always
> fall back on having the frame register equal to the SP. That would fix this
> problem and presumably other cases without special casing moves to SP.
>
> (Also, similar logic exist in 'arm-tdep.c:arm_analyze_prologue'.)
IMO, that's the way to go, but I'm no arm/gdb expert. It's a desperate
fallback, but better than nothing, and would have the same result
as your patch, in your test case. Other prologue-value parsers do
similarly, e.g., mep-tdep.c, rx-tdep.c, mn10300-tdep.c.
--
Pedro Alves