Re: [PATCH] gdb: user variables with components of dynamic type

[Andrew Burgess <andrew.burgess@embecosm.com>]

Hi Joel,

Sorry for the length of time it has taken me to get back to you.
Initially I was just chewing over your feedback, then I got
sidetracked.....

I have a question about what you wrote, see inline below.


* Joel Brobecker <brobecker@adacore.com> [2020-11-15 18:07:59 +0400]:

> Hi Andrew,
> 
> > So your third choice was the winner, the array has dynamic type and
> > includes a computed data location.
> 
> Thanks for providing the additional details.
> 
> > The problem in terms of implementation is that really everything is
> > either a real inline value, or a pointer.  All the other words are
> > just language sugar on top of these two choices.
> 
> Agreed.
> 
> > When we look at C++ references (basically pointers + automatic
> > dereferencing),
> 
> Thanks for sharing the experiment. This was actually a good refresher
> for me.
> 
> > So we get the behaviour we might expect, the pointer value underlying
> > the reference is preserved, but the value pointed too is not.
> 
> Yeah. What I took away from this experiment is that I think
> the syntax in printing the value gives some clues as to why
> some of the underlying value is not captured by the value history
> mechanism. Whether this is enough or not will likely depend on
> the level of the user, but I tend to think that a reasonably sharp
> user seeing @<some-address>: can infer that the value printed
> from it is the current value at the address shown. So good enough
> for me.
> 
> > > It's possible that the compromise you suggest (treat dynamic components
> > > the same as pointers) might be the most reasonable way out, but I think
> > > it'll invite confusion on the users' side, and probably bug reports.
> > > At the very least, I think we should warn users when we do this, so
> > > as to be sure to set expectations right, on the spot.
> > 
> > Adding a warning would be reasonably simple, we can start with (in
> > value.c:set_internalvar):
> > 
> >   if (is_dynamic_type (value_type (new_data.value)))
> >     warning ("some warning text here...");
> > 
> > There's two problems, the first is easy enough to solve:  if the top
> > level value being captured is dynamic, then we do capture the _actual_
> > value, it's only when a sub-component is dynamic that we have
> > problems.  The above check will trigger if only the top-level value is
> > dynamic, so it warns in too many places.
> > 
> > As a concrete example, given this Fortran type:
> > 
> >   type :: some_type
> >      integer, allocatable :: array_one (:,:)
> >      integer :: a_field
> >      integer, allocatable :: array_two (:,:)
> >   end type some_type
> > 
> >   type(some_type) :: some_var
> > 
> > Then in GDB:
> > 
> >   (gdb) set $foo = some_var
> > 
> > We capture the contents of the some_type struct, including the
> > pointers to the dynamic objects array_one and array_two.  But if
> > instead we do:
> > 
> >   (gdb) set $bar = some_var%array_one
> > 
> > Now we capture the full contents of array_one, there's no further
> > dynamic type resolution required.  Changing 'some_var%array_one' will
> > not change the value of $bar, but the change would be see in $foo.
> > 
> > The harder problem is, what warning do we print??  I initially went
> > with:
> > 
> >   components of dynamically typed values are not currently captured within internal variables
> > 
> > despite being a bit long, it's not immediately clear if a user will
> > know what 'dynamically typed values' means?  Maybe we end up needing a
> > language specific warning, so for Fortran:
> > 
> >   the values of allocatable fields are not currently captured within internal variables
> > 
> > thoughts or suggestions are welcome...
> 
> I admit I'm having minor headaches trying to wrap my head around
> the various scenarios and how we would want to deal with the situation.
> 
> For instance, do we auto-dereference [1] the same way for all languages?
> For Ada, for instance, I think we always auto-derefrence, regardless
> if the data we're printing is top-level or whether it's part of
> an aggregate. It sounds like the same is True for Fortran as well.
> But it does look from your experiments that C++ only dereferences
> in some of the cases (e.g. when printing a struct with a field being
> a reference, the field's value is the address, without the actual
> underlying value). This question would have an impact on the warning,
> since we would warn only when there is auto-dereferencing.
> 
> For the timing of the warning, at least, it seems clear to me that
> it would be printed when printing the value. Do we want to print it
> when accessing the value too? It would seem logical, I think, but
> are there scenarios where this could be counter-productive?
> 
> There's also the question of avoiding to get the warning printed
> in the middle of the value itself. This might be a bit minor, but
> it's something I've always found to be distracting and hard to read,
> when a warning is lost with some other data being printed.
> 
> > > Have you looked at how we handle components which are references?
> > > I wonder how well we handle those...
> > 
> > As above we treat them as pointers, but guard against possible
> > confusion by displaying them as pointers.
> > 
> > I would not like to change Fortran from displaying dynamnic types as
> > their actual value (and instead just display a pointer) as that seems
> > like a really bad change just to work around a limitation with
> > internal variables.
> 
> 100% agreed.

That's good, and not really surprising as from what you wrote I think
this is what Ada does, but then later on you write...

> 
> > What I think is super interesting is how this all interacts with
> > pretty-printers.  So, if I start with this test program:
> 
> Interesting indeed.
> 
> > I wonder if this problem should just be solved (at least in the
> > short/medium term) by improving the documentation for internal
> > variables?
> 
> For me, if I had to summarize my current thinking thanks to our
> discussions so far, I would say:
> 
>   (a) The case of fields whose type is dynamic is really very
>       similar to the case of references.
> 
>   (b) Since we handle references the same as pointers, I think
>       it's fine as a first instance to fix our immediate issue,
>       which is the internal error, by providing the exact same
>       kind of behavior for those dynamic fields as we provide
>       for references.

...this.

Aren't you here arguing that we should immediately fix this issue by
changing the way dynamic types are printed such that they are handled
more like C++ references (i.e. print the address rather than the
contents).  Which seems to be the complete opposite of your opinion
above.

I'm curious, if Ada makes use of dynamic typing then what does GDB
print when an object of dynamic type is nested within some other
object?

What happens if the parent object is assigned to a user variable?

My Ada foo is weak, but I'll try to dig into this and understand what
happens, but maybe you know the answer already.

I guess given your concerns then the idea solution here would be one
that somehow allows GDB to capture all of the dynamic content at the
time the user variable is created.  I'll try to figure out if there's
any good ways to achieve this.

Thanks,
Andrew

> 
>   (c) Finding a way to make this clearer for users would be a nice
>       enhancement, but this would be a general enhancement, not
>       something required in the context of your patch.
> 
> So you and I are converging towards the same solution in the code,
> and I agree that a documentation update might be useful as well.
> 
> Going back to your original patch, I would need more research
> than what I have time for to determine whether I'd fix it
> the same way you did at the location you did. After much staring
> with the entire function's implementation as a context, the patch
> does make sense to me, especially if I ignore a bit the asserts
> for a minute.  Hopefully others with a more complete knowledge of
> the area of value-saving for the value history can chime in.
> 
> I wish I could be more help! :-/
> 
> -- 
> Joel
> 
> [1] I know your situation has to do with dynamic types rather than
> references, but I think both cases are in the same boat and could
> be treated the same.