Re: [PATCH] Fix frame ID comparison problem on s390

[Andrew Cagney <cagney@gnu.org>]

> Andrew Cagney wrote:
> 
> 
>>> with something like:
>>> 
>>>  > extern struct frame_id frame_id_build_wild (CORE_ADDR stack_addr);
>>> 
>>> That way clients would explicitly build a wild-card frame ID, and the 
>>> frame code was free to implement that mechanism anyway it saw fit.
> 
> 
> The following patch adds bits to the struct frame_id that explicitly
> state whether each of the stack, code, or special addresses is valid.
> This removes the need to choose one particular address value to 
> signify the 'invalid/wildcard/...' state.
> 
> It also adds the frame_id_build_wild function you suggested.
> 
> However, it changes the behaviour in that nobody actually calls _wild
> at the moment, since I wasn't sure at what places the code address
> actually can be unknown right now.  Those places would need to be
> adapted later, if required.

Right.  To find them you'd have to analize all the code paths involving 
zero values being returned from the symbol table - outch!  As found they 
can be updated (and likely also fix the symtab interface).

I should note that I did see such calls in the wild[groan] but only 
while fixing bugs during a frame conversion.

> Tested on s390-ibm-linux and s390x-ibm-linux, fixes the signull
> test case failure.

Yes with some tweaks:

Change s/..._valid/..._p/:
It's a consistent, but slightly wierd, naming convention

Add some sort of qualifying remark (what, I don't care) to each of the 
comments:
   /* The frame's code address.  This shall be constant through out the
      lifetime of the frame.  While the PC (a.k.a. resume address)
so that its clear that the value is predicated by ..._p.

Initialize the id to null vis:
   struct frame_id id = null_frame_id;
   id.stack_addr = stack_addr;
   id.stack_addr_valid = 1;
   id.code_addr = code_addr;
   id.code_addr_valid = 1;
so that the code is more likely to be future proof.

Extend this comment:
!   if (!l.stack_addr_valid || !r.stack_addr_valid)
       /* Like a NaN, if either ID is invalid, the result is false.  */
to mention that since a null_frame_id is always invalid it always trips 
up on that test.

with those changes its ok,

Thanks!
Andrew