[PATCH 1/4] Distinguish sentinel frame from null frame

[Kevin Buettner <kevin at buettner dot to>]

Distinguish sentinel frame from null frame.

This patch replaces the `current_frame' static global in frame.c with
`sentinel_frame'.  It also makes the sentinel frame id unique and
different from the null frame.

By itself, there is not much point to this patch, but it makes
the code cleaner for the VALUE_FRAME_ID changes in another patch.
Since we now allow "navigation" to the sentinel frame, it removes
the necessity of adding special cases to other parts of GDB.

Note that a new function, get_next_frame_sentinel_okay, is introduced
in this patch.  It will be used by the VALUE_FRAME_ID changes that
I've made.

Thanks to Pedro Alves for this suggestion.

gdb/ChangeLog:
    
	* frame.h (enum frame_id_stack_status): Add FID_STACK_SENTINEL.
	(struct frame_id): Increase number of bits required for storing
	stack status to 3 from 2.
	(sentinel_frame_id): New declaration.
	(get_next_frame_sentinel_okay): Declare.
	(frame_find_by_id_sentinel_okay): Declare.
	* frame.c (current_frame): Rename this static global to...
	(sentinel_frame): ...this static global, which has also been
	moved an earlier location in the file.
	(fprint_frame_id): Add case for sentinel frame id.
	(get_frame_id): Return early for sentinel frame.
	(sentinel_frame_id): Define.
	(frame_find_by_id): Add case for sentinel_frame_id.
	(create_sentinel_frame): Use sentinel_frame_id for this_id.value
	instead of null_frame_id.
	(get_current_frame): Add local declaration for `current_frame'.
	Remove local declaration for `sentinel_frame.'  Add new_local,
	new_sentinel_p, and use it for knowing when to call get_frame_id().
	(get_next_frame_sentinel_okay): New function.
	(reinit_frame_cache): Use `sentinel_frame' in place of
	`current_frame'.
---
 gdb/frame.c | 101 +++++++++++++++++++++++++++++++++++++++++-------------------
 gdb/frame.h |  12 +++++++-
 2 files changed, 81 insertions(+), 32 deletions(-)

diff --git a/gdb/frame.c b/gdb/frame.c
index d3f3056..3ca8ab2 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -43,6 +43,17 @@
 #include "hashtab.h"
 #include "valprint.h"
 
+/* The sentinel frame is the innermost frame.  It generally does not
+   have any stack associated with it.  Register values may be directly
+   determined by inspection with no unwinding necessary.
+
+   The sentinel frame is constructed so that the next frame is a pointer
+   to the sentinel frame itself.
+
+   The current frame can be found at sentinel_frame->prev.  */
+
+static struct frame_info *sentinel_frame;
+
 static struct frame_info *get_prev_frame_raw (struct frame_info *this_frame);
 static const char *frame_stop_reason_symbol_string (enum unwind_stop_reason reason);
 
@@ -65,7 +76,7 @@ enum cached_copy_status
 
 /* We keep a cache of stack frames, each of which is a "struct
    frame_info".  The innermost one gets allocated (in
-   wait_for_inferior) each time the inferior stops; current_frame
+   wait_for_inferior) each time the inferior stops; sentinel_frame
    points to it.  Additional frames get allocated (in get_prev_frame)
    as needed, and are chained through the next and prev fields.  Any
    time that the frame cache becomes invalid (most notably when we
@@ -323,6 +334,8 @@ fprint_frame_id (struct ui_file *file, struct frame_id id)
     fprintf_unfiltered (file, "!stack");
   else if (id.stack_status == FID_STACK_UNAVAILABLE)
     fprintf_unfiltered (file, "stack=<unavailable>");
+  else if (id.stack_status == FID_STACK_SENTINEL)
+    fprintf_unfiltered (file, "stack=<sentinel>");
   else
     fprintf_unfiltered (file, "stack=%s", hex_string (id.stack_addr));
   fprintf_unfiltered (file, ",");
@@ -511,6 +524,9 @@ get_frame_id (struct frame_info *fi)
   if (fi == NULL)
     return null_frame_id;
 
+  if (fi == sentinel_frame)
+    return sentinel_frame_id;
+
   if (!fi->this_id.p)
     {
       int stashed;
@@ -562,6 +578,7 @@ frame_unwind_caller_id (struct frame_info *next_frame)
 }
 
 const struct frame_id null_frame_id = { 0 }; /* All zeros.  */
