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]

Re: RFC: GDB crash in Thumb prologue when executing register movs to SP


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


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