This is the mail archive of the
mailing list for the GDB project.
Re: [PATCH 2/4] Tweak meaning of VALUE_FRAME_ID
- From: Pedro Alves <palves at redhat dot com>
- To: Kevin Buettner <kevin at buettner dot to>, gdb-patches at sourceware dot org
- Date: Wed, 12 Oct 2016 14:08:21 +0100
- Subject: Re: [PATCH 2/4] Tweak meaning of VALUE_FRAME_ID
- Authentication-results: sourceware.org; auth=none
- References: <email@example.com> <firstname.lastname@example.org>
On 09/28/2016 09:52 AM, Kevin Buettner wrote:
> Tweak meaning of VALUE_FRAME_ID.
> The VALUE_FRAME_ID macro provides access to a member in struct value
> that's used to hold the frame id that's used when determining a
> register's value or when assigning to a register. The underlying
> member has a long and obscure name. I won't refer to it here, but
> will simply refer to VALUE_FRAME_ID as if it's the struct value member
> instead of being a convenient macro.
> At the moment, without this patch in place, VALUE_FRAME_ID is set in
> value_of_register_lazy() and several other locations to hold the frame
> id of the frame passed to those functions.
> VALUE_FRAME_ID is used in the lval_register case of
> value_fetch_lazy(). To fetch the register's value, it calls
> get_frame_register_value() which, in turn, calls
> frame_unwind_register_value() with frame->next.
> A python based unwinder may wish to determine the value of a register
> or evaluate an expression containing a register. When it does this,
> value_fetch_lazy() will be called under some circumstances. It will
> attempt to determine the frame id associated with the frame passed to
> it. In so doing, it will end up back in the frame sniffer of the very
> same python unwinder that's attempting to learn the value of a
> register as part of the sniffing operation. This recursion is not
> As noted above, when value_fetch_lazy() wants to fetch a register's
> value, it does so (indirectly) by unwinding from frame->next.
> With this in mind, a solution suggests itself: Change VALUE_FRAME_ID
> to hold the frame id associated with the next frame. Then, when it
> comes time to obtain the value associated with the register, we can
> simply unwind from the frame corresponding to the frame id stored in
> VALUE_FRAME_ID. This neatly avoids the python unwinder recursion
> problem by changing when the "next" operation occurs. Instead of the
> "next" operation occuring when the register value is fetched, it
> occurs earlier on when assigning a frame id to VALUE_FRAME_ID.
> (Thanks to Pedro for this suggestion.)
At the very least, The VALUE_FRAME_ID macro's documentation and
the value->frame_id's documentation must be updated to reflect
the new meaning. Ideally, we'd rename these to VALUE_NEXT_FRAME_ID
and value->frame_id. But that'd probably be best done as a
separate follow up patch.
> diff --git a/gdb/sentinel-frame.c b/gdb/sentinel-frame.c
> index 6cd1bc3..515650c 100644
> --- a/gdb/sentinel-frame.c
> +++ b/gdb/sentinel-frame.c
> @@ -51,7 +51,7 @@ sentinel_frame_prev_register (struct frame_info *this_frame,
> struct value *value;
> value = regcache_cooked_read_value (cache->regcache, regnum);
> - VALUE_FRAME_ID (value) = get_frame_id (this_frame);
> + VALUE_FRAME_ID (value) = get_frame_id (get_next_frame_sentinel_okay (this_frame));
This looks a bit odd. I think it only works because getting the
next of the sentinel frame returns back the sentinel frame again.
How about we write directly:
VALUE_FRAME_ID (value) = sentinel_frame_id;
With that, maybe we could make it an internal error to
call get_next_frame_sentinel_okay on the sentinel frame?
Otherwise this look OK to me, but I'd like to understand
better the need for get_frame_id call in the other patch.
Maybe there's something else that we should be doing.