Re: [PATCHv6 3/4] gdb: allow 'until' to work in outermost frame

[Simon Marchi <simark@simark.ca>]

On 7/10/26 10:24 AM, Andrew Burgess wrote:
> The 'until' command with an argument, e.g. 'until *ADDRESS', is
> implemented by the until_break_command in breakpoint.c.
> 
> The most important thing this function does is convert the location
> argument '*ADDRESS' in my example, into a vector of symtab_and_line
> objects.  Each of these symtab_and_line objects is then used to create
> a temporary bp_until breakpoint.
> 
> The other thing that until_break_command does is create a breakpoint
> in the caller frame.  This breakpoint is there to ensure the inferior
> stops upon exiting the frame in which the 'until' command was issued,
> if the until breakpoint was not hit.
> 
> When compiling an assembler file into a static test program, if I use
> 'starti' to stop the inferior within the outermost frame, and then try
> to use 'until *ADDRESS' I see the following error:
> 
>   (gdb) starti
>   Starting program: /tmp/hello
> 
>   Program stopped.
>   0x0000000000401000 in _start ()
>   (gdb) bt
>   #0  0x0000000000401000 in _start ()
>   (gdb) until *0x0000000000401015
>   Warning:
>   Cannot insert breakpoint 0.
>   Cannot access memory at address 0x1
> 
>   Command aborted.
>   (gdb)
> 
> The problem here is the breakpoint that 'until' tries to create in the
> caller frame.  Though the 'bt' in the above example indicates that
> there is only a single frame, the outermost frame, this is only
> because GDB has specific code in get_prev_frame to stop the backtrace
> at the outermost frame.  If we turn this off and try 'bt' again:
> 
>   (gdb) set backtrace past-entry on
>   (gdb) bt
>   #0  0x0000000000401000 in _start ()
>   #1  0x0000000000000001 in ?? ()
>   #2  0x00007fffffffac73 in ?? ()
>   #3  0x0000000000000000 in ?? ()
>   (gdb)
> 
> What we are seeing here is the garbage values that happen to be in the
> registers tricking GDB into thinking there are frames before _start.
> 
> GDB's code to handle this is in get_prev_frame, where we call
> inside_entry_func.  This checks if a frame is one of the two possible
> entry frames, the inferior entry frame or the executable entry frame.
> See the previous commit for more details.  The important thing is that
> the inferior entry frame is the absolute outer frame, the very first
> frame that the inferior executed when starting, while the executable
> entry frame is just the first frame within the main executable.
> 
> There are a number of user configurable filters in get_prev_frame, the
> backtrace past-main filter, the backtrace frame limit filter, and the
> backtrace past-entry filter that we are discussing here.
> 
> When creating the caller frame breakpoint, the 'until' command doesn't
> use get_prev_frame, it uses get_prev_frame_always.  This is so that
> the user configurable filters don't prevent the caller frame
> breakpoint from being created.  But this means that when creating the
> breakpoint, we skip the entry frame check.  As a result, the 'until'
> command will try to place a breakpoint in the bogus frame #1 shown
> above.  As the frame is at address 0x1, which is non-writable, we see
> an error when trying to insert the breakpoint.
> 
> In this commit I propose that we split the inside_entry_func handling
> in to two parts.  In get_prev_frame we will retain the check for the
> executable entry frame.  In theory it is possible that there are
> frames between the executable entry frame and the inferior entry
> frame.  These are the frames the 'backtrace past-entry' can hide or
> reveal (if the frames can be discovered).
> 
> The check for the inferior entry frame I propose moving into
> get_prev_frame_always.  I think this is a better place for it because
> any frames before the inferior entry frame are almost certainly
> bogus.  As such we want to elide those frames even for "inner" use
> cases, like the caller frame breakpoint of the 'until' command.
> 
> The test for this fix ran into an issue where the frame-id for the
> outermost frame would change between the first instruction and later
> instructions in the frame.  I created bug PR gdb/34245 for this issue.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34245
> ---
>  gdb/frame.c                                   | 103 +++++++----
>  gdb/testsuite/gdb.base/until-in-entry-frame.c |  22 +++
>  .../gdb.base/until-in-entry-frame.exp         | 166 ++++++++++++++++++
>  3 files changed, 259 insertions(+), 32 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/until-in-entry-frame.c
>  create mode 100644 gdb/testsuite/gdb.base/until-in-entry-frame.exp
> 
> diff --git a/gdb/frame.c b/gdb/frame.c
> index 782ca33abd8..a84cca5b81c 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -2232,6 +2232,63 @@ frame_register_unwind_location (const frame_info_ptr &initial_this_frame,
>      }
>  }
>  
> +/* When checking if a frame is an entry frame, there are two different
> +   entry frames to consider.  This enum is used to choose between them.  */
> +
> +enum class entry_address_type
> +{
> +  /* The executable's entry frame.  This is the first frame within the main
> +     executable.  */
> +  executable,
> +
> +  /* The inferior's entry frame.  This is the first frame within the
> +     inferior, this can be outside the main executable, e.g. for a
> +     dynamically linked executable, this will be the first frame in the
> +     dynamic linker.  */
> +  inferior,
> +};
> +
> +/* Test whether THIS_FRAME is inside the TYPE process entry point
> +   function.  */
> +
> +static bool
> +inside_entry_func (const frame_info_ptr &this_frame,
> +		   entry_address_type type)
> +{
> +  const program_space::entry_point_info &ep_info
> +    = current_program_space->get_entry_point_info ();
> +
> +  CORE_ADDR frame_func_addr;
> +  if (!get_frame_func_if_available (this_frame, &frame_func_addr))
> +    return false;
> +
> +  switch (type)
> +    {
> +    case entry_address_type::executable:
> +      return ep_info.exec_entry_address () == frame_func_addr;
> +
> +    case entry_address_type::inferior:
> +      return ep_info.inferior_entry_address () == frame_func_addr;
> +    }
> +
> +  gdb_assert_not_reached ("unknown entry address type: %d", ((int) type));
> +}

