set_value_component_location in apply_val_pretty_printer
Commit Message
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
@@ -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))
@@ -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)
@@ -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)
{
@@ -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);