From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18068 invoked by alias); 31 Mar 2008 22:05:45 -0000 Received: (qmail 18036 invoked by uid 22791); 31 Mar 2008 22:05:40 -0000 X-Spam-Check-By: sourceware.org Received: from NaN.false.org (HELO nan.false.org) (208.75.86.248) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 31 Mar 2008 22:05:04 +0000 Received: from nan.false.org (localhost [127.0.0.1]) by nan.false.org (Postfix) with ESMTP id 300AD98119; Mon, 31 Mar 2008 22:05:02 +0000 (GMT) Received: from caradoc.them.org (22.svnf5.xdsl.nauticom.net [209.195.183.55]) by nan.false.org (Postfix) with ESMTP id D64889802B; Mon, 31 Mar 2008 22:05:01 +0000 (GMT) Received: from drow by caradoc.them.org with local (Exim 4.69) (envelope-from ) id 1JgS7g-0005ik-R7; Mon, 31 Mar 2008 18:05:00 -0400 Date: Mon, 31 Mar 2008 23:41:00 -0000 From: Daniel Jacobowitz To: gdb@sourceware.org Cc: Mark Kettenis , Joel Brobecker , Ulrich Weigand Subject: [RFC] Using values to handle unwinding Message-ID: <20080331220500.GA21611@caradoc.them.org> Mail-Followup-To: gdb@sourceware.org, Mark Kettenis , Joel Brobecker , Ulrich Weigand References: <20071017160350.GA26804@caradoc.them.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20071017160350.GA26804@caradoc.them.org> User-Agent: Mutt/1.5.17 (2007-12-11) X-IsSubscribed: yes Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org X-SW-Source: 2008-03/txt/msg00284.txt.bz2 On Wed, Oct 17, 2007 at 12:03:50PM -0400, Daniel Jacobowitz wrote: > This is the patch I referenced yesterday on gdb-patches; the > background is here: > http://sourceware.org/ml/gdb-patches/2007-10/msg00432.html And the thread I'm replying to starts here for more: http://sourceware.org/ml/gdb/2007-10/msg00124.html I have finished polishing these patches. I will post (to gdb-patches, in a moment) patches for lazy register values, prev_register returning a value, and passing this_frame instead of next_frame to basically everything except the gdbarch unwind_pc method. unwind_dummy_id becomes just dummy_id, et cetera. These patches break the build for every unconverted target. I updated x86_64-* and i386-*, and all the platform-independent unwinders (tramp, trad, dummy, dwarf2, sentinel). Later in this mail are some detailed conversion instructions. I also wrote some gdbint documentation, for both new and existing frame unwinding details. It's not complete but it's a step in the right direction. One addition to the instructions: unless you have to do something particularly strange to a file, I think a single-line changelog entry is reasonable. I want to improve GDB, not the size of its ChangeLog :-) And writing the changelogs for this patch set took quite a long time. I'd love comments on the patches, the overall approach, and how to proceed. Ideally, we check this in (breaking many targets), update each target completely as someone needs that target, and make sure all targets are updated by the next release of GDB. I personally use amd64, i386, arm, mips, m68k, and powerpc; so I'm pretty likely to update all of those (mechanically). I'll do the laggards before the next release too, but not right this minute. I would appreciate assistance with other targets :-) Thoughts? === Conversion notes: Here's how you convert an unwinder. Start by finding a prev_register routine. They're pretty much all named *_prev_register so you can grep for "_prev_register ". Update it to have the new signature. After you're done converting it, be sure to convert the matching this_id routine too! The signature of this_id has changed (next_frame -> this_frame), but in a way that will not cause compilation errors. So never do a prev_register routine without the matching this_id. They come in pairs pretty much everywhere. frame_unwind_append_sniffer has gone away. It has been replaced by frame_unwind_append_unwinder, which takes a different signature (matching the pre-existing frame_unwind_prepend_unwinder). And both prepended and appended sniffers now take next_frame, just like prev_register and this_id. During execution of the sniffer, the unwinder being sniffed for is installed in the current frame. This lets simple interfaces like get_frame_func and get_frame_address_in_block behave as expected. Which means the exact same routines can be called from the sniffer as from prev_value. What this means during convertion is that you need to add the sniffer to the "struct frame_unwind" and call frame_unwind_append_unwinder instead of frame_unwind_append_sniffer. The type of the sniffer has also changed. Sniffers which just "return 1" (every architecture has one) can use default_frame_sniffer. There are not as many frame base unwinders, but keep an eye out for those too - they have the same problem as this_id, and the frame base sniffers have to take this_frame also. Similarly, the init function in struct tramp_frame now takes this_frame. I did a lot of searching files for next_frame. A few gdbarch methods still use next_frame, but most functions with an argument named next_frame are due for an update. For each affected routine, rename the next_frame argument to this_frame. Be sure to update the description in the function comment, and any other comments inside the function. Check every use of this_frame; some examples are in the table below. If it is passed to other miscellaneous target-specific functions, then you need to convert those (add them to your worklist, as it were...) and be sure to update all their callers too. I found it best to do an entire architecture at a time, e.g. amd64*.c + i386*.c all in one go. I started with amd64-tdep.c. The only nonstandard routine was called from amd64_sigtramp_frame_cache: tdep->sigcontext_addr. So from there I had to go out and do all of the amd64 and i386 OS ports and the i386 sigtramp unwinder too. You have to be careful when using get_prev_frame in an unwinder (generally don't do it - the prev frame requires this frame) and similarly for get_next_frame (it returns NULL instead of the sentinel frame, so generally don't do this either). Let me know if you run into any problems with having the wrong frame and not being able to change all the affected functions. For instance, before I converted sniffers to take this_frame, I ran into i386_linux_sigcontext_addr. That calls i386_linux_sigtramp_start, which was particularly tricky because I had not converted the sigtramp_p routine to take this_frame, which was itself tricky because I haven't converted sniffers to take this_frame being sniffed. So I had to revisit the design at that point. Until I did, I deliberately left the call untouched: pc = i386_linux_sigtramp_start (next_frame); Since I'd renamed next_frame -> this_frame in i386_linux_sigcontext_addr already, this clearly won't compile - that's a flag to come back and look at it later. It's not practical to test all platforms in advance for this sort of conversion, but the majority of unwinders are in tdep files so we can compile them easily (--enable-targets=...). I added a /* FIXME */ while I was working on it, just for good measure, and periodically scanned my diff for added FIXMEs. Then some of the files I'd already touched contained additional unwinders (e.g. amd64obsd_trapframe_sniffer) so I went back and did those too. Here's some example conversions: NEXT_FRAME THIS_FRAME ---------- ---------- frame_unwind_register get_frame_register frame_unwind_register_unsigned get_frame_register_unsigned frame_unwind_register_signed get_frame_register_signed frame_pc_unwind get_frame_pc frame_func_unwind get_frame_func (lose the extra argument) frame_unwind_address_in_block get_frame_address_in_block (lose the extra argument) get_frame_memory_unsigned *unchanged* (just rename the argument) safe_frame_unwind_memory *unchanged* (ditto, ignore the name...) Old register kind New function ----------------- ------------ not_lval frame_unwind_got_constant [For values no larger than a ULONGEST. Calculation of the value may be guarded by if (valuep). We always calculate the value in this new interface. That's the majority of calls, so this is not a noticeable loss.] not_lval frame_unwind_got_address [If the port does complicated address to pointer conversion and you have a CORE_ADDR, you'll need this instead.] lval_register frame_unwind_got_register lval_memory frame_unwind_got_memory -- Daniel Jacobowitz CodeSourcery