Re: [PATCH] Allow dynamic DW_AT_bit_stride on DW_TAG_array_type

[Keith Seitz <keiths@redhat.com>]

Hi,

On 3/20/26 5:53 AM, Tom Tromey wrote:
> In Ada, it's possible to create an array where the stride is dynamic.
> For example, this happens in array_of_symbolic_length.exp where GCC
> emits:
> 
>   <2><143a>: Abbrev Number: 12 (DW_TAG_array_type)
>      <143b>   DW_AT_byte_stride : 7 byte block: fd 86 13 0 0 34 1e 	(DW_OP_GNU_variable_value: <0x1386>; DW_OP_lit4; DW_OP_mul)
>      <1443>   DW_AT_type        : <0x13c4>
>      <1447>   DW_AT_sibling     : <0x1455>
> 
> For gnat-llvm, though, it was more convenient to always emit a bit
> stride.  Using an expression for this is a DWARF extension, but it's a
> fairly obvious one, and something similar is already used in gdb.
> 
> This patch adds support for dynamic bit strides on an array to gdb.  A
> new test case, derived from gdb.dwarf2/arr-stride.exp (the derivation
> explains the copyright dates) is included.

I think this is relatively straightforward, and aside from a few
minor observations, LGTM.

Reviewed-By: Keith Seitz <keiths@redhat.com>

Keith

> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 1d1c021a51a..e764db67f17 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -3142,10 +3142,24 @@ ada_value_slice_from_ptr (struct value *array_ptr, struct type *type,
>     type_allocator alloc (base_index_type);
>     struct type *index_type
>       = create_static_range_type (alloc, base_index_type, low, high);
> +
> +  dynamic_prop prop_storage;
> +  dynamic_prop *prop = type0->dyn_prop (DYN_PROP_BYTE_STRIDE);
> +  bool is_byte_stride = true;
> +  if (prop == nullptr)
> +    {
> +      prop = type0->dyn_prop (DYN_PROP_BIT_STRIDE);
> +      is_byte_stride = false;
> +      if (prop == nullptr)
> +	{
> +	  prop = &prop_storage;
> +	  prop->set_const_val (type0->field (0).bitsize ());
> +	}
> +    }
> +

I see the above code is duplicated below -- is it worth the bother to
separate into a function? Not an action item. Just a question.

>     struct type *slice_type = create_array_type_with_stride
>   			      (alloc, type0->target_type (), index_type,
> -			       type0->dyn_prop (DYN_PROP_BYTE_STRIDE),
> -			       type0->field (0).bitsize ());
> +			       prop, is_byte_stride);
>     LONGEST base_low = ada_discrete_type_low_bound (type0->index_type ());
>     std::optional<LONGEST> base_low_pos, low_pos;
>     CORE_ADDR base;
> @@ -3177,10 +3191,24 @@ ada_value_slice (struct value *array, LONGEST low, LONGEST high)
>     type_allocator alloc (type->index_type ());
>     struct type *index_type
>       = create_static_range_type (alloc, type->index_type (), low, high);
> +
> +  dynamic_prop prop_storage;
> +  dynamic_prop *prop = type->dyn_prop (DYN_PROP_BYTE_STRIDE);
> +  bool is_byte_stride = true;
> +  if (prop == nullptr)
> +    {
> +      prop = type->dyn_prop (DYN_PROP_BIT_STRIDE);
> +      is_byte_stride = false;
> +      if (prop == nullptr)
> +	{
> +	  prop = &prop_storage;
> +	  prop->set_const_val (type->field (0).bitsize ());
> +	}
> +    }
> +
>     struct type *slice_type = create_array_type_with_stride
>   			      (alloc, type->target_type (), index_type,
> -			       type->dyn_prop (DYN_PROP_BYTE_STRIDE),
> -			       type->field (0).bitsize ());
> +			       prop, is_byte_stride);
>     std::optional<LONGEST> low_pos, high_pos;
>   
>   
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 8b87d58dd9c..b216db137bc 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -11483,13 +11483,13 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
>     struct type *type;
>     struct type *element_type, *range_type, *index_type;
>     const char *name;
> -  unsigned int bit_stride = 0;
>   
>     /* If the stride is seen and used, byte_stride_prop will be
>        non-NULL.  In this case stride_storage will be used to store the
>        data locally.  */

I think the above comment should now refer to `stride_prop' instead of
`byte_stride_prop'.

> -  dynamic_prop *byte_stride_prop = nullptr;
> +  dynamic_prop *stride_prop = nullptr;
>     dynamic_prop stride_storage;
> +  bool is_byte_stride = true;
>   
>     element_type = die_type (die, cu);
>   
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index 139467cfd85..da9397eddf8 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -2518,21 +2523,21 @@ extern struct type *create_static_range_type (type_allocator &alloc,
>      Elements will be of type ELEMENT_TYPE, the indices will be of type
>      RANGE_TYPE.
>   
> -   BYTE_STRIDE_PROP, when not NULL, provides the array's byte stride.
> -   This byte stride property is added to the resulting array type
> -   as a DYN_PROP_BYTE_STRIDE.  As a consequence, the BYTE_STRIDE_PROP
> -   argument can only be used to create types that are objfile-owned
> -   (see add_dyn_prop), meaning that either this function must be called
> -   with an objfile-owned RESULT_TYPE, or an objfile-owned RANGE_TYPE.
> +   STRIDE_PROP, when not NULL, provides the array's stride.  If
> +   IS_BYTE_STRIDE is true, then this is a byte stride; when false this
> +   is a bit stride.  This stride property is added to the resulting
> +   array type as a DYN_PROP_BYTE_STRIDE or a DYN_PROP_BIT_SIZE, as

Should this refer to DYN_PROP_BIT_STRIDE?

> +   appropriate.  As a consequence, the STRIDE_PROP argument can only
> +   be used to create types that are objfile-owned (see add_dyn_prop),
> +   meaning that either this function must be called with an
> +   objfile-owned RESULT_TYPE, or an objfile-owned RANGE_TYPE.
>   
> -   BIT_STRIDE is taken into account only when BYTE_STRIDE_PROP is NULL.
> -   If BIT_STRIDE is not zero, build a packed array type whose element
> -   size is BIT_STRIDE.  Otherwise, ignore this parameter.  */
> +   IS_BYTE_STRIDE is ignored when STRIDE_PROP is NULL.  */
>   
>   extern struct type *create_array_type_with_stride
>        (type_allocator &alloc, struct type *element_type,
> -      struct type *range_type, struct dynamic_prop *byte_stride_prop,
> -      unsigned int bit_stride);
> +      struct type *range_type, dynamic_prop *stride_prop,
> +      bool is_byte_stride);
>   
>   /* Create a range type using ALLOC with a dynamic range from LOW_BOUND
>      to HIGH_BOUND, inclusive.  INDEX_TYPE is the underlying type.  BIAS