[PATCH] Fix frame ID comparison problem on s390

[PATCH] Fix frame ID comparison problem on s390

[Ulrich Weigand]

Hello,

even after the siglongjmp patch, there was still one failure reported
with the signull test case on s390(x).

The reason turned out to be a problem with frame ID comparison.  In
the back trace we have
  handler
  <sigtramp>
  <NULL call>
  caller

As the NULL call doesn't set up a stack frame, and because our CFA is
determined by stack pointer at function entry, this means that the 
NULL call frame and the sigtramp frame have the same 'stack_addr'
component of their respective frame IDs.

Furthermore, the NULL call frame has 0 as 'code_addr' component of the
frame ID, because the current PC is in fact 0.

Due to the way frame ID comparison works, this causes the two IDs to
compare equal: the stack_addr is equal, and a zero code_addr is
considered a wild card matching any code_addr.

This in turn causes the backtrace to abort since it encounters two
frames with the same frame ID ...

The patch below is a simple fix (cheating a bit ...) in the s390 back
end: we simply set the code_addr component to 1 if the PC is 0.  (Note
that this can never happen normally because code addresses are always
2-aligned on our platform.)

Tested on s390-ibm-linux and s390x-ibm-linux, fixes one test suite
failure.

OK?

ChangeLog:

	* s390-tdep.c (s390_stub_frame_this_id): Set code_addr part of ID
	to 1 if the PC is 0.

Index: gdb/s390-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/s390-tdep.c,v
retrieving revision 1.132
diff -c -p -r1.132 s390-tdep.c
*** gdb/s390-tdep.c	4 May 2004 18:50:26 -0000	1.132
--- gdb/s390-tdep.c	20 May 2004 13:23:44 -0000
*************** s390_stub_frame_this_id (struct frame_in
*** 2078,2086 ****
  			 void **this_prologue_cache,
  			 struct frame_id *this_id)
  {
    struct s390_stub_unwind_cache *info
      = s390_stub_frame_unwind_cache (next_frame, this_prologue_cache);
!   *this_id = frame_id_build (info->frame_base, frame_pc_unwind (next_frame));
  }
  
  static void
--- 2078,2089 ----
  			 void **this_prologue_cache,
  			 struct frame_id *this_id)
  {
+   CORE_ADDR pc = frame_pc_unwind (next_frame);
    struct s390_stub_unwind_cache *info
      = s390_stub_frame_unwind_cache (next_frame, this_prologue_cache);
!   /* For 'call via NULL function pointer' stubs we use 1 as code address
!      in order to avoid the '0 matches everything' special case.  */
!   *this_id = frame_id_build (info->frame_base, pc? pc : 1);
  }
  
  static void
-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de

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

[Daniel Jacobowitz]

On Thu, May 20, 2004 at 03:31:56PM +0200, Ulrich Weigand wrote:
> Hello,
> 
> even after the siglongjmp patch, there was still one failure reported
> with the signull test case on s390(x).
> 
> The reason turned out to be a problem with frame ID comparison.  In
> the back trace we have
>   handler
>   <sigtramp>
>   <NULL call>
>   caller
> 
> As the NULL call doesn't set up a stack frame, and because our CFA is
> determined by stack pointer at function entry, this means that the 
> NULL call frame and the sigtramp frame have the same 'stack_addr'
> component of their respective frame IDs.
> 
> Furthermore, the NULL call frame has 0 as 'code_addr' component of the
> frame ID, because the current PC is in fact 0.
> 
> Due to the way frame ID comparison works, this causes the two IDs to
> compare equal: the stack_addr is equal, and a zero code_addr is
> considered a wild card matching any code_addr.
> 
> This in turn causes the backtrace to abort since it encounters two
> frames with the same frame ID ...

I think the only reason this doesn't happen on i386 is that we appear
to use the SP at function entry rather than the SP at the call site,
and there's an implicit push of the return address.

Rather than cheat in the backend - most other backends will probably
have the same issue - I'd like to know what's actually using the code
wildcard.

-- 
Daniel Jacobowitz

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

[Ulrich Weigand]

Daniel Jacobowitz wrote:

> Rather than cheat in the backend - most other backends will probably
> have the same issue - I'd like to know what's actually using the code
> wildcard.