+const struct frame_id sentinel_frame_id = { 0, 0, 0, FID_STACK_SENTINEL, 0, 1, 0 };
 const struct frame_id outer_frame_id = { 0, 0, 0, FID_STACK_INVALID, 0, 1, 0 };
 
 struct frame_id
@@ -797,6 +814,10 @@ frame_find_by_id (struct frame_id id)
   if (!frame_id_p (id))
     return NULL;
 
+  /* Check for the sentinel frame.  */
+  if (frame_id_eq (id, sentinel_frame_id))
+    return sentinel_frame;
+
   /* Try using the frame stash first.  Finding it there removes the need
      to perform the search by looping over all frames, which can be very
      CPU-intensive if the number of frames is very high (the loop is O(n)
@@ -1474,10 +1495,9 @@ create_sentinel_frame (struct program_space *pspace, struct regcache *regcache)
   /* Link this frame back to itself.  The frame is self referential
      (the unwound PC is the same as the pc), so make it so.  */
   frame->next = frame;
-  /* Make the sentinel frame's ID valid, but invalid.  That way all
-     comparisons with it should fail.  */
+  /* The sentinel frame has a special ID.  */
   frame->this_id.p = 1;
-  frame->this_id.value = null_frame_id;
+  frame->this_id.value = sentinel_frame_id;
   if (frame_debug)
     {
       fprintf_unfiltered (gdb_stdlog, "{ create_sentinel_frame (...) -> ");
@@ -1487,10 +1507,6 @@ create_sentinel_frame (struct program_space *pspace, struct regcache *regcache)
   return frame;
 }
 
-/* Info about the innermost stack frame (contents of FP register).  */
-
-static struct frame_info *current_frame;
-
 /* Cache for frame addresses already read by gdb.  Valid only while
    inferior is stopped.  Control variables for the frame cache should
    be local to this module.  */
@@ -1511,6 +1527,9 @@ static struct frame_info *get_prev_frame_always_1 (struct frame_info *this_frame
 struct frame_info *
 get_current_frame (void)
 {
+  struct frame_info *current_frame;
+  int new_sentinel_p = 0;
+
   /* First check, and report, the lack of registers.  Having GDB
      report "No stack!" or "No memory" when the target doesn't even
      have registers is very confusing.  Besides, "printcmd.exp"
@@ -1526,30 +1545,37 @@ get_current_frame (void)
   if (get_traceframe_number () < 0)
     validate_registers_access ();
 
-  if (current_frame == NULL)
+  if (sentinel_frame == NULL)
     {
-      int stashed;
-      struct frame_info *sentinel_frame =
+      sentinel_frame =
 	create_sentinel_frame (current_program_space, get_current_regcache ());
-
-      /* Set the current frame before computing the frame id, to avoid
-	 recursion inside compute_frame_id, in case the frame's
-	 unwinder decides to do a symbol lookup (which depends on the
-	 selected frame's block).
-
-	 This call must always succeed.  In particular, nothing inside
-	 get_prev_frame_always_1 should try to unwind from the
-	 sentinel frame, because that could fail/throw, and we always
-	 want to leave with the current frame created and linked in --
-	 we should never end up with the sentinel frame as outermost
-	 frame.  */
-      current_frame = get_prev_frame_always_1 (sentinel_frame);
-      gdb_assert (current_frame != NULL);
-
-      /* No need to compute the frame id yet.  That'll be done lazily
-	 from inside get_frame_id instead.  */
+      new_sentinel_p = 1;
     }
 
+  /* Set the current frame before computing the frame id, to avoid
+     recursion inside compute_frame_id, in case the frame's
+     unwinder decides to do a symbol lookup (which depends on the
+     selected frame's block).
+
+     This call must always succeed.  In particular, nothing inside
+     get_prev_frame_always_1 should try to unwind from the
+     sentinel frame, because that could fail/throw, and we always
+     want to leave with the current frame created and linked in --
+     we should never end up with the sentinel frame as outermost
+     frame.  */
+  current_frame = get_prev_frame_always_1 (sentinel_frame);
+  gdb_assert (current_frame != NULL);
+
+  /* The call to get_frame_id, below, is not necessary when the stack is
+     well behaved.  But when it's not well behaved, we want to ensure
+     that the frame id for the current frame is known (stashed) prior to
+     finding frame id values for any outer frames.
+
+     The test gdb.dwarf2/dw2-dup-frame.exp shows an internal error
+     in the "backtrace from stop_frame" test when we don't do this here.  */
+  if (new_sentinel_p)
+    get_frame_id (current_frame);
+
   return current_frame;
 }
 
