set_value_component_location in apply_val_pretty_printer

set_value_component_location in apply_val_pretty_printer

[Yao Qi]

Hi,

I don't understand this piece of code in apply_val_pretty_printer,
why do we need to call set_value_component_location?

  set_value_component_location (value, val);
  /* set_value_component_location resets the address, so we may
     need to set it again.  */
  if (VALUE_LVAL (value) != lval_internalvar
      && VALUE_LVAL (value) != lval_internalvar_component
      && VALUE_LVAL (value) != lval_computed)
    set_value_address (value, address + embedded_offset);

It was added by Tom in
https://sourceware.org/ml/gdb-patches/2010-06/msg00132.html
There wasn't much information in email and ChangeLog.

-- 
Yao (齐尧)

Re: set_value_component_location in apply_val_pretty_printer

[Ulrich Weigand]

Yao Qi wrote:

> I don't understand this piece of code in apply_val_pretty_printer,
> why do we need to call set_value_component_location?
> 
>   set_value_component_location (value, val);
>   /* set_value_component_location resets the address, so we may
>      need to set it again.  */
>   if (VALUE_LVAL (value) !=3D lval_internalvar
>       && VALUE_LVAL (value) !=3D lval_internalvar_component
>       && VALUE_LVAL (value) !=3D lval_computed)
>     set_value_address (value, address + embedded_offset);
> 
> It was added by Tom in
> https://sourceware.org/ml/gdb-patches/2010-06/msg00132.html
> There wasn't much information in email and ChangeLog.

This the case I mentioned in my recent email to Pedro here,
where we want to create a subobject of the original object.

The problem with the value printing routines is that if you
are printing an object of some complicated type, the routines
recurse through subobjects.  However, you don't always want
to allocate a new value object for the current subobject
just so you can make the recursive call.

That's why all the val_print routines not just receive the
original value object (which remains unchanged even while
recursing through subobjects), but also an idenfication of
the subobject that is to be processed: this is a pair of
offset and type, and means, the subobject of type "type"
starting at offset "offset" within the larger object.

This works fine for the various routines in the valprint
files.  However, when using a Python pretty-printer, you
can only pass a normal value object; the pretty-printer
is not set up to handle subobjects.  Therefore, at this
point apply_val_pretty_printer does actually have to
allocate a new value object to hold the subobject.

It's reasonably simple to just create a new object of
the correct type and having the correct contents.  However,
some of the value printers also want to use the value's
*location*.  Just allocating a fresh object would have
no location information.   That's why the code above
calls set_value_component_location to set the location
of the new value to the same as the location of the old.

But this isn't really correct either, since we need the
location of the *subobject*.  Now if the value resides
in inferior memory, we can get there simply by adding
the offset to the value address.  But that's not actually
correct for values with other location types.

I think we should either add an offset parameter to
set_value_component_location, and have it attempt to
do the best thing possible to describe the subobject
location, or maybe even provide a function that creates
the subobject directly in one go, along the lines of
value_primitive_field, except not based on struct
types but simply using an offset and subobject type.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: set_value_component_location in apply_val_pretty_printer

[Tom Tromey]

>>>>> "Ulrich" == Ulrich Weigand <uweigand@de.ibm.com> writes:

>> It was added by Tom in
>> https://sourceware.org/ml/gdb-patches/2010-06/msg00132.html
>> There wasn't much information in email and ChangeLog.

What Ulrich wrote is better than anything I was likely to write.

Ulrich> That's why all the val_print routines not just receive the
Ulrich> original value object (which remains unchanged even while
Ulrich> recursing through subobjects), but also an idenfication of
Ulrich> the subobject that is to be processed: this is a pair of
Ulrich> offset and type, and means, the subobject of type "type"
Ulrich> starting at offset "offset" within the larger object.

I've thought for a long time that val_print should just be removed, and
only full values should be passed around.

Tom

Re: set_value_component_location in apply_val_pretty_printer

