From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8885 invoked by alias); 5 Jul 2009 20:58:31 -0000 Received: (qmail 8875 invoked by uid 22791); 5 Jul 2009 20:58:30 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from smtp-outbound-2.vmware.com (HELO smtp-outbound-2.vmware.com) (65.115.85.73) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 05 Jul 2009 20:58:23 +0000 Received: from jupiter.vmware.com (mailhost5.vmware.com [10.16.68.131]) by smtp-outbound-2.vmware.com (Postfix) with ESMTP id 8D75942005; Sun, 5 Jul 2009 13:58:21 -0700 (PDT) Received: from [10.20.94.141] (msnyder-server.eng.vmware.com [10.20.94.141]) by jupiter.vmware.com (Postfix) with ESMTP id 83084DC46C; Sun, 5 Jul 2009 13:58:21 -0700 (PDT) Message-ID: <4A511332.2000607@vmware.com> Date: Sun, 05 Jul 2009 20:58:00 -0000 From: Michael Snyder User-Agent: Thunderbird 1.5.0.12 (X11/20080411) MIME-Version: 1.0 To: Mark Kettenis CC: "gdb-patches@sourceware.org" , "drow@false.org" , "teawater@gmail.com" Subject: Re: [RFA] epilogue unwinder for i386 (reverse 1/2) References: <4A4EA0F7.1040004@vmware.com> <4A4EA3B3.9030107@vmware.com> <200907051235.n65CZhDb024857@brahms.sibelius.xs4all.nl> In-Reply-To: <200907051235.n65CZhDb024857@brahms.sibelius.xs4all.nl> Content-Type: multipart/mixed; boundary="------------050500080508080102090300" X-IsSubscribed: yes 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: 2009-07/txt/msg00116.txt.bz2 This is a multi-part message in MIME format. --------------050500080508080102090300 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1502 Mark Kettenis wrote: >> Date: Fri, 03 Jul 2009 17:34:59 -0700 >> From: Michael Snyder >> >> Michael Snyder wrote: >>> This comes out of a discussion with Daniel, about how gcc >>> does not generate the right dwarf info to allow correct >>> frame unwinding in function epilogues, causing frame_unwind >>> to return bad results. >>> >>> It's necessary for reverse-step, which will frequently step >>> backward to the return instruction of a function. But it also >>> provides an improvement for forward debugging, in that now, >>> without this change, if you STEPI until you are at the return >>> instruction, you will get a bad backtrace. >>> >>> The infrun changes that take advantage of this patch will follow >>> separately. >>> >>> Michael >> Oops, the patch wasn't meant to have that "#if 0" in it... >> corrected patch below. > > Still has the #if 0 in there. Sorry. ;-( > > I also think you should add a comment about the specific ordering of > this unwinder. It has to come before the dwarf2 unwinder because GCC > doesn't provide proper CFI for the epilogue, right? Right. Since the others are similarly order-dependent, I will expand on their comments as well. >> + if (target_read_memory (pc, &insn, 1) != 0) >> + return 0; /* Can't read memory at pc. */ > > For consistency's sake, can you drop the != 0 here? OK. >> + if (insn != 0xc3) /* RET */ >> + return 0; > > Please use lowercase for instruction mnemonics. OK. Revised patch attached. --------------050500080508080102090300 Content-Type: text/plain; name="epilogue.txt" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="epilogue.txt" Content-length: 4289 2009-07-03 Michael Snyder * i386-tdep.c: Add a frame unwinder for function epilogues. (i386_in_function_epilogue_p): New function. (i386_epilogue_frame_sniffer): New function. (i386_epilogue_frame_cache): New function. (i386_epilogue_frame_this_id): New function. (i386_epilogue_frame_unwind): New struct frame_unwind. (i386_gdbarch_init): Hook the new unwinder. Index: i386-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/i386-tdep.c,v retrieving revision 1.280 diff -u -p -r1.280 i386-tdep.c --- i386-tdep.c 2 Jul 2009 17:25:54 -0000 1.280 +++ i386-tdep.c 5 Jul 2009 20:56:08 -0000 @@ -1487,6 +1487,89 @@ static const struct frame_unwind i386_fr NULL, default_frame_sniffer }; + +/* Normal frames, but in a function epilogue. */ + +/* The epilogue is defined here as the RET instruction, which will + follow any instruction such as LEAVE or POP EBP that destroys the + function's stack frame. */ + +static int +i386_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc) +{ + gdb_byte insn; + + if (target_read_memory (pc, &insn, 1)) + return 0; /* Can't read memory at pc. */ + + if (insn != 0xc3) /* 'ret' instruction. */ + return 0; + + return 1; +} + +static int +i386_epilogue_frame_sniffer (const struct frame_unwind *self, + struct frame_info *this_frame, + void **this_prologue_cache) +{ + if (frame_relative_level (this_frame) == 0) + return i386_in_function_epilogue_p (get_frame_arch (this_frame), + get_frame_pc (this_frame)); + else + return 0; +} + +static struct i386_frame_cache * +i386_epilogue_frame_cache (struct frame_info *this_frame, void **this_cache) +{ + struct gdbarch *gdbarch = get_frame_arch (this_frame); + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); + struct i386_frame_cache *cache; + gdb_byte buf[4]; + + if (*this_cache) + return *this_cache; + + cache = i386_alloc_frame_cache (); + *this_cache = cache; + + /* Cache base will be ESP plus cache->sp_offset (-4). */ + get_frame_register (this_frame, I386_ESP_REGNUM, buf); + cache->base = extract_unsigned_integer (buf, 4, + byte_order) + cache->sp_offset; + + /* Cache pc will be the frame func. */ + cache->pc = get_frame_pc (this_frame); + + /* The saved ESP will be at cache->base plus 8. */ + cache->saved_sp = cache->base + 8; + + /* The saved EIP will be at cache->base plus 4. */ + cache->saved_regs[I386_EIP_REGNUM] = cache->base + 4; + + return cache; +} + +static void +i386_epilogue_frame_this_id (struct frame_info *this_frame, + void **this_cache, + struct frame_id *this_id) +{ + struct i386_frame_cache *cache = i386_epilogue_frame_cache (this_frame, + this_cache); + + (*this_id) = frame_id_build (cache->base + 8, cache->pc); +} + +static const struct frame_unwind i386_epilogue_frame_unwind = +{ + NORMAL_FRAME, + i386_epilogue_frame_this_id, + i386_frame_prev_register, + NULL, + i386_epilogue_frame_sniffer +}; /* Signal trampolines. */ @@ -5329,7 +5412,15 @@ i386_gdbarch_init (struct gdbarch_info i /* Helper for function argument information. */ set_gdbarch_fetch_pointer_argument (gdbarch, i386_fetch_pointer_argument); - /* Hook in the DWARF CFI frame unwinder. */ + /* Hook the function epilogue frame unwinder. This unwinder is + appended to the list first, so that it supercedes the Dwarf + unwinder in function epilogues (where the Dwarf unwinder + currently fails). */ + frame_unwind_append_unwinder (gdbarch, &i386_epilogue_frame_unwind); + + /* Hook in the DWARF CFI frame unwinder. This unwinder is appended + to the list before the prologue-based unwinders, so that Dwarf + CFI info will be used if it is available. */ dwarf2_append_unwinders (gdbarch); frame_base_set_default (gdbarch, &i386_frame_base); @@ -5337,6 +5428,7 @@ i386_gdbarch_init (struct gdbarch_info i /* Hook in ABI-specific overrides, if they have been registered. */ gdbarch_init_osabi (info, gdbarch); + /* Hook in the legacy prologue-based unwinders last (fallback). */ frame_unwind_append_unwinder (gdbarch, &i386_sigtramp_frame_unwind); frame_unwind_append_unwinder (gdbarch, &i386_frame_unwind); --------------050500080508080102090300--