From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18959 invoked by alias); 21 Feb 2003 16:55:03 -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 18944 invoked from network); 21 Feb 2003 16:55:03 -0000 Received: from unknown (HELO localhost.redhat.com) (172.16.49.200) by 172.16.49.205 with SMTP; 21 Feb 2003 16:55:03 -0000 Received: from redhat.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 0C37E2EFA; Fri, 21 Feb 2003 11:59:42 -0500 (EST) Message-ID: <3E565AFD.1080508@redhat.com> Date: Fri, 21 Feb 2003 16:55:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD i386; en-US; rv:1.0.2) Gecko/20030217 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Kevin Buettner Cc: Andreas Schwab , gdb-patches@sources.redhat.com Subject: Re: ppc_linux_init_extra_frame_info References: <1030221045336.ZM13013@localhost.localdomain> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2003-02/txt/msg00518.txt.bz2 > On Feb 21, 1:14am, Andreas Schwab wrote: > > >> ppc_linux_init_extra_frame_info is clobbering the frame type which >> causes dummy_frame_pop to bail out. The frame type is now set before >> INIT_FRAME_INFO is called. > > > Not always, it would appear. In create_new_frame(), the type is > set before INIT_EXTRA_FRAME_INFO() is called. However, in > legacy_get_prev_frame(), the setting of the type occurs both > before and possibly after the call to INIT_EXTRA_FRAME_INFO(). > > In fact, the "after" part has me concerned. I suspect that it could > cause a signal handler's frame to be marked as a sigtramp frame. > Andrew even comments on this: > > /* FIXME: cagney/2002-11-10: This should be moved to before the > INIT code above so that the INIT code knows what the frame's > type is (in fact, for a [generic] dummy-frame, the type can > be set and then the entire initialization can be skipped. > Unforunatly, its the INIT code that sets the PC (Hmm, catch > 22). */ Hmm, ac131313@lulu$ frep deprecated_set_frame_type frame.c:1388:deprecated_set_frame_type (struct frame_info *frame, enum frame_type type) frame.h:251:extern void deprecated_set_frame_type (struct frame_info *, i386-interix-tdep.c:166: deprecated_set_frame_type (frame, SIGTRAMP_FRAME); i386-interix-tdep.c:174: deprecated_set_frame_type (frame, SIGTRAMP_FRAME); ppc-linux-tdep.c:376: deprecated_set_frame_type (fi, SIGTRAMP_FRAME); ppc-linux-tdep.c:380: deprecated_set_frame_type (fi, NORMAL_FRAME); rs6000-tdep.c:181: deprecated_set_frame_type (fi, SIGTRAMP_FRAME); (Joel, I'm going to ignore interix :-/) So it's solely PPC GNU/Linux that is the cause of all the problems. An addition to the comment; /* FIXME: cagney/2002-11-18: Should be setting the frame's type here, before anything else, and not last. Various INIT functions are full of work-arounds for the frames type not being set correctly from the word go. Ulgh! */ prev->type = NORMAL_FRAME; is that, for legacy targets, you don't know at this point what the PC is. Why not also modify the nasty ppc-linux hack to read: if (get_frame_type () == NORMAL) if (pc in sigtramp) deprecated set frame type () Andrew >> - behavior is ever fixed.) >> - >> - PC_IN_SIGTRAMP is called from blockframe.c as well in order to set >> - the frame's type (if a SIGTRAMP_FRAME). Because of our strange >> - definition of in_sigtramp below, we can't rely on the frame's type >> - getting set correctly from within blockframe.c. This is why we >> - take pains to set it in init_extra_frame_info(). >> - >> - NOTE: cagney/2002-11-10: I suspect the real problem here is that >> - the get_prev_frame() only initializes the frame's type after the >> - call to INIT_FRAME_INFO. get_prev_frame() should be fixed, this >> - code shouldn't be working its way around a bug :-(. */ >> + behavior is ever fixed.) */ > > > I'd prefer to have these comments left in place. > > I don't agree with Andrew's comment BTW. My memory on this is rather > hazy, but I think that the problem is that we really want different > functions for PC_IN_SIGTRAMP as called from infrun.c vs frame.c. > > If we could have the frame.c PC_IN_SIGRAMP calls call > ppc_linux_at_sigtramp_return_path() and the infrun.c calls call > ppc_linux_in_sigtramp(), things would work right. This suggests > that we really need two different methods. > > (Take my current remarks with a grain of salt. It's been a while > since I've worked on this code...) > > If you'd like to add you own notes to this comment block (or anywhere > else), I have no problem with that. > > >> int >> ppc_linux_in_sigtramp (CORE_ADDR pc, char *func_name) >> @@ -374,10 +363,6 @@ ppc_linux_init_extra_frame_info (int fro >> at trampoline code */ >> if (ppc_linux_at_sigtramp_return_path (fi->pc)) >> deprecated_set_frame_type (fi, SIGTRAMP_FRAME); >> - else >> - /* FIXME: cagney/2002-11-10: Is this double bogus? What >> - happens if the frame has previously been marked as a dummy? */ >> - deprecated_set_frame_type (fi, NORMAL_FRAME); >> } >> } > > > This part is fine. (I.e, I think this part should be committed.) > > Thanks, > > Kevin >