Re: RFA: frame id enhancement

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

[-- Attachment #1: Type: text/plain, Size: 4207 bytes --]

J. Johnston wrote:
> 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 :-)
>>

Is the revised attached patch ok?

-- Jeff J.

[-- Attachment #2: frame_special.patch --]
[-- Type: text/plain, Size: 6014 bytes --]

Index: frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.110
diff -u -r1.110 frame.h
--- frame.h	10 Oct 2003 00:32:04 -0000	1.110
+++ frame.h	16 Oct 2003 23:30:50 -0000
@@ -95,8 +95,6 @@
      is used.  Watch out for all the legacy targets that still use the
      function pointer register or stack pointer register.  They are
      wrong.  */
-  /* NOTE: cagney/2002-11-16: The ia64 has two stacks and hence two
-     frame bases.  This will need to be expanded to accomodate that.  */
   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)
@@ -104,15 +102,33 @@
      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.
 
-   NOTE: Given frameless functions A and B, where A calls B (and hence
+   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 frameless, they
-   have the same .base value).  */
+   !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 and/or .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.  */
 
 /* For convenience.  All fields are zero.  */
 extern const struct frame_id null_frame_id;
@@ -120,9 +136,20 @@
 /* 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).  */
+   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).  */
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.145
diff -u -r1.145 frame.c
--- frame.c	2 Oct 2003 20:28:29 -0000	1.145
+++ frame.c	16 Oct 2003 23:30:51 -0000
@@ -144,9 +144,10 @@
 void
 fprint_frame_id (struct ui_file *file, struct frame_id id)
 {
-  fprintf_unfiltered (file, "{stack=0x%s,code=0x%s}",
+  fprintf_unfiltered (file, "{stack=0x%s,code=0x%s,special=0x%s}",
 		      paddr_nz (id.stack_addr),
-		      paddr_nz (id.code_addr));
+		      paddr_nz (id.code_addr),
+		      paddr_nz (id.special_addr));
 }
 
 static void
@@ -256,14 +257,22 @@
 const struct frame_id null_frame_id; /* All zeros.  */
 
 struct frame_id
-frame_id_build (CORE_ADDR stack_addr, CORE_ADDR code_addr)
+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)
 {
@@ -292,8 +301,14 @@
   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)
-    /* The .stack and .code are identical, the ID's are identical.  */
+  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.  */
     eq = 1;
   else
     /* No luck.  */
@@ -320,7 +335,7 @@
     /* 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).  */
+       different .code and/or .special address).  */
     inner = INNER_THAN (l.stack_addr, r.stack_addr);
   if (frame_debug)
     {