Re: RFA: frame id enhancement

["J. Johnston" <jjohnstn@redhat.com>]

Andrew Cagney wrote:
>>
>>> It's the reverse of infrun.c:2383 where the inferior is falling out 
>>> of a singnal trampoline, I think the assumptions again hold.
>>>
>>> infrun.c:2641:    if (!(frame_id_inner (current_frame, step_frame_id)))
>>>
>>> "Trust me" there's no value add.  While the comment reads:
>>>   /* In the case where we just stepped out of a function into the
>>>      middle of a line of the caller, continue stepping, but
>>>      step_frame_id must be modified to current frame */
>>> The test also updates step_frame_id when switching between frameless 
>>> stackless leaf function.  The extra test wouldn't fix that problem. 
>>> I'll try to remember to add some comments to that code.
> 
> 
> I've done this.
> 
>> Ok, that simplifies things.  I have included a revised patch that 
>> allows for the wild-card scenario.
> 
> 
> We're going to need more comments so that the next person better 
> understands what is going on:
> 
> +  /* The frame's special address.  This shall be constant through out the
> +     lifetime of the frame.  This is used for architectures that may have
> +     frames that have the same stack_addr and code_addr but are distinct
> +     due to some other qualification (e.g. the ia64 uses a register
> +     stack which is distinct from the memory stack).  */
> +  CORE_ADDR special_addr;
> 
> can you expand this definition to to note that the value isn't ordered, 
>  and that zero is treated as a wild card (its mentioned further down but 
> I think here, at the definition, is better).  For the ia64, is/can the 
> second area be described as a register spill area rather than a stack? 
> If the word "stack" can be avoided, the rationale for "special" being 
> un-ordered is stronger.
> 

It "is" a register stack on the ia64.  Registers r32 - r127 for any frame all 
come from this area.  It gets bumped up by a special alloc() instruction.  I'm 
not sure I would call it unordered.  It may be better to say that it is treated 
as unordered.  That would make the comments below much simpler - i.e. the 
special_addr field is treated as unordered so it is never used to determine 
order when comparing frames.

I can easily add the zero/wildcard comment.

> For:
> 
>    NOTE: Given frameless functions A and B, where A calls B (and hence
>    B is inner-to A).  The relationships: !eq(A,B); !eq(B,A);
>    !inner(A,B); !inner(B,A); all hold.  This is because, while B is
>    inner to A, B is not strictly inner to A (being frameless, they
>    have the same .base value).  */
> 
> an update is needed, suggest something like:
> 
>    NOTE:
> 
>    Given stackless functions A and B, where A calls B (and hence
>    B is inner-to A).  The relationships: !eq(A,B); !eq(B,A);
>    !inner(A,B); !inner(B,A); all hold.
> 
>    This is because, while B is
>    inner-to A, B is not strictly inner-to A.  Being stackless, they
>    have an identical .stack_addr value, and differ only by their 
> unordered .code_addr .special_addr values.
> 
>    Because frame_id_inner is only used as a safety net (e.g.,
>    detect a corrupt stack) the lack of strictness is not a problem.
>    Code needing to determine an exact relationship between two frames
>    must instead use frame_id_eq and frame_id_unwind.  For instance,
>    in the above, to determine that A stepped-into B, the equation
>    "A.id != B.id && A.id == id_unwind (B)" can be used.
> 
> 
> and a similar update to:
> 
> frame_id_inner (struct frame_id l, struct frame_id r)
> {
>   int inner;
>   if (l.stack_addr == 0 || r.stack_addr == 0)
>     /* Like NaN, any operation involving an invalid ID always fails.  */
>     inner = 0;
>   else
>     /* Only return non-zero when strictly inner than.  Note that, per
>        comment in "frame.h", there is some fuzz here.  Frameless
>        functions are not strictly inner than (same .stack but
>        different .code).  */
>     inner = INNER_THAN (l.stack_addr, r.stack_addr);
> 
> I can't think of a word better than "special", so I guess special it is :-)
> 
> Andrew
> 
> 
> 
>