[Yao Qi]

On Fri, Oct 28, 2016 at 08:58:33PM +0200, Ulrich Weigand wrote:
> Yao Qi wrote:
> 
> > I don't understand this piece of code in apply_val_pretty_printer,
> > why do we need to call set_value_component_location?
> > 
> >   set_value_component_location (value, val);
> >   /* set_value_component_location resets the address, so we may
> >      need to set it again.  */
> >   if (VALUE_LVAL (value) !=3D lval_internalvar
> >       && VALUE_LVAL (value) !=3D lval_internalvar_component
> >       && VALUE_LVAL (value) !=3D lval_computed)
> >     set_value_address (value, address + embedded_offset);
> > 
> > It was added by Tom in
> > https://sourceware.org/ml/gdb-patches/2010-06/msg00132.html
> > There wasn't much information in email and ChangeLog.
> 

> 
> It's reasonably simple to just create a new object of
> the correct type and having the correct contents.  However,
> some of the value printers also want to use the value's
> *location*.  Just allocating a fresh object would have
> no location information.   That's why the code above
> calls set_value_component_location to set the location
> of the new value to the same as the location of the old.
> 
> But this isn't really correct either, since we need the
> location of the *subobject*.  Now if the value resides
> in inferior memory, we can get there simply by adding
> the offset to the value address.  But that's not actually
> correct for values with other location types.

