From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 853 invoked by alias); 2 Jan 2003 19:34:29 -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 844 invoked from network); 2 Jan 2003 19:34:27 -0000 Received: from unknown (HELO localhost.redhat.com) (66.30.197.194) by 209.249.29.67 with SMTP; 2 Jan 2003 19:34:27 -0000 Received: from redhat.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 987913DE5; Thu, 2 Jan 2003 19:34:16 +0000 (GMT) Message-ID: <3E149438.3040900@redhat.com> Date: Thu, 02 Jan 2003 19:34:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.1) Gecko/20021211 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Daniel Jacobowitz Cc: gdb-patches@sources.redhat.com Subject: Re: RFC: Mostly kill FRAME_CHAIN_VALID, add user knob References: <20021226191541.GA8483@nevyn.them.org> Content-Type: multipart/mixed; boundary="------------090805080104050005020906" X-SW-Source: 2003-01/txt/msg00014.txt.bz2 This is a multi-part message in MIME format. --------------090805080104050005020906 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Content-length: 2133 > Pretty gross, neh? Well, file vs. func is merely a question of whether we > stop at main or not, so I added "set backtrace-below-main" in order to let > the user choose. Generic vs. not is a question of dummy frames, and the > generic versions work with non-generic dummy frames, so there's no reason > for that distinction earlier. It won't harm those three m68k targets (if > they still work) to use a more comprehensive frame_chain_valid. And the > five more specific ones up above can be retained, since they are only > _additional_ checks. I'm not entirely convinced that the Interix one is > necessary but I left it alone. > > So, after this patch we have FRAME_CHAIN_VALID as a predicated function that > only five architectures define; everything else just uses the new > frame_chain_valid () function, which is a more general version of > generic_func_frame_chain_valid. > > I'm more confident I got the texinfo right this time :) I tested the patch > and the new functionality on i386-linux and arm-elf, to make sure I got the > details of FRAME_CHAIN_VALID_P () right. > > I'll look to commit this in January, if no one has any comments. Andrew, > would you rather this went in frame.c? Since a purpose of that file seems > to be moving things from blockframe.c to it... FYI, Much of this is superseeded by the frame overhaul - in particular the introduction of frame_id_unwind(). The new code doesn't even call frame chain valid! Perhaphs wait for the attached [wip] to be committed and then tweak that to match your proposed policy (we can then just deprecate FRAME_CHAIN_VALID_P :-). However, making the change in parallel wouldn't hurt. Looking at my WIP, I'll need to tweak the code segment: + prev_frame->pc = frame_pc_unwind (next_frame); + if (prev_frame->pc == 0) + /* The allocated PREV_FRAME will be reclaimed when the frame + obstack is next purged. */ + return NULL; + prev_frame->type = frame_type_from_pc (prev_frame->pc); so that it checks for where the PC resides and abort accordingly. The attached is WIP since I still need to see it working once :-) Andrew --------------090805080104050005020906 Content-Type: text/plain; name="diffs" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="diffs" Content-length: 6915 2002-12-18 Andrew Cagney * frame.c (frame_type_from_pc): New function. (create_new_frame): Use frame_type_from_pc. (deprecated_get_prev_frame): New function. Move the old style previous frame code to here.... (get_prev_frame): ... from here. Re-implement using deprecated_get_prev_frame, frame_pc_unwind, frame_id_unwind and frame_type_from_pc Index: frame.c =================================================================== RCS file: /cvs/src/src/gdb/frame.c,v retrieving revision 1.47 diff -u -r1.47 frame.c --- frame.c 13 Dec 2002 23:18:56 -0000 1.47 +++ frame.c 18 Dec 2002 19:07:42 -0000 @@ -846,6 +846,29 @@ } } +/* Determine the frame's type based on its PC. */ + +static enum frame_type +frame_type_from_pc (CORE_ADDR pc) +{ + /* FIXME: cagney/2002-11-24: Can't yet directly call + pc_in_dummy_frame() as some architectures don't set + PC_IN_CALL_DUMMY() to generic_pc_in_call_dummy() (remember the + latter is implemented by simply calling pc_in_dummy_frame). */ + if (DEPRECATED_USE_GENERIC_DUMMY_FRAMES + && DEPRECATED_PC_IN_CALL_DUMMY (pc, 0, 0)) + return DUMMY_FRAME; + else + { + char *name; + find_pc_partial_function (pc, &name, NULL, NULL); + if (PC_IN_SIGTRAMP (pc, name)) + return SIGTRAMP_FRAME; + else + return NORMAL_FRAME; + } +} + /* Create an arbitrary (i.e. address specified by user) or innermost frame. Always returns a non-NULL value. */ @@ -864,37 +887,14 @@ fi->frame = addr; fi->pc = pc; - /* NOTE: cagney/2002-11-18: The code segments, found in - create_new_frame and get_prev_frame(), that initializes the - frames type is subtly different. The latter only updates ->type - when it encounters a SIGTRAMP_FRAME or DUMMY_FRAME. This stops - get_prev_frame() overriding the frame's type when the INIT code - has previously set it. This is really somewhat bogus. The - initialization, as seen in create_new_frame(), should occur - before the INIT function has been called. */ - if (DEPRECATED_USE_GENERIC_DUMMY_FRAMES - && (DEPRECATED_PC_IN_CALL_DUMMY_P () - ? DEPRECATED_PC_IN_CALL_DUMMY (pc, 0, 0) - : pc_in_dummy_frame (pc))) - /* NOTE: cagney/2002-11-11: Does this even occure? */ - type = DUMMY_FRAME; - else - { - char *name; - find_pc_partial_function (pc, &name, NULL, NULL); - if (PC_IN_SIGTRAMP (fi->pc, name)) - type = SIGTRAMP_FRAME; - else - type = NORMAL_FRAME; - } - fi->type = type; + fi->type = frame_type_from_pc (pc); if (INIT_EXTRA_FRAME_INFO_P ()) INIT_EXTRA_FRAME_INFO (0, fi); @@ -936,12 +936,10 @@ } } -/* Return a structure containing various interesting information - about the frame that called NEXT_FRAME. Returns NULL - if there is no such frame. */ +/* Create the previous frame using the old style methods. */ -struct frame_info * -get_prev_frame (struct frame_info *next_frame) +static struct frame_info * +deprecated_get_prev_frame (struct frame_info *next_frame) { CORE_ADDR address = 0; struct frame_info *prev; @@ -1185,6 +1183,102 @@ } return prev; +} + +/* Return a structure containing various interesting information + about the frame that called NEXT_FRAME. Returns NULL + if there is no such frame. */ + +struct frame_info * +get_prev_frame (struct frame_info *next_frame) +{ + struct frame_info *prev_frame; + + if (DEPRECATED_INIT_FRAME_PC_P () + || DEPRECATED_INIT_FRAME_PC_FIRST_P ()) + return deprecated_get_prev_frame (next_frame); + + /* There is always a frame. If this assertion fails, suspect that + something should be calling get_selected_frame() or + get_current_frame(). */ + gdb_assert (next_frame != NULL); + + /* Only try to do the unwind once. */ + if (next_frame->prev_p) + return next_frame->prev; + next_frame->prev_p = 1; + + /* Allocate the new frame but do not wire it in. Some (bad) code + tries to look along frame->next to pull some fancy tricks (of + course such code is, by definition, recursive). Try to prevent + it. */ + prev_frame = frame_obstack_alloc (sizeof (struct frame_info)); + memset (prev_frame, 0, sizeof (struct frame_info)); + prev_frame->level = next_frame->level + 1; + + /* Try to unwind the PC. If that doesn't work, assume we've reached + the oldest frame and simply return. Is there a better sentinal + value? The unwound PC value is then used to initialize the new + previous frame's type. + + Note that the pc-unwind is intentionally performed before the + frame chain. This is ok since, for old targets, both + frame_pc_unwind (nee, FRAME_SAVED_PC) and FRAME_CHAIN()) assume + NEXT_FRAME's data structures have already been initialized (using + INIT_EXTRA_FRAME_INFO) and hence the call order doesn't matter. + + By unwinding the PC first, it becomes possible to, in the case of + a dummy frame, avoid also unwinding the frame ID. This is + because (well ignoring the PPC) a dummy frame can be located + using NEXT_FRAME's frame ID. */ + + prev_frame->pc = frame_pc_unwind (next_frame); + if (prev_frame->pc == 0) + /* The allocated PREV_FRAME will be reclaimed when the frame + obstack is next purged. */ + return NULL; + prev_frame->type = frame_type_from_pc (prev_frame->pc); + + /* Set the unwind functions based on that identified PC. */ + set_unwind_by_pc (prev_frame->pc, &prev_frame->register_unwind, + &prev_frame->pc_unwind, &prev_frame->id_unwind); + + /* Now figure out how to initialize this new frame. Perhaphs one + day, this will too, be selected by set_unwind_by_pc(). */ + if (prev_frame->type != DUMMY_FRAME) + { + /* A dummy frame doesn't need to unwind the frame ID because the + frame ID comes from the previous frame. The other frames do + though. True? */ +#if 0 + /* Oops, the frame doesn't chain. Treat this as the last frame. */ + prev_frame->id = frame_id_unwind (next_frame); + if (!frame_id_p (prev_frame->id)) + return NULL; +#else + /* FIXME: cagney/2002-12-18: Instead of this hack, should just + save the frame ID directly. */ + struct frame_id id = frame_id_unwind (next_frame); + if (!frame_id_p (id)) + return NULL; + prev_frame->frame = id.base; +#endif + } + + /* Link it in. */ + next_frame->prev = prev_frame; + prev_frame->next = next_frame; + + /* NOTE: cagney/2002-12-18: Eventually this call will go away. + Instead of initializing extra info, all frames will use the + frame_cache (passed to the unwind functions) to store extra frame + info. */ + if (INIT_EXTRA_FRAME_INFO_P ()) + /* NOTE: This code doesn't bother trying to sort out frameless + functions. That is left to the target. */ + INIT_EXTRA_FRAME_INFO (0, prev_frame); + + return prev_frame; } CORE_ADDR --------------090805080104050005020906--