From: Sandra Loosemore <sandra@codesourcery.com>
To: Tobias Burnus <tobias@codesourcery.com>,
"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
"fortran@gcc.gnu.org" <fortran@gcc.gnu.org>
Subject: Re: [patch v2] Fortran: fix sm computation in CFI_allocate [PR93524]
Date: Mon, 21 Jun 2021 14:31:21 -0600 [thread overview]
Message-ID: <bcc538d3-22a0-ac4f-dc51-b561228e1e58@codesourcery.com> (raw)
In-Reply-To: <6f1a1d90-f555-6def-71c4-6b790b6b33d0@codesourcery.com>
[-- Attachment #1: Type: text/plain, Size: 560 bytes --]
On 6/21/21 5:42 AM, Tobias Burnus wrote:
> On 21.06.21 08:05, Sandra Loosemore wrote:
>
>> I ran into this bug in CFI_allocate while testing something else and
>> then realized there was already a PR open for it. It seems like an
>> easy fix, and I've used Tobias's test case from the issue more or less
>> verbatim.
>>
>> There were some other bugs added on to this issue but I think they
>> have all been fixed already except for this one.
>>
>> OK to check in?
> OK – but see some comments below.
Revised patch attached. How's this one?
-Sandra
[-- Attachment #2: pr93524.patch --]
[-- Type: text/x-patch, Size: 3344 bytes --]
commit 323fda07729fa0b0f2d1f8b4269db874280ac318
Author: Sandra Loosemore <sandra@codesourcery.com>
Date: Mon Jun 21 13:25:55 2021 -0700
Fortran: fix sm computation in CFI_allocate [PR93524]
This patch fixes a bug in setting the step multiplier field in the
C descriptor for array dimensions > 2.
2021-06-21 Sandra Loosemore <sandra@codesourcery.com>
Tobias Burnus <tobias@codesourcery.com>
libgfortran/
PR fortran/93524
* runtime/ISO_Fortran_binding.c (CFI_allocate): Fix
sm computation.
gcc/testsuite/
PR fortran/93524
* gfortran.dg/pr93524.c: New.
* gfortran.dg/pr93524.f90: New.
diff --git a/gcc/testsuite/gfortran.dg/pr93524.c b/gcc/testsuite/gfortran.dg/pr93524.c
new file mode 100644
index 0000000..24e5e09
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr93524.c
@@ -0,0 +1,33 @@
+/* Test the fix for PR93524, in which CFI_allocate was computing
+ sm incorrectly for dimensions > 2. */
+
+#include <stdlib.h> // For size_t
+#include "../../../libgfortran/ISO_Fortran_binding.h"
+
+void my_fortran_sub_1 (CFI_cdesc_t *dv);
+void my_fortran_sub_2 (CFI_cdesc_t *dv);
+
+int main ()
+{
+ CFI_CDESC_T (3) a;
+ CFI_cdesc_t *dv = (CFI_cdesc_t *) &a;
+ // dv, base_addr, attribute, type, elem_len, rank, extents
+ CFI_establish (dv, NULL, CFI_attribute_allocatable, CFI_type_float, 0, 3, NULL);
+
+ if (dv->base_addr != NULL)
+ return 1; // shall not be allocated
+
+ CFI_index_t lower_bounds[] = {-10, 0, 3};
+ CFI_index_t upper_bounds[] = {10, 5, 10};
+ size_t elem_len = 0; // only needed for strings
+ if (CFI_SUCCESS != CFI_allocate (dv, lower_bounds, upper_bounds, elem_len))
+ return 2;
+
+ if (!CFI_is_contiguous (dv))
+ return 2; // allocatables shall be contiguous,unless a strided section is used
+
+ my_fortran_sub_1 (dv);
+ my_fortran_sub_2 (dv);
+ CFI_deallocate (dv);
+ return 0;
+}
diff --git a/gcc/testsuite/gfortran.dg/pr93524.f90 b/gcc/testsuite/gfortran.dg/pr93524.f90
new file mode 100644
index 0000000..0cebc8f
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr93524.f90
@@ -0,0 +1,17 @@
+! { dg-additional-sources pr93524.c }
+! { dg-do run }
+!
+! Test the fix for PR93524. The main program is in pr93524.c.
+
+subroutine my_fortran_sub_1 (A) bind(C)
+ real :: A(:, :, :)
+ if (any (lbound(A) /= 1)) stop 1
+ if (any (ubound(A) /= [21,6,8])) stop 2
+ if (.not. is_contiguous (A)) stop 3
+end
+subroutine my_fortran_sub_2 (A) bind(C)
+ real, ALLOCATABLE :: A(:, :, :)
+ if (any (lbound(A) /= [-10,0,3])) stop 1
+ if (any (ubound(A) /= [10,5,10])) stop 2
+ if (.not. is_contiguous (A)) stop 3
+end subroutine my_fortran_sub_2
diff --git a/libgfortran/runtime/ISO_Fortran_binding.c b/libgfortran/runtime/ISO_Fortran_binding.c
index 20833ad..0978832 100644
--- a/libgfortran/runtime/ISO_Fortran_binding.c
+++ b/libgfortran/runtime/ISO_Fortran_binding.c
@@ -254,10 +254,7 @@ CFI_allocate (CFI_cdesc_t *dv, const CFI_index_t lower_bounds[],
{
dv->dim[i].lower_bound = lower_bounds[i];
dv->dim[i].extent = upper_bounds[i] - dv->dim[i].lower_bound + 1;
- if (i == 0)
- dv->dim[i].sm = dv->elem_len;
- else
- dv->dim[i].sm = dv->elem_len * dv->dim[i - 1].extent;
+ dv->dim[i].sm = dv->elem_len * arr_len;
arr_len *= dv->dim[i].extent;
}
}
next prev parent reply other threads:[~2021-06-21 20:31 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-21 6:05 [patch] " Sandra Loosemore
2021-06-21 11:42 ` Tobias Burnus
2021-06-21 20:31 ` Sandra Loosemore [this message]
2021-06-22 6:03 ` [patch v2] " Tobias Burnus
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=bcc538d3-22a0-ac4f-dc51-b561228e1e58@codesourcery.com \
--to=sandra@codesourcery.com \
--cc=fortran@gcc.gnu.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=tobias@codesourcery.com \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).