Re: [patch 11/12] entryval#2: @entry values even for references

[Pedro Alves <pedro at codesourcery dot com>]

On Friday 16 September 2011 19:24:34, Jan Kratochvil wrote:
> On Fri, 16 Sep 2011 14:26:09 +0200, Pedro Alves wrote:
> > On Tuesday 13 September 2011 20:50:47, Jan Kratochvil wrote:
> > > +             struct value *deref_val;
> > > +
> > > +             deref_val = coerce_ref_if_computed (original_value);
> > 
> > Hmm, this doesn't look right.  It ignores embedded_offset.  What if the reference
> > we're currently printing is a field of a struct at offset != 0?
> > E.g., `struct { long l; long &r } foo;', and we're `p foo', which descends
> > into printing R at FOO + offset of R ?
> 
> It is not perfect but I do not know about some short way out.
> 
> coerce_ref_if_computed currently can have effect only for @entry values
> (only entry_data_value_funcs has coerce_ref != NULL) and in such case the
> first printing code will have embedded_offset == 0 and further printing gets

But not in the example I gave?  Or are you saying that there can't be
an entryval whose type is a struct?  If that's the case, I'm okay with
the assert for now.

> resolved via common_val_print passed the dereferenced value with
> original_value no longer present anywhere.

>
> If it should work really correctly there are more cases to catch such as
> f77_print_array (*) being passed original_value etc.  The correct way would be
> the rework to full struct value * printing
> 	http://sourceware.org/ml/gdb-patches/2010-10/msg00127.html
> but I did not want to get deeper in the patchset dependencies.

I'm not sure that'd be necessary.  (the first 2 steps are
complete, btw).  That would change the running-offset scheme,
allowing value contents to be even more lazy, but that does not
mean the current scheme doesn't work.

E.g, if entryval's lval_funcs->coerce_ref took an embedded_offset,
and returned the target value with value_offset adjusted you'd be
almost there?  The problem is that you share/refcount the target
value, so we'd either need to unshare it so we could have copies with
different offsets set, or we'd add a new lval_computed type that serves
as view with offset into a current value, as I proposed in that URL,
but without changing everything else in valprint.

> (*) Fortran support is planned for entry_values, it was even AFAIK one of the
>     reasons for DW_AT_GNU_call_site_data_value at all, it waits now on
>     gcc PR debug/49981 but that can be incremental GDB update later.
> 
> Is it OK this way?
> 
> +	      deref_val = coerce_ref_if_computed (original_value);
> +	      if (deref_val != NULL)
> +		{
> +		  /* More complicated computed references are not supported.  */
> +		  gdb_assert (embedded_offset == 0);
> +		}
> +	      else

-- 
Pedro Alves