Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Andrew Burgess <andrew.burgess@embecosm.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <andrew.burgess@embecosm.com>
Subject: [PUSHED] gdb/fortran: Support negative array stride in one limited case
Date: Tue, 25 Feb 2020 16:35:00 -0000	[thread overview]
Message-ID: <20200225163500.13749-1-andrew.burgess@embecosm.com> (raw)
In-Reply-To: <87eev9kn0u.fsf@redhat.com>

I've pushed this fix now.  The exact patch I pushed is included below.


Thanks,
Andrew




---

This commit adds support for negative Fortran array strides in one
limited case, that is the case of a single element array with a
negative array stride.

The changes in this commit will be required in order for more general
negative array stride support to work correctly, however, right now
other problems in GDB prevent negative array strides from working in
the general case.

The reason negative array strides don't currently work in the general
case is that when dealing with such arrays, the base address for the
objects data is actually the highest addressed element, subsequent
elements are then accessed with a negative offset from that address,
and GDB is not currently happy with this configuration.

The changes here can be summarised as, stop treating signed values as
unsigned, specifically, the array stride, and offsets calculated using
the array stride.

This issue was identified on the mailing list by Sergio:

  https://sourceware.org/ml/gdb-patches/2020-01/msg00360.html

The test for this issue is a new one written by me as the copyright
status of the original test is currently unknown.

gdb/ChangeLog:

	* gdbtypes.c (create_array_type_with_stride): Handle negative
	array strides.
	* valarith.c (value_subscripted_rvalue): Likewise.

gdb/testsuite/ChangeLog:

	* gdb.fortran/derived-type-striding.exp: Add a new test.
	* gdb.fortran/derived-type-striding.f90: Add pointer variable for
	new test.