@@ -1729,6 +1755,19 @@ get_next_frame (struct frame_info *this_frame)
     return NULL;
 }
 
+/* Return the frame that THIS_FRAME calls.  If THIS_FRAME is the innermost
+   frame, we return the sentinel frame.  Thus, NULL will never be returned.  */
+
+struct frame_info *
+get_next_frame_sentinel_okay (struct frame_info *this_frame)
+{
+  gdb_assert (this_frame != NULL);
+
+  /* This always works, since the sentinel frame refers to itself via the
+     ``next'' field.  */
+  return this_frame->next;
+}
+
 /* Observer for the target_changed event.  */
 
 static void
@@ -1745,7 +1784,7 @@ reinit_frame_cache (void)
   struct frame_info *fi;
 
   /* Tear down all frame caches.  */
-  for (fi = current_frame; fi != NULL; fi = fi->prev)
+  for (fi = sentinel_frame; fi != NULL; fi = fi->prev)
     {
       if (fi->prologue_cache && fi->unwind->dealloc_cache)
 	fi->unwind->dealloc_cache (fi, fi->prologue_cache);
@@ -1757,10 +1796,10 @@ reinit_frame_cache (void)
   obstack_free (&frame_cache_obstack, 0);
   obstack_init (&frame_cache_obstack);
 
-  if (current_frame != NULL)
+  if (sentinel_frame != NULL)
     annotate_frames_invalid ();
 
-  current_frame = NULL;		/* Invalidate cache */
+  sentinel_frame = NULL;		/* Invalidate cache */
   select_frame (NULL);
   frame_stash_invalidate ();
   if (frame_debug)
diff --git a/gdb/frame.h b/gdb/frame.h
index 5f21bb8..849a83d 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -89,6 +89,9 @@ enum frame_id_stack_status
   /* Stack address is valid, and is found in the stack_addr field.  */
   FID_STACK_VALID = 1,
 
+  /* Sentinel frame.  Stack may or may not be valid.  */
+  FID_STACK_SENTINEL = 2,
+
   /* Stack address is unavailable.  I.e., there's a valid stack, but
      we don't know where it is (because memory or registers we'd
      compute it from were not collected).  */
@@ -149,7 +152,7 @@ struct frame_id
   CORE_ADDR special_addr;
 
   /* Flags to indicate the above fields have valid contents.  */
-  ENUM_BITFIELD(frame_id_stack_status) stack_status : 2;
+  ENUM_BITFIELD(frame_id_stack_status) stack_status : 3;
   unsigned int code_addr_p : 1;
   unsigned int special_addr_p : 1;
 
@@ -165,6 +168,9 @@ struct frame_id
 /* For convenience.  All fields are zero.  This means "there is no frame".  */
 extern const struct frame_id null_frame_id;
 
+/* Sentinel frame.  */
+extern const struct frame_id sentinel_frame_id;
+
 /* This means "there is no frame ID, but there is a frame".  It should be
    replaced by best-effort frame IDs for the outermost frame, somehow.
    The implementation is only special_addr_p set.  */
@@ -309,6 +315,10 @@ extern void select_frame (struct frame_info *);
 extern struct frame_info *get_prev_frame (struct frame_info *);
 extern struct frame_info *get_next_frame (struct frame_info *);
 
+/* Like get_next_frame(), but allows return of the sentinel frame.  NULL
+   is never returned.  */
+extern struct frame_info *get_next_frame_sentinel_okay (struct frame_info *);
+
 /* Return a "struct frame_info" corresponding to the frame that called
    THIS_FRAME.  Returns NULL if there is no such frame.