From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26217 invoked by alias); 21 Apr 2002 05:01:26 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 25596 invoked from network); 21 Apr 2002 04:59:43 -0000 Received: from unknown (HELO localhost.redhat.com) (24.112.240.27) by sources.redhat.com with SMTP; 21 Apr 2002 04:59:43 -0000 Received: from cygnus.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id C61353D1A for ; Sun, 21 Apr 2002 00:59:36 -0400 (EDT) Message-ID: <3CC24738.9060104@cygnus.com> Date: Sat, 20 Apr 2002 22:01:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:0.9.9) Gecko/20020328 X-Accept-Language: en-us, en MIME-Version: 1.0 To: gdb-patches@sources.redhat.com Subject: [rfc] frame->register_unwind() - different way of unwinding a frame Content-Type: multipart/mixed; boundary="------------070205080500010409040306" X-SW-Source: 2002-04/txt/msg00714.txt.bz2 This is a multi-part message in MIME format. --------------070205080500010409040306 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1167 Ref: http://sources.redhat.com/ml/gdb/2002-04/msg00245.html The attached is a revised frame->unwind and frame cache patch. I think this one is looking much better - it now drops into place for existing targets. It introduces two changes: frame->register_unwind() Per other e-mail, there is a per-frame unwind function. Two implementations are provided: - a generic dummy frame - the traditional ->saved_regs frame The method for each frame is currently selected according to the PC (override mechanism?). Those functions can make use of a cacheing mechanism (although in the patch it is #if 0'd out). Running restore.exp native suggests a 2% reduction in the system time (which makes sense :-). Testing continues. I suspect the interface to this method will evolve over time. ``info frame'' I've updated this to use the register-unwind function. This eliminates the problem of using ->saved_regs to determine the location of all the registers. In the case of the SP, it will now also indicate a value on the stack or in another register. Doesn't show any regressions for the rs6000. I'll have to try it on a few other targets. Andrew --------------070205080500010409040306 Content-Type: text/plain; name="diffs" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="diffs" Content-length: 19115 2002-04-20 Andrew Cagney * stack.c (frame_info): Use frame_register_unwind instead of saved_regs. Mention when the SP is on the stack or in a register. * frame.h (frame_register_unwind_ftype): Define. Document. (struct frame_info): Add field register_unwind and register_unwind_cache. (frame_register_unwind): Declare. (generic_unwind_get_saved_register): Declare. * frame.c (frame_register_unwind): New function. (generic_unwind_get_saved_register): New function. * blockframe.c (generic_call_dummy_register_unwind): New function. (frame_saved_regs_register_unwind): New function. (set_unwind_by_pc): New function. (create_new_frame): New function. (get_prev_frame): New function. Index: blockframe.c =================================================================== RCS file: /cvs/src/src/gdb/blockframe.c,v retrieving revision 1.24 diff -u -r1.24 blockframe.c --- blockframe.c 14 Apr 2002 13:38:06 -0000 1.24 +++ blockframe.c 21 Apr 2002 04:31:37 -0000 @@ -34,9 +34,28 @@ #include "inferior.h" /* for read_pc */ #include "annotate.h" #include "regcache.h" +#include "gdb_assert.h" /* Prototypes for exported functions. */ +static void generic_call_dummy_register_unwind (struct frame_info *frame, + void **cache, + int regnum, + int *optimized, + enum lval_type *lval, + CORE_ADDR *addrp, + int *realnum, + void *raw_buffer); +static void frame_saved_regs_register_unwind (struct frame_info *frame, + void **cache, + int regnum, + int *optimized, + enum lval_type *lval, + CORE_ADDR *addrp, + int *realnum, + void *buffer); + + void _initialize_blockframe (void); /* A default FRAME_CHAIN_VALID, in the form that is suitable for most @@ -208,6 +227,27 @@ current_frame = frame; } + +/* Using the PC, select a mechanism for unwinding a frame returning + the previous frame. The register unwind function should, on + demand, initialize the ->context object. */ + +static void +set_unwind_by_pc (CORE_ADDR pc, CORE_ADDR fp, + frame_register_unwind_ftype **unwind) +{ + if (!USE_GENERIC_DUMMY_FRAMES) + /* Still need to set this to something. The ``info frame'' code + calls this function to find out where the saved registers are. + Hopefully this is robust enough to stop any core dumps and + return vaguely correct values.. */ + *unwind = frame_saved_regs_register_unwind; + else if (PC_IN_CALL_DUMMY (pc, fp, fp)) + *unwind = generic_call_dummy_register_unwind; + else + *unwind = frame_saved_regs_register_unwind; +} + /* Create an arbitrary (i.e. address specified by user) or innermost frame. Always returns a non-NULL value. */ @@ -232,6 +272,9 @@ if (INIT_EXTRA_FRAME_INFO_P ()) INIT_EXTRA_FRAME_INFO (0, fi); + /* Select/initialize an unwind function. */ + set_unwind_by_pc (fi->pc, fi->frame, &fi->register_unwind); + return fi; } @@ -456,6 +499,12 @@ } } + /* Initialize the code used to unwind the frame PREV based on the PC + (and probably other architectural information). The PC lets you + check things like the debug info at that point (dwarf2cfi?) and + use that to decide how the frame should be unwound. */ + set_unwind_by_pc (prev->pc, prev->frame, &prev->register_unwind); + find_pc_partial_function (prev->pc, &name, (CORE_ADDR *) NULL, (CORE_ADDR *) NULL); if (IN_SIGTRAMP (prev->pc, name)) @@ -1241,6 +1290,141 @@ struct value **args, struct type *type, int gcc_p) { return; +} + +/* Given a call-dummy dummy-frame, return the registers. Here the + register value is taken from the local copy of the register buffer. */ + +static void +generic_call_dummy_register_unwind (struct frame_info *frame, void **cache, + int regnum, int *optimized, + enum lval_type *lvalp, CORE_ADDR *addrp, + int *realnum, void *bufferp) +{ + gdb_assert (frame != NULL); + gdb_assert (PC_IN_CALL_DUMMY (frame->pc, frame->frame, frame->frame)); + + /* Describe the register's location. Generic dummy frames always + have the register value in an ``expression''. */ + *optimized = 0; + *lvalp = not_lval; + *addrp = 0; + *realnum = -1; + + /* If needed, find and return the value of the register. */ + if (bufferp != NULL) + { + char *registers; +#if 0 + /* Get the address of the register buffer that contains all the + saved registers for this dummy frame. Cache that address. */ + registers = (*cache); + if (registers == NULL) + { + registers = generic_find_dummy_frame (frame->pc, frame->frame); + (*cache) = registers; + } +#else + /* Get the address of the register buffer that contains the + saved registers and then extract the value from that. */ + registers = generic_find_dummy_frame (frame->pc, frame->frame); +#endif + gdb_assert (registers != NULL); + /* Return the actual value. */ + memcpy (bufferp, registers + REGISTER_BYTE (regnum), + REGISTER_RAW_SIZE (regnum)); + } +} + +/* Return the register saved in the simplistic ``saved_regs'' cache. + If the value isn't here AND a value is needed, try the next inner + most frame. */ + +static void +frame_saved_regs_register_unwind (struct frame_info *frame, void **cache, + int regnum, int *optimizedp, + enum lval_type *lvalp, CORE_ADDR *addrp, + int *realnump, void *bufferp) +{ + /* There is always a frame at this point. And THIS is the frame + we're interested in. */ + gdb_assert (frame != NULL); + gdb_assert (!PC_IN_CALL_DUMMY (frame->pc, frame->frame, frame->frame)); + + /* Load the saved_regs register cache. */ + if (frame->saved_regs == NULL) + FRAME_INIT_SAVED_REGS (frame); + + if (frame->saved_regs != NULL + && frame->saved_regs[regnum] != 0) + { + if (regnum == SP_REGNUM) + { + /* SP register treated specially. */ + *optimizedp = 0; + *lvalp = not_lval; + *addrp = 0; + *realnump = -1; + if (bufferp != NULL) + store_address (bufferp, REGISTER_RAW_SIZE (regnum), + frame->saved_regs[regnum]); + } + else + { + /* Any other register is saved in memory, fetch it but cache + a local copy of its value. */ + *optimizedp = 0; + *lvalp = lval_memory; + *addrp = frame->saved_regs[regnum]; + *realnump = -1; + if (bufferp != NULL) + { +#if 0 + /* Save each register value, as it is read in, in a + frame based cache. */ + void **regs = (*cache); + if (regs == NULL) + { + int sizeof_cache = ((NUM_REGS + NUM_PSEUDO_REGS) + * sizeof (void *)); + regs = frame_obstack_alloc (sizeof_cache); + memset (regs, 0, sizeof_cache); + (*cache) = regs; + } + if (regs[regnum] == NULL) + { + regs[regnum] + = frame_obstack_alloc (REGISTER_RAW_SIZE (regnum)); + read_memory (frame->saved_regs[regnum], regs[regnum], + REGISTER_RAW_SIZE (regnum)); + } + memcpy (bufferp, regs[regnum], REGISTER_RAW_SIZE (regnum)); +#else + /* Read the value in from memory. */ + read_memory (frame->saved_regs[regnum], bufferp, + REGISTER_RAW_SIZE (regnum)); +#endif + } + } + return; + } + + /* No luck, assume this and the next frame have the same register + value. If a value is needed, pass the request on down the chain; + otherwise just return an indication that the value is in the same + register as the next frame. */ + if (bufferp == NULL) + { + *optimizedp = 0; + *lvalp = lval_register; + *addrp = 0; + *realnump = regnum; + } + else + { + frame_register_unwind (frame->next, regnum, optimizedp, lvalp, addrp, + realnump, bufferp); + } } /* Function: get_saved_register Index: frame.c =================================================================== RCS file: /cvs/src/src/gdb/frame.c,v retrieving revision 1.7 diff -u -r1.7 frame.c --- frame.c 17 Apr 2002 21:55:12 -0000 1.7 +++ frame.c 21 Apr 2002 04:31:38 -0000 @@ -1,4 +1,4 @@ -/* Cache and manage the values of registers for GDB, the GNU debugger. +/* Cache and manage frames for GDB, the GNU debugger. Copyright 1986, 1987, 1989, 1991, 1994, 1995, 1996, 1998, 2000, 2001, 2002 Free Software Foundation, Inc. @@ -26,6 +26,7 @@ #include "value.h" #include "inferior.h" /* for inferior_ptid */ #include "regcache.h" +#include "gdb_assert.h" /* FIND_SAVED_REGISTER () @@ -157,6 +158,95 @@ } if (addrp != NULL) *addrp = addr; +} + +void +frame_register_unwind (struct frame_info *frame, int regnum, + int *optimizedp, enum lval_type *lvalp, + CORE_ADDR *addrp, int *realnump, void *bufferp) +{ + struct frame_unwind_cache *cache; + + /* Require all but BUFFERP to be valid. A NULL BUFFERP indicates + that the value proper does not need to be fetched. */ + gdb_assert (optimizedp != NULL); + gdb_assert (lvalp != NULL); + gdb_assert (addrp != NULL); + gdb_assert (realnump != NULL); + /* gdb_assert (bufferp != NULL); */ + + /* NOTE: cagney/2002-04-14: It would be nice if, instead of a + special case, there was always an inner frame dedicated to the + hardware registers. Unfortunatly, there is too much unwind code + around that looks up/down the frame chain while making the + assumption that each frame level is using the same unwind code. */ + + if (frame == NULL) + { + /* We're in the inner-most frame, get the value direct from the + register cache. */ + *optimizedp = 0; + *lvalp = lval_register; + *addrp = 0; + /* Should this code test ``register_cached (regnum) < 0'' and do + something like set realnum to -1 when the register isn't + available? */ + *realnump = regnum; + if (bufferp) + read_register_gen (regnum, bufferp); + return; + } + + /* Ask this frame to unwind its register. */ + frame->register_unwind (frame, &frame->register_unwind_cache, regnum, + optimizedp, lvalp, addrp, realnump, bufferp); +} + + +void +generic_unwind_get_saved_register (char *raw_buffer, + int *optimizedp, + CORE_ADDR *addrp, + struct frame_info *frame, + int regnum, + enum lval_type *lvalp) +{ + int optimizedx; + CORE_ADDR addrx; + int realnumx; + enum lval_type lvalx; + + if (!target_has_registers) + error ("No registers."); + + /* Keep things simple, ensure that all the pointers (except valuep) + are non NULL. */ + if (optimizedp == NULL) + optimizedp = &optimizedx; + if (lvalp == NULL) + lvalp = &lvalx; + if (addrp == NULL) + addrp = &addrx; + + /* Reached the the bottom (youngest, inner most) of the frame chain + (youngest, inner most) frame, go direct to the hardware register + cache (do not pass go, do not try to cache the value, ...). The + unwound value would have been cached in frame->next but that + doesn't exist. This doesn't matter as the hardware register + cache is stopping any unnecessary accesses to the target. */ + + /* NOTE: cagney/2002-04-14: It would be nice if, instead of a + special case, there was always an inner frame dedicated to the + hardware registers. Unfortunatly, there is too much unwind code + around that looks up/down the frame chain while making the + assumption that each frame level is using the same unwind code. */ + + if (frame == NULL) + frame_register_unwind (NULL, regnum, optimizedp, lvalp, addrp, &realnumx, + raw_buffer); + else + frame_register_unwind (frame->next, regnum, optimizedp, lvalp, addrp, + &realnumx, raw_buffer); } #if !defined (GET_SAVED_REGISTER) Index: frame.h =================================================================== RCS file: /cvs/src/src/gdb/frame.h,v retrieving revision 1.12 diff -u -r1.12 frame.h --- frame.h 12 Apr 2002 18:18:57 -0000 1.12 +++ frame.h 21 Apr 2002 04:31:40 -0000 @@ -23,6 +23,29 @@ #if !defined (FRAME_H) #define FRAME_H 1 +/* Return the location (and possibly value) of REGNUM for the previous + (older, up) frame. All parameters except VALUEP can be assumed to + be non NULL. When VALUEP is NULL, just the location of the + register should be returned. + + UNWIND_CACHE is provided as mechanism for implementing a per-frame + local cache. It's initial value being NULL. Memory for that cache + should be allocated using frame_obstack_alloc(). + + Register window architectures (eg SPARC) should note that REGNUM + identifies the register for the previous frame. For instance, a + request for the value of "o1" for the previous frame would be found + in the register "i1" in this FRAME. */ + +typedef void (frame_register_unwind_ftype) (struct frame_info *frame, + void **unwind_cache, + int regnum, + int *optimized, + enum lval_type *lvalp, + CORE_ADDR *addrp, + int *realnump, + void *valuep); + /* Describe the saved registers of a frame. */ #if defined (EXTRA_FRAME_INFO) || defined (FRAME_FIND_SAVED_REGS) @@ -112,6 +135,11 @@ related unwind data. */ struct unwind_contect *context; + /* See description above. Return the register value for the + previous frame. */ + frame_register_unwind_ftype *register_unwind; + void *register_unwind_cache; + /* Pointers to the next (down, inner) and previous (up, outer) frame_info's in the frame cache. */ struct frame_info *next; /* down, inner */ @@ -276,6 +304,22 @@ extern void generic_get_saved_register (char *, int *, CORE_ADDR *, struct frame_info *, int, enum lval_type *); + +extern void generic_unwind_get_saved_register (char *raw_buffer, + int *optimized, + CORE_ADDR * addrp, + struct frame_info *frame, + int regnum, + enum lval_type *lval); + +/* Unwind the stack frame so that the value of REGNUM, in the previous + frame is returned. If VALUEP is NULL, don't fetch/compute the + value. Instead just return the location of the value. */ + +extern void frame_register_unwind (struct frame_info *frame, int regnum, + int *optimizedp, enum lval_type *lvalp, + CORE_ADDR *addrp, int *realnump, + void *valuep); extern void get_saved_register (char *raw_buffer, int *optimized, CORE_ADDR * addrp, Index: rs6000-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v retrieving revision 1.59 diff -u -r1.59 rs6000-tdep.c --- rs6000-tdep.c 20 Apr 2002 03:09:28 -0000 1.59 +++ rs6000-tdep.c 21 Apr 2002 04:31:47 -0000 @@ -2655,7 +2655,7 @@ set_gdbarch_pc_in_call_dummy (gdbarch, generic_pc_in_call_dummy); set_gdbarch_call_dummy_p (gdbarch, 1); set_gdbarch_call_dummy_stack_adjust_p (gdbarch, 0); - set_gdbarch_get_saved_register (gdbarch, generic_get_saved_register); + set_gdbarch_get_saved_register (gdbarch, generic_unwind_get_saved_register); set_gdbarch_fix_call_dummy (gdbarch, rs6000_fix_call_dummy); set_gdbarch_push_dummy_frame (gdbarch, generic_push_dummy_frame); set_gdbarch_save_dummy_frame_tos (gdbarch, generic_save_dummy_frame_tos); Index: stack.c =================================================================== RCS file: /cvs/src/src/gdb/stack.c,v retrieving revision 1.33 diff -u -r1.33 stack.c --- stack.c 10 Apr 2002 23:32:33 -0000 1.33 +++ stack.c 21 Apr 2002 04:31:54 -0000 @@ -918,39 +918,84 @@ } } - FRAME_INIT_SAVED_REGS (fi); - if (fi->saved_regs != NULL) - { - /* The sp is special; what's returned isn't the save address, but - actually the value of the previous frame's sp. */ - printf_filtered (" Previous frame's sp is "); - print_address_numeric (fi->saved_regs[SP_REGNUM], 1, gdb_stdout); - printf_filtered ("\n"); - count = 0; - numregs = NUM_REGS + NUM_PSEUDO_REGS; - for (i = 0; i < numregs; i++) - if (fi->saved_regs[i] && i != SP_REGNUM) + if (fi->saved_regs == NULL) + FRAME_INIT_SAVED_REGS (fi); + /* Print as much information as possible on the location of all the + registers. */ + { + enum lval_type lval; + int optimized; + CORE_ADDR addr; + int realnum; + int count; + int i; + int need_nl = 1; + + /* The sp is special; what's displayed isn't the save address, but + the value of the previous frame's sp. This is a legacy thing, + at one stage the frame cached the previous frame's SP instead + of its address, hence it was easiest to just display the cached + value. */ + if (SP_REGNUM >= 0) + { + /* Find out the location of the saved stack pointer with out + actually evaluating it. */ + frame_register_unwind (fi, SP_REGNUM, &optimized, &lval, &addr, + &realnum, NULL); + if (!optimized && lval == not_lval) { - if (count == 0) - puts_filtered (" Saved registers:\n "); - else - puts_filtered (","); - wrap_here (" "); - printf_filtered (" %s at ", REGISTER_NAME (i)); - print_address_numeric (fi->saved_regs[i], 1, gdb_stdout); - count++; + void *value = alloca (MAX_REGISTER_RAW_SIZE); + CORE_ADDR sp; + frame_register_unwind (fi, SP_REGNUM, &optimized, &lval, &addr, + &realnum, value); + sp = extract_address (value, REGISTER_RAW_SIZE (SP_REGNUM)); + printf_filtered (" Previous frame's sp is "); + print_address_numeric (sp, 1, gdb_stdout); + printf_filtered ("\n"); + need_nl = 0; } - if (count) - puts_filtered ("\n"); - } - else - { - /* We could get some information about saved registers by - calling get_saved_register on each register. Which info goes - with which frame is necessarily lost, however, and I suspect - that the users don't care whether they get the info. */ + else if (!optimized && lval == lval_memory) + { + printf_filtered (" Previous frame's sp at "); + print_address_numeric (addr, 1, gdb_stdout); + printf_filtered ("\n"); + need_nl = 0; + } + else if (!optimized && lval == lval_register) + { + printf_filtered (" Previous frame's sp in %s\n", + REGISTER_NAME (realnum)); + need_nl = 0; + } + /* else keep quiet. */ + } + + count = 0; + numregs = NUM_REGS + NUM_PSEUDO_REGS; + for (i = 0; i < numregs; i++) + if (i != SP_REGNUM) + { + /* Find out the location of the saved register without + fetching the corresponding value. */ + frame_register_unwind (fi, i, &optimized, &lval, &addr, &realnum, + NULL); + /* For moment, only display registers that were saved on the + stack. */ + if (!optimized && lval == lval_memory) + { + if (count == 0) + puts_filtered (" Saved registers:\n "); + else + puts_filtered (","); + wrap_here (" "); + printf_filtered (" %s at ", REGISTER_NAME (i)); + print_address_numeric (addr, 1, gdb_stdout); + count++; + } + } + if (count || need_nl) puts_filtered ("\n"); - } + } } #if 0 --------------070205080500010409040306--