I don't see why it is not correct to set the value's location to the same as
the location of old.  The 'whole' object and 'component' object should have
the same location with different offsets (internalvar is an exception since 
the component's location is interanlvar_component), so
set_value_component_location  looks right to me.  However,
gdb{py,scm}_apply_val_pretty_printer are wrong to me that they use
value_from_contents_and_address and set_value_address, like this,

  value = value_from_contents_and_address (type, valaddr + embedded_offset,
					  address + embedded_offset);

  set_value_component_location (value, val);
  /* set_value_component_location resets the address, so we may
     need to set it again.  */
  if (VALUE_LVAL (value) != lval_internalvar
      && VALUE_LVAL (value) != lval_internalvar_component
      && VALUE_LVAL (value) != lval_computed)
    set_value_address (value, address + embedded_offset);

because the 'whole' object may not be from memory, as you pointed out.  I 
give a patch to this.

> 
> I think we should either add an offset parameter to
> set_value_component_location, and have it attempt to
> do the best thing possible to describe the subobject
> location, or maybe even provide a function that creates
> the subobject directly in one go, along the lines of
> value_primitive_field, except not based on struct
> types but simply using an offset and subobject type.
> 

value_primitive_field gives me some hints, and I use some in the patch.
Regression tested on x86_64-linux.  This patch fixes some asserts if I
restrict value_has_address that only returns true for lval_memory and
lval_register.  value_has_address patch is here,
https://sourceware.org/ml/gdb-patches/2016-10/msg00741.html

-- 
Yao (齐尧)

From d02fbc6bf894588300bb0ff88b5dc059415f1e03 Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Fri, 11 Nov 2016 10:51:23 +0000
Subject: [PATCH] Create subobject value in pretty printer

Nowadays, we create a value of subobject in pretty printer with 'address'
being used,

  value = value_from_contents_and_address (type, valaddr + embedded_offset,
					   address + embedded_offset);

  set_value_component_location (value, val);
  /* set_value_component_location resets the address, so we may
     need to set it again.  */
  if (VALUE_LVAL (value) != lval_internalvar
      && VALUE_LVAL (value) != lval_internalvar_component
      && VALUE_LVAL (value) != lval_computed)
    set_value_address (value, address + embedded_offset);

value_from_contents_and_address creates a value from memory, but the
value we are pretty-printing may not from memory at all.

Instead of using value_from_contents_and_address, we create a value
of subobject with the same location as object's but different offset.
We avoid using address in this way.  As a result, parameter 'address'
in apply_val_pretty_printer is no longer needed, we can remove it in
next step.

We've already had the location of the 'whole' value, so it is safe
to assume we can create a value of 'component' or 'suboject' value
at the same location but with different offset.

gdb:

2016-11-11  Yao Qi  <yao.qi@linaro.org>

	* guile/scm-pretty-print.c (gdbscm_apply_val_pretty_printer):
	Don't call value_from_contents_and_address and
	set_value_address.  Call value_from_component.
	* python/py-prettyprint.c (gdbpy_apply_val_pretty_printer):
	Likewise.
	* value.c (value_from_component): New function.
	* value.h (value_from_component): Likewise.

diff --git a/gdb/guile/scm-pretty-print.c b/gdb/guile/scm-pretty-print.c
index 5253def..648ca53 100644
--- a/gdb/guile/scm-pretty-print.c
+++ b/gdb/guile/scm-pretty-print.c
@@ -985,16 +985,7 @@ gdbscm_apply_val_pretty_printer (const struct extension_language_defn *extlang,
   cleanups = make_cleanup (null_cleanup, NULL);
 
   /* Instantiate the printer.  */
-  value = value_from_contents_and_address (type, valaddr + embedded_offset,
-					   address + embedded_offset);
-
-  set_value_component_location (value, val);
-  /* set_value_component_location resets the address, so we may
-     need to set it again.  */
-  if (VALUE_LVAL (value) != lval_internalvar
-      && VALUE_LVAL (value) != lval_internalvar_component
-      && VALUE_LVAL (value) != lval_computed)
-    set_value_address (value, address + embedded_offset);
+  value = value_from_component (val, type, embedded_offset);
 
   val_obj = vlscm_scm_from_value (value);
   if (gdbscm_is_exception (val_obj))
diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index cbc168d..4f5e7f7 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -726,16 +726,7 @@ gdbpy_apply_val_pretty_printer (const struct extension_language_defn *extlang,
   cleanups = ensure_python_env (gdbarch, language);
 
   /* Instantiate the printer.  */
-  value = value_from_contents_and_address (type, valaddr + embedded_offset,
-					   address + embedded_offset);
-
-  set_value_component_location (value, val);
-  /* set_value_component_location resets the address, so we may
-     need to set it again.  */
-  if (VALUE_LVAL (value) != lval_internalvar
-      && VALUE_LVAL (value) != lval_internalvar_component
-      && VALUE_LVAL (value) != lval_computed)
-    set_value_address (value, address + embedded_offset);
+  value = value_from_component (val, type, embedded_offset);
 
   val_obj = value_to_value_object (value);
   if (! val_obj)
diff --git a/gdb/value.c b/gdb/value.c
index 9def1b3..1bb61f1 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3804,6 +3804,31 @@ value_from_history_ref (const char *h, const char **endp)
   return access_value_history (index);
 }
 
+/* Get the component value (offset by OFFSET bytes) of a struct or
+   union WHOLE.  Component's type is TYPE.  */
+
+struct value *
+value_from_component (struct value *whole, struct type *type, LONGEST offset)
+{
+  struct value *v;
+
+  if (value_lazy (whole))
+    v = allocate_value_lazy (type);
+  else
+    {
+      v = allocate_value (type);
+      value_contents_copy_raw (v, value_embedded_offset (v),
+			       whole, value_embedded_offset (whole) + offset,
+			       type_length_units (type));
+    }
+  v->offset = value_offset (whole) + offset + value_embedded_offset (whole);
+  set_value_component_location (v, whole);
+  VALUE_REGNUM (v) = VALUE_REGNUM (whole);
+  VALUE_FRAME_ID (v) = VALUE_FRAME_ID (whole);
+
+  return v;
+}
+
 struct value *
 coerce_ref_if_computed (const struct value *arg)
 {
diff --git a/gdb/value.h b/gdb/value.h
index f962508..c36eb6c 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -637,6 +637,8 @@ extern struct value *value_from_double (struct type *type, DOUBLEST num);
 extern struct value *value_from_decfloat (struct type *type,
 					  const gdb_byte *decbytes);
 extern struct value *value_from_history_ref (const char *, const char **);
+extern struct value *value_from_component (struct value *, struct type *,
+					   LONGEST);
 
 extern struct value *value_at (struct type *type, CORE_ADDR addr);
 extern struct value *value_at_lazy (struct type *type, CORE_ADDR addr);

Re: set_value_component_location in apply_val_pretty_printer

[Ulrich Weigand]

Yao Qi wrote:
> On Fri, Oct 28, 2016 at 08:58:33PM +0200, Ulrich Weigand wrote:
> > But this isn't really correct either, since we need the
> > location of the *subobject*.  Now if the value resides
> > in inferior memory, we can get there simply by adding
> > the offset to the value address.  But that's not actually
> > correct for values with other location types.
> 
> I don't see why it is not correct to set the value's location to the same as
> the location of old.  The 'whole' object and 'component' object should have
> the same location with different offsets (internalvar is an exception since 
> the component's location is interanlvar_component), so
> set_value_component_location  looks right to me.

I think in the past we have avoided using value_offset for lval_memory
values for some reason.  It's been used only for values in registers
(or other non-memory values).  I'm not really sure why this is the
case -- maybe value_offset isn't always taken into account everywhere?

But maybe it is indeed the best way to use value_offset for creating
subobjects, and if there's any fallout, just fix that.  (In the 
alternative, we might use value_offset for non-lval_memory values,
and just increment the address for lval_memory ones ...).

B.t.w. I noticed that the bottom half of value_subscripted_rvalue
now looks very similar to your new value_from_component routine;
maybe it ought to be used there too.

Otherwise the patch looks good to me.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: set_value_component_location in apply_val_pretty_printer

[Yao Qi]

On Mon, Nov 14, 2016 at 05:38:32PM +0100, Ulrich Weigand wrote:
> 
> B.t.w. I noticed that the bottom half of value_subscripted_rvalue
> now looks very similar to your new value_from_component routine;
> maybe it ought to be used there too.

I updated value_from_component so that it can be used in
value_subscripted_rvalue.  There are some tiny differences between
them, and I tweak value_from_component a little bit.

> 
> Otherwise the patch looks good to me.
> 

Patch below is regression tested on x86_64-linux and is pushed in.

-- 
Yao (齐尧)

From c1f6385f46e7e1d43d332cc24d2eadb51c4f3a3b Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Fri, 11 Nov 2016 10:51:23 +0000
Subject: [PATCH] Create subobject value in pretty printer

Nowadays, we create a value of subobject in pretty printer with 'address'
being used,

  value = value_from_contents_and_address (type, valaddr + embedded_offset,
					   address + embedded_offset);

  set_value_component_location (value, val);
  /* set_value_component_location resets the address, so we may
     need to set it again.  */
  if (VALUE_LVAL (value) != lval_internalvar
      && VALUE_LVAL (value) != lval_internalvar_component
      && VALUE_LVAL (value) != lval_computed)
    set_value_address (value, address + embedded_offset);

value_from_contents_and_address creates a value from memory, but the
value we are pretty-printing may not from memory at all.

Instead of using value_from_contents_and_address, we create a value
of subobject with the same location as object's but different offset.
We avoid using address in this way.  As a result, parameter 'address'
in apply_val_pretty_printer is no longer needed, we can remove it in
next step.

We've already had the location of the 'whole' value, so it is safe
to assume we can create a value of 'component' or 'suboject' value
at the same location but with different offset.

gdb:

2016-11-21  Yao Qi  <yao.qi@linaro.org>

	* guile/scm-pretty-print.c (gdbscm_apply_val_pretty_printer):
	Don't call value_from_contents_and_address and
	set_value_address.  Call value_from_component.
	* python/py-prettyprint.c (gdbpy_apply_val_pretty_printer):
	Likewise.
	* value.c (value_from_component): New function.
	* value.h (value_from_component): Likewise.
	* valarith.c (value_subscripted_rvalue): Call
	value_from_component.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3797e8b..768a1b3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,15 @@
+2016-11-21  Yao Qi  <yao.qi@linaro.org>
+
+	* guile/scm-pretty-print.c (gdbscm_apply_val_pretty_printer):
+	Don't call value_from_contents_and_address and
+	set_value_address.  Call value_from_component.
+	* python/py-prettyprint.c (gdbpy_apply_val_pretty_printer):
+	Likewise.
+	* value.c (value_from_component): New function.
+	* value.h (value_from_component): Likewise.
+	* valarith.c (value_subscripted_rvalue): Call
+	value_from_component.
+
 2016-11-19  Joel Brobecker  <brobecker@adacore.com>
 
 	* contrib/ari/gdb_ari.sh: Add detection of printf_vma and
diff --git a/gdb/guile/scm-pretty-print.c b/gdb/guile/scm-pretty-print.c
index 5253def..648ca53 100644
--- a/gdb/guile/scm-pretty-print.c
+++ b/gdb/guile/scm-pretty-print.c
@@ -985,16 +985,7 @@ gdbscm_apply_val_pretty_printer (const struct extension_language_defn *extlang,
   cleanups = make_cleanup (null_cleanup, NULL);
 
   /* Instantiate the printer.  */
-  value = value_from_contents_and_address (type, valaddr + embedded_offset,
-					   address + embedded_offset);
-
-  set_value_component_location (value, val);
-  /* set_value_component_location resets the address, so we may
-     need to set it again.  */
-  if (VALUE_LVAL (value) != lval_internalvar
-      && VALUE_LVAL (value) != lval_internalvar_component
-      && VALUE_LVAL (value) != lval_computed)
-    set_value_address (value, address + embedded_offset);
+  value = value_from_component (val, type, embedded_offset);
 
   val_obj = vlscm_scm_from_value (value);
   if (gdbscm_is_exception (val_obj))
diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index cbc168d..4f5e7f7 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -726,16 +726,7 @@ gdbpy_apply_val_pretty_printer (const struct extension_language_defn *extlang,
   cleanups = ensure_python_env (gdbarch, language);
 
   /* Instantiate the printer.  */
-  value = value_from_contents_and_address (type, valaddr + embedded_offset,
-					   address + embedded_offset);
-
-  set_value_component_location (value, val);
-  /* set_value_component_location resets the address, so we may
-     need to set it again.  */
-  if (VALUE_LVAL (value) != lval_internalvar
-      && VALUE_LVAL (value) != lval_internalvar_component
-      && VALUE_LVAL (value) != lval_computed)
-    set_value_address (value, address + embedded_offset);
+  value = value_from_component (val, type, embedded_offset);
 
   val_obj = value_to_value_object (value);
   if (! val_obj)
diff --git a/gdb/valarith.c b/gdb/valarith.c
index d532ecf..fb4bc7d 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -206,7 +206,6 @@ value_subscripted_rvalue (struct value *array, LONGEST index, int lowerbound)
   struct type *elt_type = check_typedef (TYPE_TARGET_TYPE (array_type));
   ULONGEST elt_size = type_length_units (elt_type);
   ULONGEST elt_offs = elt_size * (index - lowerbound);
-  struct value *v;
 
   if (index < lowerbound || (!TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type)
 			     && elt_offs >= type_length_units (array_type)))
@@ -227,21 +226,7 @@ value_subscripted_rvalue (struct value *array, LONGEST index, int lowerbound)
       elt_type = resolve_dynamic_type (elt_type, NULL, address);
     }
 
-  if (VALUE_LVAL (array) == lval_memory && value_lazy (array))
-    v = allocate_value_lazy (elt_type);
-  else
-    {
-      v = allocate_value (elt_type);
-      value_contents_copy (v, value_embedded_offset (v),
-			   array, value_embedded_offset (array) + elt_offs,
-			   elt_size);
-    }
-
-  set_value_component_location (v, array);
-  VALUE_REGNUM (v) = VALUE_REGNUM (array);
-  VALUE_NEXT_FRAME_ID (v) = VALUE_NEXT_FRAME_ID (array);
-  set_value_offset (v, value_offset (array) + elt_offs);
-  return v;
+  return value_from_component (array, elt_type, elt_offs);
 }
 
 \f
diff --git a/gdb/value.c b/gdb/value.c
index 0fc43d5..085784c 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3806,6 +3806,31 @@ value_from_history_ref (const char *h, const char **endp)
   return access_value_history (index);
 }
 
+/* Get the component value (offset by OFFSET bytes) of a struct or
+   union WHOLE.  Component's type is TYPE.  */
+
+struct value *
+value_from_component (struct value *whole, struct type *type, LONGEST offset)
+{
+  struct value *v;
+
+  if (VALUE_LVAL (whole) == lval_memory && value_lazy (whole))
+    v = allocate_value_lazy (type);
+  else
+    {
+      v = allocate_value (type);
+      value_contents_copy (v, value_embedded_offset (v),
+			   whole, value_embedded_offset (whole) + offset,
+			   type_length_units (type));
+    }
+  v->offset = value_offset (whole) + offset + value_embedded_offset (whole);
+  set_value_component_location (v, whole);
+  VALUE_REGNUM (v) = VALUE_REGNUM (whole);
+  VALUE_FRAME_ID (v) = VALUE_FRAME_ID (whole);
+
+  return v;
+}
+
 struct value *
 coerce_ref_if_computed (const struct value *arg)
 {
diff --git a/gdb/value.h b/gdb/value.h
index afcbcae..281b5a8 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -645,6 +645,8 @@ extern struct value *value_from_double (struct type *type, DOUBLEST num);
 extern struct value *value_from_decfloat (struct type *type,
 					  const gdb_byte *decbytes);
 extern struct value *value_from_history_ref (const char *, const char **);
+extern struct value *value_from_component (struct value *, struct type *,
+					   LONGEST);
 
 extern struct value *value_at (struct type *type, CORE_ADDR addr);
 extern struct value *value_at_lazy (struct type *type, CORE_ADDR addr);

Re: set_value_component_location in apply_val_pretty_printer

[Yao Qi]

On Mon, Nov 21, 2016 at 2:14 PM, Yao Qi <qiyaoltc@gmail.com> wrote:
>
> -  if (VALUE_LVAL (array) == lval_memory && value_lazy (array))
> -    v = allocate_value_lazy (elt_type);
> -  else
> -    {
> -      v = allocate_value (elt_type);
> -      value_contents_copy (v, value_embedded_offset (v),
> -                          array, value_embedded_offset (array) + elt_offs,
> -                          elt_size);
> -    }
> -
> -  set_value_component_location (v, array);
> -  VALUE_REGNUM (v) = VALUE_REGNUM (array);
> -  VALUE_NEXT_FRAME_ID (v) = VALUE_NEXT_FRAME_ID (array);
> -  set_value_offset (v, value_offset (array) + elt_offs);
> -  return v;
> +  return value_from_component (array, elt_type, elt_offs);
>  }
>
>

I removed the code setting VALUE_NEXT_FRAME_ID, but
value_from_component only sets VALUE_FRAME_ID.  The change
above is not equivalent, and it causes some regressions in
https://sourceware.org/ml/gdb-testers/2016-q4/msg03381.html

Sorry about that.  The fix would be setting both
VALUE_NEXT_FRAME_ID and VALUE_FRAME_ID in
value_from_component.  I'll give a fix.

-- 
Yao (齐尧)

Re: set_value_component_location in apply_val_pretty_printer

[Yao Qi]

On Mon, Nov 21, 2016 at 06:23:47PM +0000, Yao Qi wrote:
> I removed the code setting VALUE_NEXT_FRAME_ID, but
> value_from_component only sets VALUE_FRAME_ID.  The change
> above is not equivalent, and it causes some regressions in
> https://sourceware.org/ml/gdb-testers/2016-q4/msg03381.html
> 
> Sorry about that.  The fix would be setting both
> VALUE_NEXT_FRAME_ID and VALUE_FRAME_ID in
> value_from_component.  I'll give a fix.
>

Oh, looks we renamed VALUE_FRAME_ID to VALUE_NEXT_FRAME_ID recently,
https://sourceware.org/ml/gdb-patches/2016-11/msg00018.html
and I regression tested against a copy of one-week-old gdb, but
didn't see the regression.  The fix would be replacing VALUE_FRAME_ID
with VALUE_NEXT_FRAME_ID in my patch.  Testing the fix...

-- 
Yao 

Re: set_value_component_location in apply_val_pretty_printer

[Yao Qi]

On Mon, Nov 21, 2016 at 08:37:24PM +0000, Yao Qi wrote:
> On Mon, Nov 21, 2016 at 06:23:47PM +0000, Yao Qi wrote:
> > I removed the code setting VALUE_NEXT_FRAME_ID, but
> > value_from_component only sets VALUE_FRAME_ID.  The change
> > above is not equivalent, and it causes some regressions in
> > https://sourceware.org/ml/gdb-testers/2016-q4/msg03381.html
> > 
> > Sorry about that.  The fix would be setting both
> > VALUE_NEXT_FRAME_ID and VALUE_FRAME_ID in
> > value_from_component.  I'll give a fix.
> >
> 
> Oh, looks we renamed VALUE_FRAME_ID to VALUE_NEXT_FRAME_ID recently,
> https://sourceware.org/ml/gdb-patches/2016-11/msg00018.html
> and I regression tested against a copy of one-week-old gdb, but
> didn't see the regression.  The fix would be replacing VALUE_FRAME_ID
> with VALUE_NEXT_FRAME_ID in my patch.  Testing the fix...
>

The patch below fixes the regressions by replacing VALUE_FRAME_ID
with VALUE_NEXT_FRAME_ID in value_from_component.  I pushed it in.
 
-- 
Yao (齐尧)

From c5acd8159633cfde315b01431099e1ce5b23dcf7 Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Tue, 22 Nov 2016 08:53:34 +0000
Subject: [PATCH] Use VALUE_NEXT_FRAME_ID in value_from_component

We renamed VALUE_FRAME_ID to VALUE_NEXT_FRAME_ID recently,
https://sourceware.org/ml/gdb-patches/2016-11/msg00018.html
and we should use VALUE_NEXT_FRAME_ID in value_from_component
too.

gdb:

2016-11-22  Yao Qi  <yao.qi@linaro.org>

	* value.c (value_from_component): Use VALUE_NEXT_FRAME_ID
	instead of VALUE_FROM_ID.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index dfaddd1..5b88917 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2016-11-22  Yao Qi  <yao.qi@linaro.org>
+
+	* value.c (value_from_component): Use VALUE_NEXT_FRAME_ID
+	instead of VALUE_FROM_ID.
+
 2016-11-21  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* Makefile.in (%o: $(srcdir)/mi/%.c): Add missing POSTCOMPILE
diff --git a/gdb/value.c b/gdb/value.c
index 200d61d..8d33501 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3815,7 +3815,7 @@ value_from_component (struct value *whole, struct type *type, LONGEST offset)
   v->offset = value_offset (whole) + offset + value_embedded_offset (whole);
   set_value_component_location (v, whole);
   VALUE_REGNUM (v) = VALUE_REGNUM (whole);
-  VALUE_FRAME_ID (v) = VALUE_FRAME_ID (whole);
+  VALUE_NEXT_FRAME_ID (v) = VALUE_NEXT_FRAME_ID (whole);
 
   return v;
 }