---
 gdb/ChangeLog                                       |  6 ++++++
 gdb/gdbtypes.c                                      | 17 +++++++++++++----
 gdb/testsuite/ChangeLog                             |  6 ++++++
 gdb/testsuite/gdb.fortran/derived-type-striding.exp |  2 ++
 gdb/testsuite/gdb.fortran/derived-type-striding.f90 |  2 ++
 gdb/valarith.c                                      |  4 ++--
 6 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 85758930491..ef110b30445 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1223,7 +1223,7 @@ create_array_type_with_stride (struct type *result_type,
 	  && !type_not_allocated (result_type)))
     {
       LONGEST low_bound, high_bound;
-      unsigned int stride;
+      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
@@ -1241,9 +1241,18 @@ create_array_type_with_stride (struct type *result_type,
 	 In such cases, the array length should be zero.  */
       if (high_bound < low_bound)
 	TYPE_LENGTH (result_type) = 0;
-      else if (stride > 0)
-	TYPE_LENGTH (result_type) =
-	  (stride * (high_bound - low_bound + 1) + 7) / 8;
+      else if (stride != 0)
+	{
+	  /* Ensure that the type length is always positive, even in the
+	     case where (for example in Fortran) we have a negative
+	     stride.  It is possible to have a single element array with a
+	     negative stride in Fortran (this doesn't mean anything
+	     special, it's still just a single element array) so do
+	     consider that case when touching this code.  */
+	  LONGEST element_count = abs (high_bound - low_bound + 1);
+	  TYPE_LENGTH (result_type)
+	    = ((abs (stride) * element_count) + 7) / 8;
+	}
       else
 	TYPE_LENGTH (result_type) =
 	  TYPE_LENGTH (element_type) * (high_bound - low_bound + 1);
diff --git a/gdb/testsuite/gdb.fortran/derived-type-striding.exp b/gdb/testsuite/gdb.fortran/derived-type-striding.exp
index 094843ca8b1..639dc4c9528 100644
--- a/gdb/testsuite/gdb.fortran/derived-type-striding.exp
+++ b/gdb/testsuite/gdb.fortran/derived-type-striding.exp
@@ -41,3 +41,5 @@ gdb_test "p point_dimension" "= \\\(2, 2, 2, 2, 2, 2, 2, 2, 2\\\)"
 # Test mixed type derived type.
 if { $gcc_with_broken_stride } { setup_kfail *-*-* gcc/92775 }
 gdb_test "p point_mixed_dimension" "= \\\(3, 3, 3, 3\\\)"
+
+gdb_test "p cloud_slice" " = \\\(\\\( x = 1, y = 2, z = 3 \\\)\\\)"
diff --git a/gdb/testsuite/gdb.fortran/derived-type-striding.f90 b/gdb/testsuite/gdb.fortran/derived-type-striding.f90
index 26829f51dc0..fb537579faa 100644
--- a/gdb/testsuite/gdb.fortran/derived-type-striding.f90
+++ b/gdb/testsuite/gdb.fortran/derived-type-striding.f90
@@ -28,9 +28,11 @@ program derived_type_member_stride
     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()
+    type(cartesian), dimension(:), pointer :: cloud_slice => null()
     cloud(:)%x = 1
     cloud(:)%y = 2
     cloud(:)%z = 3
+    cloud_slice => cloud(3:2:-2)
     point_dimension => cloud(1:9)%y
     mixed_cloud(:)%x = 1
     mixed_cloud(:)%y = 2
diff --git a/gdb/valarith.c b/gdb/valarith.c
index 79b148602bb..be0e0731bee 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -187,7 +187,7 @@ value_subscripted_rvalue (struct value *array, LONGEST index, LONGEST lowerbound
 {
   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 elt_size = type_length_units (elt_type);
 
   /* Fetch the bit stride and convert it to a byte stride, assuming 8 bits
      in a byte.  */
@@ -199,7 +199,7 @@ value_subscripted_rvalue (struct value *array, LONGEST index, LONGEST lowerbound
       elt_size = stride / (unit_size * 8);
     }
 
-  ULONGEST elt_offs = elt_size * (index - lowerbound);
+  LONGEST elt_offs = elt_size * (index - lowerbound);
 
   if (index < lowerbound
       || (!TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type)
-- 
2.14.5


  reply	other threads:[~2020-02-25 16:35 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-14 14:56 [review] gdb/fortran: array stride support Andrew Burgess (Code Review)
2019-11-15 22:36 ` Tom Tromey (Code Review)
2019-11-15 23:54 ` Andrew Burgess (Code Review)
2019-11-18 18:58 ` Tom Tromey (Code Review)
2019-11-18 21:47 ` [review v2] " Andrew Burgess (Code Review)
2019-11-18 21:50 ` Andrew Burgess (Code Review)
2019-11-18 21:55 ` [review v3] " Andrew Burgess (Code Review)
2019-11-22 10:10 ` [review v4] " Andrew Burgess (Code Review)
2019-11-22 10:12 ` Andrew Burgess (Code Review)
2019-11-22 13:06 ` Simon Marchi (Code Review)
2019-11-22 17:30 ` [review v5] " Andrew Burgess (Code Review)
2019-11-22 17:31 ` Andrew Burgess (Code Review)
2019-11-22 17:46 ` Simon Marchi (Code Review)
2019-11-28  0:45 ` [review v6] " Andrew Burgess (Code Review)
2019-11-29 23:32 ` [review v7] " Andrew Burgess (Code Review)
2019-11-29 23:35 ` Andrew Burgess (Code Review)
2019-11-30 21:47 ` [review v8] " Andrew Burgess (Code Review)
2019-11-30 22:10 ` [review v9] " Andrew Burgess (Code Review)
2019-11-30 22:11 ` Andrew Burgess (Code Review)
2019-12-01  0:09 ` Simon Marchi (Code Review)
2019-12-01  0:09 ` Simon Marchi (Code Review)
2019-12-01 22:33 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2019-12-01 22:33 ` Sourceware to Gerrit sync (Code Review)
2020-01-14  4:11 ` [PATCH] Add gdb.fortran/vla-stride.exp and report a bug (was: Re: [review] gdb/fortran: array stride support) Sergio Durigan Junior
2020-01-19  1:59   ` Andrew Burgess
2020-02-05 16:38     ` [PATCH] Add gdb.fortran/vla-stride.exp and report a bug Sergio Durigan Junior
2020-02-25 16:35       ` Andrew Burgess [this message]
2020-08-06 10:41         ` Copyright status gdb.fortran/vla-stride.exp test-case Tom de Vries
2020-08-06 13:35           ` Andrew Burgess
2020-08-18  9:50             ` Tom de Vries
2020-08-18 10:12               ` Andrew Burgess
2020-12-12  6:33         ` [PUSHED] gdb/fortran: Support negative array stride in one limited case Simon Marchi via Gdb-patches
2020-12-12 22:18           ` Andrew Burgess
2020-12-13  0:51             ` Simon Marchi via Gdb-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200225163500.13749-1-andrew.burgess@embecosm.com \
    --to=andrew.burgess@embecosm.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox