Patchwork [review] gdb/fortran: array stride support

login
register
mail settings
Submitter Simon Marchi (Code Review)
Date Nov. 14, 2019, 2:56 p.m.
Message ID <gerrit.1573743387000.I9af2bcd1f2d4c56f76f5f3f9f89d8f06bef10d9a@gnutoolchain-gerrit.osci.io>
Download mbox | patch
Permalink /patch/35885/
State New
Headers show

Comments

Simon Marchi (Code Review) - Nov. 14, 2019, 2:56 p.m.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/627
......................................................................

gdb/fortran: array stride support

Currently GDB supports a byte or bit stride on arrays, in DWARF this
would be DW_AT_bit_stride or DW_AT_byte_stride on DW_TAG_array_type.
However, DWARF can also support DW_AT_byte_stride or DW_AT_bit_stride
on DW_TAG_subrange_type, the tag used to describe each dimension of an
array.

Strides on subranges are used by gFortran to represent Fortran arrays,
and this commit adds support for this to GDB.

I've extended the range_bounds struct to include the stride
information.  The name is possibly a little inaccurate now, but this
still sort of makes sense, the structure represents information about
the bounds of the range, and also how to move from the lower to the
upper bound (the stride).

I've added initial support for bit strides, but I've never actually
seen an example of this being generated.  Further, I don't really see
right now how GDB would currently handle a bit stride that was not a
multiple of the byte size as the code in, for example,
valarith.c:value_subscripted_rvalue seems geared around byte
addressing.  As a consequence if we see a bit stride that is not a
multiple of 8 then GDB will give a warning and then carry on, even
though the results are likely to be wrong.

gdb/ChangeLog:

	* dwarf2read.c (read_subrange_type): Read bit and byte stride and
	create a range with stride where appropriate.
	* f-valprint.c (f77_print_array_1): Take the stride into account
	when walking the array.
	* gdbtypes.c (create_range_type): Initialise the stride to
	constant zero.
	(create_range_type_with_stride): Initialise the range as normal,
	and then setup the stride.
	(has_static_range): Include the stride here.
	(create_array_type_with_stride): Consider the range stride if the
	array isn't given its own stride.
	(resolve_dynamic_range): Resolve the stride if needed.
	* gdbtypes.h (struct range_bounds) <stride>: New member variable.
	(struct range_bounds) <byte_stride_p>: New member variable.
	(TYPE_BIT_STRIDE): Define.
	(TYPE_ARRAY_BIT_STRIDE): Define.
	(create_range_type_with_stride): Declare.
	* valarith.c (value_subscripted_rvalue): Take range stride into
	account when walking the array.

gdb/testsuite/ChangeLog:

	* gdb.fortran/derived-type-striding.exp: New file.
	* gdb.fortran/derived-type-striding.f90: New file.
	* gdb.fortran/array-slices.exp: New file.
	* gdb.fortran/array-slices.f90: New file.

Change-Id: I9af2bcd1f2d4c56f76f5f3f9f89d8f06bef10d9a
---
M gdb/ChangeLog
M gdb/dwarf2read.c
M gdb/f-valprint.c
M gdb/gdbtypes.c
M gdb/gdbtypes.h
M gdb/testsuite/ChangeLog
A gdb/testsuite/gdb.fortran/array-slices.exp
A gdb/testsuite/gdb.fortran/array-slices.f90
A gdb/testsuite/gdb.fortran/derived-type-striding.exp
A gdb/testsuite/gdb.fortran/derived-type-striding.f90
M gdb/valarith.c
11 files changed, 369 insertions(+), 9 deletions(-)
Simon Marchi (Code Review) - Nov. 15, 2019, 10:36 p.m.
Tom Tromey has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/627
......................................................................


Patch Set 1:

(3 comments)

Thanks for doing this.  It looks essentially good to me, though I had
a few notes.

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +20,21 @@ information.  The name is possibly a little inaccurate now, but this
| +still sort of makes sense, the structure represents information about
| +the bounds of the range, and also how to move from the lower to the
| +upper bound (the stride).
| +
| +I've added initial support for bit strides, but I've never actually
| +seen an example of this being generated.  Further, I don't really see
| +right now how GDB would currently handle a bit stride that was not a
| +multiple of the byte size as the code in, for example,
| +valarith.c:value_subscripted_rvalue seems geared around byte
| +addressing.  As a consequence if we see a bit stride that is not a
| +multiple of 8 then GDB will give a warning and then carry on, even
| +though the results are likely to be wrong.

PS1, Line 31:

I wonder if it would be better to "error" when evaluating
such an expression, to avoid giving incorrect answers.

| +
| +gdb/ChangeLog:
| +
| +	* dwarf2read.c (read_subrange_type): Read bit and byte stride and
| +	create a range with stride where appropriate.
| +	* f-valprint.c (f77_print_array_1): Take the stride into account
| +	when walking the array.
| +	* gdbtypes.c (create_range_type): Initialise the stride to
| +	constant zero.
| --- gdb/gdbtypes.c
| +++ gdb/gdbtypes.c
| @@ -978,17 +1007,18 @@ create_static_range_type (struct type *result_type, struct type *index_type,
|  /* Predicate tests whether BOUNDS are static.  Returns 1 if all bounds values
|     are static, otherwise returns 0.  */
|  
|  static int
|  has_static_range (const struct range_bounds *bounds)
|  {
|    return (bounds->low.kind == PROP_CONST
| -	  && bounds->high.kind == PROP_CONST);
| +	  && bounds->high.kind == PROP_CONST
| +	  && bounds->stride.kind == PROP_CONST);

PS1, Line 1015:

It seems like this will not be true for types that don't
have a stride -- changing the semantics of this function.
Won't this negatively affect other cases?

|  }
|  
|  
|  /* Set *LOWP and *HIGHP to the lower and upper bounds of discrete type
|     TYPE.  Return 1 if type is a range type, 0 if it is discrete (and
|     bounds will fit in LONGEST), or -1 otherwise.  */
|  
|  int
|  get_discrete_bounds (struct type *type, LONGEST *lowp, LONGEST *highp)
| --- gdb/gdbtypes.h
| +++ gdb/gdbtypes.h
| @@ -620,8 +620,19 @@ struct range_bounds
| +  /* The stride value for this range.  This can be stored in bits or bytes
| +     based on the value of BYTE_STRIDE_P.  It is optional to have a stride
| +     value, if this range has no stride value defined then this will be set
| +     to the constant zero.  */
| +
| +  struct dynamic_prop stride;
| +
| +  /* If this is true this STRIDE is in bytes, otherwise STRIDE is in bits.  */
| +
| +  bool byte_stride_p;

