Re: set_value_component_location in apply_val_pretty_printer

[Yao Qi <qiyaoltc at gmail dot com>]

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);
 }
 
 
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);