From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2778 invoked by alias); 24 Apr 2002 22:09:15 -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 2766 invoked from network); 24 Apr 2002 22:09:14 -0000 Received: from unknown (HELO cygnus.com) (205.180.83.203) by 209.249.29.67 with SMTP; 24 Apr 2002 22:09:14 -0000 Received: from redhat.com (reddwarf.sfbay.redhat.com [172.16.24.50]) by runyon.cygnus.com (8.8.7-cygnus/8.8.7) with ESMTP id PAA07630; Wed, 24 Apr 2002 15:09:02 -0700 (PDT) Message-ID: <3CC72A2B.B9C88580@redhat.com> Date: Wed, 24 Apr 2002 15:09:00 -0000 From: Michael Snyder Organization: Red Hat, Inc. X-Accept-Language: en MIME-Version: 1.0 To: Andrew Cagney CC: gdb-patches@sources.redhat.com, Kevin Buettner Subject: Re: [patch/rfa:rs6000] Don't use ->prev References: <3CB9B3B1.90007@cygnus.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-SW-Source: 2002-04/txt/msg00977.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. > 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. ;-) > The rs6000 has me puzzled. I think the ->frame contains the address of > the wrong end of the frame! If ->frame pointed at the frame's start, > the code below wouldn't even be needed. Anyway, I've changed it to use > frame_chain() (I don't see regressions on NetBSD/PPC. The other > possability would be to risk a (recursive) get_prev_frame() call. > > Anyway, is the rs6000 ok? > > Andrew > > ------------------------------------------------------------------------ > 2002-04-14 Andrew Cagney > > * sparc-tdep.c (sparc_get_saved_register): Use get_prev_frame > instead of ->prev. > * z8k-tdep.c (z8k_frame_chain): Do not use ->prev. > * s390-tdep.c (s390_frame_chain): Do not use ->prev. > * rs6000-tdep.c (frame_get_saved_regs): Use rs6000_frame_chain() > instead of ->prev. > > Index: rs6000-tdep.c > =================================================================== > RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v > retrieving revision 1.55 > diff -u -r1.55 rs6000-tdep.c > --- rs6000-tdep.c 12 Apr 2002 19:48:36 -0000 1.55 > +++ rs6000-tdep.c 14 Apr 2002 16:48:56 -0000 > @@ -1388,10 +1388,12 @@ > && fdatap->cr_offset == 0 > && fdatap->vr_offset == 0) > frame_addr = 0; > - else if (fi->prev && fi->prev->frame) > - frame_addr = fi->prev->frame; > else > - frame_addr = read_memory_addr (fi->frame, wordsize); > + /* NOTE: cagney/2002-04-14: The ->frame points to the inner-most > + address of the current frame. Things might be easier if the > + ->frame pointed to the outer-most address of the frame. In the > + mean time, the address of the prev frame is used. */ > + frame_addr = rs6000_frame_chain (fi); > > /* if != -1, fdatap->saved_fpr is the smallest number of saved_fpr. > All fpr's from saved_fpr to fp31 are saved. */ > Index: s390-tdep.c > =================================================================== > RCS file: /cvs/src/src/gdb/s390-tdep.c,v > retrieving revision 1.43 > diff -u -r1.43 s390-tdep.c > --- s390-tdep.c 6 Apr 2002 00:02:50 -0000 1.43 > +++ s390-tdep.c 14 Apr 2002 16:49:11 -0000 > @@ -1009,9 +1009,7 @@ > { > 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, thisframe->frame, > S390_SP_REGNUM); > else > Index: sparc-tdep.c > =================================================================== > RCS file: /cvs/src/src/gdb/sparc-tdep.c,v > retrieving revision 1.26 > diff -u -r1.26 sparc-tdep.c > --- sparc-tdep.c 12 Apr 2002 18:18:57 -0000 1.26 > +++ sparc-tdep.c 14 Apr 2002 16:49:23 -0000 > @@ -829,11 +829,11 @@ > addr = frame1->frame + (regnum - G0_REGNUM) * SPARC_INTREG_SIZE > - (FP_REGISTER_BYTES + 8 * SPARC_INTREG_SIZE); > else if (regnum >= I0_REGNUM && regnum < I0_REGNUM + 8) > - addr = (frame1->prev->extra_info->bottom > + addr = (get_prev_frame (frame1)->extra_info->bottom > + (regnum - I0_REGNUM) * SPARC_INTREG_SIZE > + FRAME_SAVED_I0); > else if (regnum >= L0_REGNUM && regnum < L0_REGNUM + 8) > - addr = (frame1->prev->extra_info->bottom > + addr = (get_prev_frame (frame1)->extra_info->bottom > + (regnum - L0_REGNUM) * SPARC_INTREG_SIZE > + FRAME_SAVED_L0); > else if (regnum >= O0_REGNUM && regnum < O0_REGNUM + 8) > @@ -875,11 +875,11 @@ > { > /* Normal frame. Local and In registers are saved on stack. */ > if (regnum >= I0_REGNUM && regnum < I0_REGNUM + 8) > - addr = (frame1->prev->extra_info->bottom > + addr = (get_prev_frame (frame1)->extra_info->bottom > + (regnum - I0_REGNUM) * SPARC_INTREG_SIZE > + FRAME_SAVED_I0); > else if (regnum >= L0_REGNUM && regnum < L0_REGNUM + 8) > - addr = (frame1->prev->extra_info->bottom > + addr = (get_prev_frame (frame1)->extra_info->bottom > + (regnum - L0_REGNUM) * SPARC_INTREG_SIZE > + FRAME_SAVED_L0); > else if (regnum >= O0_REGNUM && regnum < O0_REGNUM + 8) > Index: z8k-tdep.c > =================================================================== > RCS file: /cvs/src/src/gdb/z8k-tdep.c,v > retrieving revision 1.7 > diff -u -r1.7 z8k-tdep.c > --- z8k-tdep.c 12 Apr 2002 18:18:57 -0000 1.7 > +++ z8k-tdep.c 14 Apr 2002 16:49:25 -0000 > @@ -160,10 +160,6 @@ > CORE_ADDR > z8k_frame_chain (struct frame_info *thisframe) > { > - if (thisframe->prev == 0) > - { > - /* This is the top of the stack, let's get the sp for real */ > - } > if (!inside_entry_file (thisframe->pc)) > { > return read_memory_pointer (thisframe->frame);