From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 116018 invoked by alias); 28 Sep 2016 08:53:01 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 115988 invoked by uid 89); 28 Sep 2016 08:52:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_NONE autolearn=no version=3.3.2 spammy=unwinder, neatly, 28317, 8158 X-HELO: emailserver1.aplushosting.com Received: from emailserver1.asdf456.com (HELO emailserver1.aplushosting.com) (72.18.207.136) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with SMTP; Wed, 28 Sep 2016 08:52:58 +0000 Received: (qmail 30219 invoked by uid 0); 28 Sep 2016 08:52:56 -0000 Received: from unknown (HELO pinnacle.lan) (70.176.31.165) by emailserver1.asdf456.com with SMTP; Wed, 28 Sep 2016 08:52:56 +0000 Date: Wed, 28 Sep 2016 08:56:00 -0000 From: Kevin Buettner To: gdb-patches@sourceware.org Subject: [PATCH 2/4] Tweak meaning of VALUE_FRAME_ID Message-ID: <20160928015255.3d8463af@pinnacle.lan> In-Reply-To: <20160928014455.438266a2@pinnacle.lan> References: <20160928014455.438266a2@pinnacle.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-SW-Source: 2016-09/txt/msg00394.txt.bz2 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 desirable. 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.) This patch implements this idea. It builds on the patch "Distinguish sentinel frame from null frame". Without that work in place, it's necessary to check for null_id at several places and then obtain the sentinel frame. In the course of developing this patch, I found that the lval_register case in value_assign() required some special care. It was passing the frame associated with VALUE_FRAME_ID (for the value being assigned) to put_frame_register_bytes(). But put_frame_register_bytes() calls put_frame_register(), which calls frame_register, which does frame_register_unwind() on frame->next. I briefly considered adjusting frame_register_unwind to work on the frame passed to it instead of frame->next, but this would have required more extensive changes throughout more of GDB. Instead, I opted for changing value_assign() so that frame->prev is passed to put_frame_register_bytes(). gdb/ChangeLog: * findvar.c (value_of_register_lazy): VALUE_FRAME_ID for register value now refers to the next frame. (default_value_from_register): Likewise. (value_from_register): Likewise. * frame_unwind.c (frame_unwind_got_optimized): Likewise. * sentinel-frame.c (sentinel_frame_prev_register): Likewise. * valops.c (value_assign): Obtain `prev' frame for passing to put_frame_register_bytes(). * value.c (value_fetch_lazy): Call frame_unwind_register_value() instead of get_frame_register_value(). --- gdb/findvar.c | 22 ++++++++++++++++++---- gdb/frame-unwind.c | 2 +- gdb/sentinel-frame.c | 2 +- gdb/valops.c | 11 +++++++++++ gdb/value.c | 2 +- 5 files changed, 32 insertions(+), 7 deletions(-) diff --git a/gdb/findvar.c b/gdb/findvar.c index 6e28a29..4d4ae49 100644 --- a/gdb/findvar.c +++ b/gdb/findvar.c @@ -283,17 +283,23 @@ value_of_register_lazy (struct frame_info *frame, int regnum) { struct gdbarch *gdbarch = get_frame_arch (frame); struct value *reg_val; + struct frame_info *next_frame; gdb_assert (regnum < (gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch))); - /* We should have a valid (i.e. non-sentinel) frame. */ - gdb_assert (frame_id_p (get_frame_id (frame))); + gdb_assert (frame != NULL); + + next_frame = get_next_frame_sentinel_okay (frame); + + /* We should have a valid next frame. */ + gdb_assert (frame_id_p (get_frame_id (next_frame))); reg_val = allocate_value_lazy (register_type (gdbarch, regnum)); VALUE_LVAL (reg_val) = lval_register; VALUE_REGNUM (reg_val) = regnum; - VALUE_FRAME_ID (reg_val) = get_frame_id (frame); + VALUE_FRAME_ID (reg_val) = get_frame_id (next_frame); + return reg_val; } @@ -815,8 +821,16 @@ default_value_from_register (struct gdbarch *gdbarch, struct type *type, { int len = TYPE_LENGTH (type); struct value *value = allocate_value (type); + struct frame_info *frame; VALUE_LVAL (value) = lval_register; + frame = frame_find_by_id (frame_id); + + if (frame == NULL) + frame_id = null_frame_id; + else + frame_id = get_frame_id (get_next_frame_sentinel_okay (frame)); + VALUE_FRAME_ID (value) = frame_id; VALUE_REGNUM (value) = regnum; @@ -902,7 +916,7 @@ value_from_register (struct type *type, int regnum, struct frame_info *frame) including the location. */ v = allocate_value (type); VALUE_LVAL (v) = lval_register; - VALUE_FRAME_ID (v) = get_frame_id (frame); + VALUE_FRAME_ID (v) = get_frame_id (get_next_frame_sentinel_okay (frame)); VALUE_REGNUM (v) = regnum; ok = gdbarch_register_to_value (gdbarch, frame, regnum, type1, value_contents_raw (v), &optim, diff --git a/gdb/frame-unwind.c b/gdb/frame-unwind.c index 187e6c2..0dd98a2 100644 --- a/gdb/frame-unwind.c +++ b/gdb/frame-unwind.c @@ -210,7 +210,7 @@ frame_unwind_got_optimized (struct frame_info *frame, int regnum) mark_value_bytes_optimized_out (val, 0, TYPE_LENGTH (type)); VALUE_LVAL (val) = lval_register; VALUE_REGNUM (val) = regnum; - VALUE_FRAME_ID (val) = get_frame_id (frame); + VALUE_FRAME_ID (val) = get_frame_id (get_next_frame_sentinel_okay (frame)); return val; } 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)); return value; } diff --git a/gdb/valops.c b/gdb/valops.c index 40392e8..c3305fa 100644 --- a/gdb/valops.c +++ b/gdb/valops.c @@ -1114,6 +1114,17 @@ value_assign (struct value *toval, struct value *fromval) /* Figure out which frame this is in currently. */ frame = frame_find_by_id (VALUE_FRAME_ID (toval)); + + /* value_of_register_lazy() stores what amounts to frame->next + in VALUE_FRAME_ID. But our eventual call to + put_frame_register_bytes() will do its own next. So, to + make things work out, we must pass it frame->prev instead + of just frame. Otherwise, it'll (essentially) be + frame->next->next. */ + + if (frame != NULL) + frame = get_prev_frame (frame); + value_reg = VALUE_REGNUM (toval); if (!frame) diff --git a/gdb/value.c b/gdb/value.c index b825aec..92afc45 100644 --- a/gdb/value.c +++ b/gdb/value.c @@ -4004,7 +4004,7 @@ value_fetch_lazy (struct value *val) gdb_assert (!gdbarch_convert_register_p (get_frame_arch (frame), regnum, type)); - new_val = get_frame_register_value (frame, regnum); + new_val = frame_unwind_register_value (frame, regnum); /* If we get another lazy lval_register value, it means the register is found by reading it from the next frame.