[commit] frame ID cleanup

[Andrew Cagney <ac131313 at redhat dot com>]

Hello,

Attached is a cleanup to the frame ID related code.  It:

- renames "pc" -> "code_addr"; "base" -> "stack_addr"
Names like "pc" and "base" come with heaps of baggage; the rename gets 
way from that, putting some needed distance between them and a frame ID.

- reworks frame_id_eq(), frame_id_p() and frame_id_inner()
The new code better handles edge cases such as frame_id_eq() being 
called with an invalid ID.  It also makes the way for a follow-on patch 
that will significantly increase the output from `set debug frame 1'.

committed,
Andrew

2003-04-09  Andrew Cagney  <cagney at redhat dot com>

	* frame.h (struct frame_id): Replace "pc" and "base" with
	"stack_addr" and "code_addr".  Update comments.
	(frame_id_build): Update parameter names and comment.
	(struct frame_info): Replace "id_p" and "id" with "this_id".
	* dummy-frame.c (dummy_frame_this_id): Update.
	* breakpoint.c (print_one_breakpoint): Update.
	* frame.c (get_frame_id): Update.
	(get_frame_base, frame_id_build): Update.
	(create_sentinel_frame, legacy_get_prev_frame): Update.
	(deprecated_update_frame_base_hack): Update.
	(frame_id_p, frame_id_eq): Rework, return 0 when an invalid ID.
	(frame_id_inner): Ditto.

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.117
diff -u -r1.117 breakpoint.c
--- breakpoint.c	31 Mar 2003 19:01:18 -0000	1.117
+++ breakpoint.c	10 Apr 2003 03:08:06 -0000
@@ -3418,7 +3418,7 @@
       ui_out_text (uiout, "\tstop only in stack frame at ");
       /* FIXME: cagney/2002-12-01: Shouldn't be poeking around inside
          the frame ID.  */
-      ui_out_field_core_addr (uiout, "frame", b->frame_id.base);
+      ui_out_field_core_addr (uiout, "frame", b->frame_id.stack_addr);
       ui_out_text (uiout, "\n");
     }
   
Index: dummy-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dummy-frame.c,v
retrieving revision 1.17
diff -u -r1.17 dummy-frame.c
--- dummy-frame.c	5 Apr 2003 03:55:59 -0000	1.17
+++ dummy-frame.c	10 Apr 2003 03:08:13 -0000
@@ -397,7 +397,8 @@
       (*this_id) = null_frame_id;
       return;
     }
-  (*this_prologue_cache) = find_dummy_frame ((*this_id).pc, (*this_id).base);
+  (*this_prologue_cache) = find_dummy_frame ((*this_id).code_addr,
+					     (*this_id).stack_addr);
 }
 
 static struct frame_unwind dummy_frame_unwind =
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.103
diff -u -r1.103 frame.c
--- frame.c	9 Apr 2003 20:49:22 -0000	1.103
+++ frame.c	10 Apr 2003 03:08:50 -0000
@@ -58,53 +58,74 @@
     {
       return null_frame_id;
     }
-  if (!fi->id_p)
+  if (!fi->this_id.p)
     {
       gdb_assert (!legacy_frame_p (current_gdbarch));
       /* Find THIS frame's ID.  */
-      fi->unwind->this_id (fi->next, &fi->prologue_cache, &fi->id);
-      fi->id_p = 1;
+      fi->unwind->this_id (fi->next, &fi->prologue_cache, &fi->this_id.value);
+      fi->this_id.p = 1;
     }
-  return frame_id_build (fi->id.base, get_frame_pc (fi));
+  return frame_id_build (fi->this_id.value.stack_addr, get_frame_pc (fi));
 }
 
 const struct frame_id null_frame_id; /* All zeros.  */
 
 struct frame_id
-frame_id_build (CORE_ADDR base, CORE_ADDR func_or_pc)
+frame_id_build (CORE_ADDR stack_addr, CORE_ADDR code_addr)
 {
   struct frame_id id;
-  id.base = base;
-  id.pc = func_or_pc;
+  id.stack_addr = stack_addr;
+  id.code_addr = code_addr;
   return id;
 }
 
 int
 frame_id_p (struct frame_id l)
 {
-  /* The .func can be NULL but the .base cannot.  */
-  return (l.base != 0);
+  int p;
+  /* The .code can be NULL but the .stack cannot.  */
+  p = (l.stack_addr != 0);
+  return p;
 }
 
 int
 frame_id_eq (struct frame_id l, struct frame_id r)
 {
-  /* If .base is different, the frames are different.  */
-  if (l.base != r.base)
-    return 0;
-  /* Add a test to check that the frame ID's are for the same function
-     here.  */
-  return 1;
+  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)
+    /* The .stack and .code are identical, the ID's are identical.  */
+    eq = 1;
+  else
+    /* FIXME: cagney/2003-04-06: This should be zero.  Can't yet do
+       this because most frame ID's are not being initialized
+       correctly.  */
+    eq = 1;
+  return eq;
 }
 
 int
 frame_id_inner (struct frame_id l, struct frame_id r)
 {
-  /* 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 .base but different
-     .func).  */
-  return INNER_THAN (l.base, r.base);
+  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);
+  return inner;
 }
 
 struct frame_info *
@@ -520,8 +541,8 @@
   frame->next = frame;
   /* Make the sentinel frame's ID valid, but invalid.  That way all
      comparisons with it should fail.  */
-  frame->id_p = 1;
-  frame->id = null_frame_id;
+  frame->this_id.p = 1;
+  frame->this_id.value = null_frame_id;
   return frame;
 }
 
@@ -1069,7 +1090,7 @@
   prev->type = UNKNOWN_FRAME;
 
   /* A legacy frame's ID is always computed here.  Mark it as valid.  */
-  prev->id_p = 1;
+  prev->this_id.p = 1;
 
   /* Handle sentinel frame unwind as a special case.  */
   if (this_frame->level < 0)
@@ -1132,7 +1153,8 @@
 	     dummy ID from the next frame.  Note that this method uses
 	     frame_register_unwind to obtain the register values
 	     needed to determine the dummy frame's ID.  */
-	  prev->id = gdbarch_unwind_dummy_id (current_gdbarch, this_frame);
+	  prev->this_id.value = gdbarch_unwind_dummy_id (current_gdbarch,
+							 this_frame);
 	}
       else
 	{
@@ -1141,11 +1163,11 @@
 	     using the same sequence as is found a traditional
 	     unwinder.  Once all architectures supply the
 	     unwind_dummy_id method, this code can go away.  */
-	  prev->id = frame_id_build (read_fp (), read_pc ());
+	  prev->this_id.value = frame_id_build (read_fp (), read_pc ());
 	}
 
       /* Check that the unwound ID is valid.  */
-      if (!frame_id_p (prev->id))
+      if (!frame_id_p (prev->this_id.value))
 	{
 	  if (frame_debug)
 	    fprintf_unfiltered (gdb_stdlog,
@@ -1664,7 +1686,7 @@
 CORE_ADDR
 get_frame_base (struct frame_info *fi)
 {
-  return get_frame_id (fi).base;
+  return get_frame_id (fi).stack_addr;
 }
 
 /* High-level offsets into the frame.  Used by the debug info.  */
@@ -1786,7 +1808,7 @@
 deprecated_update_frame_base_hack (struct frame_info *frame, CORE_ADDR base)
 {
   /* See comment in "frame.h".  */
-  frame->id.base = base;
+  frame->this_id.value.stack_addr = base;
 }
 
 void
Index: frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.88
diff -u -r1.88 frame.h
--- frame.h	9 Apr 2003 21:53:28 -0000	1.88
+++ frame.h	10 Apr 2003 03:08:55 -0000
@@ -44,15 +44,24 @@
 
 struct frame_id
 {
-  /* The frame's address.  This should be constant through out the
-     lifetime of a frame.  */
+  /* The frame's stack address.  This shall be constant through out
+     the lifetime of a frame.  Note that this requirement applies to
+     not just the function body, but also the prologue and (in theory
+     at least) the epilogue.  Since that value needs to fall either on
+     the boundary, or within the frame's address range, the frame's
+     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.  */
   /* 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 base;
-  /* The frame's current PC.  While the PC within the function may
-     change, the function that contains the PC does not.  Should this
-     instead be the frame's function?  */
-  CORE_ADDR pc;
+  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;
 };
 
 /* Methods for constructing and comparing Frame IDs.
@@ -66,12 +75,12 @@
 /* For convenience.  All fields are zero.  */
 extern const struct frame_id null_frame_id;
 
-/* Construct a frame ID.  The second parameter isn't yet well defined.
-   It might be the containing function, or the resume PC (see comment
-   above in `struct frame_id')?  A func/pc of zero indicates a
-   wildcard (i.e., do not use func in frame ID comparisons).  */
-extern struct frame_id frame_id_build (CORE_ADDR base,
-				       CORE_ADDR func_or_pc);
+/* 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).  */
+extern struct frame_id frame_id_build (CORE_ADDR stack_addr,
+				       CORE_ADDR code_addr);
 
 /* Returns non-zero when L is a valid frame (a valid frame has a
    non-zero .base).  */
@@ -397,10 +406,12 @@
       int p;
     } prev_func;
 
-    /* This frame's ID.  Note that the frame's ID, base and PC contain
-       redundant information.  */
-    int id_p;
-    struct frame_id id;
+    /* This frame's ID.  */
+    struct
+    {
+      int p;
+      struct frame_id value;
+    } this_id;
 
     /* The frame's high-level base methods, and corresponding cache.
        The high level base methods are selected based on the frame's