Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Guinevere Larsen <guinevere@redhat.com>
To: Andrew Burgess <aburgess@redhat.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH] gdb: allow 'until' to work in outermost frame
Date: Thu, 11 Jun 2026 16:24:47 -0300	[thread overview]
Message-ID: <02e967d8-a06a-4774-8618-e8fddd665c26@redhat.com> (raw)
In-Reply-To: <41fe591d58ba010fa771e80ca674b61e30feef2f.1780942441.git.aburgess@redhat.com>

On 6/8/26 3:14 PM, 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.
>
> 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, however, when 'until'
> creates the caller frame breakpoint it calls frame_unwind_caller_pc
> which calls frame_unwind_caller_frame, which skips get_prev_frame and
> calls get_prev_frame_always.  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 share the outer frame detection logic
> between get_prev_frame and frame_unwind_caller_frame.  When 'backtrace
> past-entry' is off, which is the default, frame_unwind_caller_frame
> will return NULL for the outermost frame.  This means that a command
> like 'until *ADDRESS' will no longer try to create a breakpoint in the
> caller frame when used in the outermost frame.

Why not simply change frame_unwind_caller_frame to call get_prev_frame 
instead of get_prev_frame_always? feels like it would be a better idea 
to not assume that the caller is always available anyway, rather than 
porting the bits of work-arounds back to it

