PATCH: Update x86 stack align analyzer
H.J. Lu
hjl.tools@gmail.com
Wed Aug 6 21:36:00 GMT 2008
On Wed, Aug 6, 2008 at 2:05 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>
>> introduced new ways to align stack. This patch teaches gdb how to
>> recognize the new stack prologue. OK for trunk?
>
> A few comments inline below.
Thanks for you review.
>
>> --- ../gdb/src/gdb/amd64-tdep.c 2008-07-16 22:30:04.000000000 -0700
>> +++ gdb/gdb/amd64-tdep.c 2008-07-25 11:57:37.000000000 -0700
>> @@ -680,6 +680,7 @@ struct amd64_frame_cache
>> /* Saved registers. */
>> CORE_ADDR saved_regs[AMD64_NUM_SAVED_REGS];
>> CORE_ADDR saved_sp;
>> + enum i386_regnum saved_sp_reg;
>
> I suppose you meant 'enum amd64_regnum' here. I'd prefer if you'd
> simply use 'int' as the type for registers instead of the enum.
> That's what all the other code in GDB does.
>
Done.
>> +/* GCC 4.4 and later, can put code in the prologue to realign the
>> + stack pointer. Check whether PC points to such code, and update
>> + CACHE accordingly. Return the first instruction after the code
>> + sequence or CURRENT_PC, whichever is smaller. If we don't
>> + recognize the code, return PC. */
>
> There are older versions of GCC that use at least method #2, so this
> comment is a bit misleading.
Gcc 4.1 can align stack to 16byte, but only for ia32, not for x86-64. We
added this capability to gcc 4.4.
>
>> +static CORE_ADDR
>> +amd64_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc,
>> + struct amd64_frame_cache *cache)
>> +{
>> + /* There are 3 code sequences to re-align stack:
>> +
>> + 1. Use %ebp:
>> +
>> + pushq %rbp
>> + movq %rsp, %rbp
>> + andq $-XXX, %rsp
>
> But this is basically the standard prologue sequence. I don't see why
> you need to handle this sequence here, since it is already handled in
> amd64_analyze_prologue().
We need it to set saved_sp_reg.
>
>> - get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
>> - cache->base = extract_unsigned_integer (buf, 8) + cache->sp_offset;
>> + if (cache->saved_sp_reg != AMD64_RSP_REGNUM)
>> + {
>> + /* We're halfway aligning the stack. Saved stack pointer
>> + has been saved in saved_sp_reg. */
>> + get_frame_register (this_frame, cache->saved_sp_reg, buf);
>> + cache->saved_sp = extract_unsigned_integer(buf, 8);
>> + cache->base = ((cache->saved_sp - 8) & 0xfffffffffffffff0LL) - 8;
>> + cache->saved_regs[AMD64_RIP_REGNUM] = cache->saved_sp - 8;
>
> I don't see how this could possibly work if saved_regs[AMD64_RIP_REGNUM]
> is set to 8 unconditionally a few lines further on.
I changed it not to set %rip to 8 when halfway aligning the stack. That
is similar to i386 code.
>
> See my comment about amd64.
>
I changed to int.
I am enclosed my updated patch.
Thanks.
--
H.J.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: gdb-stack-3.patch.txt
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20080806/80065e47/attachment.txt>
More information about the Gdb-patches
mailing list