PS1, Line 629:

Perhaps this should be at the end of the struct, so it packs better.

| +
|    /* * The bias.  Sometimes a range value is biased before storage.
|       The bias is added to the stored bits to form the true value.  */
|  
|    LONGEST bias;
|  
|    /* True if HIGH range bound contains the number of elements in the
|       subrange.  This affects how the final high bound is computed.  */
|
Simon Marchi (Code Review) - Nov. 15, 2019, 11:54 p.m.
Andrew Burgess has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/627
......................................................................


Patch Set 1:

(3 comments)

I'll roll a new revision - I just wanted to check how you feel about the gdbtypes.c change given my comment below... thanks.

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +20,21 @@ information.  The name is possibly a little inaccurate now, but this
| +still sort of makes sense, the structure represents information about
| +the bounds of the range, and also how to move from the lower to the
| +upper bound (the stride).
| +
| +I've added initial support for bit strides, but I've never actually
| +seen an example of this being generated.  Further, I don't really see
| +right now how GDB would currently handle a bit stride that was not a
| +multiple of the byte size as the code in, for example,
| +valarith.c:value_subscripted_rvalue seems geared around byte
| +addressing.  As a consequence if we see a bit stride that is not a
| +multiple of 8 then GDB will give a warning and then carry on, even
| +though the results are likely to be wrong.

PS1, Line 31:

I'll change to an error.

| +
| +gdb/ChangeLog:
| +
| +	* dwarf2read.c (read_subrange_type): Read bit and byte stride and
| +	create a range with stride where appropriate.
| +	* f-valprint.c (f77_print_array_1): Take the stride into account
| +	when walking the array.
| +	* gdbtypes.c (create_range_type): Initialise the stride to
| +	constant zero.
| --- gdb/gdbtypes.c
| +++ gdb/gdbtypes.c
| @@ -978,17 +1007,18 @@ create_static_range_type (struct type *result_type, struct type *index_type,
|  /* Predicate tests whether BOUNDS are static.  Returns 1 if all bounds values
|     are static, otherwise returns 0.  */
|  
|  static int
|  has_static_range (const struct range_bounds *bounds)
|  {
|    return (bounds->low.kind == PROP_CONST
| -	  && bounds->high.kind == PROP_CONST);
| +	  && bounds->high.kind == PROP_CONST
| +	  && bounds->stride.kind == PROP_CONST);

PS1, Line 1015:

If the range is initialised without a stride then the stride property
is set to a constant 0, its only if there _is_ a stride that this
could ever be non-constant.  I'll add a comment to this effect here,
but does this sound like it addresses your concern?

|  }
|  
|  
|  /* Set *LOWP and *HIGHP to the lower and upper bounds of discrete type
|     TYPE.  Return 1 if type is a range type, 0 if it is discrete (and
|     bounds will fit in LONGEST), or -1 otherwise.  */
|  
|  int
|  get_discrete_bounds (struct type *type, LONGEST *lowp, LONGEST *highp)
| --- gdb/gdbtypes.h
| +++ gdb/gdbtypes.h
| @@ -620,8 +620,19 @@ struct range_bounds
| +  /* The stride value for this range.  This can be stored in bits or bytes
| +     based on the value of BYTE_STRIDE_P.  It is optional to have a stride
| +     value, if this range has no stride value defined then this will be set
| +     to the constant zero.  */
| +
| +  struct dynamic_prop stride;
| +
| +  /* If this is true this STRIDE is in bytes, otherwise STRIDE is in bits.  */
| +
| +  bool byte_stride_p;

PS1, Line 629:

I'll move it.

| +
|    /* * The bias.  Sometimes a range value is biased before storage.
|       The bias is added to the stored bits to form the true value.  */
|  
|    LONGEST bias;
|  
|    /* True if HIGH range bound contains the number of elements in the
|       subrange.  This affects how the final high bound is computed.  */
|
Simon Marchi (Code Review) - Nov. 18, 2019, 6:58 p.m.
Tom Tromey has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/627
......................................................................


Patch Set 1:

(1 comment)

| --- gdb/gdbtypes.c
| +++ gdb/gdbtypes.c
| @@ -978,17 +1007,18 @@ create_static_range_type (struct type *result_type, struct type *index_type,
|  /* Predicate tests whether BOUNDS are static.  Returns 1 if all bounds values
|     are static, otherwise returns 0.  */
|  
|  static int
|  has_static_range (const struct range_bounds *bounds)
|  {
|    return (bounds->low.kind == PROP_CONST
| -	  && bounds->high.kind == PROP_CONST);
| +	  && bounds->high.kind == PROP_CONST
| +	  && bounds->stride.kind == PROP_CONST);

PS1, Line 1015:

Yes, sorry about this -- I missed the hunk that initializes this
field in create_range_type.

|  }
|  
|  
|  /* Set *LOWP and *HIGHP to the lower and upper bounds of discrete type
|     TYPE.  Return 1 if type is a range type, 0 if it is discrete (and
|     bounds will fit in LONGEST), or -1 otherwise.  */
|  
|  int
|  get_discrete_bounds (struct type *type, LONGEST *lowp, LONGEST *highp)
Simon Marchi (Code Review) - Nov. 18, 2019, 9:50 p.m.
Andrew Burgess has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/627
......................................................................


