[flang-commits] [PATCH] D101659: [flang] Fix handling of elem_len in CFI_establish

[Diana Picus via Phabricator via flang-commits]

rovka created this revision.
rovka added reviewers: klausler, tskeith.
rovka added a project: Flang.
Herald added a subscriber: jdoerfert.
rovka requested review of this revision.

The current code computes the minimum element length based on the `type`
used to create the descriptor and uses that as the element length
whenever it is greater than 0. This means that the `elem_len` parameter
is essentially ignored for any type where we can compute a minimum
element length (which includes `CFI_type_char[16|32]_t`), and we may
therefore end up with descriptors with a lower element length than
expected.

This patch fixes the issue by explicitly doing what the standard says,
i.e. it uses the given `elem_len` for character types, `CFI_type_struct`
and `CFI_type_other`, and ignores it (falls back to the minimum element
length) for everything else.


https://reviews.llvm.org/D101659

Files:
  flang/runtime/ISO_Fortran_binding.cpp
  flang/unittests/Evaluate/ISO-Fortran-binding.cpp


Index: flang/unittests/Evaluate/ISO-Fortran-binding.cpp
===================================================================
--- flang/unittests/Evaluate/ISO-Fortran-binding.cpp
+++ flang/unittests/Evaluate/ISO-Fortran-binding.cpp
@@ -114,6 +114,7 @@
       }
     }
     if (type == CFI_type_struct || type == CFI_type_char ||
+        type == CFI_type_char16_t || type == CFI_type_char32_t ||
         type == CFI_type_other) {
       MATCH(elem_len, res->ElementBytes());
     }
@@ -136,6 +137,7 @@
   }
 
   if ((type == CFI_type_struct || type == CFI_type_char ||
+          type == CFI_type_char16_t || type == CFI_type_char32_t ||
           type == CFI_type_other) &&
       elem_len <= 0) {
     ++numErr;
@@ -166,7 +168,8 @@
   CFI_attribute_t attrCases[]{
       CFI_attribute_pointer, CFI_attribute_allocatable, CFI_attribute_other};
   CFI_type_t typeCases[]{CFI_type_int, CFI_type_struct, CFI_type_double,
-      CFI_type_char, CFI_type_other, CFI_TYPE_LAST + 1};
+      CFI_type_char, CFI_type_char16_t, CFI_type_char32_t, CFI_type_other,
+      CFI_TYPE_LAST + 1};
   CFI_index_t *extentCases[]{extents, nullptr};
   void *baseAddrCases[]{dummyAddr, nullptr};
   CFI_rank_t rankCases[]{0, 1, CFI_MAX_RANK, CFI_MAX_RANK + 1};
Index: flang/runtime/ISO_Fortran_binding.cpp
===================================================================
--- flang/runtime/ISO_Fortran_binding.cpp
+++ flang/runtime/ISO_Fortran_binding.cpp
@@ -234,11 +234,13 @@
   if (!descriptor) {
     return CFI_INVALID_DESCRIPTOR;
   }
-  std::size_t minElemLen{MinElemLen(type)};
-  if (minElemLen > 0) {
-    elem_len = minElemLen;
-  } else if (elem_len <= 0) {
-    return CFI_INVALID_ELEM_LEN;
+  if (type == CFI_type_struct || type == CFI_type_other ||
+      IsCharacterType(type)) {
+    if (elem_len <= 0)
+      return CFI_INVALID_ELEM_LEN;
+  } else {
+    elem_len = MinElemLen(type);
+    assert(elem_len > 0 && "Unknown element length for type");
   }
   descriptor->base_addr = base_addr;
   descriptor->elem_len = elem_len;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D101659.342011.patch
Type: text/x-patch
Size: 2026 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/flang-commits/attachments/20210430/23552d49/attachment.bin>