This is the mail archive of the gdb-cvs@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[binutils-gdb] Distinguish sentinel frame from null frame.


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=df433d316277ff5293832d3cd6cbc30b5c38dec0

commit df433d316277ff5293832d3cd6cbc30b5c38dec0
Author: Kevin Buettner <kevinb@redhat.com>
Date:   Tue Sep 27 20:33:38 2016 -0700

    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.'
        	(get_next_frame_sentinel_okay): New function.
        	(reinit_frame_cache): Use `sentinel_frame' in place of
        	`current_frame'.

Diff:
---
 gdb/ChangeLog | 23 +++++++++++++++
 gdb/frame.c   | 94 ++++++++++++++++++++++++++++++++++++++---------------------
 gdb/frame.h   | 12 +++++++-
 3 files changed, 94 insertions(+), 35 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 666beb6..9740a23 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,26 @@
+2016-11-16  Kevin Buettner  <kevinb@redhat.com>
+
+	* 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.'
+	(get_next_frame_sentinel_okay): New function.
+	(reinit_frame_cache): Use `sentinel_frame' in place of
+	`current_frame'.
+
 2016-11-15  Pedro Alves  <palves@redhat.com>
 
 	* gnulib/update-gnulib.sh (GNULIB_COMMIT_SHA1): Set to
diff --git a/gdb/frame.c b/gdb/frame.c
index d3f3056..54dc833 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -43,6 +43,15 @@
 #include "hashtab.h"
 #include "valprint.h"
 
+/* The sentinel frame terminates the innermost end of the frame chain.
+   If unwound, it returns the information needed to construct an
+   innermost frame.
+
+   The current frame, which is the innermost 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 +74,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 +332,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, ",");
@@ -562,6 +573,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 +809,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 +1490,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 +1502,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 +1522,8 @@ 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;
+
   /* 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,29 +1539,23 @@ get_current_frame (void)
   if (get_traceframe_number () < 0)
     validate_registers_access ();
 
-  if (current_frame == NULL)
-    {
-      int stashed;
-      struct frame_info *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.  */
-    }
+  if (sentinel_frame == NULL)
+    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);
 
   return current_frame;
 }
@@ -1729,6 +1736,25 @@ get_next_frame (struct frame_info *this_frame)
     return NULL;
 }
 
+/* Return the frame that THIS_FRAME calls.  If THIS_FRAME is the
+   innermost (i.e. current) frame, return the sentinel frame.  Thus,
+   unlike get_next_frame(), NULL will never be returned.  */
+
+struct frame_info *
+get_next_frame_sentinel_okay (struct frame_info *this_frame)
+{
+  gdb_assert (this_frame != NULL);
+
+  /* Note that, due to the manner in which the sentinel frame is
+     constructed, this_frame->next still works even when this_frame
+     is the sentinel frame.  But we disallow it here anyway because
+     calling get_next_frame_sentinel_okay() on the sentinel frame
+     is likely a coding error.  */
+  gdb_assert (this_frame != sentinel_frame);
+
+  return this_frame->next;
+}
+
 /* Observer for the target_changed event.  */
 
 static void
@@ -1745,7 +1771,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 +1783,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 a05ac82..dc25ce9 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -90,6 +90,9 @@ enum frame_id_stack_status
   /* Stack address is valid, and is found in the stack_addr field.  */
   FID_STACK_VALID = 1,
 
+  /* Sentinel frame.  */
+  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).  */
@@ -150,7 +153,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;
 
@@ -166,6 +169,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.  */
@@ -310,6 +316,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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]