Re: [PATCH] gdb: return raw frame_info pointer from create_sentinel_frame

[Simon Marchi <simark@simark.ca>]

On 2026-06-18 13:00, Andrew Burgess wrote:
> The create_sentinel_frame function is static within frame.c and
> currently returns a frame_info_ptr.  The frame_info_ptr is created at
> the last moment, as part of the 'return' statement.
> 
> Every user of create_sentinel_frame immediately pulls the raw
> frame_info pointer out of the frame_info_ptr and then discards the
> frame_info_ptr object.
> 
> Creating a frame_info_ptr isn't zero work.  Not that I'm trying to
> sell this as a performance improvement, the work saved here is
> negligible, but it's so obviously redundant in this case that I figure
> we can just remove the frame_info_ptr and pass back a raw frame_info
> pointer instead.
> 
> This patch changes create_sentinel_frame to return a raw frame_info
> pointer, which is what the callers actually want, and avoids the
> unnecessary creation and destruction of a frame_info_ptr object.
> 
> There should be no user visible changes after this commit.

Your patch looks obviously correct to me, so:

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

I was wondering if you instead considered making the sentinel_frame
global a frame_info_ptr itself.  I think there would be a concern in
construction ordering, because the frame_info_ptr constructor uses the
frame_info_ptr::frame_list list, and that one is defined later in the
file, but that would be easy to fix.  There are two spots where we do

  frame_info_ptr (sentinel_frame))

that would be simplified a little bit.  But other than that, I don't
think it would provide any tangible benefit.

Another random thing I noticed while reading the code, do you know why
we have this in frame_find_by_id?

  /* Check for the sentinel frame.  */
  if (id == frame_id_build_sentinel (0, 0))
    return frame_info_ptr (sentinel_frame);

Given that sentinel frames are put in the frame stash (see
create_sentinel_frame), I don't see why this special case is needed.
Moreover, this would only match the sentinel frames constructed with
stack and code addresses 0, which I don't think is the typical case.
Hmm, it's probably because sentinel frames were not put in the frame
cache before my commit here...

  https://gitlab.com/gnutools/binutils-gdb/-/commit/19f988359a62889b00d67fb59ef8c7d6d759fc98

There is probably something to clean up here.

Simon