2002-12-03 Andrew Cagney <ac131313@redhat.com>

	* frame.h (get_frame_id): Convert to a function.
	(null_frame_id, frame_id_p): Declare.
	(frame_id_eq, frame_id_inner): Declare.
	(frame_id_build): New function.
	* frame.c (get_frame_id): Update.  Use null_frame_id.
	(frame_find_by_id): Rewrite using frame_id_p, frame_id_eq and
	frame_id_inner.
	(null_frame_id, frame_id_p): Define.
	(frame_id_eq, frame_id_inner): Define.
	(frame_id_build): New function.
	* varobj.c (varobj_create): Update.
	(varobj_update): Update.
	* valops.c (value_assign): Update.
	(new_root_variable): Update.
	* infrun.c (save_inferior_status): Update.
	* breakpoint.c (watch_command_1): Update.

7 files changed, 120 insertions, 34 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a5d5a93..e23b20e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,22 @@
+2002-12-03  Andrew Cagney  <ac131313@redhat.com>
+
+	* frame.h (get_frame_id): Convert to a function.
+	(null_frame_id, frame_id_p): Declare.
+	(frame_id_eq, frame_id_inner): Declare.
+	(frame_id_build): New function.
+	* frame.c (get_frame_id): Update.  Use null_frame_id.
+	(frame_find_by_id): Rewrite using frame_id_p, frame_id_eq and
+	frame_id_inner.
+	(null_frame_id, frame_id_p): Define.
+	(frame_id_eq, frame_id_inner): Define.
+	(frame_id_build): New function.
+	* varobj.c (varobj_create): Update.
+	(varobj_update): Update.
+	* valops.c (value_assign): Update.
+	(new_root_variable): Update.
+	* infrun.c (save_inferior_status): Update.
+	* breakpoint.c (watch_command_1): Update.
+
 2002-12-03  J. Brobecker  <brobecker@gnat.com>
 
 	* config/pa/tm-hppah.h (SNAP1): Remove unused macro.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index eda76c2..7418c41 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5400,7 +5400,7 @@ watch_command_1 (char *arg, int accessflag, int from_tty)
   if (frame)
     {
       prev_frame = get_prev_frame (frame);
-      get_frame_id (frame, &b->watchpoint_frame);
+      b->watchpoint_frame = get_frame_id (frame);
     }
   else
     {
diff --git a/gdb/frame.c b/gdb/frame.c
index 3fd5bb3..b4e2cfd 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -35,24 +35,64 @@
 #include "annotate.h"
 #include "language.h"
 
-/* Return a frame uniq ID that can be used to, later re-find the
+/* Return a frame uniq ID that can be used to, later, re-find the
    frame.  */
 
-void
-get_frame_id (struct frame_info *fi, struct frame_id *id)
+struct frame_id
+get_frame_id (struct frame_info *fi)
 {
   if (fi == NULL)
     {
-      id->base = 0;
-      id->pc = 0;
+      return null_frame_id;
     }
   else
     {
-      id->base = fi->frame;
-      id->pc = fi->pc;
+      struct frame_id id;
+      id.base = fi->frame;
+      id.pc = fi->pc;
+      return id;
     }
 }
 
+const struct frame_id null_frame_id; /* All zeros.  */
+
+struct frame_id
+frame_id_build (CORE_ADDR base, CORE_ADDR func_or_pc)
+{
+  struct frame_id id;
+  id.base = base;
+  id.pc = func_or_pc;
+  return id;
+}
+
+int
+frame_id_p (struct frame_id l)
+{
+  /* The .func can be NULL but the .base cannot.  */
+  return (l.base != 0);
+}
+
+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
+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);
+}
+
 struct frame_info *
 frame_find_by_id (struct frame_id id)
 {
@@ -60,29 +100,24 @@ frame_find_by_id (struct frame_id id)
 
   /* ZERO denotes the null frame, let the caller decide what to do
      about it.  Should it instead return get_current_frame()?  */
-  if (id.base == 0 && id.pc == 0)
+  if (!frame_id_p (id))
     return NULL;
 
   for (frame = get_current_frame ();
        frame != NULL;
        frame = get_prev_frame (frame))
     {
-      struct frame_id this;
-      get_frame_id (frame, &this);
-      if (INNER_THAN (this.base, id.base))
-	/* ``inner/current < frame < id.base''.  Keep looking along
-           the frame chain.  */
-	continue;
-      if (INNER_THAN (id.base, this.base))
-	/* ``inner/current < id.base < frame''.  Oops, gone past it.
-           Just give up.  */
+      struct frame_id this = get_frame_id (frame);
+      if (frame_id_eq (id, this))
+	/* An exact match.  */
+	return frame;
+      if (frame_id_inner (id, this))
+	/* Gone to far.  */
 	return NULL;
-      /* FIXME: cagney/2002-04-21: This isn't sufficient.  It should
-	 use id.pc / this.pc to check that the two frames belong to
-	 the same function.  Otherwise we'll do things like match
-	 dummy frames or mis-match frameless functions.  However,
-	 until someone notices, stick with the existing behavour.  */
-      return frame;
+      /* Either, we're not yet gone far enough out along the frame
+         chain (inner(this,id), or we're comparing frameless functions
+         (same .base, different .func, no test available).  Struggle
+         on until we've definitly gone to far.  */
     }
   return NULL;
 }
