Re: [PATCHv6] gdb: prevent an assertion when computing the frame_id for an inline frame

Simon Marchi via Gdb-patches
2021-09-22T14:14:34Z

> static struct frame_info *
> -get_prev_frame_if_no_cycle (struct frame_info *this_frame)
> +get_prev_frame_maybe_check_cycle (struct frame_info *this_frame)
> {
> struct frame_info *prev_frame;
>
> + /* This assert primarily checks that CYCLE_DETECTION_P is only false for
> + inline frames. However, this assertion also makes some claims about
> + what the state of GDB should be when we enter this function and
> + THIS_FRAME is an inline frame.
> +
> + If frame #0 is an inline frame then we put off calculating the
> + frame_id until we specifically make a call to get_frame_id(). As a
> + result we can enter this function in two possible states. If GDB
> + asked for the previous frame of frame #0 then THIS_FRAME will be frame
> + #0 (an inline frame), and the frame_id will be in the NOT_COMPUTED
> + state. However, if GDB asked for the frame_id of frame #0, then, as
> + getting the frame_id of an inline frame requires us to get the
> + frame_id of the previous frame, we will still end up in here, and the
> + frame_id status will be COMPUTING.
> +
> + If we consider an inline frame at a level greater than #0 then things
> + are simpler. For these frames we immediately compute the frame_id,
> + and so, for those frames, we will always reenter this function with
> + the frame_id status of COMPUTING. */
> +
> + bool cycle_detection_p = get_frame_type (this_frame) != INLINE_FRAME;

I'm just wondering whether the "This assert primarily ..." comment above
needs a bit of re-wording following the latest code changes. As it
looks right now, the comment says that we make sure that cycle detection
is only false for inline frame, then cycle_detection_p is defined
exactly as false for inline frames. So it's kind of stating the
obvious.

Other than that, this is fine with me.

Simon