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

[Andrew Burgess <aburgess@redhat.com>]

"Rohr, Stephan" <stephan.rohr@intel.com> writes:

> Hello Andrew,
>
> I think changing 'get_prev_frame_raw' to return a raw pointer
> and changing 'compute_frame_id' to accept a raw pointer seems
> reasonable.
>
> I was hesitant to propose this change as calling
> 'frame_unwind_find_by_frame' in 'compute_frame_id' requires
> a frame_info_ptr.  Same for ' fi->unwind->this_id (..)'.  It feels wrong
> to create a temporary frame_info_ptr for these calls, though I observed
> similar patterns, e.g., as done in 'create_new_frame'.

Yeah, after sending the email I started implementing this change and ran
into this exact case.  I agree that changing frame_unwind_find_by_frame
probably isn't going to be ideal right now, so I think my original
suggestion might be the way to go.

I think long term it might be right to downgrade some part of the frame
building API back to raw frame_info pointers, but I think this will need
some wider changes to prevent those pointers becoming invalidated,
e.g. frame_unwind_find_by_frame can invoke Python code, which can then
invalidate the frame cache (yuck), and though that probably is never
going to do anything useful, right now we run into problems reinflating
(due to missing frame-ids), and if we switch to raw frame_info pointer
we end up with use after free issues.

I'll go through your feedback to my earlier patch, and write up a commit
message and repost it shortly.

Thanks,
Andrew