diff --git a/gdb/frame.h b/gdb/frame.h
index ab123b3..62b1ce3 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -31,8 +31,8 @@ struct frame_info;
 
 /* The frame object's ID.  This provides a per-frame unique identifier
    that can be used to relocate a `struct frame_info' after a target
-   resume or a frame cache destruct (assuming the target hasn't
-   unwound the stack past that frame - a problem handled elsewhere).  */
+   resume or a frame cache destruct.  It of course assumes that the
+   inferior hasn't unwound the stack past that frame.  */
 
 struct frame_id
 {
@@ -47,6 +47,38 @@ struct frame_id
   CORE_ADDR pc;
 };
 
+/* Methods for constructing and comparing Frame IDs.
+
+   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).  */
+
+/* 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);
+
+/* 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);
+
+/* Returns non-zero when L and R identify the same frame, or, if
+   either L or R have a zero .func, then the same frame base.  */
+extern int frame_id_eq (struct frame_id l, struct frame_id r);
+
+/* Returns non-zero when L is strictly inner-than R (they have
+   different frame .bases).  Neither L, nor R can be `null'.  See note
+   above about frameless functions.  */
+extern int frame_id_inner (struct frame_id l, struct frame_id r);
+
+
 /* For every stopped thread, GDB tracks two frames: current and
    selected.  Current frame is the inner most frame of the selected
    thread.  Selected frame is the one being examined by the the GDB
@@ -176,8 +208,9 @@ extern void find_frame_sal (struct frame_info *frame,
 extern CORE_ADDR get_frame_base (struct frame_info *);
 
 /* Return the per-frame unique identifer.  Can be used to relocate a
-   frame after a frame cache flush (and other similar operations).  */
-extern void get_frame_id (struct frame_info *fi, struct frame_id *id);
+   frame after a frame cache flush (and other similar operations).  If
+   FI is NULL, return the null_frame_id.  */
+extern struct frame_id get_frame_id (struct frame_info *fi);
 
 /* The frame's level: 0 for innermost, 1 for its caller, ...; or -1
    for an invalid frame).  */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index c6dda16..6998d41 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3856,7 +3856,7 @@ save_inferior_status (int restore_stack_info)
 
   inf_status->registers = regcache_dup (current_regcache);
 
-  get_frame_id (deprecated_selected_frame, &inf_status->selected_frame_id);
+  inf_status->selected_frame_id = get_frame_id (deprecated_selected_frame);
   return inf_status;
 }
 
diff --git a/gdb/valops.c b/gdb/valops.c
index c0b534f..b7d50e5 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -649,7 +649,7 @@ value_assign (struct value *toval, struct value *fromval)
 	/* Since modifying a register can trash the frame chain, we
            save the old frame and then restore the new frame
            afterwards.  */
-	get_frame_id (deprecated_selected_frame, &old_frame);
+	old_frame = get_frame_id (deprecated_selected_frame);
 
 	/* Figure out which frame this is in currently.  */
 	if (VALUE_LVAL (toval) == lval_register)
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 35275c9..0c9f048 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -487,7 +487,7 @@ varobj_create (char *objname,
          Since select_frame is so benign, just call it for all cases. */
       if (fi != NULL)
 	{
-	  get_frame_id (fi, &var->root->frame);
+	  var->root->frame = get_frame_id (fi);
 	  old_fi = deprecated_selected_frame;
 	  select_frame (fi);
 	}
@@ -898,7 +898,7 @@ varobj_update (struct varobj **varp, struct varobj ***changelist)
 
   /* Save the selected stack frame, since we will need to change it
      in order to evaluate expressions. */
-  get_frame_id (deprecated_selected_frame, &old_fid);
+  old_fid = get_frame_id (deprecated_selected_frame);
 
   /* Update the root variable. value_of_root can return NULL
      if the variable is no longer around, i.e. we stepped out of
@@ -1344,8 +1344,7 @@ new_root_variable (void)
   var->root->lang = NULL;
   var->root->exp = NULL;
   var->root->valid_block = NULL;
-  var->root->frame.base = 0;
-  var->root->frame.pc = 0;
+  var->root->frame = null_frame_id;
   var->root->use_selected_frame = 0;
   var->root->rootvar = NULL;