Re: RFA: frame id enhancement

["J. Johnston" <jjohnstn at redhat dot 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