From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6071 invoked by alias); 24 Jan 2008 17:50:36 -0000 Received: (qmail 6063 invoked by uid 22791); 24 Jan 2008 17:50:35 -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; Thu, 24 Jan 2008 17:50:18 +0000 Received: from brahms.sibelius.xs4all.nl (kettenis@localhost.sibelius.xs4all.nl [127.0.0.1]) by brahms.sibelius.xs4all.nl (8.14.1/8.14.1) with ESMTP id m0OHoDjA001417; Thu, 24 Jan 2008 18:50:13 +0100 (CET) Received: (from kettenis@localhost) by brahms.sibelius.xs4all.nl (8.14.1/8.14.1/Submit) id m0OHoD5k007103; Thu, 24 Jan 2008 18:50:13 +0100 (CET) Date: Thu, 24 Jan 2008 17:51:00 -0000 Message-Id: <200801241750.m0OHoD5k007103@brahms.sibelius.xs4all.nl> From: Mark Kettenis To: muller@ics.u-strasbg.fr CC: gdb-patches@sourceware.org In-reply-to: <003101c85696$6f4d9e20$4de8da60$@u-strasbg.fr> (muller@ics.u-strasbg.fr) Subject: Re: [RFC-v2] Enhance backtrace for microsoft system DLL calls References: <000001c83b4a$573b4560$05b1d020$@u-strasbg.fr> <200712101854.lBAIs91J031646@brahms.sibelius.xs4all.nl> <002701c83be2$ac2a9a60$047fcf20$@u-strasbg.fr> <003101c85696$6f4d9e20$4de8da60$@u-strasbg.fr> 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-01/txt/msg00590.txt.bz2 > From: "Pierre Muller" > Date: Mon, 14 Jan 2008 11:15:49 +0100 Hi Pierre, I've been down with a nasty flu, so I didn't get around to properly reviewing your diff until today. > I wrote a i386_skip_noop function. Thanks. > It currently only tests for 'nop' and 'mov %edi,%edi' > instructions, but the way it is written, other > instructions should be easy to add. > I also tried to explain the reason of the presence > of the 'mov %edi,%edi' instruction in the win32 system DLL prologue, > as explained by Pedro. The comment is badly formatted and I must say it is not very clear I must say. Would you mind if I rewrote it after you committed the diff? > Tested on cygwin target, no regressions found. > The patch allows to get the backtrace of the main thread of gdb > to come up to the functions that called the systems DLL. > If I use ./gdb ./gdb with 'set new-console on' > and use Ctrl-C on the debuggee gdb window. > Without the patch, the backtrace only shows > 3 levels in ntdll.dll and kernel32.dll > > Questions: > 1) Is the 'nop' test useful or should it be removed? What do you mean by this? Are you saying you added the single-instruction nop even though you've never seen it in those DLL's? > 2) Should we add other possible no-ops? When we encounter them in the real world. Starting with what's in your diff is fine. > As said in my previous email, the number of > possible no-ops is big, and it is probably not wise to test all of > them. > > 3) this call is used for all i386 targets, but it > is probably useless for all operating systems but Microsoft Windows, > so should it be called only for that OS, and if yes, how should > we code this? I've heard of a couple of code generation tools that do something similar as Microsoft and insert nop instructions at the start of a function to be patched up later. So other targets could benefit from the same code. And calling this function unconditionally keeps the code simple. > 4) Any suggestions to make the comment clearer will be > most appreciated. See above. > ChangeLog entry: > > 2008-01-14 Pierre Muller > > * i386-tdep.c (i386_skip_noop): New function. > (i386_analyze_prologue): Call i386_skip_noop function. > > > Index: gdb/i386-tdep.c > =================================================================== > RCS file: /cvs/src/src/gdb/i386-tdep.c,v > retrieving revision 1.248 > diff -u -p -r1.248 i386-tdep.c > --- gdb/i386-tdep.c 11 Jan 2008 13:20:00 -0000 1.248 > +++ gdb/i386-tdep.c 14 Jan 2008 08:11:27 -0000 > @@ -632,6 +632,51 @@ struct i386_insn i386_frame_setup_skip_i > { 0 } > }; > > + > +/* Check whether PC points to a no-op instruction. */ > +static CORE_ADDR > +i386_skip_noop (CORE_ADDR pc) > +{ > + gdb_byte op; > + int check = 1; > + > + read_memory_nobpt (pc, &op, 1); > + > + while (check) > + { > + check = 0; > + /* Ignore `nop' instruction. */ > + if (op == 0x90) > + { > + pc += 1; > + read_memory_nobpt (pc, &op, 1); > + check = 1; > + } > + /* Ignore no-op instruction `mov %edi, %edi'. > + Microsoft system dlls often start with > + a `mov %edi,%edi' instruction. > + The 5 bytes before the function start are > + filled with `nop' instructions. > + This pattern can be used for hot-patching: > + The `mov %edi, %edi' instruction can be replaced by a > + near jump to the location of the 5 `nop' instructions > + which can be replaced by a 32-bit jump to anywhere > + in the 32-bit address space. */ > + > + else if (op == 0x8b) > + { > + read_memory_nobpt (pc + 1, &op, 1); > + if (op == 0xff) > + { > + pc += 2; > + read_memory_nobpt (pc, &op, 1); > + check = 1; > + } > + } > + } > + return pc; > +} > + > /* Check whether PC points at a code that sets up a new stack frame. > If so, it updates CACHE and returns the address of the first > instruction after the sequence that sets up the frame or LIMIT, > @@ -817,6 +862,7 @@ static CORE_ADDR > i386_analyze_prologue (CORE_ADDR pc, CORE_ADDR current_pc, > struct i386_frame_cache *cache) > { > + pc = i386_skip_noop (pc); > pc = i386_follow_jump (pc); > pc = i386_analyze_struct_return (pc, current_pc, cache); > pc = i386_skip_probe (pc); > > > > >