[PATCH 2/4] Tweak meaning of VALUE_FRAME_ID

[Kevin Buettner <kevin at buettner dot to>]

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.