Re: [PATCHv8 3/4] gdb: allow 'until' to work in outermost frame
Simon Marchi <[email protected]>
| Newsgroups | gmane.comp.gdb.patches |
|---|---|
| Message-ID | <[email protected]> |
On 7/20/26 5:52 AM, Andrew Burgess wrote:
> The 'until' command with an argument, e.g. 'until *ADDRESS', is
> implemented by 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 frame_unwind_caller_frame, which calls
> 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 add an entry frame check into
> frame_unwind_caller_frame, similar to the one found in
> get_prev_frame. However, while get_prev_frame checks for both the
> inferior and executable entry frames (see previous commit for details
> on the differences), in frame_unwind_caller_frame I think we only need
> to check for the inferior entry frame.
>
> My reasoning here is that, in most cases, any frames before the
> inferior entry frame are likely to be invalid, and any attempt to
> write to them will trigger an error.
>
> In contrast, frames before the executable entry frame sit between
> standard libraries entry code and the executable's entry function,
> these frames, if GDB can find them, are likely to be valid.
>
> An earlier version of this patch tried placing this check into
> get_prev_frame_always, however, if get_prev_frame_always is unable to
> unwind a frame then it must set the stop reason on the last frame,
> this can be seen in `get_frame_unwind_stop_reason` where the assert
> `gdb_assert (frame->prev_p);` will trigger if the previous frame is
> not setup correctly.
>
> However, setting the stop reason means the decision about whether
> there's a previous frame or not is permanent (at least until the next
> frame cache flush), but we need commands like 'bt -past-entry' to
> work, which means the choice for whether there's a previous frame or
> not needs to remain dynamic.
>
> Placing the check in frame_unwind_caller_frame seemed like the best
> solution. This is the function that is called by other parts of GDB
> when they need 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
I don't know if I'm not testing correctly, but I'm trying with a simple
binary, compiled with "gcc test.c -O0 -o test" (no debug info, if that
matters).
$ ./gdb -nx -q --data-directory=data-directory ./test
Reading symbols from ./test...
This GDB supports auto-downloading debuginfo from the following URLs:
<https://debuginfod.archlinux.org>
Enable debuginfod for this session? (y or [n]) n
Debuginfod has been disabled.
To make this setting permanent, add 'set debuginfod enabled off' to .gdbinit.
(No debugging symbols found in ./test)
(gdb) b _start
Breakpoint 1 at 0x1020
(gdb) run
Starting program: /home/smarchi/build/binutils-gdb/gdb/test
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1".
Breakpoint 1, 0x0000555555555020 in _start ()
(gdb) disas
Dump of assembler code for function _start:
=> 0x0000555555555020 <+0>: endbr64
0x0000555555555024 <+4>: xor %ebp,%ebp
0x0000555555555026 <+6>: mov %rdx,%r9
0x0000555555555029 <+9>: pop %rsi
0x000055555555502a <+10>: mov %rsp,%rdx
0x000055555555502d <+13>: and $0xfffffffffffffff0,%rsp
0x0000555555555031 <+17>: push %rax
0x0000555555555032 <+18>: push %rsp
0x0000555555555033 <+19>: xor %r8d,%r8d
0x0000555555555036 <+22>: xor %ecx,%ecx
0x0000555555555038 <+24>: lea 0xda(%rip),%rdi # 0x555555555119 <main>
0x000055555555503f <+31>: call *0x2f7b(%rip) # 0x555555557fc0
0x0000555555555045 <+37>: hlt
End of assembler dump.
(gdb) bt
#0 0x0000555555555020 in _start ()
(gdb) bt -past-entry
#0 0x0000555555555020 in _start ()
#1 0x0000000000000001 in ?? ()
#2 0x00007fffffffdfc4 in ?? ()
#3 0x0000000000000000 in ?? ()
(gdb) until *0x0000555555555024
❌️ Warning:
Cannot insert breakpoint 0.
Cannot access memory at address 0x1
❌️ Command aborted.
I can see that "inside_entry_func" returns false while I'm in _start,
which is surprising given that the "bt" command seems to understand that
it is the entry frame.
> diff --git a/gdb/frame.c b/gdb/frame.c
> index e61bfd8d1f7..caa208c6d42 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -734,6 +734,52 @@ get_stack_frame_id (const frame_info_ptr &next_frame)
> return get_frame_id (skip_artificial_frames (next_frame));
> }
>
> +/* When checking if a frame is an entry frame, there are two different
> + entry frames to consider. This enum is used to select which entry
> + frame(s) we wish to consider. See the inside_entry_func function. */
> +
> +enum class entry_address_type_flag
> +{
> + /* The executable's entry frame. This is the first frame within the main
> + executable. */
> + executable = (1 << 0),
> +
> + /* 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 = (1 << 1),
> +};
> +DEF_ENUM_FLAGS_TYPE (enum entry_address_type_flag, entry_address_type_flags);
> +
> +/* Return true if THIS_FRAME is inside the either of the possible entry
> + frames based on ENTRY_ADDR_TYPES, otherwise return false. */
> +
> +static bool
> +inside_entry_func (const frame_info_ptr &this_frame,
> + entry_address_type_flags entry_addr_types)
> +{
> + /* It doesn't make sense to call this with no flag bits set. */
> + gdb_assert (entry_addr_types != (entry_address_type_flags) 0);
> +
> + CORE_ADDR frame_func_addr;
> + if (!get_frame_func_if_available (this_frame, &frame_func_addr))
> + return false;
> +
> + const entry_point_info &ep_info
> + = current_program_space->get_entry_point_info ();
> +
> + /* For each flag set in ENTRY_ADDR_TYPES if FRAME_FUNC_ADDR matches the
> + corresponding entry address from EP_INFO then THIS_FRAME is an entry
> + frame. */
> + return (((entry_addr_types & entry_address_type_flag::executable)
> + == entry_address_type_flag::executable
> + && ep_info.exec_entry_address () == frame_func_addr)
> + || ((entry_addr_types & entry_address_type_flag::inferior)
> + == entry_address_type_flag::inferior
> + && ep_info.inferior_entry_address () == frame_func_addr));
For readability, I really wouldn't mind if you split that in two. Also,
you can compare the result of the bitwise and to 0, it's less verbose (I
think it would be as, if not more readable, without the explicit
comparison, but I guess that it you want to strictly follow the GNU
style you need it).
if ((entry_addr_types & entry_address_type_flag::executable) != 0
&& ep_info.exec_entry_address () == frame_func_addr)
return true;
if ((entry_addr_types & entry_address_type_flag::inferior) != 0
&& ep_info.inferior_entry_address () == frame_func_addr)
return true;
return false;
> +}
> +
> /* 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.
> @@ -744,6 +790,15 @@ 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)
> {
> + /* Avoid returning a frame which is possibly before the inferior's entry
> + frame, any such frame is likely invalid. However, if the user has
> + turned on 'backtrace past-entry' then we assume they know what they
> + are doing and allow these frames. */
> + if (!user_set_backtrace_options.backtrace_past_entry
> + && inside_entry_func (initial_next_frame,
> + entry_address_type_flag::inferior))
> + return nullptr;
I'm trying to put words on why we would only check for
entry_address_type_flag::inferior here, and not
entry_address_type_flag::executable, but I don't really understand.
Perhaps you can explain it? Now that I say it, that could explain why
the example above didn't work, _start is the exec entry point, not the
inferior entry point.
It seemed strange at first for "set backtrace past-entry" to be checked
here, influencing the behavior of other commands like "until". I was
going to suggest that the decision of going past the entry frame should
perhaps be done on a case by case basis *. But perhaps that "set
backtrace past-entry" should be understood as: "for everything you (GDB)
are doing (not just the backtrace command), pretend that there exists
stack frames under what you think is the entry frame". If the user is
telling this to GDB, then it makes sense for "until" to try and put a
breakpoint in the calling frame, since the user told GDB to assume that
there are more stack frames there. So maybe it's just that the setting
name is a bit too specific, not a big deal.
* For the record, my idea was, instead of taking the decision here
whether or not to stop the unwind, perhaps we could mark the
frame_info as "this is the exec entry point", "this is the inferior
entry point", and even "this is the main entry point". And then, each
command could decide what to do with it, whether to ignore frames past
the entry point/main or not. But as noted above, it perhaps makes
sense after all to do it in a central spot.
Simon