Patch Set 2:

Updated the patch to add a comment in has_static_range.  I also made that function return a bool.
Simon Marchi (Code Review) - Nov. 22, 2019, 10:12 a.m.
Andrew Burgess has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/627
......................................................................


Patch Set 4:

Rebase and slightly improved the test - it now covers dynamically allocated arrays as well.
Simon Marchi (Code Review) - Nov. 22, 2019, 1:06 p.m.
Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/627
......................................................................


Patch Set 4:

(3 comments)

I can't assess the correctness of the patch, as I don't really know the domain, but I noted a few points.

| --- gdb/dwarf2read.c
| +++ gdb/dwarf2read.c
| @@ -18053,8 +18053,18 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
|    if (low.kind == PROP_CONST
|        && !TYPE_UNSIGNED (base_type) && (low.data.const_val & negative_mask))
|      low.data.const_val |= negative_mask;
|    if (high.kind == PROP_CONST
|        && !TYPE_UNSIGNED (base_type) && (high.data.const_val & negative_mask))
|      high.data.const_val |= negative_mask;
|  
| -  range_type = create_range_type (NULL, orig_base_type, &low, &high, bias);
| +  /* Check for bit and byte strides.  */
| +  struct attribute *attr_bit_stride, *attr_byte_stride;

PS4, Line 18061:

You might as well declare them on the line where you assign them.

| +  struct dynamic_prop bit_stride_prop, byte_stride_prop;
| +  attr_byte_stride = dwarf2_attr (die, DW_AT_byte_stride, cu);
| +  if (attr_byte_stride != nullptr)
| +    {
| +      struct type *prop_type
| +	= dwarf2_per_cu_addr_sized_int_type (cu->per_cu, false);
| +      attr_to_dynamic_prop (attr_byte_stride, die, cu, &byte_stride_prop,
| +			    prop_type);
| +    }
| --- gdb/gdbtypes.c
| +++ gdb/gdbtypes.c
| @@ -942,9 +946,21 @@ create_range_type (struct type *result_type, struct type *index_type,
|       less than the lower bound, so checking the lower bound is not
|       enough.  Make sure we do not mark a range type whose upper bound
|       is negative as unsigned.  */
|    if (high_bound->kind == PROP_CONST && high_bound->data.const_val < 0)
|      TYPE_UNSIGNED (result_type) = 0;
|  
|    return result_type;
|  }
|  
| +/* Like CREATE_RANGE_TYPE but also sets up a stride.  When BYTE_STRIDE_P
| +   is true the value in STRIDE is a byte stride, otherwise STRIDE is a bit
| +   stride.  */

PS4, Line 957:

This comment should be

 /* See gdbtypes.h.  */

I know that other functions around don't adhere to this rule, but I
think for a new function, or when modifying an existing function
comment, we should do it right.

| +
| +struct type *
| +create_range_type_with_stride (struct type *result_type,
| +			       struct type *index_type,
| +			       const struct dynamic_prop *low_bound,
| +			       const struct dynamic_prop *high_bound,
| +			       LONGEST bias,
| +			       const struct dynamic_prop *stride,
| +			       bool byte_stride_p)

 ...

