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

[Ulrich Weigand <weigand@i1.informatik.uni-erlangen.de>]

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.

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

OK?

Bye,
Ulrich


ChangeLog:

	* frame.h (struct frame_id): New fields stack_addr_valid,
	code_addr_valid, and special_addr_valid.
	(frame_id_build, frame_id_build_special): Update comments.
	(frame_id_build_wild): New prototype.
	* frame.c (frame_id_build, frame_id_build_special): Fill in new
	struct frame_id fields.
	(frame_id_build_wild): New function.
	(frame_id_eq, frame_id_inner): Use new struct frame_id fields.

Index: gdb/frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.182
diff -c -p -r1.182 frame.c
*** gdb/frame.c	10 Jun 2004 13:22:05 -0000	1.182
--- gdb/frame.c	11 Jun 2004 16:10:13 -0000
*************** frame_id_build_special (CORE_ADDR stack_
*** 272,292 ****
    id.stack_addr = stack_addr;
    id.code_addr = code_addr;
    id.special_addr = special_addr;
    return id;
  }
  
  struct frame_id
  frame_id_build (CORE_ADDR stack_addr, CORE_ADDR code_addr)
  {
!   return frame_id_build_special (stack_addr, code_addr, 0);
  }
  
  int
  frame_id_p (struct frame_id l)
  {
    int p;
!   /* The .code can be NULL but the .stack cannot.  */
!   p = (l.stack_addr != 0);
    if (frame_debug)
      {
        fprintf_unfiltered (gdb_stdlog, "{ frame_id_p (l=");
--- 272,315 ----
    id.stack_addr = stack_addr;
    id.code_addr = code_addr;
    id.special_addr = special_addr;
+   id.stack_addr_valid = 1;
+   id.code_addr_valid = 1;
+   id.special_addr_valid = 1;
    return id;
  }
  
  struct frame_id
  frame_id_build (CORE_ADDR stack_addr, CORE_ADDR code_addr)
  {
!   struct frame_id id;
!   id.stack_addr = stack_addr;
!   id.code_addr = code_addr;
!   id.special_addr = 0;
!   id.stack_addr_valid = 1;
!   id.code_addr_valid = 1;
!   id.special_addr_valid = 0;
!   return id;
! }
! 
! struct frame_id
! frame_id_build_wild (CORE_ADDR stack_addr)
! {
!   struct frame_id id;
!   id.stack_addr = stack_addr;
!   id.code_addr = 0;
!   id.special_addr = 0;
!   id.stack_addr_valid = 1;
!   id.code_addr_valid = 0;
!   id.special_addr_valid = 0;
!   return id;
  }
  
  int
  frame_id_p (struct frame_id l)
  {
    int p;
!   /* The frame is valid iff it has a valid stack address.  */
!   p = l.stack_addr_valid;
    if (frame_debug)
      {
        fprintf_unfiltered (gdb_stdlog, "{ frame_id_p (l=");
*************** int
*** 300,319 ****
  frame_id_eq (struct frame_id l, struct frame_id r)
  {
    int eq;
!   if (l.stack_addr == 0 || r.stack_addr == 0)
      /* Like a NaN, if either ID is invalid, the result is false.  */
      eq = 0;
    else if (l.stack_addr != r.stack_addr)
      /* If .stack addresses are different, the frames are different.  */
      eq = 0;
!   else if (l.code_addr == 0 || r.code_addr == 0)
!     /* A zero code addr is a wild card, always succeed.  */
      eq = 1;
    else if (l.code_addr != r.code_addr)
      /* If .code addresses are different, the frames are different.  */
      eq = 0;
!   else if (l.special_addr == 0 || r.special_addr == 0)
!     /* A zero special addr is a wild card (or unused), always succeed.  */
      eq = 1;
    else if (l.special_addr == r.special_addr)
      /* Frames are equal.  */
--- 323,342 ----
  frame_id_eq (struct frame_id l, struct frame_id r)
  {
    int eq;
!   if (!l.stack_addr_valid || !r.stack_addr_valid)
      /* Like a NaN, if either ID is invalid, the result is false.  */
      eq = 0;
    else if (l.stack_addr != r.stack_addr)
      /* If .stack addresses are different, the frames are different.  */
      eq = 0;
!   else if (!l.code_addr_valid || !r.code_addr_valid)
!     /* An invalid code addr is a wild card, always succeed.  */
      eq = 1;
    else if (l.code_addr != r.code_addr)
      /* If .code addresses are different, the frames are different.  */
      eq = 0;
!   else if (!l.special_addr_valid || !r.special_addr_valid)
!     /* An invalid special addr is a wild card (or unused), always succeed.  */
      eq = 1;
    else if (l.special_addr == r.special_addr)
      /* Frames are equal.  */
*************** int
*** 336,342 ****
  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
--- 359,365 ----
  frame_id_inner (struct frame_id l, struct frame_id r)
  {
    int inner;
!   if (!l.stack_addr_valid || !r.stack_addr_valid)
      /* Like NaN, any operation involving an invalid ID always fails.  */
      inner = 0;
    else
Index: gdb/frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.134
diff -c -p -r1.134 frame.h
*** gdb/frame.h	10 Jun 2004 13:22:05 -0000	1.134
--- gdb/frame.h	11 Jun 2004 16:10:13 -0000
*************** struct frame_id
*** 107,116 ****
       frames that do not change the stack but are still distinct and have 
       some form of distinct identifier (e.g. the ia64 which uses a 2nd 
       stack for registers).  This field is treated as unordered - i.e. will
!      not be used in frame ordering comparisons such as frame_id_inner().
!      A zero in this field will be treated as a wild-card when comparing
!      frames for equality.  */
    CORE_ADDR special_addr;
  };
  
  /* Methods for constructing and comparing Frame IDs.
--- 107,122 ----
       frames that do not change the stack but are still distinct and have 
       some form of distinct identifier (e.g. the ia64 which uses a 2nd 
       stack for registers).  This field is treated as unordered - i.e. will
!      not be used in frame ordering comparisons such as frame_id_inner().  */
    CORE_ADDR special_addr;
+ 
+   /* Flags to indicate the above fields have valid contents.  An invalid
+      stack address indicates a null frame, while invalid code and special
+      addresses are treated as wild cards that compare equal with every
+      address.  */
+   int stack_addr_valid : 1;
+   int code_addr_valid : 1;
+   int special_addr_valid : 1;
  };
  
  /* Methods for constructing and comparing Frame IDs.
*************** extern const struct frame_id null_frame_
*** 135,156 ****
  
  /* Construct a frame ID.  The first parameter is the frame's constant
     stack address (typically the outer-bound), and the second the
!    frame's constant code address (typically the entry point) (or zero,
!    to indicate a wild card).  The special identifier address is
!    defaulted to zero.  */
  extern struct frame_id frame_id_build (CORE_ADDR stack_addr,
  				       CORE_ADDR code_addr);
  
  /* Construct a special frame ID.  The first parameter is the frame's constant
     stack address (typically the outer-bound), the second is the
!    frame's constant code address (typically the entry point) (or zero,
!    to indicate a wild card), and the third parameter is the frame's
!    special identifier address (or zero to indicate a wild card or 
!    unused default).  */
  extern struct frame_id frame_id_build_special (CORE_ADDR stack_addr,
  					       CORE_ADDR code_addr,
  					       CORE_ADDR special_addr);
  
  /* Returns non-zero when L is a valid frame (a valid frame has a
     non-zero .base).  */
  extern int frame_id_p (struct frame_id l);
--- 141,164 ----
  
  /* Construct a frame ID.  The first parameter is the frame's constant
     stack address (typically the outer-bound), and the second the
!    frame's constant code address (typically the entry point).
!    The special identifier address is set to indicate a wild card.  */
  extern struct frame_id frame_id_build (CORE_ADDR stack_addr,
  				       CORE_ADDR code_addr);
  
  /* Construct a special frame ID.  The first parameter is the frame's constant
     stack address (typically the outer-bound), the second is the
!    frame's constant code address (typically the entry point),
!    and the third parameter is the frame's special identifier address. */
  extern struct frame_id frame_id_build_special (CORE_ADDR stack_addr,
  					       CORE_ADDR code_addr,
  					       CORE_ADDR special_addr);
  
+ /* Construct a wild card frame ID.  The parameter is the frame's constant
+    stack address (typically the outer-bound).  The code address as well
+    as the special identifier address are set to indicate wild cards.  */
+ extern struct frame_id frame_id_build_wild (CORE_ADDR stack_addr);
+ 
  /* Returns non-zero when L is a valid frame (a valid frame has a
     non-zero .base).  */
  extern int frame_id_p (struct frame_id l);

-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de