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

Simon Marchi
2020-10-12T14:10:59Z

On 2020-10-11 2:12 p.m., Andrew Burgess wrote:
> 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:

"to selected"

>
> (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

"unapacked"

> diff --git a/gdb/f-array-walker.h b/gdb/f-array-walker.h
> new file mode 100644
> index 00000000000..395c26e5350
> --- /dev/null
> +++ b/gdb/f-array-walker.h
> @@ -0,0 +1,255 @@
> +/* Copyright (C) 2020 Free Software Foundation, Inc.
> +
> + This file is part of GDB.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +/* Support classes to wrap up the process of iterating over a
> + multi-dimensional Fortran array. */
> +
> +#ifndef F_ARRAY_WALKER_H
> +#define F_ARRAY_WALKER_H
> +
> +#include "defs.h"
> +#include "gdbtypes.h"
> +#include "f-lang.h"
> +
> +/* Class for calculating the byte offset for elements within a single
> + dimension of a Fortran array. */
> +class fortran_array_offset_calculator
> +{
> +public:
> + /* Create a new offset calculator for TYPE, which is either an array or a
> + string. */
> + fortran_array_offset_calculator (struct type *type)
> + {
> + /* Validate the type. */
> + type = check_typedef (type);
> + if (type->code () != TYPE_CODE_ARRAY
> + && (type->code () != TYPE_CODE_STRING))
> + error (_("can only compute offsets for arrays and strings"));
> +
> + /* Get the range, and extract the bounds. */
> + struct type *range_type = type->index_type ();
> + if (get_discrete_bounds (range_type, &m_lowerbound, &m_upperbound) < 0)
> + error ("unable to read array bounds");
> +
> + /* Figure out the stride for this array. */
> + struct type *elt_type = check_typedef (TYPE_TARGET_TYPE (type));
> + m_stride = type->index_type ()->bounds ()->bit_stride ();
> + if (m_stride == 0)
> + m_stride = type_length_units (elt_type);
> + else
> + {
> + struct gdbarch *arch = get_type_arch (elt_type);
> + int unit_size = gdbarch_addressable_memory_unit_size (arch);
> + m_stride /= (unit_size * 8);
> + }
> + };
> +
> + /* Get the byte offset for element INDEX within the type we are working
> + on. There is no bounds checking done on INDEX. If the stride is
> + negative then we still assume that the base address (for the array
> + object) points to the element with the lowest memory address, we then
> + calculate an offset assuming that index 0 will be the element at the
> + highest address, index 1 the next highest, and so on. This is not
> + quite how Fortran works in reality; in reality the base address of
> + the object would point at the element with the highest address, and
> + we would index backwards from there in the "normal" way, however,
> + GDB's current value contents model doesn't support having the base
> + address be near to the end of the value contents, so we currently
> + adjust the base address of Fortran arrays with negative strides so
> + their base address points at the lowest memory address. This code
> + here is part of working around this weirdness. */

I didn't quite catch this, but I think it's just because I'm missing
some knowledge about how Fortran arrays work. But it looks well
explained enough that if I needed to fix something here, I could
understand what is happening.

> + LONGEST index_offset (LONGEST index)
> + {
> + LONGEST offset;
> + if (m_stride < 0)
> + offset = std::abs (m_stride) * (m_upperbound - index);
> + else
> + offset = std::abs (m_stride) * (index - m_lowerbound);
> + return offset;
> + }
> +
> +private:
> +
> + /* The stride for the type we are working with. */
> + LONGEST m_stride;
> +
> + /* The upper bound for the type we are working with. */
> + LONGEST m_upperbound;
> +
> + /* The lower bound for the type we are working with. */
> + LONGEST m_lowerbound;
> +};
> +
> +/* A base class used by fortran_array_walker. */
> +class fortran_array_walker_base_impl
> +{
> +public:
> + /* Constructor. */
> + explicit fortran_array_walker_base_impl ()
> + { /* Nothing. */ }
> +
> + /* Called when iterating between the lower and upper bounds of each
> + dimension of the array. Return true if GDB should continue iterating,
> + otherwise, return false.
> +
> + SHOULD_CONTINUE indicates if GDB is going to stop anyway, and should
> + be taken into consideration when deciding what to return. If
> + SHOULD_CONTINUE is false then this function must also return false,
> + the function is still called though in case extra work needs to be
> + done as part of the stopping process. */
> + bool continue_walking (bool should_continue)
> + { return should_continue; }
> +
> + /* Called when GDB starts iterating over a dimension of the array. This
> + function will be called once for each of the bounds in this dimension.
> + DIM is the current dimension number, NDIM is the total number of
> + dimensions, and FIRST_P is true for the first bound of this
> + dimension, and false in all other cases. */
> + void start_dimension (int dim, int ndim, bool first_p)
> + { /* Nothing. */ }
> +
> + /* Called when GDB finishes iterating over a dimension of the array.
> + This function will be called once for each of the bounds in this
> + dimension. DIM is the current dimension number, NDIM is the total
> + number of dimensions, and LAST_P is true for the last bound of this
> + dimension, and false in all other cases. */
> + void finish_dimension (int dim, int ndim, bool last_p)
> + { /* Nothing. */ }
> +
> + /* Called when processing the inner most dimension of the array, for
> + every element in the array. PARENT_VALUE is the value from which
> + elements are being extracted, ELT_TYPE is the type of the element
> + being extracted, and ELT_OFF is the offset of the element from the
> + start of PARENT_VALUE. */
> + void process_element (struct value *parent_value, struct type *elt_type,
> + LONGEST elt_off)
> + { /* Nothing. */ }
> +};

I don't understand when start_dimension and finish_dimension get called
exactly. Looking at the implementation, it looks like it will be
something like this:

start_dimension(1, 3, true)
start_dimension(2, 3, true)
start_dimension(3, 3, true)
process_element(element at (1,1,1))
finish_dimension(3, 3, false)
start_dimension(3, 3, false)
process_element(element (1,1,2))
finish_dimension(3, 3, true)
finish_dimension(2, 3, false)
start_dimension(2, 3, false)
start_dimension(3, 3, true)
process_element(element at (1,2,1))
finish_dimension(3, 3, false)
start_dimension(3, 3, false)
process_element(element (1,2,2))
finish_dimension(3, 3, true)
finish_dimension(2, 3, false)
finish_dimension(1, 3, false)
start_dimension(1, 3, false)
start_dimension(2, 3, true)
start_dimension(3, 3, true)
process_element(element at (2,1,1))
finish_dimension(3, 3, false)
start_dimension(3, 3, false)
process_element(element (2,1,2))
finish_dimension(3, 3, true)
finish_dimension(2, 3, false)
start_dimension(2, 3, false)
start_dimension(3, 3, true)
process_element(element at (2,2,1))
finish_dimension(3, 3, false)
start_dimension(3, 3, false)
process_element(element (2,2,2))
finish_dimension(3, 3, true)
finish_dimension(2, 3, false)
finish_dimension(1, 3, false)

Essentially for the inner dimention, walk_1 calls start_dimension and
finish_dimension around each single element. That's more than I
expected to given my understanding of start_dimension and
finish_dimension. I expected them to be called just once around once
"scan" of the inner dimension, so every two elements. For the outer
dimension, I would expect a single call to start and finish, as there is
a single "scan" in this dimension. I would expect basically this:

start_dimension(1, 3, ?)
start_dimension(2, 3, ?)
start_dimension(3, 3, ?)
process_element(element at (1,1,1))
process_element(element (1,1,2))
finish_dimension(3, 3, ?)
start_dimension(3, 3, ?)
process_element(element at (1,2,1))
process_element(element (1,2,2))
finish_dimension(3, 3, ?)
finish_dimension(2, 3, ?)
start_dimension(2, 3, ?)
start_dimension(3, 3, ?)
process_element(element at (2,1,1))
process_element(element (2,1,2))
finish_dimension(3, 3, ?)
start_dimension(3, 3, ?)
process_element(element at (2,2,1))
process_element(element (2,2,2))
finish_dimension(3, 3, ?)
finish_dimension(2, 3, ?)
finish_dimension(1, 3, ?)

I omitted the first_p/last_p values, because I am not sure what they
would be. Did I understand the meaning of
start_dimension/finish_dimension wrong?

I don't have time to look at the rest of the patch now, so that is all
for now :).

Simon