Re: [PATCHv2 1/2] gdb: remove sentinel frame check in frame_find_by_id

[Simon Marchi <simark@simark.ca>]

On 6/22/26 5:33 PM, Andrew Burgess wrote:
> While reviewing another patch Simon pointed out that the code in
> frame_find_by_id for looking up the sentinel frame is no longer needed
> since commit:
> 
>   commit 19f988359a62889b00d67fb59ef8c7d6d759fc98
>   Date:   Mon Jan 30 15:02:49 2023 -0500
> 
>     gdb: give sentinel for user frames distinct IDs, register sentinel frames to the frame cache
> 
> Prior to this commit the sentinel_frame was not added to the frame
> stash, so we needed a manual check to lookup the sentinel frame.
> After this commit the sentinel frame is added to the stash so the
> manual check, though perfectly valid, is just unnecessary complexity,
> and can be removed.  A frame stash lookup is just a hash lookup, and
> is relatively cheap, so switching to this isn't much extra cost.
> 
> While I was working in frame_find_by_id I made a couple of additional
> changes:
> 
>   1. I moved the declarations of frame and prev_frame locals into the
>      function body, closer to where they are first defined.
> 
>   2. Instead of treating a frame_info_ptr as a bool, I made use of the
>      frame_info_ptr::is_null method.
> 
> There should be no user visible changes after this commit.
> ---
>  gdb/frame.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/gdb/frame.c b/gdb/frame.c
> index f64f5554f8c..c1a7c41dc4a 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -959,17 +959,11 @@ frame_id_inner (struct gdbarch *gdbarch, struct frame_id l, struct frame_id r)
>  frame_info_ptr
>  frame_find_by_id (struct frame_id id)
>  {
> -  frame_info_ptr frame, prev_frame;
> -
>    /* ZERO denotes the null frame, let the caller decide what to do
>       about it.  Should it instead return get_current_frame()?  */
>    if (!frame_id_p (id))
>      return NULL;
>  
> -  /* Check for the sentinel frame.  */
> -  if (id == frame_id_build_sentinel (0, 0))
> -    return frame_info_ptr (sentinel_frame);

From what you know, was this check even working today?  This compares
the id we're looking for (which presumably has "real" addresses) with
"code" and "special" addresses set to 0.

In any case, LGTM.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon