Re: [PATCH] New function value_has_address

[Yao Qi <qiyaoltc at gmail dot com>]

On Thu, Oct 27, 2016 at 2:03 PM, Pedro Alves <palves@redhat.com> wrote:
>
> I think the answer is here:
>
>   /* Location of value (if lval).  */
>   union
>   {
>     /* If lval == lval_memory, this is the address in the inferior.
>        If lval == lval_register, this is the byte offset into the
>        registers structure.  */
>     CORE_ADDR address;
>
>     /* Pointer to internal variable.  */
>     struct internalvar *internalvar;
>
>     /* Pointer to xmethod worker.  */
>     struct xmethod_worker *xm_worker;
>
>     /* If lval == lval_computed, this is a set of function pointers
>        to use to access and describe the value, and a closure pointer
>        for them to use.  */
>     struct
>     {
>       /* Functions to call.  */
>       const struct lval_funcs *funcs;
>
>       /* Closure for those functions to use.  */
>       void *closure;
>     } computed;
>   } location;
>
>
> That's a union.  We should only ever access the "address"
> on lval types that use the "address" field above.

So looks we can restrict value_has_address,

/* Return true if VALUE->location.address is valid, otherwise return
   false.  */

static int
value_has_address (const struct value *value)
{
  return (value->lval == lval_memory || value->lval == lval_register);
}

This triggers some GDB internal errors.  I fixed some of them, but
still don't know how to fix one in gdbpy_apply_val_pretty_printer,

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

still can't figure out why do we call set_value_component_location
here.  These code was added in
https://sourceware.org/ml/gdb-patches/2010-06/msg00132.html
I'll ask Tom.

>
> Note that we call value_address on lval_computed values and that
> incorrectly returns "value->location.address + value->offset",
> which is completely bogus.  (Try running to main, and then doing
> p $_siginfo).
>
> The value printing routines want to pass around a value address,
> but since we actually pass the value pointer around nowadays too,
> I think that parameter could be eliminated, and the result is
> likely to be that value_address ends up called is much fewer
> places where it doesn't really make sense.

value_address () return value is not always passed to val_print.
We pass zero to val_print in some places in ada-valprint.c.
I don't see how to eliminate tat parameter, if I understand you
correctly.

>
> IMO, struct value and its lval types would be nice candidates
> for converting to a class hierarchy, with lval_funcs
> expanded into virtual methods in struct value directly, that are
> then used by all value lval types, including lval_memory/lval_register.
>

Yes, I agree, but I don't know how much memory usage increase
if "struct value" becomes "class value".  We have such comments
to "struct value",

/* Note that the fields in this structure are arranged to save a bit
   of memory.  */

I am not ready converting value to C++ as I still don't think I fully
understand it.

-- 
Yao (齐尧)