I have no idea -- maybe this is obsolete by now?

The following patch simply removes the wild card feature, and it works
for s390 without test suite regressions (and fixes the signull failure
as well).

I've tried to look at the other platforms, but they appear not to be
using the wild card feature either ...

OK?

Bye,
Ulrich


ChangeLog:

	* frame.c (frame_id_eq): Do not treat zero code addr as wild card.
	* frame.h (frame_id_build, frame_id_build_special): Update comment.

Index: gdb/frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.181
diff -c -p -r1.181 frame.c
*** gdb/frame.c	8 May 2004 19:03:04 -0000	1.181
--- gdb/frame.c	24 May 2004 13:14:02 -0000
*************** frame_id_eq (struct frame_id l, struct f
*** 306,314 ****
    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;
--- 306,311 ----
Index: gdb/frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.133
diff -c -p -r1.133 frame.h
*** gdb/frame.h	7 May 2004 23:19:14 -0000	1.133
--- gdb/frame.h	24 May 2004 13:14:02 -0000
*************** extern const struct frame_id null_frame_
*** 135,152 ****
  
  /* 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);
--- 135,150 ----
  
  /* 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 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),
!    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);


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

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

[Andrew Cagney]

Daniel Jacobowitz wrote:


Rather than cheat in the backend - most other backends will probably
have the same issue - I'd like to know what's actually using the code
wildcard.


I have no idea -- maybe this is obsolete by now?

The following patch simply removes the wild card feature, and it works
for s390 without test suite regressions (and fixes the signull failure
as well).
I've tried to look at the other platforms, but they appear not to be
using the wild card feature either ...
Symbol table code often returns 0 to indicate a failed lookup (here a 
search for the function containing pc).  That zero can end up in the 
frame ID.  Look at calls to get_frame_func / frame_func_unwind (which 
I've proposed eliminating).

From memory architectures that do not implement dummy ID unwind also 
end up with wild-card IDs (fortunatly the dummy-frame code works around 
this).

Broken tramp unwinders often leave the .code address zero (see paragraph 
#1 for why).

Andrew

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

[Ulrich Weigand]

Andrew Cagney wrote:

> Symbol table code often returns 0 to indicate a failed lookup (here a 
> search for the function containing pc).  That zero can end up in the 
> frame ID.  Look at calls to get_frame_func / frame_func_unwind (which 
> I've proposed eliminating).
> 
>  From memory architectures that do not implement dummy ID unwind also 
> end up with wild-card IDs (fortunatly the dummy-frame code works around 
> this).
> 
> Broken tramp unwinders often leave the .code address zero (see paragraph 
> #1 for why).

So, what would you recommend to solve the problem of 'wildcard zero pc'
being confused with 'NULL pointer call'?  Is my original back-end hack
OK with or, or do you have another target-independent suggestion?

Thanks,
Ulrich

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

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

[Andrew Cagney]

> Andrew Cagney wrote:
> 
> 
>>> Symbol table code often returns 0 to indicate a failed lookup (here a 
>>> search for the function containing pc).  That zero can end up in the 
>>> frame ID.  Look at calls to get_frame_func / frame_func_unwind (which 
>>> I've proposed eliminating).
>>> 
>>>  From memory architectures that do not implement dummy ID unwind also 
>>> end up with wild-card IDs (fortunatly the dummy-frame code works around 
>>> this).
>>> 
>>> Broken tramp unwinders often leave the .code address zero (see paragraph 
>>> #1 for why).
> 
> 
> So, what would you recommend to solve the problem of 'wildcard zero pc'
> being confused with 'NULL pointer call'?  Is my original back-end hack
> OK with or, or do you have another target-independent suggestion?

No. Not really.

The wild card mechanism is needed (for when the function entry point 
address isn't known) but it needs to be made more robust.  One step on 
that path would be to extend the build interface:

> /* 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);

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.

Andrew

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

[Ulrich Weigand]

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

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

[Andrew Cagney]

> 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

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

[Ulrich Weigand]

Andrew Cagney wrote:

> with those changes its ok,

Thanks; here's the version I finally committed.

(Sorry it took so long, I got sidetracked by a kernel bug triggered
by running the gdb test suite that caused random kernel memory 
corruption :-/)

Bye,
Ulrich


ChangeLog:

        * frame.h (struct frame_id): New fields stack_addr_p, code_addr_p
	and special_addr_p.
        (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	16 Jun 2004 18:53:20 -0000
*************** struct frame_id
*** 268,292 ****
  frame_id_build_special (CORE_ADDR stack_addr, CORE_ADDR code_addr,
                          CORE_ADDR special_addr)
  {
!   struct frame_id id;
    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=");
--- 268,309 ----
  frame_id_build_special (CORE_ADDR stack_addr, CORE_ADDR code_addr,
                          CORE_ADDR special_addr)
  {
!   struct frame_id id = null_frame_id;
    id.stack_addr = stack_addr;
+   id.stack_addr_p = 1;
    id.code_addr = code_addr;
+   id.code_addr_p = 1;
    id.special_addr = special_addr;
+   id.special_addr_p = 1;
    return id;
  }
  
  struct frame_id
  frame_id_build (CORE_ADDR stack_addr, CORE_ADDR code_addr)
  {
!   struct frame_id id = null_frame_id;
!   id.stack_addr = stack_addr;
!   id.stack_addr_p = 1;
!   id.code_addr = code_addr;
!   id.code_addr_p = 1;
!   return id;
! }
! 
! struct frame_id
! frame_id_build_wild (CORE_ADDR stack_addr)
! {
!   struct frame_id id = null_frame_id;
!   id.stack_addr = stack_addr;
!   id.stack_addr_p = 1;
!   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_p;
    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.  */
--- 317,337 ----
  frame_id_eq (struct frame_id l, struct frame_id r)
  {
    int eq;
!   if (!l.stack_addr_p || !r.stack_addr_p)
!     /* Like a NaN, if either ID is invalid, the result is false.
!        Note that a frame ID is invalid iff it is the null frame ID.  */
      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_p || !r.code_addr_p)
!     /* 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_p || !r.special_addr_p)
!     /* 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
--- 354,360 ----
  frame_id_inner (struct frame_id l, struct frame_id r)
  {
    int inner;
!   if (!l.stack_addr_p || !r.stack_addr_p)
      /* 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	16 Jun 2004 18:53:20 -0000
*************** struct frame_id
*** 94,116 ****
       outer-most address (the inner-most address of the previous frame)
       is used.  Watch out for all the legacy targets that still use the
       function pointer register or stack pointer register.  They are
!      wrong.  */
    CORE_ADDR stack_addr;
    /* The frame's code address.  This shall be constant through out the
       lifetime of the frame.  While the PC (a.k.a. resume address)
       changes as the function is executed, this code address cannot.
       Typically, it is set to the address of the entry point of the
!      frame's function (as returned by frame_func_unwind().  */
    CORE_ADDR code_addr;
    /* 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 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.
--- 94,132 ----
       outer-most address (the inner-most address of the previous frame)
       is used.  Watch out for all the legacy targets that still use the
       function pointer register or stack pointer register.  They are
!      wrong.
! 
!      This field is valid only if stack_addr_p is true.  Otherwise, this
!      frame represents the null frame.  */
    CORE_ADDR stack_addr;
+ 
    /* The frame's code address.  This shall be constant through out the
       lifetime of the frame.  While the PC (a.k.a. resume address)
       changes as the function is executed, this code address cannot.
       Typically, it is set to the address of the entry point of the
!      frame's function (as returned by frame_func_unwind().  
! 
!      This field is valid only if code_addr_p is true.  Otherwise, this
!      frame is considered to have a wildcard code address, i.e. one that
!      matches every address value in frame comparisons.  */
    CORE_ADDR code_addr;
+ 
    /* 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 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().
! 
!      This field is valid only if special_addr_p is true.  Otherwise, this
!      frame is considered to have a wildcard special address, i.e. one that
!      matches every address value in frame comparisons.  */
    CORE_ADDR special_addr;
+ 
+   /* Flags to indicate the above fields have valid contents.  */
+   int stack_addr_p : 1;
+   int code_addr_p : 1;
+   int special_addr_p : 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);
--- 151,174 ----
  
  /* 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

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

[Daniel Jacobowitz]

On Sun, Jun 27, 2004 at 10:47:58PM +0200, Ulrich Weigand wrote:
> Andrew Cagney wrote:
> 
> > with those changes its ok,
> 
> Thanks; here's the version I finally committed.
> 
> (Sorry it took so long, I got sidetracked by a kernel bug triggered
> by running the gdb test suite that caused random kernel memory 
> corruption :-/)

Thanks!  Just one small nit.  If you're going to do this:

> +   id.stack_addr_p = 1;
>     id.code_addr = code_addr;
> +   id.code_addr_p = 1;
>     id.special_addr = special_addr;
> +   id.special_addr_p = 1;

Then these should be "unsigned int : 1":

> +   /* Flags to indicate the above fields have valid contents.  */
> +   int stack_addr_p : 1;
> +   int code_addr_p : 1;
> +   int special_addr_p : 1;
>   };
>   
>   /* Methods for constructing and comparing Frame IDs.

Some future version of GCC will warn about this mistake.  The only
valid values for "int : 1" are 0 and -1.

-- 
Daniel Jacobowitz

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

[Ulrich Weigand]

Daniel Jacobowitz wrote:

> Some future version of GCC will warn about this mistake.  The only
> valid values for "int : 1" are 0 and -1.

Good point, I had forgotten about this.  B.t.w. according to C99,
it is actually implementation-defined whether a bit field type 'int'
is signed or unsigned.  (Which still means it is a good idea to
explicitly use 'unsigned int', of course.)

Fixed by the following patch.  Tested on s390-ibm-linux and
s390x-ibm-linux, committed to mainline as obvious.

Bye,
Ulrich


ChangeLog:

	* frame.h (struct frame_id): Change bit field type of stack_addr_p, 
	code_addr_p and special_addr_p to 'unsigned int'.

Index: gdb/frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.135
diff -c -p -r1.135 frame.h
*** gdb/frame.h	27 Jun 2004 20:45:05 -0000	1.135
--- gdb/frame.h	27 Jun 2004 22:12:00 -0000
*************** struct frame_id
*** 124,132 ****
    CORE_ADDR special_addr;
  
    /* Flags to indicate the above fields have valid contents.  */
