From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24594 invoked by alias); 5 May 2002 01:39:02 -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 24585 invoked from network); 5 May 2002 01:39:01 -0000 Received: from unknown (HELO localhost.redhat.com) (24.112.240.27) by sources.redhat.com with SMTP; 5 May 2002 01:39:01 -0000 Received: from cygnus.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 8EB243DAA; Sat, 4 May 2002 21:39:03 -0400 (EDT) Message-ID: <3CD48D37.5040700@cygnus.com> Date: Sat, 04 May 2002 18:39:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0rc1) Gecko/20020429 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Michael Snyder Cc: gdb-patches@sources.redhat.com, Kevin Buettner Subject: Re: [patch/rfa:rs6000] Don't use ->prev References: <3CB9B3B1.90007@cygnus.com> <3CC72A2B.B9C88580@redhat.com> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2002-05/txt/msg00115.txt.bz2 > Andrew Cagney wrote: > >> >> Hello, >> >> A number of targets contained code that tested ``->prev'' as part of >> doing their frame analysis. I think this ``cheating''. The objective >> of the frame analysis code is to create the ->prev frame using >> information from the current frame, not to use the still being created >> ->prev frame :-) >> >> Two cases - frame_chain for z8k and s390 - were, I think, simply wrong. >> The deleted z8k comment is telling - someone was 180 degrees out! :-) > > > I notice that your patch does not preserve the functionality on s390 -- > if the condition is false, the old code would set prev_fp, but yours > will not. Yes. That shouldn't matter. { CORE_ADDR prev_fp = 0; - if (thisframe->prev && thisframe->prev->frame) - prev_fp = thisframe->prev->frame; - else if (generic_find_dummy_frame (thisframe->pc, thisframe->frame)) + if (generic_find_dummy_frame (thisframe->pc, thisframe->frame)) return generic_read_register_dummy (thisframe->pc, The function (frame_chain()) will simply compute prev_fp using thisframe. >> The other two cases - get_saved_regs for SPARC and rs6000 - are more >> interesting. >> >> The SPARC code clearly relies on there already being a ->prev frame so >> I've used get_prev_frame(). Since ->prev must exist, there is no risk >> of recursion - get_prev_frame() calling get_saved_regs() (yes, grotty). > > > Maybe a comment to that effect? > Otherwise the sparc bit is (belatedly) approved. ;-) I've added this comment: /* NOTE: cagney/2002-05-04: The call to get_prev_frame() is safe/cheap - there will always be a prev frame. This is because frame1 is initialized to frame->next (frame1->prev == frame) and is then advanced towards the innermost (next) frame. */ Andrew