Given that this code path allows reading just one of the two entry
address types, it would perhaps be good to by pass
program_spaceget_entry_point_info (which returns both), and just go
fetch the one that is requested.  The other one will inevitably be
wasted.  Not a big deal if both branches are reasonably fast to fetch,
but still it shouldn't be difficult to fetch just the right one.

> +
> +/* Debug routine to print a NULL frame being returned.  */
> +
> +static void
> +frame_debug_got_null_frame (const frame_info_ptr &this_frame,
> +			    const char *reason)
> +{
> +  if (frame_debug)
> +    {
> +      if (this_frame != NULL)
> +	frame_debug_printf ("this_frame=%d -> %s", this_frame->level, reason);
> +      else
> +	frame_debug_printf ("this_frame=nullptr -> %s", reason);
> +    }
> +}
> +
>  /* Get the previous raw frame, and check that it is not identical to
>     same other frame frame already in the chain.  If it is, there is
>     most likely a stack cycle, so we discard it, and mark THIS_FRAME as
> @@ -2538,6 +2595,17 @@ get_prev_frame_always_1 (const frame_info_ptr &this_frame)
>  frame_info_ptr
>  get_prev_frame_always (const frame_info_ptr &this_frame)
>  {
> +  if (this_frame->level >= 0
> +      && get_frame_type (this_frame) == NORMAL_FRAME
> +      && !user_set_backtrace_options.backtrace_past_entry
> +      && inside_entry_func (this_frame, entry_address_type::inferior))

get_prev_frame_always is called quite often, inside_entry_func is going
to be called quite often (the other parts of the conjunction are often
going to be true), inside_entry_func calls
program_space::get_entry_point_info, which calls
solib_ops::inferior_entry_point_address.
svr4_solib_ops::inferior_entry_point_address is somewhat heavy (ELF
parsing and all).  I wonder if the inferior entry point should be
cached, as it's not going to change often.

I put a printf in svr4_solib_ops::inferior_entry_point_address, attached
to gnome-calculator (with debuginfod, so debug info for all the libs)
and ran a backtrace.  For 10 frames,
svr4_solib_ops::inferior_entry_point_address was called 242 times.

Perhaps it's premature optimization, but it just feels wrong.

> +    {
> +      this_frame->prev_p = true;
> +      this_frame->stop_reason = UNWIND_OUTERMOST;
> +      frame_debug_got_null_frame (this_frame, "inside inferior entry func");
> +      return nullptr;
> +    }

I notice that this makes get_prev_frame_always modify a persistent
state (stop_reason) of `this_frame` based on a setting that could change
from one call to another (options.backtrace_past_entry).  For instance,
what happens to "bt -past-entry" after this runs?

>  static bool
> @@ -2689,19 +2742,6 @@ inside_main_func (const frame_info_ptr &this_frame)
>    return sym_addr == get_frame_func (this_frame);
>  }
>  
> -/* Test whether THIS_FRAME is inside the process entry point function.  */
> -
> -static bool
> -inside_entry_func (const frame_info_ptr &this_frame)
> -{
> -  const program_space::entry_point_info &ep_info
> -    = current_program_space->get_entry_point_info ();
> -
> -  CORE_ADDR frame_func_addr = get_frame_func (this_frame);
> -  return (ep_info.exec_entry_address () == frame_func_addr
> -	  || ep_info.inferior_entry_address () == frame_func_addr);
> -}
> -
>  /* Return a structure containing various interesting information about
>     the frame that called THIS_FRAME.  Returns NULL if there is either
>     no such frame or the frame fails any of a set of target-independent
> @@ -2783,11 +2823,10 @@ get_prev_frame (const frame_info_ptr &this_frame)
>    if (this_frame->level >= 0
>        && get_frame_type (this_frame) == NORMAL_FRAME
>        && !user_set_backtrace_options.backtrace_past_entry
> -      && frame_pc.has_value ()

Can you explain the removal of this frame_pc.has_value() line?  Probably
fine, but I want to be sure.

Simon