>
> If the user turns 'backtrace past-entry' on then we assume that the
> user knows best, and will try to place the breakpoint in the caller
> frame.
>
> 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                                   |  37 ++++-
>   gdb/testsuite/gdb.base/until-in-entry-frame.c |  22 +++
>   .../gdb.base/until-in-entry-frame.exp         | 153 ++++++++++++++++++
>   3 files changed, 207 insertions(+), 5 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 f64f5554f8c..5561ba0f2bd 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -74,6 +74,7 @@ set_backtrace_options user_set_backtrace_options;
>   static frame_info_ptr get_prev_frame_raw (const frame_info_ptr &this_frame);
>   static const char *frame_stop_reason_symbol_string (enum unwind_stop_reason reason);
>   static frame_info_ptr create_new_frame (frame_id id);
> +static bool inside_entry_func (const frame_info_ptr &this_frame);
>   
>   /* Status of some values cached in the frame_info object.  */
>   
> @@ -697,6 +698,21 @@ get_stack_frame_id (const frame_info_ptr &next_frame)
>     return get_frame_id (skip_artificial_frames (next_frame));
>   }
>   
> +/* If FRAME is the outermost frame, and the user has not turned on
> +   'backtrace past-entry', then return true, otherwise, return false.  */
> +
> +static bool
> +stop_unwinding_due_to_outermost_frame (const frame_info_ptr &frame)
> +{
> +  std::optional<CORE_ADDR> frame_pc = get_frame_pc_if_available (frame);
> +
> +  return (frame->level >= 0
> +	  && get_frame_type (frame) == NORMAL_FRAME
> +	  && !user_set_backtrace_options.backtrace_past_entry
> +	  && frame_pc.has_value ()
> +	  && inside_entry_func (frame));
> +}
> +
>   /* Helper for the various frame_unwind_caller_* functions.  Unwind
>      INITIAL_NEXT_FRAME at least one frame, but skip any artificial frames,
>      that is inline or tailcall frames.
> @@ -707,6 +723,21 @@ get_stack_frame_id (const frame_info_ptr &next_frame)
>   static frame_info_ptr
>   frame_unwind_caller_frame (const frame_info_ptr &initial_next_frame)
>   {
> +  /* The frames prior to the entry frame (not main, but the actual entry
> +     frame) are usually invalid.  If INITIAL_NEXT_FRAME is the entry frame
> +     then just claim that there is no caller frame.  This prevents things
> +     like 'until' from trying to place breakpoints in the invalid frame
> +     which GDB thinks called the entry frame.
> +
> +     Of course, if the user has turned on 'backtrace past-entry' then we
> +     assume that the user knows best, and that those frames are valid, in
> +     which case we do unwind past the entry frame.  */
> +  if (stop_unwinding_due_to_outermost_frame (initial_next_frame))
> +    {
> +      frame_debug_printf ("inside entry func");
> +      return nullptr;
> +    }
> +
>     frame_info_ptr this_frame = get_prev_frame_always (initial_next_frame);
>     if (this_frame == nullptr)
>       return nullptr;
> @@ -2785,11 +2816,7 @@ get_prev_frame (const frame_info_ptr &this_frame)
>        from main returns directly to the caller of main.  Since we don't
>        stop at main, we should at least stop at the entry point of the
>        application.  */
> -  if (this_frame->level >= 0
> -      && get_frame_type (this_frame) == NORMAL_FRAME
> -      && !user_set_backtrace_options.backtrace_past_entry
> -      && frame_pc.has_value ()
> -      && inside_entry_func (this_frame))
> +  if (stop_unwinding_due_to_outermost_frame (this_frame))
>       {
>         frame_debug_got_null_frame (this_frame, "inside entry func");
>         return NULL;
> diff --git a/gdb/testsuite/gdb.base/until-in-entry-frame.c b/gdb/testsuite/gdb.base/until-in-entry-frame.c
> new file mode 100644
> index 00000000000..9b08ea9e1d9
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/until-in-entry-frame.c
> @@ -0,0 +1,22 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2026 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see  <http://www.gnu.org/licenses/>.  */
> +
> +int
> +main (void)
> +{
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/until-in-entry-frame.exp b/gdb/testsuite/gdb.base/until-in-entry-frame.exp
> new file mode 100644
> index 00000000000..aaf117ac3aa
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/until-in-entry-frame.exp
> @@ -0,0 +1,153 @@
> +# Copyright 2026 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Check that 'until' in the outermost frame works as expected.
> +
> +standard_testfile
> +
> +if { [build_executable "failed to build" ${testfile} $srcfile \
> +	  { debug ldflags=-static } ] } {
> +    return
> +}
> +
> +# Use 'bt 1' to get the name of the current function.  Returns the
> +# name of the current function, or the empty string if the current
> +# function name cannot be established.
> +proc current_function_name { testname } {
> +    set func_name ""
> +    gdb_test_multiple "bt 1" $testname {
> +	-re -wrap "#0\\s+\[^\r\n\]*\\s+in (\[^( \]+) \\(.*" {
> +	    set func_name $expect_out(1,string)
> +	}
> +    }
> +
> +    return $func_name
> +}
> +
> +# Start the inferior with 'starti', then use 'until' within the
> +# outermost frame.  This is the real outermost frame, not 'main'.
> +#
> +# PAST_ENTRY should be 'on' or 'off' and is used in a 'set backtrace
> +# past-entry ...' command.
> +proc run_test { use_stepi past_entry } {
> +    clean_restart $::testfile
> +
> +    if { [gdb_starti_cmd] < 0 } {
> +	untested starti
> +	return
> +    }
> +
> +    gdb_test "" "Program stopped\\..*" "starti"
> +
> +    set func_name [current_function_name \
> +		       "function name for first function"]
> +
> +    gdb_test_no_output "set backtrace past-entry $past_entry"
> +
> +    # On x86-64 GDB does get a valid frame-id for the very first
> +    # instruction, but from the second instruction onwards it gets
> +    # outer_frame_id.  As a result an 'until' setup at the first
> +    # instruction doesn't match later on within the same function as
> +    # the frame-ids no longer match.
> +    #
> +    # Try to work around this by issuing a 'stepi' to move to the
> +    # second instruction.
> +    #
> +    # This is only a heuristic though.  On different architectures GDB
> +    # might have a valid frame-id for multiple instructions within the
> +    # outer frame, or might not have outer_frame_id for all
> +    # instructions.
> +    #
> +    # Also, we need to consider that the very first instruction might
> +    # be a control flow instruction, so using stepi could send the
> +    # inferior to another function.  We try to spot this case and
> +    # perform an early return if that happens.
> +    if { $use_stepi } {
> +	gdb_test "stepi"
> +	set func_name_after [current_function_name \
> +				 "function name after stepi"]
> +	if { $func_name ne $func_name_after } {
> +	    unsupported "stepi moved to another function"
> +	    return
> +	}
> +    }
> +
> +    # If the 'until' doesn't trigger then stop in 'main' as a backup.
> +    gdb_breakpoint main
> +
> +    # Find an address to use in the 'until' command.
> +    set until_address ""
> +    set capture_address false
> +    gdb_test_multiple "disassemble" "find address for until" {
> +	-re "^=> $::hex \[^\r\n\]+\r\n" {
> +	    set capture_address true
> +	    exp_continue
> +	}
> +
> +	-re "^\\s+($::hex) \[^\r\n\]+\r\n" {
> +	    if { $capture_address } {
> +		set until_address $expect_out(1,string)
> +		set capture_address false
> +	    }
> +
> +	    exp_continue
> +	}
> +
> +	-re "^$::gdb_prompt $" {
> +	    set found_address [expr { $until_address ne "" }]
> +	    gdb_assert { $found_address } $gdb_test_name
> +	    if { !$found_address } {
> +		return
> +	    }
> +	}
> +
> +	-re "^\[^\r\n\]*\r\n" {
> +	    exp_continue
> +	}
> +    }
> +
> +    # Send the 'until' command and then wait for GDB to stop.  See the
> +    # notes above about the 'stepi' for why we sometimes expect to see
> +    # GDB reach 'main' here.
> +    send_gdb "until *${until_address}\n"
> +    gdb_test_multiple "" "after until" {
> +	-re -wrap "Breakpoint $::decimal, (?:\[^\r\n\]+ in )?main \\(\\).*" {
> +	    kfail "gdb/34245" "$gdb_test_name (skipped until breakpoint)"
> +	}
> +
> +	-re -wrap "$::hex in $func_name \\(\\).*" {
> +	    pass $gdb_test_name
> +	}
> +
> +	-re -wrap "Warning:\r\nCannot insert breakpoint 0\\.\r\nCannot access memory at address $::hex.*Command aborted\\." {
> +	    # With PAST_ENTRY 'on', GDB discovers some invalid frames
> +	    # past the entry frame based on whatever happens to be in
> +	    # the registers when the entry function is called.  These
> +	    # invalid frames are often non-writable, so GDB will fail
> +	    # to insert the breakpoint when the inferior resumes.
> +	    #
> +	    # We still count this as a pass though; the user should
> +	    # only turn on 'backtrace past-entry' when they know that
> +	    # is needed, so failure here is totally expected.
> +	    gdb_assert { $past_entry eq "on" } "$gdb_test_name (caller breakpoint failed)"
> +	}
> +    }
> +}
> +
> +foreach_with_prefix past_entry { off on } {
> +    foreach_with_prefix use_stepi { true false } {
> +	run_test $use_stepi $past_entry
> +    }
> +}
>
> base-commit: 4562eab73d375e57f3c5a67f62d2678d7730d7ab


-- 
Cheers,
Guinevere Larsen
it/its
she/her (deprecated)


  reply	other threads:[~2026-06-11 19:25 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-08 18:14 Andrew Burgess
2026-06-11 19:24 ` Guinevere Larsen [this message]
2026-06-11 20:40   ` Andrew Burgess
2026-06-12 12:49     ` Guinevere Larsen
2026-06-11 21:59 ` [PATCHv2 0/3] " Andrew Burgess
2026-06-11 21:59   ` [PATCHv2 1/3] gdb: rename program_space::entry_point_address* functions Andrew Burgess
2026-06-12 15:41     ` Pedro Alves
2026-06-11 21:59   ` [PATCHv2 2/3] gdb: introduce program_space::get_entry_point_info function Andrew Burgess
2026-06-12  6:07     ` Eli Zaretskii
2026-06-15 10:14       ` Andrew Burgess
2026-06-15 12:01         ` Eli Zaretskii
2026-06-12 15:41     ` Pedro Alves
2026-06-15 10:29       ` Andrew Burgess
2026-06-16 18:36         ` Pedro Alves
2026-06-23  9:46           ` Andrew Burgess
2026-06-23 10:20             ` Pedro Alves
2026-06-11 21:59   ` [PATCHv2 3/3] gdb: allow 'until' to work in outermost frame Andrew Burgess
2026-06-12 15:57     ` Pedro Alves
2026-06-16 19:47   ` [PATCHv3 0/4] " Andrew Burgess
2026-06-16 19:47     ` [PATCHv3 1/4] gdb: rename program_space::entry_point_address* functions Andrew Burgess
2026-06-16 19:47     ` [PATCHv3 2/4] gdb: introduce program_space::get_entry_point_info function Andrew Burgess
2026-06-17 11:52       ` Eli Zaretskii
2026-06-16 19:47     ` [PATCHv3 3/4] gdb: allow 'until' to work in outermost frame Andrew Burgess
2026-06-16 19:47     ` [PATCHv3 4/4] gdb: cache program space entry point information Andrew Burgess
2026-06-23 10:47     ` [PATCHv4 0/4] gdb: allow 'until' to work in outermost frame Andrew Burgess
2026-06-23 10:47       ` [PATCHv4 1/4] gdb: rename program_space::entry_point_address* functions Andrew Burgess
2026-06-23 10:47       ` [PATCHv4 2/4] gdb: introduce program_space::get_entry_point_info function Andrew Burgess
2026-06-23 10:47       ` [PATCHv4 3/4] gdb: allow 'until' to work in outermost frame Andrew Burgess
2026-06-23 10:47       ` [PATCHv4 4/4] gdb: cache program space entry point information Andrew Burgess
2026-06-25 15:26       ` [PATCHv5 0/4] gdb: allow 'until' to work in outermost frame Andrew Burgess
2026-06-25 15:26         ` [PATCHv5 1/4] gdb: rename program_space::entry_point_address* functions Andrew Burgess
2026-06-25 15:26         ` [PATCHv5 2/4] gdb: introduce program_space::get_entry_point_info function Andrew Burgess
2026-06-25 15:26         ` [PATCHv5 3/4] gdb: allow 'until' to work in outermost frame Andrew Burgess
2026-06-25 15:26         ` [PATCHv5 4/4] gdb: cache program space entry point information Andrew Burgess
2026-07-10 14:24         ` [PATCHv6 0/4] gdb: allow 'until' to work in outermost frame Andrew Burgess
2026-07-10 14:24           ` [PATCHv6 1/4] gdb: rename program_space::entry_point_address* functions Andrew Burgess
2026-07-10 14:24           ` [PATCHv6 2/4] gdb: introduce program_space::get_entry_point_info function Andrew Burgess
2026-07-10 15:39             ` Simon Marchi
2026-07-10 14:24           ` [PATCHv6 3/4] gdb: allow 'until' to work in outermost frame Andrew Burgess
2026-07-10 17:00             ` Simon Marchi
2026-07-10 17:01               ` Simon Marchi
2026-07-16 15:06               ` Andrew Burgess
2026-07-10 14:24           ` [PATCHv6 4/4] gdb: cache program space entry point information Andrew Burgess
2026-07-10 17:07             ` Simon Marchi
2026-07-18 13:11           ` [PATCHv7 0/4] gdb: allow 'until' to work in outermost frame Andrew Burgess
2026-07-18 13:11             ` [PATCHv7 1/4] gdb: rename program_space::entry_point_address* functions Andrew Burgess
2026-07-18 13:11             ` [PATCHv7 2/4] gdb: introduce program_space::get_entry_point_info function Andrew Burgess
2026-07-18 13:11             ` [PATCHv7 3/4] gdb: allow 'until' to work in outermost frame Andrew Burgess
2026-07-18 13:11             ` [PATCHv7 4/4] gdb: cache program space entry point information Andrew Burgess
2026-07-20  9:52             ` [PATCHv8 0/4] gdb: allow 'until' to work in outermost frame Andrew Burgess
2026-07-20  9:52               ` [PATCHv8 1/4] gdb: rename program_space::entry_point_address* functions Andrew Burgess
2026-07-20  9:52               ` [PATCHv8 2/4] gdb: introduce program_space::get_entry_point_info function Andrew Burgess
2026-07-20  9:52               ` [PATCHv8 3/4] gdb: allow 'until' to work in outermost frame Andrew Burgess
2026-07-20  9:52               ` [PATCHv8 4/4] gdb: cache program space entry point information Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=02e967d8-a06a-4774-8618-e8fddd665c26@redhat.com \
    --to=guinevere@redhat.com \
    --cc=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox