public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, Fortran] Bind(c): CFI_signed_char is not a Fortran character type
@ 2021-07-16  3:46 Sandra Loosemore
  2021-07-16  7:52 ` Thomas Koenig
  2021-07-22  8:03 ` [PATCH, Fortran] Bind(c): CFI_signed_char is not a Fortran character type Tobias Burnus
  0 siblings, 2 replies; 8+ messages in thread
From: Sandra Loosemore @ 2021-07-16  3:46 UTC (permalink / raw)
  To: gcc-patches, fortran

[-- Attachment #1: Type: text/plain, Size: 841 bytes --]

When I was reading code in conjunction with fixing PR101317, I noticed 
an unrelated bug in the implementation of CFI_allocate and 
CFI_select_part:  they were mis-handling the CFI_signed_char type as if 
it were a Fortran character type for the purposes of deciding whether to 
use the elem_len argument to those functions.  It's really an integer 
type that has the size of signed char.  I checked similar code in other 
functions in ISO_Fortran_binding.c and these were the only two that were 
incorrect.

The part of the patch to add tests for this goes on top of my base 
TS29113 testsuite patch, which hasn't been reviewed or committed yet.  I 
can refactor that into the same commit as the rest of the testsuite, 
assuming everything eventually gets approved.

https://gcc.gnu.org/pipermail/gcc-patches/2021-July/574576.html

-Sandra

[-- Attachment #2: signed-char.patch --]
[-- Type: text/x-patch, Size: 4969 bytes --]

commit 45190d9eb5123df77bd60a1d6712f05a3af5f42c
Author: Sandra Loosemore <sandra@codesourcery.com>
Date:   Thu Jul 15 16:51:55 2021 -0700

    Bind(c): signed char is not a Fortran character type
    
    CFI_allocate and CFI_select_part were incorrectly treating
    CFI_type_signed_char as a Fortran character type for the purpose of
    deciding whether or not to use the elem_len argument.  It is a Fortran
    integer type per table 18.2 in the 2018 Fortran standard.
    
    Other functions in ISO_Fortran_binding.c appeared to handle this case
    correctly already.
    
    2021-07-15  Sandra Loosemore  <sandra@codesourcery.com>
    
    gcc/testsuite/
    	* gfortran.dg/ts29113/library/allocate-c.c (ctest): Also test
    	handling of elem_len for CFI_type_char vs CFI_type_signed_char.
    	* gfortran.dg/ts29113/library/select-c.c (ctest): Likewise.
    
    libgfortran/
    	* runtime/ISO_Fortran_binding.c (CFI_allocate)

diff --git a/gcc/testsuite/gfortran.dg/ts29113/library/allocate-c.c b/gcc/testsuite/gfortran.dg/ts29113/library/allocate-c.c
index 0208e5a..6343d28 100644
--- a/gcc/testsuite/gfortran.dg/ts29113/library/allocate-c.c
+++ b/gcc/testsuite/gfortran.dg/ts29113/library/allocate-c.c
@@ -135,5 +135,34 @@ ctest (void)
 		    CFI_deallocate (dv));
   if (dv->base_addr != NULL)
     abort ();
+
+  /* Signed char is not a Fortran character type.  Here we expect it to
+     ignore the elem_len argument and use the size of the type.  */
+  ex[0] = 3;
+  ex[1] = 4;
+  ex[2] = 5;
+  check_CFI_status ("CFI_establish",
+		    CFI_establish (dv, NULL, CFI_attribute_allocatable,
+				   CFI_type_signed_char, 4, 3, ex));
+  lb[0] = 1;
+  lb[1] = 2;
+  lb[2] = 3;
+  ub[0] = 10;
+  ub[1] = 5;
+  ub[2] = 10;
+  sm = sizeof (double);
+  check_CFI_status ("CFI_allocate",
+		    CFI_allocate (dv, lb, ub, sm));
+  dump_CFI_cdesc_t (dv);
+  if (dv->base_addr == NULL)
+    abort ();
+  if (dv->elem_len != sizeof (signed char))
+    abort ();
+
+  check_CFI_status ("CFI_deallocate",
+		    CFI_deallocate (dv));
+  if (dv->base_addr != NULL)
+    abort ();
+
 }
 
