RE: [PATCH 1/1] gdb: set the cache information in 'get_prev_frame_maybe_check_cycle'

["Rohr, Stephan" <stephan.rohr@intel.com>]

Hi Tom,

thank you for your feedback!  I will fix the typos in v2 of the patch.

I'll try to explain the problem in more detail now:

  1. If you run an inferior call after the up command invalidates
      the selected frame.
  2. The selected frame is rebuild following the chain

        get_selected_frame () -> lookup_selected_frame ()
        -> get_prev_frame_always_1 ()
        -> get_prev_frame_maybe_check_cycle ()

  3. At this point, the frame_info_ptr is constructed before the
      frame-id is computed.  The cache Information is not updated as
      'compute_frame_id' only updates the frame id of the internal
      'frame_info' pointer.

  4. If we now run the "frame" command in combination with a
      pretty-printer, this invokes another infcall to evaluate the
      pretty-printer.

  5. This clears the 'm_ptr' member of the frame_info_ptr. Since the cache
       id is not set, a subsequent reinflate fails.

A very simple fix for this could be:

@@ -2332,7 +2361,7 @@ get_prev_frame_maybe_check_cycle (const frame_info_ptr &this_frame)
       throw;
     }

-  return prev_frame;
+  return frame_info_ptr (prev_frame.get ());
 }

It constructs a new frame_info_ptr using the updated raw pointer
of 'prev_frame'.  This is somewhat redundant as another frame_info_ptr
is added to the frame list in the frame_info_ptr ctor.

The frame list entry from 

   frame_info_ptr prev_frame = get_prev_frame_raw (this_frame);

is removed again by the frame_info_ptr's dtor.

This would add some overhead. 

I didn't find a better solution.  We could modify 'get_prev_frame_raw' to
return a raw pointer instead (it is the only location where is this called at
all).  But we'd need a temporary copy of the frame_info_ptr anyways,
either to pass a frame_info_ptr to 'compute_frame_id' or inside of
'compute_frame_id' (to forward the frame id to
'frame_unwind_find_by_frame').  Changing these functions to
accept a raw frame_info pointer is not desired in my point of view.

I appreciate your feedback.

Thanks
Stephan

> -----Original Message-----
> From: Tom Tromey <tom@tromey.com>
> Sent: Friday, 26 June 2026 17:07
> To: Rohr, Stephan <stephan.rohr@intel.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH 1/1] gdb: set the cache information in
> 'get_prev_frame_maybe_check_cycle'
> 
> >>>>> Stephan Rohr <stephan.rohr@intel.com> writes:
> 
> > From: "Rohr, Stephan" <stephan.rohr@intel.com>
> > 'get_prev_frame_maybe_check_cycle' retrieves the previous frame and
> > computes the frame id.  At this point, the frame id is not cached yet,
> > which prevents the frame from being reinflated once the frane cache is
> 
> s/frane/frame/
> 
> >   1. Build the 'prev_frame' and eventually calls
> >      'get_prev_frame_maybe_check_cycle'.  The frame_id is only computed
> >      for the raw frame pointer, the wrapping 'frame_info_ptr' it not
> >      updated.
> 
> s/it not/is not/
> 
> > +  /* No cycle and the frame is added to the frame stash, update the cache
> > +     information to ensure the frame can be reinflated after the cache is
> > +     flushed.  */
> > +  if (prev_frame)
> > +    prev_frame.update_cache ();
> > +
> 
> I wonder if some other approach could be taken here.
> It seems unfortunate to have to add a new public method to this class
> just to work around some sort of ordering issue.
> 
> That said I'm not sure I understand the problem well enough to suggest a
> better solution.  Like should maybe get_prev_frame_raw not return a
> frame_info_ptr?  And then the frame_info_ptr be created after the call
> to frame_stash_add in get_prev_frame_maybe_check_cycle?
> 
> Tom
Intel Deutschland GmbH

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany
Tel: +49 89 991 430, www.intel.de
Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell
Chairperson of the Supervisory Board: Nicole Lau
Registered Seat: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928