Re: [PATCHv5 4/4] gdb/fortran: Add support for Fortran array slices at the GDB prompt

[Tom Tromey <tom@tromey.com>]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> The problem is that, as I see it, the current value contents model
Andrew> assumes that an object base address will be the lowest address within
Andrew> that object, and that the contents of the object start at this base
Andrew> address and occupy the TYPE_LENGTH bytes after that.

Andrew> ( We do have the embedded_offset, which is used for C++ sub-classes,
Andrew> such that an object can start at some offset from the content buffer,
Andrew> however, the assumption that the object then occupies the next
Andrew> TYPE_LENGTH bytes is still true within GDB. )

Relatedly, we had a bug for a while where the Python val-print code
could be given just a virtual base of an object, and it would then try
to examine memory outside this buffer.

I've also considered separating values from memory in some ways.

Here's something weird in gdb... debug this program:

    #include <stdio.h>
    char b[] = "hello";
    struct x {
      char *x;
    };
    int main()
    {
      struct x val; val.x = b;
      printf("%s\n", val.x);
      b[1] = 'q';
      printf("%s\n", val.x);
      return 0;
    }

If you stop at the first printf and "print val" you get:

    (gdb) p val
    $1 = {
      x = 0x40200c <b> "hello"
    }

Then at the second you can see:

    (gdb) p val
    $2 = {
      x = 0x40200c <b> "hqllo"
    }
    (gdb) p $1
    $3 = {
      x = 0x40200c <b> "hqllo"
    }

That is, the apparent value of the string in "$1" changed.  This happens
because the value only holds the pointer, the contents are read during
printing.

So, sometimes I've wondered if we want to fix that, by say attaching
more memory to the value, say as it is printed.

Another thing I've considered is maybe letting multiple values share
some memory (to avoid duplicating large arrays); or maybe going the
other way and lazily populating arrays when they are used purely as
intermediate values.

Kind of random thoughts, though I suppose the lazy array thing is
similar to something you've done in this patch.

Andrew> Where this hack will show through to the user is if they ask for the
Andrew> address of an array in their program with a negative array stride, the
Andrew> address they get from GDB will not match the address that would be
Andrew> computed within the Fortran program.

Calls for a second hack ;)

FWIW I don't think I really have a problem with your proposed hack.

Andrew> +  /* Create a new offset calculator for TYPE, which is either an array or a
Andrew> +     string.  */
Andrew> +  fortran_array_offset_calculator (struct type *type)

Single-argument constructors should normally be explicit.

Andrew> +/* A base class used by fortran_array_walker.  */
Andrew> +class fortran_array_walker_base_impl
Andrew> +{
Andrew> +public:

A class with only public methods (and no data) can just be a "struct".

Andrew> +  /* Constructor.  */
Andrew> +  explicit fortran_array_walker_base_impl ()
Andrew> +  { /* Nothing.  */ }

Doesn't need the constructor.

Andrew> +/* A class to wrap up the process of iterating over a multi-dimensional
Andrew> +   Fortran array.  IMPL is used to specialise what happens as we walk over
Andrew> +   the array.  See class FORTRAN_ARRAY_WALKER_BASE_IMPL (above) for the
Andrew> +   methods than can be used to customise the array walk.  */
Andrew> +template<typename Impl>
Andrew> +class fortran_array_walker

This seems to mix compile-time- and runtime- polymorphism.

Maybe the idea was not to have virtual methods?  But in that case this:

Andrew> +  /* Ensure that Impl is derived from the required base class.  This just
Andrew> +     ensures that all of the required API methods are available and have a
Andrew> +     sensible default implementation.  */
Andrew> +  gdb_static_assert ((std::is_base_of<fortran_array_walker_base_impl,Impl>::value));

... seems weird.  I guess the idea is to use method hiding as a kind of
static overriding?  But if any method is missing, compilation will just
fail anyway.

Tom