diff --git a/gcc/testsuite/gfortran.dg/ts29113/library/select-c.c b/gcc/testsuite/gfortran.dg/ts29113/library/select-c.c
index df6172c..9bcbc01 100644
--- a/gcc/testsuite/gfortran.dg/ts29113/library/select-c.c
+++ b/gcc/testsuite/gfortran.dg/ts29113/library/select-c.c
@@ -8,6 +8,8 @@
 
 /* Declare some source arrays.  */
 struct ss {
+  char c[4];
+  signed char b[4];
   int i, j, k;
 } s[10][5][3];
 
@@ -61,6 +63,31 @@ ctest (void)
   if (result->dim[2].sm != source->dim[2].sm)
     abort ();
 
+  /* Check that we use the given elem_size for char but not for
+     signed char, which is considered an integer type instead of a Fortran
+     character type.  */
+  check_CFI_status ("CFI_establish", 
+		    CFI_establish (result, NULL, CFI_attribute_pointer,
+				   CFI_type_char, 4, 3, NULL));
+  if (result->elem_len != 4)
+    abort ();
+  offset = offsetof (struct ss, c);
+  check_CFI_status ("CFI_select_part",
+		    CFI_select_part (result, source, offset, 4));
+  if (result->elem_len != 4)
+    abort ();
+
+  check_CFI_status ("CFI_establish", 
+		    CFI_establish (result, NULL, CFI_attribute_pointer,
+				   CFI_type_signed_char, 4, 3, NULL));
+  if (result->elem_len != sizeof (signed char))
+    abort ();
+  offset = offsetof (struct ss, c);
+  check_CFI_status ("CFI_select_part",
+		    CFI_select_part (result, source, offset, 4));
+  if (result->elem_len != sizeof (signed char))
+    abort ();
+
   /* Extract an array of character substrings.  */
   offset = 2;
   check_CFI_status ("CFI_establish",
diff --git a/libgfortran/runtime/ISO_Fortran_binding.c b/libgfortran/runtime/ISO_Fortran_binding.c
index 78953d0..9fe3a85 100644
--- a/libgfortran/runtime/ISO_Fortran_binding.c
+++ b/libgfortran/runtime/ISO_Fortran_binding.c
@@ -229,10 +229,9 @@ CFI_allocate (CFI_cdesc_t *dv, const CFI_index_t lower_bounds[],
 	}
     }
 
-  /* If the type is a character, the descriptor's element length is replaced
-     by the elem_len argument. */
-  if (dv->type == CFI_type_char || dv->type == CFI_type_ucs4_char ||
-      dv->type == CFI_type_signed_char)
+  /* If the type is a Fortran character type, the descriptor's element
+     length is replaced by the elem_len argument. */
+  if (dv->type == CFI_type_char || dv->type == CFI_type_ucs4_char)
     dv->elem_len = elem_len;
 
   /* Dimension information and calculating the array length. */
@@ -759,9 +758,9 @@ int CFI_select_part (CFI_cdesc_t *result, const CFI_cdesc_t *source,
 	}
     }
 
-  /* Element length. */
-  if (result->type == CFI_type_char || result->type == CFI_type_ucs4_char ||
-      result->type == CFI_type_signed_char)
+  /* Element length is ignored unless result->type specifies a Fortran
+     character type.  */
+  if (result->type == CFI_type_char || result->type == CFI_type_ucs4_char)
     result->elem_len = elem_len;
 
   if (unlikely (compile_options.bounds_check))

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-07-22  8:03 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-16  3:46 [PATCH, Fortran] Bind(c): CFI_signed_char is not a Fortran character type Sandra Loosemore
2021-07-16  7:52 ` Thomas Koenig
2021-07-16 15:32   ` Pushing XFAILed test cases (was: [PATCH, Fortran] Bind(c): CFI_signed_char is not a Fortran character type) Thomas Schwinge
2021-07-16 17:26     ` Pushing XFAILed test cases Martin Sebor
2021-07-16 18:22     ` Sandra Loosemore
2021-07-17  7:25       ` Thomas Koenig
2021-07-21  9:20         ` Tobias Burnus
2021-07-22  8:03 ` [PATCH, Fortran] Bind(c): CFI_signed_char is not a Fortran character type Tobias Burnus

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).