!   int stack_addr_p : 1;
!   int code_addr_p : 1;
!   int special_addr_p : 1;
  };
  
  /* Methods for constructing and comparing Frame IDs.
--- 124,132 ----
    CORE_ADDR special_addr;
  
    /* Flags to indicate the above fields have valid contents.  */
!   unsigned int stack_addr_p : 1;
!   unsigned int code_addr_p : 1;
!   unsigned int special_addr_p : 1;
  };
  
  /* Methods for constructing and comparing Frame IDs.


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

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

[Andreas Schwab]

Daniel Jacobowitz <drow@false.org> writes:

>> +   /* Flags to indicate the above fields have valid contents.  */
>> +   int stack_addr_p : 1;
>> +   int code_addr_p : 1;
>> +   int special_addr_p : 1;
>>   };
>>   
>>   /* Methods for constructing and comparing Frame IDs.
>
> Some future version of GCC will warn about this mistake.  The only
> valid values for "int : 1" are 0 and -1.

Actually it can be both 0/-1 and 0/1, since it is implementation defined
whether unadorned int bitfields are signed or unsigned.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

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

[Daniel Jacobowitz]

On Mon, Jun 28, 2004 at 12:53:43AM +0200, Andreas Schwab wrote:
> Daniel Jacobowitz <drow@false.org> writes:
> 
> >> +   /* Flags to indicate the above fields have valid contents.  */
> >> +   int stack_addr_p : 1;
> >> +   int code_addr_p : 1;
> >> +   int special_addr_p : 1;
> >>   };
> >>   
> >>   /* Methods for constructing and comparing Frame IDs.
> >
> > Some future version of GCC will warn about this mistake.  The only
> > valid values for "int : 1" are 0 and -1.
> 
> Actually it can be both 0/-1 and 0/1, since it is implementation defined
> whether unadorned int bitfields are signed or unsigned.

Huh, thank you.  In any case, thanks for making this explicit, Ulrich.

-- 
Daniel Jacobowitz