From df433d316277ff5293832d3cd6cbc30b5c38dec0 Mon Sep 17 00:00:00 2001 From: Kevin Buettner Date: Tue, 27 Sep 2016 20:33:38 -0700 Subject: [PATCH] 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'. --- 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 666beb6dc6d..9740a239494 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,26 @@ +2016-11-16 Kevin Buettner + + * 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 * gnulib/update-gnulib.sh (GNULIB_COMMIT_SHA1): Set to diff --git a/gdb/frame.c b/gdb/frame.c index d3f30561dc0..54dc833202a 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="); + else if (id.stack_status == FID_STACK_SENTINEL) + fprintf_unfiltered (file, "stack="); 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 a05ac82f909..dc25ce96a26 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. -- 2.30.2