| @@ -2017,0 +2061,19 @@ resolve_dynamic_range (struct type *dyn_range_type,
| +    {
| +      stride.kind = PROP_CONST;
| +      stride.data.const_val = value;
| +
| +      /* If we have a bit stride that is not a multiple of the byte stride
| +	 then I really don't think this is going to work with current GDB.
| +	 The array indexing code in GDB seems to be pretty heavily tied to
| +	 byte offsets right now.  If this comes up then we warn the user
| +	 and set up a known incorrect stride.  */
| +      if (!byte_stride_p && (value % HOST_CHAR_BIT) != 0)

PS4, Line 2070:

Using HOST_CHAR_BIT (a few occurences in the patch) when manipulating
target data is probably not right.  You probably want to use
gdbarch_addressable_memory_unit_size, using the gdbarch from the type.

| +	error (_("bit strides that are not a multiple of the byte size "
| +		 "are currently not supported"));
| +    }
| +  else
| +    {
| +      stride.kind = PROP_UNDEFINED;
| +      stride.data.const_val = 0;
| +      byte_stride_p = true;
| +    }
Simon Marchi (Code Review) - Nov. 22, 2019, 5:31 p.m.
Andrew Burgess has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/627
......................................................................


Patch Set 5:

(3 comments)

All issues addressed.

| --- gdb/dwarf2read.c
| +++ gdb/dwarf2read.c
| @@ -18053,8 +18053,18 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
|    if (low.kind == PROP_CONST
|        && !TYPE_UNSIGNED (base_type) && (low.data.const_val & negative_mask))
|      low.data.const_val |= negative_mask;
|    if (high.kind == PROP_CONST
|        && !TYPE_UNSIGNED (base_type) && (high.data.const_val & negative_mask))
|      high.data.const_val |= negative_mask;
|  
| -  range_type = create_range_type (NULL, orig_base_type, &low, &high, bias);
| +  /* Check for bit and byte strides.  */
| +  struct attribute *attr_bit_stride, *attr_byte_stride;

PS4, Line 18061:

Done.

| +  struct dynamic_prop bit_stride_prop, byte_stride_prop;
| +  attr_byte_stride = dwarf2_attr (die, DW_AT_byte_stride, cu);
| +  if (attr_byte_stride != nullptr)
| +    {
| +      struct type *prop_type
| +	= dwarf2_per_cu_addr_sized_int_type (cu->per_cu, false);
| +      attr_to_dynamic_prop (attr_byte_stride, die, cu, &byte_stride_prop,
| +			    prop_type);
| +    }
| --- gdb/gdbtypes.c
| +++ gdb/gdbtypes.c
| @@ -942,9 +946,21 @@ create_range_type (struct type *result_type, struct type *index_type,
|       less than the lower bound, so checking the lower bound is not
|       enough.  Make sure we do not mark a range type whose upper bound
|       is negative as unsigned.  */
|    if (high_bound->kind == PROP_CONST && high_bound->data.const_val < 0)
|      TYPE_UNSIGNED (result_type) = 0;
|  
|    return result_type;
|  }
|  
| +/* Like CREATE_RANGE_TYPE but also sets up a stride.  When BYTE_STRIDE_P
| +   is true the value in STRIDE is a byte stride, otherwise STRIDE is a bit
| +   stride.  */

PS4, Line 957:

Done.

| +
| +struct type *
| +create_range_type_with_stride (struct type *result_type,
| +			       struct type *index_type,
| +			       const struct dynamic_prop *low_bound,
| +			       const struct dynamic_prop *high_bound,
| +			       LONGEST bias,
| +			       const struct dynamic_prop *stride,
| +			       bool byte_stride_p)

 ...

| @@ -2017,0 +2061,19 @@ resolve_dynamic_range (struct type *dyn_range_type,
| +    {
| +      stride.kind = PROP_CONST;
| +      stride.data.const_val = value;
| +
| +      /* If we have a bit stride that is not a multiple of the byte stride
| +	 then I really don't think this is going to work with current GDB.
| +	 The array indexing code in GDB seems to be pretty heavily tied to
| +	 byte offsets right now.  If this comes up then we warn the user
| +	 and set up a known incorrect stride.  */
| +      if (!byte_stride_p && (value % HOST_CHAR_BIT) != 0)

PS4, Line 2070:

I don't think that gdbarch_addressable_memory_unit_size is what I
want.  From it's comment in gdbarch.h:

  /* Return the size in 8-bit bytes of an addressable memory unit on this
   architecture.  This corresponds to the number of 8-bit bytes associated to
   each address in memory. */

While what I want is the number of bits in a byte, which is available
as... wait for it.... TARGET_CHAR_BIT.  I don't know what I was
thinking when I originally used HOST_CHAR_BIT.  Thanks for pointing
this out.

| +	error (_("bit strides that are not a multiple of the byte size "
| +		 "are currently not supported"));
| +    }
| +  else
| +    {
| +      stride.kind = PROP_UNDEFINED;
| +      stride.data.const_val = 0;
| +      byte_stride_p = true;
| +    }
Simon Marchi (Code Review) - Nov. 22, 2019, 5:46 p.m.
Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/627
......................................................................


Patch Set 5:

(1 comment)

| --- gdb/gdbtypes.c
| +++ gdb/gdbtypes.c
| @@ -2017,0 +2061,19 @@ resolve_dynamic_range (struct type *dyn_range_type,
| +    {
| +      stride.kind = PROP_CONST;
| +      stride.data.const_val = value;
| +
| +      /* If we have a bit stride that is not a multiple of the byte stride
| +	 then I really don't think this is going to work with current GDB.
| +	 The array indexing code in GDB seems to be pretty heavily tied to
| +	 byte offsets right now.  If this comes up then we warn the user
| +	 and set up a known incorrect stride.  */
| +      if (!byte_stride_p && (value % HOST_CHAR_BIT) != 0)

PS4, Line 2070:

Sorry, I meant gdbarch_addressable_memory_unit_size * 8, to have the
number of bits in a "byte" on that architecture.

I think TARGET_CHAR_BIT is actually broken and should be removed,
because it is a compile time constant.  Given that you can debug code
from different architectures (some with 8 bit bytes and some with 16
bit bytes) with the same GDB, TARGET_CHAR_BIT will necessarily be
wrong for some of these architectures.

| +	error (_("bit strides that are not a multiple of the byte size "
| +		 "are currently not supported"));
| +    }
| +  else
| +    {
| +      stride.kind = PROP_UNDEFINED;
| +      stride.data.const_val = 0;
| +      byte_stride_p = true;
| +    }
Simon Marchi (Code Review) - Nov. 29, 2019, 11:35 p.m.
Andrew Burgess has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/627
......................................................................


Patch Set 7:

(1 comment)

| --- gdb/gdbtypes.c
| +++ gdb/gdbtypes.c
| @@ -2017,0 +2061,19 @@ resolve_dynamic_range (struct type *dyn_range_type,
| +    {
| +      stride.kind = PROP_CONST;
| +      stride.data.const_val = value;
| +
| +      /* If we have a bit stride that is not a multiple of the byte stride
| +	 then I really don't think this is going to work with current GDB.
| +	 The array indexing code in GDB seems to be pretty heavily tied to
| +	 byte offsets right now.  If this comes up then we warn the user
| +	 and set up a known incorrect stride.  */
| +      if (!byte_stride_p && (value % HOST_CHAR_BIT) != 0)

PS4, Line 2070:

OK, I see.  I'll get this change made ASAP.  Thanks.

| +	error (_("bit strides that are not a multiple of the byte size "
| +		 "are currently not supported"));
| +    }
| +  else
| +    {
| +      stride.kind = PROP_UNDEFINED;
| +      stride.data.const_val = 0;
| +      byte_stride_p = true;
| +    }
Simon Marchi (Code Review) - Nov. 30, 2019, 10:11 p.m.
Andrew Burgess has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/627
......................................................................


Patch Set 9:

(1 comment)

| --- gdb/gdbtypes.c
| +++ gdb/gdbtypes.c
| @@ -2017,0 +2061,19 @@ resolve_dynamic_range (struct type *dyn_range_type,
| +    {
| +      stride.kind = PROP_CONST;
| +      stride.data.const_val = value;
| +
| +      /* If we have a bit stride that is not a multiple of the byte stride
| +	 then I really don't think this is going to work with current GDB.
| +	 The array indexing code in GDB seems to be pretty heavily tied to
| +	 byte offsets right now.  If this comes up then we warn the user
| +	 and set up a known incorrect stride.  */
| +      if (!byte_stride_p && (value % HOST_CHAR_BIT) != 0)

PS4, Line 2070:

I think that the latest version should address this issue.  There were
four places I'd made use of TARGET_CHAR_BIT, in f-valprint.c,
valarith.c, and gdbtypes.c I'm now also using
gdbarch_addressable_memory_unit_size to convert the byte stride into
address unit stride.  In gdbtypes.h I now just use '8' because here we
really want to convert bytes to bits, there's no address units
involved here I think.

| +	error (_("bit strides that are not a multiple of the byte size "
| +		 "are currently not supported"));
| +    }
| +  else
| +    {
| +      stride.kind = PROP_UNDEFINED;
| +      stride.data.const_val = 0;
| +      byte_stride_p = true;
| +    }
Simon Marchi (Code Review) - Dec. 1, 2019, 12:09 a.m.
Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/627
......................................................................


Patch Set 9:

(1 comment)

LGTM, I have just noted something about a comment.

| --- gdb/gdbtypes.h
| +++ gdb/gdbtypes.h
| @@ -1960,13 +1977,21 @@ extern struct type *create_static_range_type (struct type *, struct type *,
|  extern struct type *create_array_type_with_stride
|    (struct type *, struct type *, struct type *,
|     struct dynamic_prop *, unsigned int);
|  
|  extern struct type *create_range_type (struct type *, struct type *,
|  				       const struct dynamic_prop *,
|  				       const struct dynamic_prop *,
|  				       LONGEST);
|  
| +/* Like CREATE_RANGE_TYPE but also sets up a stride.  When BYTE_STRIDE_P
| +   is true the value in STRIDE is a byte stride, otherwise STRIDE is a bit
| +   stride.  */

PS9, Line 1988:

I know the surrounding code is like that, but I don't find it very
readable to refer to parameter names when the declaration doesn't
include parameters names.  I'd be much inclined to adding the
parameter names below at the same time.

| +
| +extern struct type * create_range_type_with_stride
| +  (struct type *, struct type *, const struct dynamic_prop *,
| +   const struct dynamic_prop *, LONGEST, const struct dynamic_prop *, bool);
| +
|  extern struct type *create_array_type (struct type *, struct type *,
|  				       struct type *);
|  
|  extern struct type *lookup_array_range_type (struct type *, LONGEST, LONGEST);
Simon Marchi (Code Review) - Dec. 1, 2019, 12:09 a.m.
Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/627
......................................................................


Patch Set 9: Code-Review+2

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8be8efb..210af76 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,25 @@ 
+2019-11-14  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* dwarf2read.c (read_subrange_type): Read bit and byte stride and
+	create a range with stride where appropriate.
+	* f-valprint.c (f77_print_array_1): Take the stride into account
+	when walking the array.
+	* gdbtypes.c (create_range_type): Initialise the stride to
+	constant zero.
+	(create_range_type_with_stride): Initialise the range as normal,
+	and then setup the stride.
+	(has_static_range): Include the stride here.
+	(create_array_type_with_stride): Consider the range stride if the
+	array isn't given its own stride.
+	(resolve_dynamic_range): Resolve the stride if needed.
+	* gdbtypes.h (struct range_bounds) <stride>: New member variable.
+	(struct range_bounds) <byte_stride_p>: New member variable.
+	(TYPE_BIT_STRIDE): Define.
+	(TYPE_ARRAY_BIT_STRIDE): Define.
+	(create_range_type_with_stride): Declare.
+	* valarith.c (value_subscripted_rvalue): Take range stride into
+	account when walking the array.
+
 2019-11-14  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
 	* python/py-finishbreakpoint.c (gdbpy_breakpoint_created):
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index bbfa442..14d294d 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -18047,7 +18047,51 @@ 
       && !TYPE_UNSIGNED (base_type) && (high.data.const_val & negative_mask))
     high.data.const_val |= negative_mask;
 
-  range_type = create_range_type (NULL, orig_base_type, &low, &high, bias);
+  /* Check for bit and byte strides.  */
+  struct attribute *attr_bit_stride, *attr_byte_stride;
+  struct dynamic_prop bit_stride_prop, byte_stride_prop;
+  attr_byte_stride = dwarf2_attr (die, DW_AT_byte_stride, cu);
+  if (attr_byte_stride != nullptr)
+    {
+      struct type *prop_type
+	= dwarf2_per_cu_addr_sized_int_type (cu->per_cu, false);
+      attr_to_dynamic_prop (attr_byte_stride, die, cu, &byte_stride_prop,
+			    prop_type);
+    }
+  attr_bit_stride = dwarf2_attr (die, DW_AT_bit_stride, cu);
+  if (attr_bit_stride != nullptr)
+    {
+      /* It only makes sense to have either a bit or byte stride.  */
+      if (attr_byte_stride != nullptr)
+	{
+	  complaint (_("Found DW_AT_bit_stride and DW_AT_byte_stride "
+		       "- DIE at %s [in module %s]"),
+		     sect_offset_str (die->sect_off),
+		     objfile_name (cu->per_cu->dwarf2_per_objfile->objfile));
+	  attr_bit_stride = nullptr;
+	}
+      else
+	{
+	  struct type *prop_type
+	    = dwarf2_per_cu_addr_sized_int_type (cu->per_cu, false);
+	  attr_to_dynamic_prop (attr_bit_stride, die, cu, &bit_stride_prop,
+				prop_type);
+	}
+    }
+
+  if (attr_byte_stride != nullptr
+      || attr_bit_stride != nullptr)
+    {
+      bool byte_stride_p = (attr_byte_stride != nullptr);
+      struct dynamic_prop *stride
+	= byte_stride_p ? &byte_stride_prop : &bit_stride_prop;
+
+      range_type
+	= create_range_type_with_stride (NULL, orig_base_type, &low,
+					 &high, bias, stride, byte_stride_p);
+    }
+  else
+    range_type = create_range_type (NULL, orig_base_type, &low, &high, bias);
 
   if (high_bound_is_count)
     TYPE_RANGE_DATA (range_type)->flag_upper_bound_is_count = 1;
diff --git a/gdb/f-valprint.c b/gdb/f-valprint.c
index f9d4923..a2330a9 100644
--- a/gdb/f-valprint.c
+++ b/gdb/f-valprint.c
@@ -121,6 +121,9 @@ 
   if (nss != ndimensions)
     {
       size_t dim_size = TYPE_LENGTH (TYPE_TARGET_TYPE (type));
+      size_t stride = TYPE_ARRAY_BIT_STRIDE (type) / HOST_CHAR_BIT;
+      if (stride == 0)
+	stride = dim_size;
       size_t offs = 0;
 
       for (i = lowerbound;
@@ -137,7 +140,7 @@ 
 			     value_embedded_offset (subarray),
 			     value_address (subarray),
 			     stream, recurse, subarray, options, elts);
-	  offs += dim_size;
+	  offs += stride;
 	  fprintf_filtered (stream, ") ");
 	}
       if (*elts >= options->print_max && i < upperbound)
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index fd1c765..968aeb2 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -935,6 +935,10 @@ 
   TYPE_RANGE_DATA (result_type)->high = *high_bound;
   TYPE_RANGE_DATA (result_type)->bias = bias;
 
+  /* Initialize the stride to be a constant, the value will already be zero
+     thanks to the use of TYPE_ZALLOC above.  */
+  TYPE_RANGE_DATA (result_type)->stride.kind = PROP_CONST;
+
   if (low_bound->kind == PROP_CONST && low_bound->data.const_val >= 0)
     TYPE_UNSIGNED (result_type) = 1;
 
@@ -948,6 +952,31 @@ 
   return result_type;
 }
 
+/* Like CREATE_RANGE_TYPE but also sets up a stride.  When BYTE_STRIDE_P
+   is true the value in STRIDE is a byte stride, otherwise STRIDE is a bit
+   stride.  */
+
+struct type *
+create_range_type_with_stride (struct type *result_type,
+			       struct type *index_type,
+			       const struct dynamic_prop *low_bound,
+			       const struct dynamic_prop *high_bound,
+			       LONGEST bias,
+			       const struct dynamic_prop *stride,
+			       bool byte_stride_p)
+{
+  result_type = create_range_type (result_type, index_type, low_bound,
+				   high_bound, bias);
+
+  gdb_assert (stride != nullptr);
+  TYPE_RANGE_DATA (result_type)->stride = *stride;
+  TYPE_RANGE_DATA (result_type)->byte_stride_p = byte_stride_p;
+
+  return result_type;
+}
+
+
+
 /* Create a range type using either a blank type supplied in
    RESULT_TYPE, or creating a new type, inheriting the objfile from
    INDEX_TYPE.
@@ -982,7 +1011,8 @@ 
 has_static_range (const struct range_bounds *bounds)
 {
   return (bounds->low.kind == PROP_CONST
-	  && bounds->high.kind == PROP_CONST);
+	  && bounds->high.kind == PROP_CONST
+	  && bounds->stride.kind == PROP_CONST);
 }
 
 
@@ -1189,6 +1219,15 @@ 
 	  && !type_not_allocated (result_type)))
     {
       LONGEST low_bound, high_bound;
+      unsigned int stride;
+
+      /* If the array itself doesn't provide a stride value then take
+	 whatever stride the range provides.  Don't update BIT_STRIDE as
+	 we don't want to place the stride value from the range into this
+	 arrays bit size field.  */
+      stride = bit_stride;
+      if (stride == 0)
+	stride = TYPE_BIT_STRIDE (range_type);
 
       if (get_discrete_bounds (range_type, &low_bound, &high_bound) < 0)
 	low_bound = high_bound = 0;
@@ -1198,9 +1237,9 @@ 
 	 In such cases, the array length should be zero.  */
       if (high_bound < low_bound)
 	TYPE_LENGTH (result_type) = 0;
-      else if (bit_stride > 0)
+      else if (stride > 0)
 	TYPE_LENGTH (result_type) =
-	  (bit_stride * (high_bound - low_bound + 1) + 7) / 8;
+	  (stride * (high_bound - low_bound + 1) + 7) / 8;
       else
 	TYPE_LENGTH (result_type) =
 	  TYPE_LENGTH (element_type) * (high_bound - low_bound + 1);
@@ -1982,7 +2021,7 @@ 
   CORE_ADDR value;
   struct type *static_range_type, *static_target_type;
   const struct dynamic_prop *prop;
-  struct dynamic_prop low_bound, high_bound;
+  struct dynamic_prop low_bound, high_bound, stride;
 
   gdb_assert (TYPE_CODE (dyn_range_type) == TYPE_CODE_RANGE);
 
@@ -2014,13 +2053,40 @@ 
       high_bound.data.const_val = 0;
     }
 
+  bool byte_stride_p = TYPE_RANGE_DATA (dyn_range_type)->byte_stride_p;
+  prop = &TYPE_RANGE_DATA (dyn_range_type)->stride;
+  if (dwarf2_evaluate_property (prop, NULL, addr_stack, &value))
+    {
+      /* If we have a bit stride that is not a multiple of the byte stride
+	 then I really don't think this is going to work with current GDB.
+	 The array indexing code in GDB seems to be pretty heavily tied to
+	 byte offsets right now.  If this comes up then we warn the user
+	 and set up a known incorrect stride.  */
+      if (!byte_stride_p && (value % HOST_CHAR_BIT) != 0)
+	{
+	  warning (_("bit strides that are not a multiple of the byte "
+		     "size are currently not supported"));
+	  value = value / HOST_CHAR_BIT;
+	  byte_stride_p = true;
+	}
+
+      stride.kind = PROP_CONST;
+      stride.data.const_val = value;
+    }
+  else
+    {
+      stride.kind = PROP_UNDEFINED;
+      stride.data.const_val = 0;
+      byte_stride_p = true;
+    }
+
   static_target_type
     = resolve_dynamic_type_internal (TYPE_TARGET_TYPE (dyn_range_type),
 				     addr_stack, 0);
   LONGEST bias = TYPE_RANGE_DATA (dyn_range_type)->bias;
-  static_range_type = create_range_type (copy_type (dyn_range_type),
-					 static_target_type,
-					 &low_bound, &high_bound, bias);
+  static_range_type = create_range_type_with_stride
+    (copy_type (dyn_range_type), static_target_type,
+     &low_bound, &high_bound, bias, &stride, byte_stride_p);
   TYPE_RANGE_DATA (static_range_type)->flag_bound_evaluated = 1;
   return static_range_type;
 }
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 6d6ff59..c0940e1 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -617,6 +617,17 @@ 
 
   struct dynamic_prop high;
 
+  /* The stride value for this range.  This can be stored in bits or bytes
+     based on the value of BYTE_STRIDE_P.  It is optional to have a stride
+     value, if this range has no stride value defined then this will be set
+     to the constant zero.  */
+
+  struct dynamic_prop stride;
+
+  /* If this is true this STRIDE is in bytes, otherwise STRIDE is in bits.  */
+
+  bool byte_stride_p;
+
   /* * The bias.  Sometimes a range value is biased before storage.
      The bias is added to the stored bits to form the true value.  */
 
@@ -1345,6 +1356,9 @@ 
   TYPE_RANGE_DATA(range_type)->high.kind
 #define TYPE_LOW_BOUND_KIND(range_type) \
   TYPE_RANGE_DATA(range_type)->low.kind
+#define TYPE_BIT_STRIDE(range_type) \
+  (TYPE_RANGE_DATA(range_type)->stride.data.const_val \
+   * (TYPE_RANGE_DATA(range_type)->byte_stride_p ? HOST_CHAR_BIT : 1))
 
 /* Property accessors for the type data location.  */
 #define TYPE_DATA_LOCATION(thistype) \
@@ -1387,6 +1401,9 @@ 
 #define TYPE_ARRAY_LOWER_BOUND_VALUE(arraytype) \
    (TYPE_LOW_BOUND(TYPE_INDEX_TYPE((arraytype))))
 
+#define TYPE_ARRAY_BIT_STRIDE(arraytype) \
+  (TYPE_BIT_STRIDE(TYPE_INDEX_TYPE((arraytype))))
+
 /* C++ */
 
 #define TYPE_SELF_TYPE(thistype) internal_type_self_type (thistype)
@@ -1959,6 +1976,10 @@ 
 				       const struct dynamic_prop *,
 				       LONGEST);
 
+extern struct type * create_range_type_with_stride
+  (struct type *, struct type *, const struct dynamic_prop *,
+   const struct dynamic_prop *, LONGEST, const struct dynamic_prop *, bool);
+
 extern struct type *create_array_type (struct type *, struct type *,
 				       struct type *);
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 7416b82..0e09ca2 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,11 @@ 
+2019-11-14  Richard Bunt  <richard.bunt@arm.com>
+	    Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.fortran/derived-type-striding.exp: New file.
+	* gdb.fortran/derived-type-striding.f90: New file.
+	* gdb.fortran/array-slices.exp: New file.
+	* gdb.fortran/array-slices.f90: New file.
+
 2019-11-12  Tom Tromey  <tom@tromey.com>
 
 	* lib/tuiterm.exp (_accept): Add wait_for parameter.  Check output
diff --git a/gdb/testsuite/gdb.fortran/array-slices.exp b/gdb/testsuite/gdb.fortran/array-slices.exp
new file mode 100644
index 0000000..afd030b
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/array-slices.exp
@@ -0,0 +1,55 @@ 
+# Copyright 2019 Free Software Foundation, Inc.
+
+# 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/> .
+
+# Print a 2 dimensional assumed shape array.  We pass different slices
+# of the array to a subroutine and print the array as recieved within
+# the subroutine.  This should exercise GDB's ability to handle
+# different strides for the different dimensions.
+
+if {[skip_fortran_tests]} { return -1 }
+
+standard_testfile ".f90"
+
+if {[prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} \
+	 {debug f90}]} {
+    return -1
+}
+
+if ![runto_main] {
+    untested "could not run to main"
+    return -1
+}
+
+gdb_breakpoint "show"
+gdb_breakpoint [gdb_get_line_number "Final Breakpoint"]
+
+set array_contents \
+    [list \
+	 " = \\(\\( 1, 2, 3, 4, 5, 6, 7, 8, 9, 10\\) \\( 11, 12, 13, 14, 15, 16, 17, 18, 19, 20\\) \\( 21, 22, 23, 24, 25, 26, 27, 28, 29, 30\\) \\( 31, 32, 33, 34, 35, 36, 37, 38, 39, 40\\) \\( 41, 42, 43, 44, 45, 46, 47, 48, 49, 50\\) \\( 51, 52, 53, 54, 55, 56, 57, 58, 59, 60\\) \\( 61, 62, 63, 64, 65, 66, 67, 68, 69, 70\\) \\( 71, 72, 73, 74, 75, 76, 77, 78, 79, 80\\) \\( 81, 82, 83, 84, 85, 86, 87, 88, 89, 90\\) \\( 91, 92, 93, 94, 95, 96, 97, 98, 99, 100\\) \\)" \
+	 " = \\(\\( 1, 2, 3, 4, 5\\) \\( 11, 12, 13, 14, 15\\) \\( 21, 22, 23, 24, 25\\) \\( 31, 32, 33, 34, 35\\) \\( 41, 42, 43, 44, 45\\) \\)" \
+	 " = \\(\\( 1, 3, 5, 7, 9\\) \\( 21, 23, 25, 27, 29\\) \\( 41, 43, 45, 47, 49\\) \\( 61, 63, 65, 67, 69\\) \\( 81, 83, 85, 87, 89\\) \\)" \
+	 " = \\(\\( 1, 4, 7, 10\\) \\( 21, 24, 27, 30\\) \\( 41, 44, 47, 50\\) \\( 61, 64, 67, 70\\) \\( 81, 84, 87, 90\\) \\)" \
+	 " = \\(\\( 1, 5, 9\\) \\( 31, 35, 39\\) \\( 61, 65, 69\\) \\( 91, 95, 99\\) \\)" ]
+
+set i 0
+foreach result $array_contents {
+    incr i
+    with_test_prefix "test $i" {
+	gdb_continue_to_breakpoint "show"
+	gdb_test "p array" $result
+    }
+}
+
+gdb_continue_to_breakpoint "continue to Final Breakpoint"
diff --git a/gdb/testsuite/gdb.fortran/array-slices.f90 b/gdb/testsuite/gdb.fortran/array-slices.f90
new file mode 100644
index 0000000..6f80a51
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/array-slices.f90
@@ -0,0 +1,56 @@ 
+! Copyright 2019 Free Software Foundation, Inc.
+!
+! 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/>.
+
+subroutine show (message, array)
+  character (len=*) :: message
+  integer, dimension (:,:) :: array
+
+  print *, message
+  do i=LBOUND (array, 2), UBOUND (array, 2), 1
+     do j=LBOUND (array, 1), UBOUND (array, 1), 1
+        write(*, fmt="(i4)", advance="no") array (j, i)
+     end do
+     print *, ""
+ end do
+ print *, array
+ print *, ""
+
+end subroutine show
+
+program test
+
+  interface
+     subroutine show (message, array)
+       character (len=*) :: message
+       integer, dimension(:,:) :: array
+     end subroutine show
+  end interface
+
+  integer, dimension (1:10,1:10) :: array
+
+  do i=LBOUND (array, 2), UBOUND (array, 2), 1
+     do j=LBOUND (array, 1), UBOUND (array, 1), 1
+        array (j,i) = ((i - 1) * UBOUND (array, 2)) + j
+     end do
+  end do
+
+  call show ("array", array)
+  call show ("array (1:5,1:5)", array (1:5,1:5))
+  call show ("array (1:10:2,1:10:2)", array (1:10:2,1:10:2))
+  call show ("array (1:10:3,1:10:2)", array (1:10:3,1:10:2))
+  call show ("array (1:10:5,1:10:3)", array (1:10:4,1:10:3))
+
+  print *, "" ! Final Breakpoint.
+end program test
diff --git a/gdb/testsuite/gdb.fortran/derived-type-striding.exp b/gdb/testsuite/gdb.fortran/derived-type-striding.exp
new file mode 100644
index 0000000..a2590a9
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/derived-type-striding.exp
@@ -0,0 +1,37 @@ 
+# Copyright 2019 Free Software Foundation, Inc.
+
+# 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/> .
+
+# Print some single dimensional integer arrays that will have a byte
+# stride in the debug information.
+
+if {[skip_fortran_tests]} { return -1 }
+
+standard_testfile ".f90"
+
+if {[prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} \
+	 {debug f90}]} {
+    return -1
+}
+
+if {![runto [gdb_get_line_number "post_init"]]} then {
+    perror "couldn't run to breakpoint post_init"
+    continue
+}
+
+# Test homogeneous derived type.
+gdb_test "p point_dimension" "= \\\(2, 2, 2, 2, 2, 2, 2, 2, 2\\\)"
+
+# Test mixed type derived type.
+gdb_test "p point_mixed_dimension" "= \\\(3, 3, 3, 3\\\)"
diff --git a/gdb/testsuite/gdb.fortran/derived-type-striding.f90 b/gdb/testsuite/gdb.fortran/derived-type-striding.f90
new file mode 100644
index 0000000..8189ad3
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/derived-type-striding.f90
@@ -0,0 +1,43 @@ 
+! Copyright 2019 Free Software Foundation, Inc.
+!
+! 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/>.
+
+program derived_type_member_stride
+    type cartesian
+        integer(kind=8) :: x
+        integer(kind=8) :: y
+        integer(kind=8) :: z
+    end type
+    type mixed_cartesian
+        integer(kind=8) :: x
+        integer(kind=4) :: y
+        integer(kind=8) :: z
+    end type
+    type(cartesian), dimension(10), target :: cloud
+    type(mixed_cartesian), dimension(10), target :: mixed_cloud
+    integer(kind=8), dimension(:), pointer :: point_dimension => null()
+    integer(kind=8), dimension(:), pointer :: point_mixed_dimension => null()
+    cloud(:)%x = 1
+    cloud(:)%y = 2
+    cloud(:)%z = 3
+    point_dimension => cloud(1:9)%y
+    mixed_cloud(:)%x = 1
+    mixed_cloud(:)%y = 2
+    mixed_cloud(:)%z = 3
+    point_mixed_dimension => mixed_cloud(1:4)%z
+    ! Prevent the compiler from optimising the work out.
+    print *, cloud(:)%x ! post_init
+    print *, point_dimension
+    print *, point_mixed_dimension
+end program
diff --git a/gdb/valarith.c b/gdb/valarith.c
index 7f1b24f..fb42688 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -188,6 +188,11 @@ 
   struct type *array_type = check_typedef (value_type (array));
   struct type *elt_type = check_typedef (TYPE_TARGET_TYPE (array_type));
   ULONGEST elt_size = type_length_units (elt_type);
+
+  LONGEST stride = TYPE_ARRAY_BIT_STRIDE (array_type);
+  if (stride != 0)
+    elt_size = stride / HOST_CHAR_BIT;
+
   ULONGEST elt_offs = elt_size * (index - lowerbound);
 
   if (index < lowerbound