From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7275 invoked by alias); 9 Aug 2008 12:03:38 -0000 Received: (qmail 7260 invoked by uid 22791); 9 Aug 2008 12:03:36 -0000 X-Spam-Check-By: sourceware.org Received: from sibelius.xs4all.nl (HELO sibelius.xs4all.nl) (82.92.89.47) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 09 Aug 2008 12:02:49 +0000 Received: from brahms.sibelius.xs4all.nl (kettenis@localhost.sibelius.xs4all.nl [127.0.0.1]) by brahms.sibelius.xs4all.nl (8.14.3/8.14.3) with ESMTP id m79C2e5q015759; Sat, 9 Aug 2008 14:02:40 +0200 (CEST) Received: (from kettenis@localhost) by brahms.sibelius.xs4all.nl (8.14.3/8.14.3/Submit) id m79C2dbV001412; Sat, 9 Aug 2008 14:02:39 +0200 (CEST) Date: Sat, 09 Aug 2008 12:03:00 -0000 Message-Id: <200808091202.m79C2dbV001412@brahms.sibelius.xs4all.nl> From: Mark Kettenis To: hjl.tools@gmail.com CC: gdb-patches@sourceware.org, xuepeng.guo@intel.com, joey.ye@intel.com In-reply-to: <6dc9ffc80808061436y7ba7c35fl4844b5d9bce52a1@mail.gmail.com> (hjl.tools@gmail.com) Subject: Re: PATCH: Update x86 stack align analyzer References: <20080730211152.GA9645@lucon.org> <200808062105.m76L51xN016378@brahms.sibelius.xs4all.nl> <6dc9ffc80808061436y7ba7c35fl4844b5d9bce52a1@mail.gmail.com> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-08/txt/msg00232.txt.bz2 > Date: Wed, 6 Aug 2008 14:36:01 -0700 > From: "H.J. Lu" > > >> +/* 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. Ah ok, guess I confused amd64 with i386 there. Sorry 'bout that. > >> +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. I feel, rather strongly, that the stack align stuff stuff should only deal with stack alignment done *before* a frame gets setup. I think setting saved_sp_reg to -1, and proceeding as usual if it is still set at that value after the stack alignment analysis, works just fine. I also noticed a problem with your diff in that it doesn't use the proper saved sp value if you do have a frame (not sure GCC actually generates such code). How about the attached diff? Does it work with GCC 4.4? Index: amd64-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/amd64-tdep.c,v retrieving revision 1.52 diff -u -p -r1.52 amd64-tdep.c --- amd64-tdep.c 16 May 2008 00:27:22 -0000 1.52 +++ amd64-tdep.c 9 Aug 2008 11:56:43 -0000 @@ -680,6 +680,7 @@ struct amd64_frame_cache /* Saved registers. */ CORE_ADDR saved_regs[AMD64_NUM_SAVED_REGS]; CORE_ADDR saved_sp; + int saved_sp_reg; /* Do we have a frame? */ int frameless_p; @@ -702,6 +703,7 @@ amd64_init_frame_cache (struct amd64_fra for (i = 0; i < AMD64_NUM_SAVED_REGS; i++) cache->saved_regs[i] = -1; cache->saved_sp = 0; + cache->saved_sp_reg = -1; /* Frameless until proven otherwise. */ cache->frameless_p = 1; @@ -719,6 +721,179 @@ amd64_alloc_frame_cache (void) return cache; } +/* 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. */ + +static CORE_ADDR +amd64_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc, + struct amd64_frame_cache *cache) +{ + /* There are 2 code sequences to re-align stack before the frame + gets set up: + + 1. Use a caller-saved saved register: + + leaq 8(%rsp), %reg + andq $-XXX, %rsp + pushq -8(%reg) + + 2. Use a callee-saved saved register: + + pushq %reg + leaq 16(%rsp), %reg + andq $-XXX, %rsp + pushq -8(%reg) + + "andq $-XXX, %rsp" can be either 4 bytes or 7 bytes: + + 0x48 0x83 0xe4 0xf0 andq $-16, %rsp + 0x48 0x81 0xe4 0x00 0xff 0xff 0xff andq $-256, %rsp + */ + + gdb_byte buf[18]; + int reg, r; + int offset, offset_and; + static int regnums[16] = { + AMD64_RAX_REGNUM, /* %rax */ + AMD64_RCX_REGNUM, /* %rcx */ + AMD64_RDX_REGNUM, /* %rdx */ + AMD64_RBX_REGNUM, /* %rbx */ + AMD64_RSP_REGNUM, /* %rsp */ + AMD64_RBP_REGNUM, /* %rbp */ + AMD64_RSI_REGNUM, /* %rsi */ + AMD64_RDI_REGNUM, /* %rdi */ + AMD64_R8_REGNUM, /* %r8 */ + AMD64_R9_REGNUM, /* %r9 */ + AMD64_R10_REGNUM, /* %r10 */ + AMD64_R11_REGNUM, /* %r11 */ + AMD64_R12_REGNUM, /* %r12 */ + AMD64_R13_REGNUM, /* %r13 */ + AMD64_R14_REGNUM, /* %r14 */ + AMD64_R15_REGNUM, /* %r15 */ + }; + + if (target_read_memory (pc, buf, sizeof buf)) + return pc; + + /* Check caller-saved saved register. The first instruction has + to be "leaq 8(%rsp), %reg". */ + if ((buf[0] & 0xfb) == 0x48 + && buf[1] == 0x8d + && buf[3] == 0x24 + && buf[4] == 0x8) + { + /* MOD must be binary 10 and R/M must be binary 100. */ + if ((buf[2] & 0xc7) != 0x44) + return pc; + + /* REG has register number. */ + reg = (buf[2] >> 3) & 7; + + /* Check the REX.R bit. */ + if (buf[0] == 0x4c) + reg += 8; + + offset = 5; + } + else + { + /* Check callee-saved saved register. The first instruction + has to be "pushq %reg". */ + reg = 0; + if ((buf[0] & 0xf8) == 0x50) + offset = 0; + else if ((buf[0] & 0xf6) == 0x40 + && (buf[1] & 0xf8) == 0x50) + { + /* Check the REX.B bit. */ + if ((buf[0] & 1) != 0) + reg = 8; + + offset = 1; + } + else + return pc; + + /* Get register. */ + reg += buf[offset] & 0x7; + + offset++; + + /* The next instruction has to be "leaq 16(%rsp), %reg". */ + if ((buf[offset] & 0xfb) != 0x48 + || buf[offset + 1] != 0x8d + || buf[offset + 3] != 0x24 + || buf[offset + 4] != 0x10) + return pc; + + /* MOD must be binary 10 and R/M must be binary 100. */ + if ((buf[offset + 2] & 0xc7) != 0x44) + return pc; + + /* REG has register number. */ + r = (buf[offset + 2] >> 3) & 7; + + /* Check the REX.R bit. */ + if (buf[offset] == 0x4c) + r += 8; + + /* Registers in pushq and leaq have to be the same. */ + if (reg != r) + return pc; + + offset += 5; + } + + /* Rigister can't be %rsp nor %rbp. */ + if (reg == 4 || reg == 5) + return pc; + + /* The next instruction has to be "andq $-XXX, %rsp". */ + if (buf[offset] != 0x48 + || buf[offset + 2] != 0xe4 + || (buf[offset + 1] != 0x81 && buf[offset + 1] != 0x83)) + return pc; + + offset_and = offset; + offset += buf[offset + 1] == 0x81 ? 7 : 4; + + /* The next instruction has to be "pushq -8(%reg)". */ + r = 0; + if (buf[offset] == 0xff) + offset++; + else if ((buf[offset] & 0xf6) == 0x40 + && buf[offset + 1] == 0xff) + { + /* Check the REX.B bit. */ + if ((buf[offset] & 0x1) != 0) + r = 8; + offset += 2; + } + else + return pc; + + /* 8bit -8 is 0xf8. REG must be binary 110 and MOD must be binary + 01. */ + if (buf[offset + 1] != 0xf8 + || (buf[offset] & 0xf8) != 0x70) + return pc; + + /* R/M has register. */ + r += buf[offset] & 7; + + /* Registers in leaq and pushq have to be the same. */ + if (reg != r) + return pc; + + if (current_pc > pc + offset_and) + cache->saved_sp_reg = regnums[reg]; + + return min (pc + offset + 2, current_pc); +} + /* Do a limited analysis of the prologue at PC and update CACHE accordingly. Bail out early if CURRENT_PC is reached. Return the address where the analysis stopped. @@ -742,6 +917,8 @@ amd64_analyze_prologue (CORE_ADDR pc, CO if (current_pc <= pc) return current_pc; + pc = amd64_analyze_stack_align (pc, current_pc, cache); + op = read_memory_unsigned_integer (pc, 1); if (op == 0x55) /* pushq %rbp */ @@ -804,6 +981,13 @@ amd64_frame_cache (struct frame_info *th if (cache->pc != 0) amd64_analyze_prologue (cache->pc, get_frame_pc (this_frame), cache); + if (cache->saved_sp_reg != -1) + { + /* Stack pointer has been saved. */ + get_frame_register (this_frame, cache->saved_sp_reg, buf); + cache->saved_sp = extract_unsigned_integer(buf, 8); + } + if (cache->frameless_p) { /* We didn't find a valid frame. If we're at the start of a @@ -813,8 +997,20 @@ amd64_frame_cache (struct frame_info *th at the stack pointer. For truly "frameless" functions this might work too. */ - get_frame_register (this_frame, AMD64_RSP_REGNUM, buf); - cache->base = extract_unsigned_integer (buf, 8) + cache->sp_offset; + if (cache->saved_sp_reg != -1) + { + /* We're halfway aligning the stack. */ + cache->base = ((cache->saved_sp - 8) & 0xfffffffffffffff0LL) - 8; + cache->saved_regs[AMD64_RIP_REGNUM] = cache->saved_sp - 8; + + /* This will be added back below. */ + cache->saved_regs[AMD64_RIP_REGNUM] -= cache->base; + } + else + { + get_frame_register (this_frame, AMD64_RSP_REGNUM, buf); + cache->base = extract_unsigned_integer (buf, 8) + cache->sp_offset; + } } else { @@ -828,8 +1024,10 @@ amd64_frame_cache (struct frame_info *th /* For normal frames, %rip is stored at 8(%rbp). If we don't have a frame we find it at the same offset from the reconstructed base - address. */ - cache->saved_regs[AMD64_RIP_REGNUM] = 8; + address. If we're halfway aligning the stack, %rip is handled + differently (see above). */ + if (!cache->frameless_p || cache->saved_sp_reg == -1) + cache->saved_regs[AMD64_RIP_REGNUM] = 8; /* Adjust all the saved registers such that they contain addresses instead of offsets. */ Index: amd64-tdep.h =================================================================== RCS file: /cvs/src/src/gdb/amd64-tdep.h,v retrieving revision 1.11 diff -u -p -r1.11 amd64-tdep.h --- amd64-tdep.h 1 Jan 2008 22:53:09 -0000 1.11 +++ amd64-tdep.h 9 Aug 2008 11:56:43 -0000 @@ -39,8 +39,14 @@ enum amd64_regnum AMD64_RDI_REGNUM, /* %rdi */ AMD64_RBP_REGNUM, /* %rbp */ AMD64_RSP_REGNUM, /* %rsp */ - AMD64_R8_REGNUM = 8, /* %r8 */ - AMD64_R15_REGNUM = 15, /* %r15 */ + AMD64_R8_REGNUM, /* %r8 */ + AMD64_R9_REGNUM, /* %r9 */ + AMD64_R10_REGNUM, /* %r10 */ + AMD64_R11_REGNUM, /* %r11 */ + AMD64_R12_REGNUM, /* %r12 */ + AMD64_R13_REGNUM, /* %r13 */ + AMD64_R14_REGNUM, /* %r14 */ + AMD64_R15_REGNUM, /* %r15 */ AMD64_RIP_REGNUM, /* %rip */ AMD64_EFLAGS_REGNUM, /* %eflags */ AMD64_CS_REGNUM, /* %cs */ Index: i386-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/i386-tdep.c,v retrieving revision 1.261 diff -u -p -r1.261 i386-tdep.c --- i386-tdep.c 3 Jul 2008 00:19:58 -0000 1.261 +++ i386-tdep.c 9 Aug 2008 11:56:44 -0000 @@ -518,7 +518,7 @@ struct i386_frame_cache /* Saved registers. */ CORE_ADDR saved_regs[I386_NUM_SAVED_REGS]; CORE_ADDR saved_sp; - int stack_align; + int saved_sp_reg; int pc_in_eax; /* Stack space reserved for local variables. */ @@ -545,7 +545,7 @@ i386_alloc_frame_cache (void) for (i = 0; i < I386_NUM_SAVED_REGS; i++) cache->saved_regs[i] = -1; cache->saved_sp = 0; - cache->stack_align = 0; + cache->saved_sp_reg = -1; cache->pc_in_eax = 0; /* Frameless until proven otherwise. */ @@ -707,37 +707,111 @@ static CORE_ADDR i386_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc, struct i386_frame_cache *cache) { - /* The register used by the compiler to perform the stack re-alignment - is, in order of preference, either %ecx, %edx, or %eax. GCC should - never use %ebx as it always treats it as callee-saved, whereas - the compiler can only use caller-saved registers. */ - static const gdb_byte insns_ecx[10] = { - 0x8d, 0x4c, 0x24, 0x04, /* leal 4(%esp), %ecx */ - 0x83, 0xe4, 0xf0, /* andl $-16, %esp */ - 0xff, 0x71, 0xfc /* pushl -4(%ecx) */ - }; - static const gdb_byte insns_edx[10] = { - 0x8d, 0x54, 0x24, 0x04, /* leal 4(%esp), %edx */ - 0x83, 0xe4, 0xf0, /* andl $-16, %esp */ - 0xff, 0x72, 0xfc /* pushl -4(%edx) */ - }; - static const gdb_byte insns_eax[10] = { - 0x8d, 0x44, 0x24, 0x04, /* leal 4(%esp), %eax */ - 0x83, 0xe4, 0xf0, /* andl $-16, %esp */ - 0xff, 0x70, 0xfc /* pushl -4(%eax) */ + /* There are 2 code sequences to re-align stack before the frame + gets set up: + + 1. Use a caller-saved saved register: + + leal 4(%esp), %reg + andl $-XXX, %esp + pushl -4(%reg) + + 2. Use a callee-saved saved register: + + pushl %reg + leal 8(%esp), %reg + andl $-XXX, %esp + pushl -4(%reg) + + "andl $-XXX, %esp" can be either 3 bytes or 6 bytes: + + 0x83 0xe4 0xf0 andl $-16, %esp + 0x81 0xe4 0x00 0xff 0xff 0xff andl $-256, %esp + */ + + gdb_byte buf[14]; + int reg; + int offset, offset_and; + static int regnums[8] = { + I386_EAX_REGNUM, /* %eax */ + I386_ECX_REGNUM, /* %ecx */ + I386_EDX_REGNUM, /* %edx */ + I386_EBX_REGNUM, /* %ebx */ + I386_ESP_REGNUM, /* %esp */ + I386_EBP_REGNUM, /* %ebp */ + I386_ESI_REGNUM, /* %esi */ + I386_EDI_REGNUM /* %edi */ }; - gdb_byte buf[10]; - if (target_read_memory (pc, buf, sizeof buf) - || (memcmp (buf, insns_ecx, sizeof buf) != 0 - && memcmp (buf, insns_edx, sizeof buf) != 0 - && memcmp (buf, insns_eax, sizeof buf) != 0)) + if (target_read_memory (pc, buf, sizeof buf)) + return pc; + + /* Check caller-saved saved register. The first instruction has + to be "leal 4(%esp), %reg". */ + if (buf[0] == 0x8d && buf[2] == 0x24 && buf[3] == 0x4) + { + /* MOD must be binary 10 and R/M must be binary 100. */ + if ((buf[1] & 0xc7) != 0x44) + return pc; + + /* REG has register number. */ + reg = (buf[1] >> 3) & 7; + offset = 4; + } + else + { + /* Check callee-saved saved register. The first instruction + has to be "pushl %reg". */ + if ((buf[0] & 0xf8) != 0x50) + return pc; + + /* Get register. */ + reg = buf[0] & 0x7; + + /* The next instruction has to be "leal 8(%esp), %reg". */ + if (buf[1] != 0x8d || buf[3] != 0x24 || buf[4] != 0x8) + return pc; + + /* MOD must be binary 10 and R/M must be binary 100. */ + if ((buf[2] & 0xc7) != 0x44) + return pc; + + /* REG has register number. Registers in pushl and leal have to + be the same. */ + if (reg != ((buf[2] >> 3) & 7)) + return pc; + + offset = 5; + } + + /* Rigister can't be %esp nor %ebp. */ + if (reg == 4 || reg == 5) + return pc; + + /* The next instruction has to be "andl $-XXX, %esp". */ + if (buf[offset + 1] != 0xe4 + || (buf[offset] != 0x81 && buf[offset] != 0x83)) + return pc; + + offset_and = offset; + offset += buf[offset] == 0x81 ? 6 : 3; + + /* The next instruction has to be "pushl -4(%reg)". 8bit -4 is + 0xfc. REG must be binary 110 and MOD must be binary 01. */ + if (buf[offset] != 0xff + || buf[offset + 2] != 0xfc + || (buf[offset + 1] & 0xf8) != 0x70) + return pc; + + /* R/M has register. Registers in leal and pushl have to be the + same. */ + if (reg != (buf[offset + 1] & 7)) return pc; - if (current_pc > pc + 4) - cache->stack_align = 1; + if (current_pc > pc + offset_and) + cache->saved_sp_reg = regnums[reg]; - return min (pc + 10, current_pc); + return min (pc + offset + 3, current_pc); } /* Maximum instruction length we need to handle. */ @@ -1241,10 +1315,10 @@ i386_frame_cache (struct frame_info *thi if (cache->pc != 0) i386_analyze_prologue (cache->pc, get_frame_pc (this_frame), cache); - if (cache->stack_align) + if (cache->saved_sp_reg != -1) { - /* Saved stack pointer has been saved in %ecx. */ - get_frame_register (this_frame, I386_ECX_REGNUM, buf); + /* Saved stack pointer has been saved. */ + get_frame_register (this_frame, cache->saved_sp_reg, buf); cache->saved_sp = extract_unsigned_integer(buf, 4); } @@ -1258,7 +1332,7 @@ i386_frame_cache (struct frame_info *thi frame by looking at the stack pointer. For truly "frameless" functions this might work too. */ - if (cache->stack_align) + if (cache->saved_sp_reg != -1) { /* We're halfway aligning the stack. */ cache->base = ((cache->saved_sp - 4) & 0xfffffff0) - 4;