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

Eli Zaretskii
2020-08-13T13:31:54Z

> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Date: Thu, 13 Aug 2020 13:58:45 +0100
>
> This commit brings array slice support to GDB.
>
> WARNING: This patch contains a rather big hack which is limited to
> Fortran arrays, this can be seen in gdbtypes.c and f-lang.c. More
> details on this below.
>
> This patch rewrites two areas of GDB's Fortran support, the code to
> extract an array slice, and the code to print an array.
>
> After this commit a user can, from the GDB prompt, ask for a slice of
> a Fortran array and should get the correct result back. Slices can
> (optionally) have the lower bound, upper bound, and a stride
> specified. Slices can also have a negative stride.
>
> Fortran has the concept of repacking array slices. Within a compiled
> Fortran program if a user passes a non-contiguous array slice to a
> function then the compiler may have to repack the slice, this involves
> copying the elements of the slice to a new area of memory before the
> call, and copying the elements back to the original array after the
> call. Whether repacking occurs will depend on which version of
> Fortran is being used, and what type of function is being called.
>
> This commit adds support for both packed, and unpacked array slicing,
> with the default being unpacked.
>
> With an unpacked array slice, when the user asks for a slice of an
> array GDB creates a new type that accurately describes where the
> elements of the slice can be found within the original array, a
> value of this type is then returned to the user. The address of an
> element within the slice will be equal to the address of an element
> within the original array.
>
> A user can choose to selected packed array slices instead using:
>
> (gdb) set fortran repack-array-slices on|off
> (gdb) show fortran repack-array-slices
>
> With packed array slices GDB creates a new type that reflects how the
> elements of the slice would look if they were laid out in contiguous
> memory, allocates a value of this type, and then fetches the elements
> from the original array and places then into the contents buffer of
> the new value.
>
> One benefit of using packed slices over unapacked slices is the memory
> usage, taking a small slice of N elements from a large array will
> require (in GDB) N * ELEMENT_SIZE bytes of memory, while an unpacked
> array will also include all of the "padding" between the
> non-contiguous elements. There are new tests added that highlight
> this difference.
>
> There is also a new debugging flag added with this commit that
> introduces these commands:
>
> (gdb) set debug fortran-array-slicing on|off
> (gdb) show debug fortran-array-slicing
>
> This prints information about how the array slices are being built.
>
> As both the repacking, and the array printing requires GDB to walk
> through a multi-dimensional Fortran array visiting each element, this
> commit adds the file f-array-walk.h, which introduces some
> infrastructure to support this process. This means the array printing
> code in f-valprint.c is significantly reduced.
>
> The only slight issue with this commit is the "rather big hack" that I
> mentioned above. This hack allows us to handle one specific case,
> array slices with negative strides. This is something that I don't
> believe the current GDB value contents model will allow us to
> "correctly" handle, and rather than rewrite the value contents code
> right now, I'm hoping to slip this hack in as a work around.
>
> The problem is that as I see it the current value contents model
> assumes that an object base address will be the lowest address within
> that object, and that the contents of the object start at this base
> address and occupy the TYPE_LENGTH bytes after that.
>
> We do have the embedded_offset, which is used for C++ sub-classes,
> such that an object can start at some offset from the content buffer,
> however, the assumption that the object then occupies the next
> TYPE_LENGTH bytes is still true within GDB.
>
> The problem is that Fortran arrays with a negative stride don't follow
> this pattern. In this case the base address of the object points to
> the element with the highest address, the contents of the array then
> start at some offset _before_ the base address, and proceed for one
> element _past_ the base address.
>
> This is further complicated because arrays with negative strides like
> this are always dynamic types, the program being debugged has passed a
> slice with a negative stride to a function, and it is only when we
> actually try to look at the slice within the function that the dynamic
> type is resolved, and the negative type is seen. When dealing with
> dynamic types like this the address is actually stored on the _type_,
> not the value, this dynamic address then overrides the value's address
> in the value_address function.
>
> I currently don't see any way to handle this address configuration
> with GDB's current dynamic type and value system, which is why I've
> added this hack:
>
> When we resolve a dynamic Fortran array type, if the stride is
> negative, then we adjust the base address to point to the lowest
> address required by the array. The printing and slicing code is aware
> of this adjustment and will correctly slice and print Fortran arrays.
>
> Where this hack will show through to the user is if they ask for the
> address of an array in their program with a negative array stride, the
> address they get from GDB will not match the address that would be
> computed within the Fortran program.
>
> gdb/ChangeLog:
>
> * Makefile.in (HFILES_NO_SRCDIR): Add f-array-walker.h.
> * NEWS: Mention new options.
> * expprint.c (dump_subexp_body_standard): Print RANGE_HAS_STRIDE.
> * expression.h (enum range_type): Add RANGE_HAS_STRIDE.
> * f-array-walker.h: New file.
> * f-exp.y (arglist): Allow for a series of subranges.
> (subrange): Add cases for subranges with strides.
> * f-lang.c: Include 'gdbcmd.h' and 'f-array-walker.h'.
> (repack_array_slices): New static global.
> (show_repack_array_slices): New function.
> (fortran_array_slicing_debug): New static global.
> (show_fortran_array_slicing_debug): New function.
> (value_f90_subarray): Delete.
> (skip_undetermined_arglist): Delete.
> (class fortran_array_repacker_base_impl): New class.
> (class fortran_lazy_array_repacker_impl): New class.
> (class fortran_array_repacker_impl): New class.
> (fortran_value_subarray): Complete rewrite.
> (set_fortran_list): New static global.
> (show_fortran_list): Likewise.
> (_initialize_f_language): Register new commands.
> (fortran_adjust_dynamic_array_base_address_hack): New function.
> * f-lang.h (fortran_adjust_dynamic_array_base_address_hack):
> Declare.
> * f-valprint.c: Include 'f-array-walker.h'.
> (class fortran_array_printer_impl): New class.
> (f77_print_array_1): Delete.
> (f77_print_array): Delete.
> (fortran_print_array): New.
> (f_value_print_inner): Update to call fortran_print_array.
> * gdbtypes.c: Include 'f-lang.h'.
> (resolve_dynamic_type_internal): Call
> fortran_adjust_dynamic_array_base_address_hack.
> * parse.c (operator_length_standard): Handle RANGE_HAS_STRIDE.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.fortran/array-slices-bad.exp: New file.
> * gdb.fortran/array-slices-bad.f90: New file.
> * gdb.fortran/array-slices-sub-slices.exp: New file.
> * gdb.fortran/array-slices-sub-slices.f90: New file.
> * gdb.fortran/array-slices.exp: Rewrite tests.
> * gdb.fortran/array-slices.f90: Rewrite tests.
> * gdb.fortran/vla-sizeof.exp: Correct expected results.
>
> gdb/doc/ChangeLog:
>
> * gdb.texinfo (Debugging Output): Document 'set/show debug
> fotran-array-slicing'.
> (Special Fortran Commands): Document 'set/show fortran
> repack-array-slices'.

Thanks, the documentation parts are okay.