Re: [patch/rfa:rs6000] Don't use ->prev

[Andrew Cagney <ac131313@cygnus.com>]

> 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