From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id aVE1FpbzWGrZzgwAWB0awg (envelope-from ) for ; Thu, 16 Jul 2026 11:07:02 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=av2E+cb2; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 54FD71E033; Thu, 16 Jul 2026 11:07:02 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-6.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=unavailable autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [IPv6:2620:52:6:3111::32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 3BC411E033 for ; Thu, 16 Jul 2026 11:07:01 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id F3B374BA2E23 for ; Thu, 16 Jul 2026 15:06:53 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org F3B374BA2E23 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=av2E+cb2 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 0DA764BA2E07 for ; Thu, 16 Jul 2026 15:06:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0DA764BA2E07 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 0DA764BA2E07 Authentication-Results: sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1784214387; cv=none; b=iBOPI7xsI+D61grvCKqxUxfaQxw2lZ0pjXcIfcXg/hq+AgBgAUS67bzMQBkvYq6om3S1YNj+aqF90UP5/qBrvAzoU4BoDphc14BN916XfztokUhEF5Mq8rPOgVJJ42uLjGp1OfegiKRPn9MeF5zuiTb2OOYUQandTtFQNNoEOv0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1784214387; c=relaxed/simple; bh=rDLRQEVzhY/gLUF1tcO5g5huV5LAGmRpgkJIBspEwgA=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=tfNEfoygCqLHJv30fYH+mIJcTm52YYcVX/pCMG5O4kgTpoJzVrHcBQzli5477MuahprGXk/trcd3vtGSYApv+1jAJAqlNZoRzNYDNdxUPvqVYoJyGJe+hx7jNdhSto45iv+RXSduTfywvbPUZAJmdPdThFVWLGJpSnTmf5/MSh8= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=av2E+cb2 DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 0DA764BA2E07 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1784214386; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=mO6ZyEl+72mpMNonXXTxCKUt/b3Curnb7C9VOinGqZ0=; b=av2E+cb2gOU7woZQkffOS/g7E7qkmrC6X42Wwnbb2y+TlFJTpxIQt0vrHJPiOLRowsKLFz BSQQhipNn9WDlseLkNRzTqMlutc/7m1uIubESP0gHoZ6X9G61UoxEcYB5sJE22Gf7TzzcU U3NBfuuZrHQVg5DqaO3V/jnI2eNkCtg= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-74-Zx_apGbsMDmpac9kzjY6uw-1; Thu, 16 Jul 2026 11:06:25 -0400 X-MC-Unique: Zx_apGbsMDmpac9kzjY6uw-1 X-Mimecast-MFC-AGG-ID: Zx_apGbsMDmpac9kzjY6uw_1784214383 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-495427ed72eso10794225e9.2 for ; Thu, 16 Jul 2026 08:06:24 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784214383; x=1784819183; h=content-type:mime-version:message-id:date:references:in-reply-to :subject:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=mO6ZyEl+72mpMNonXXTxCKUt/b3Curnb7C9VOinGqZ0=; b=rPiIWAFIJuAzxYlmHyH2jZmViEHr1rWdaDTWA8T2Cmc3YRq+huhiuvU6JmfkRAsoWd MG61niO7O3jGccK5rhlg9odmbKHgMNhkroqiWcFcX/io60JFZIUjhfKLZRfKesqARbbE ys1XD5hiINhFHRyRKVXKTrFeJ1UtE3R05boMGHpfrqnNbC+g44kx7ZYHktZmkVRJEOia zWl6r+HyoNtRuDMeCH2AfmTGAGK2sTiN9t6BGwOAfYbzO6ofkJPBi/3FDtpLdf1a2OCi Sn9B7r5kpKOC8D3CFbBqfYPMRAdKLsVzIW/DHgqVaQve4NoaR/dTPoxkhTMYv5gTkKk9 i+iQ== X-Forwarded-Encrypted: i=1; AHgh+RoPxJYFmgbolYmo35hfgs6mj5j+1LSjcrfQ5kBewfyRRAzkVZQ5ti/yU9Lv76kxi4lRFiuMOk+vOR677g==@sourceware.org X-Gm-Message-State: AOJu0YwVP8p9pB/POVnDYfZTXp41AkAIicrfxYlKhnZbcTzFnD9DU8pK TRzYLnC0OkxhEFt9SJZJBFoz2JI28ACrdPmWUw/MGIrJsqAwcBvj8N1yss0GTTqDzXX9t2c6s9u lddPhwidjt0kaGP6EV0pCFZ2CKCbpllnM7facnCE1EI7nV2agjmeJsRxot7hNtmU= X-Gm-Gg: AfdE7cnB26stAWQRjNihlRjCqhQcKcyYeqeAQFIMdxjUBbhl40NMixUXO9xdaCRFfkI 02vdPr8mGRCxYIyLjVIJSNUST3wGCtN+h/jWVwjvnjykLq6fjiSH/cT1+PZeNiMZiDy4uHb+Dy1 w9U1XI3UVuXXCjFX4fazLYP4zxLuuFRr8zGWdsS19769UHrAj9tKroZ07KbL8o64neRMVV0mHBn KKl/faT33/VMiBAXCbQwKZhfpHP1JoNV/XQUKWwX0guYCImM7G2V/KpqCvuwEaXjx78zFelqCd8 mI8z12shnHduy24k99P4u35fUblOUpZNlF8Lu63F6t4FORouQmFQlTD4YA8b6czKijadidz0 X-Received: by 2002:a05:600c:8284:b0:493:b647:1acd with SMTP id 5b1f17b1804b1-493f8845b3amr244596435e9.36.1784214381730; Thu, 16 Jul 2026 08:06:21 -0700 (PDT) X-Received: by 2002:a05:600c:8284:b0:493:b647:1acd with SMTP id 5b1f17b1804b1-493f8845b3amr244595625e9.36.1784214380855; Thu, 16 Jul 2026 08:06:20 -0700 (PDT) Received: from localhost ([31.111.209.233]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-49541e9425asm102034215e9.11.2026.07.16.08.06.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 16 Jul 2026 08:06:19 -0700 (PDT) From: Andrew Burgess To: Simon Marchi , gdb-patches@sourceware.org Subject: Re: [PATCHv6 3/4] gdb: allow 'until' to work in outermost frame In-Reply-To: <848a1bb5-78d6-4bc3-93b1-ec195b8615c9@simark.ca> References: <2a4ac1c0763d6ea9c3f97855b19ef91c07d35f19.1783693321.git.aburgess@redhat.com> <848a1bb5-78d6-4bc3-93b1-ec195b8615c9@simark.ca> Date: Thu, 16 Jul 2026 16:06:16 +0100 Message-ID: <8733xjdlef.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: KNJjQDac30vPcV_rPkEgeG-0ygAFRgqgjKGxKzT1yC0_1784214383 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Simon Marchi writes: > 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. The reason I didn't do this is the caching added in the next patch. I currently cache the whole entry_point_info structure. I could push that caching down and cache each field of the entry_point_info separately, then we can fetch each field. But as it currently stands with this complete series (including the next patch), we pay a price the first time inside_entry_func is called, but after that fetching the entry_point_info is pretty cheap. Also, we always end up calling inside_entry_func twice, once for each entry address that it can check, so the "price" we pay isn't wasted, it just means the first call has to prime the cache, then we're good. > >> + >> +/* 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. As you spotted, patch #4 adds the caching. I'll add a sentence to the commit message to make it clear that sub-optimal things in this patch should be resolved by the next one. > >> + { >> + 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? I got rid of this and just return NULL, which is what the entry frame detection in get_prev_frame does, and that works just fine too. I also extended the test in the previous commit to test 'bt -past-entry', which continues to pass after this patch has been applied. > >> 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. It seemed redundant given that `inside_entry_func`, which is also called by this `if` check uses `get_frame_func_if_available`, which will cover the same logic. But looking again, I think there are multiple instances of this `frame_pc.has_value()` that could be removed, so this is probably better done in a separate commit. As such, I've added this check back in for now. I'm just re-running the tests, but will post a v7 shortly. Thanks, Andrew