From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id FQDMC3RSVmrW/QcAWB0awg (envelope-from ) for ; Tue, 14 Jul 2026 11:15:00 -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=CInlLJ0t; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 1E7471E09E; Tue, 14 Jul 2026 11:15:00 -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=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.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 71E6C1E033 for ; Tue, 14 Jul 2026 11:14:58 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A41964BA23C7 for ; Tue, 14 Jul 2026 15:14:56 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A41964BA23C7 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=CInlLJ0t 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 2524F4BA2E17 for ; Tue, 14 Jul 2026 15:14:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2524F4BA2E17 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 2524F4BA2E17 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=1784042070; cv=none; b=GozSYaPw/OAywF4kafV947drfMBrASpWzKk80aEl6Ikvs0i4VORQUkPB/8wX5jPbKBVZQs+lQoX3DGJNl5Rl4nc1NYNk1nTgoqyKptCSjlFKmjQV4Gmv1edhsZws3zBTFKdrQd5rlo20Olvm8Mv5q89Zoxkk7KHLxzgfFIlOF5E= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1784042070; c=relaxed/simple; bh=02VAJZNVCRXuosoCgEgBUZc4keWl65lxGtgOfDX+AJU=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=sj+DGIfl/SlZ10lpLinaoCOhhEeeD1/tu/EFejiJt2hV9iopbXMGt5Wn0YqVGbvlFgr6uHMFy7+1/VLhAb1b2c8HRnBJaglprQUfGlRUrHarS3yYXWZeAKUElrOCeXz7neAF96hCm9SpE3Hr/cFfVHU+uKaXMJmzGXQIjeZB0VI= 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=CInlLJ0t DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2524F4BA2E17 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1784042069; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=5exeJDSWvHjaAOYowk6F+2etcC6ONI2CpPGYtmj+kjo=; b=CInlLJ0twcBMFh3fnRwcybEoCzYzJwejfam5/fDeT1X/L7ZgtFSDhvVJXaiGjP/ESFl9ng a+rzdjQUbQG1dfbzmU3hpOnC8Xcp2OxTJMA/StsshV+NbDWyzzwcsWLewlcoEuj1eeVwT7 EuwGHa3BHBHeE4av846nCp+RCwfKOJY= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-12-63mBNYfkNVeUmESWWVkRIg-1; Tue, 14 Jul 2026 11:14:28 -0400 X-MC-Unique: 63mBNYfkNVeUmESWWVkRIg-1 X-Mimecast-MFC-AGG-ID: 63mBNYfkNVeUmESWWVkRIg_1784042067 Received: by mail-wr1-f70.google.com with SMTP id ffacd0b85a97d-473e18559b2so645561f8f.0 for ; Tue, 14 Jul 2026 08:14:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784042067; x=1784646867; h=content-type:mime-version:message-id:date:references:in-reply-to :subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to:content-type; bh=5exeJDSWvHjaAOYowk6F+2etcC6ONI2CpPGYtmj+kjo=; b=X+uLFgC4AYHWGSrMwtZ0lfL/Vcw9vLyNY7n3dyPPYj6v9lRNGmhHE71MSVYN2DahGI ubGM2fXj3SGG+0l1b3UX5LeTqhzh5Qx75tSZBejxyNqoVzU3ClLg3lAAikua1TF7mrrH Aj8wwPSuz8DglEkX8dhPvNj2yh88Eoq09Lh50XYhi4UaGKTda9EcMrbaxR0nPSIMIy6T jBzJRzYRfUfFQ7F7I3g9O8ULLulgHgdJ3E8Ch9gFjdE4rq3m7ZdVV8o5a+8oJh2AkT7U Ar2hzfEh80807lOjFiGUNYK1kH52WROrXuWZEFXmXKQMbHa6A6GSlJ+F/41f3CTXfs+j aIoA== X-Gm-Message-State: AOJu0YyYjopZaE6JAo2xbvEFYQy3KtYtemq/0lwMe1O6XqGYlym48guO 7xkSS8xMfBFHBBbjYEnIwcmWP7+ckuZp0COgu2xDfqS5q9pYzRhKcYm7uAcwT5zEgmAxW1C6P8s psDpPEhU8A9u1eAR74eQHUEOtZLA/ZzBlkeFuL4XBz1PP2LlvtuQJLNS9afEr60QyiJKU5AU= X-Gm-Gg: AfdE7cmkN8JToMU23w8j/3LdMSGNt0BnCCO0AyN6XLpohOTGH/xT0Ow5097VBzHKyXs bCWcKEjB+bjuI5bqu/6YScIWJfZmmZqdIJGWCcK2xkSWlcok+roQ7vbvlpFVWe0mHPdgHckUT5v JRz5cqC6N+XVw8oUDq2pZYMJs1jYA819fCSrzr3pPc9y5DOWLHOZH5vDjPrQbfjrYgQ9GlPxdud B5ljZeg0I5nBidkMofhRiT1O+8UFq3cjMuEUbYXfjJxL04xSfa9Bdq3VwDFKCF5VxREPe0fUWiQ vJaGnOHqk0inE0Hr3V3ACtlJ4N8GJV1jF6morMoLJcy9dmgisBZWm7IHaNA0c2EBcaTXpwswhe5 vOqAB9RA= X-Received: by 2002:a5d:5d86:0:b0:474:d7a5:4b6d with SMTP id ffacd0b85a97d-47f2dc9bff2mr14618727f8f.21.1784042066921; Tue, 14 Jul 2026 08:14:26 -0700 (PDT) X-Received: by 2002:a5d:5d86:0:b0:474:d7a5:4b6d with SMTP id ffacd0b85a97d-47f2dc9bff2mr14618662f8f.21.1784042066126; Tue, 14 Jul 2026 08:14:26 -0700 (PDT) Received: from localhost ([31.111.209.233]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-47f4635ac2esm8968658f8f.13.2026.07.14.08.14.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Jul 2026 08:14:25 -0700 (PDT) From: Andrew Burgess To: "Rohr, Stephan" Cc: "gdb-patches@sourceware.org" , Tom Tromey Subject: RE: [PATCH 1/1] gdb: set the cache information in 'get_prev_frame_maybe_check_cycle' In-Reply-To: References: <20260625145850.3104079-1-stephan.rohr@intel.com> <20260625145850.3104079-2-stephan.rohr@intel.com> <87se69wdbt.fsf@tromey.com> <87zf04tawh.fsf@redhat.com> <87ldbmu6py.fsf@redhat.com> <87h5m9fuqi.fsf@redhat.com> Date: Tue, 14 Jul 2026 16:14:24 +0100 Message-ID: <87a4rteh7z.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: 4RzXQjnqdC4FxRT0TuP08nzVmQMZCeBjullEv5PZa7M_1784042067 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 "Rohr, Stephan" writes: > Hi Andrew, > > Two more nits for the commit message, see my comment bellow. Hi Stephan, Sorry for the delay, I took some time to think about one of the issue you raised before responding. > >> -----Original Message----- >> From: Andrew Burgess >> Sent: Wednesday, 8 July 2026 15:59 >> To: Rohr, Stephan >> Cc: gdb-patches@sourceware.org; Tom Tromey >> Subject: RE: [PATCH 1/1] gdb: set the cache information in >> 'get_prev_frame_maybe_check_cycle' >> >> "Rohr, Stephan" writes: >> >> > HI Andrew, >> > >> > Thanks for sharing. The patch itself looks good. >> > >> > I have a few comments regarding the commit message, see below. >> >> Thanks Stephan. Below is an updated patch with an improved commit >> message. I also tweaked some of the comments in the actual code as, >> upon re-reading, I found some of them not ideal. >> >> Let me know what you think. >> >> Thanks, >> Andrew >> >> --- >> >> commit cec35caa5c173de5e98a9ac6cbc0b8ee8e15d912 >> Author: Andrew Burgess >> Date: Thu Jun 25 14:58:50 2026 +0000 >> >> gdb: set frame_info_ptr::m_cached_id during invalidation >> >> Currently frame_info_ptr caches the frame_id at construction time, see >> frame_info_ptr::frame_info_ptr in frame.c. The problem with this is >> that a frame's frame-id might not be known at this point. >> >> Consider get_prev_frame_maybe_check_cycle, this calls >> get_prev_frame_raw to create the previous frame, placing the result >> into a frame_info_ptr PREV_FRAME. Then (for frames other than frame >> 0) compute_frame_id is called, however, this only computes the >> frame_id for the frame_info object pointed to by the frame_info_ptr, >> the cached frame_id within the frame_info_ptr itself is not updated. >> > > This is a long sentence. Consider splitting into two to improve readability. > I've reworked this, see the updated commit message below. >> What this means is that in get_prev_frame_maybe_check_cycle, the >> PREV_FRAME local has no cached frame-id. >> >> If we consider the call stack: >> >> get_selected_frame >> lookup_selected_frame >> frame_find_by_id >> get_prev_frame >> get_prev_frame_always >> get_prev_frame_always_1 >> get_prev_frame_maybe_check_cycle >> >> Then what we see is that the frame_info_ptr created in >> get_prev_frame_maybe_check_cycle, which lacks a cached frame_id, can >> be passed all the way back to lookup_selected_frame, where it will be >> stored in the SELECTED_FRAME global by a call to select_frame. The >> outer get_selected_frame call (in the above backtrace) will then >> return the SELECTED_FRAME global, which lacks a cached frame-id. >> >> If GDB ever tries to reinflate the SELECTED_FRAME frame_info_ptr (or a >> copy of it), then we will trigger the assert: >> `gdb_assert (frame_id_p (m_cached_id));` which can be found in >> `frame_info_ptr::reinflate` in frame.c. >> >> An example of how this can be triggered is included in the updated >> test case: >> >> - The 'up' command sets the selected frame to a frame with >> level > 0. >> - An inferior call invalidates the selected frame. >> - The selected frame is rebuilt following the call-stack above. >> The wrapping frame_info_ptr object doesn't cache the frame-id. >> - The 'frame' command invokes another inferior call for the pretty >> printer, which flushes the frame cache. >> - The frame_info_ptr is reinflated, e.g., to print the next >> argument, and this hits the assertion mentioned above. >> >> The problem is that frames don't always know their frame-id when they >> are placed into a frame_info_ptr, but they always do (for frames other >> than #0) after get_prev_frame_maybe_check_cycle has finished. This >> commit defers caching the frame-id in the frame_info_ptr until the >> frame cache is being flushed, at which point the frame-id is known. >> > > I think we shouldn't focus on 'get_prev_frame_maybe_check_cycle' solely, though > I didn't find any other location where the described behaviour could reproduce > (which doesn't mean it doesn't exist). I think in GDB right now get_prev_frame_maybe_check_cycle is the only place this bug exists. There are 3 places where new frame_info objects are created: create_sentinel_frame, create_new_frame, and get_prev_frame_raw. In the first two of these the frame_info is assigned an ID before being placed into the frame_info_ptr, so these are not problems. Only in get_prev_frame_raw is the frame_info placed into a frame_info_ptr before having an ID assigned, and that is only called from get_prev_frame_maybe_check_cycle. As we discussed in another thread, an ideal solution would be to have get_prev_frame_raw not create the frame_info_ptr at all, and defer this until get_prev_frame_maybe_check_cycle has finished, but this would require changing the frame sniffer API to not expect a frame_info_ptr, which seems like a bigger change than I'd like to make right now. But as I wrote the above I'm finding it harder to justify the churn of moving the frame_id caching into the frame_info_ptr::invalidate method. So, sorry to pivot again, but how about the patch below? It's far simpler than the original suggestion and is targets just get_prev_frame_maybe_check_cycle, which is where the broken frame_info_ptr objects always come from. Let me know what you think. Thanks, Andrew -- commit b6dba04e0e21f1a83b43ca616ae5fcce1a66eb9b Author: Andrew Burgess Date: Thu Jun 25 14:58:50 2026 +0000 gdb: recreate the frame_info_ptr in get_prev_frame_maybe_check_cycle Currently frame_info_ptr caches the frame_id at construction time, see frame_info_ptr::frame_info_ptr in frame.c. The problem with this is that a frame's frame-id might not be known at this point. The function get_prev_frame_maybe_check_cycle calls get_prev_frame_raw to create the previous frame, placing the result into a frame_info_ptr PREV_FRAME. For frames other than frame 0, compute_frame_id is then called computing the frame-id. However, the call to compute_frame_id only updates the frame_info object itself, the frame_info_ptr PREV_FRAME is not updated with the new frame-id. What this means is that in get_prev_frame_maybe_check_cycle, the PREV_FRAME local has no cached frame-id. Consider the call stack: get_selected_frame lookup_selected_frame frame_find_by_id get_prev_frame get_prev_frame_always get_prev_frame_always_1 get_prev_frame_maybe_check_cycle What we see is that the frame_info_ptr created in get_prev_frame_maybe_check_cycle, which lacks a cached frame_id, can be passed all the way back to lookup_selected_frame, where it will be stored in the SELECTED_FRAME global by a call to select_frame. The outer get_selected_frame call (in the above backtrace) will then return the SELECTED_FRAME global, which lacks a cached frame-id. If GDB ever tries to reinflate the SELECTED_FRAME frame_info_ptr (or a copy of it), then we will trigger the assert: `gdb_assert (frame_id_p (m_cached_id));` which can be found in `frame_info_ptr::reinflate` in frame.c. An example of how this can be triggered is included in the updated test case: - The 'up' command sets the selected frame to a frame with level > 0. - An inferior call invalidates the selected frame. - The selected frame is rebuilt following the call-stack above. The wrapping frame_info_ptr object doesn't cache the frame-id. - The 'frame' command invokes another inferior call for the pretty printer, which flushes the frame cache. - The frame_info_ptr is reinflated, e.g., to print the next argument, and this hits the assertion mentioned above. There are only 3 places in GDB where new frame_info objects are created: create_sentinel_frame, create_new_frame, and get_prev_frame_raw. Of these, the first two always calculate the frame_id before placing the frame_info object into a frame_info_ptr. Only get_prev_frame_raw, which is only called from get_prev_frame_maybe_check_cycle, creates the frame_info_ptr before the frame_id is calculated. There are two places where PREV_FRAME is returned from get_prev_frame_maybe_check_cycle. The first is only for frame #0. The frame_info_ptr::reinflate method doesn't need a frame_id for frame #0, so the first return is not a problem. The second return from get_prev_frame_maybe_check_cycle is done after the frame_id has been calculated, and it is here that the problem can be fixed. If we create a new frame_info_ptr to replace PREV_FRAME then this new frame_info_ptr will have a cached frame_id and the problem described above will no longer occur. Co-Authored-By: Rohr, Stephan diff --git a/gdb/frame.c b/gdb/frame.c index cefdde5ed1e..b91e18fad99 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -2332,7 +2332,16 @@ get_prev_frame_maybe_check_cycle (const frame_info_ptr &this_frame) throw; } - return prev_frame; + /* When PREV_FRAME was initially created it had no cached frame_id as the + frame_id had not yet been computed. Without a frame_id however + PREV_FRAME will not be able to reinflate. Recreate the frame_info_ptr + now that the frame_id is known, this new frame_info_ptr will have a + cached frame_id. + + You might wonder about the earlier return of PREV_FRAME within the + function. That is fine as reinflating a frame_info_ptr at level 0 + doesn't require a cached frame_id. */ + return frame_info_ptr (prev_frame.get ()); } /* Helper function for get_prev_frame_always, this is called inside a diff --git a/gdb/testsuite/gdb.python/pretty-print-call-by-hand.exp b/gdb/testsuite/gdb.python/pretty-print-call-by-hand.exp index 52162fc9952..a2a29c4d0f8 100644 --- a/gdb/testsuite/gdb.python/pretty-print-call-by-hand.exp +++ b/gdb/testsuite/gdb.python/pretty-print-call-by-hand.exp @@ -108,6 +108,8 @@ with_test_prefix "frame movement down" { with_test_prefix "frame movement up" { if { [start_test "TAG: final frame"] == 0 } { gdb_test "up" [multi_line "#1 .*in g \\(mt=mytype is .*\\, depth=1\\).*" ".*first frame.*"] + gdb_test "p f ()" " = 2" + gdb_test "frame" [multi_line "#1 .*in g \\(mt=mytype is .*\\, depth=1\\).*" ".*first frame.*"] } }