[flang] Fix runtime crash on TRIM('')

[klausler]

Summary

The implementation of CFI_establish() incorrectly rejects an
element length of zero, causing a crash when a runtime intrinsic
function tries to return an empty CHARACTER value. Fix.

[PeteSteinfeld]

This builds OK, but when I run tests, I see failures in the ISO-Fortran-binding test:

FAIL: flang-OldUnit :: Evaluate/ISO-Fortran-binding.test (608 of 738)
******************** TEST 'flang-OldUnit :: Evaluate/ISO-Fortran-binding.test' FAILED ********************
/local/home/psteinfeld/main/trim/flang/unittests/Evaluate/ISO-Fortran-binding.cpp:151: FAIL: expectedRetCode == 0x3, not 0x0
/local/home/psteinfeld/main/trim/flang/unittests/Evaluate/ISO-Fortran-binding.cpp:151: FAIL: expectedRetCode == 0x3, not 0x0
/local/home/psteinfeld/main/trim/flang/unittests/Evaluate/ISO-Fortran-binding.cpp:151: FAIL: expectedRetCode == 0x3, not 0x0
/local/home/psteinfeld/main/trim/flang/unittests/Evaluate/ISO-Fortran-binding.cpp:151: FAIL: expectedRetCode == 0x3, not 0x0

[jeanPerier]

Regarding CFI_establish, the Fortran standard says in 18.5.5.5 about elem_len :

elem_len: If type is equal to CFI_type_struct, CFI_type_other, or a Fortran character type code, elem_len shall be greater than zero and equal to the storage size in bytes of an element of the object. Otherwise, elem_len will be ignored.

So it look like the implementation/unittests are following the standard. But it is weird that it should be forbidden to establish zero length characters in the C interoperability context. I see no harm in that, so I am OK with your change.

However, do zero sized struct/ CFI_type_other make sense ? I guess in the C interoperability context, empty structs are not thing since they are technically illegal in C, but outside of the C interoperability context, I do not think empty derived types are forbidden.

[klausler]

In D99296#2649900, @jeanPerier wrote:

Regarding CFI_establish, the Fortran standard says in 18.5.5.5 about elem_len :

elem_len: If type is equal to CFI_type_struct, CFI_type_other, or a Fortran character type code, elem_len shall be greater than zero and equal to the storage size in bytes of an element of the object. Otherwise, elem_len will be ignored.

So it look like the implementation/unittests are following the standard. But it is weird that it should be forbidden to establish zero length characters in the C interoperability context. I see no harm in that, so I am OK with your change.

However, do zero sized struct/ CFI_type_other make sense ? I guess in the C interoperability context, empty structs are not thing since they are technically illegal in C, but outside of the C interoperability context, I do not think empty derived types are forbidden.

I'm going to rework things so that CFI_establish will adhere to the standard and then change the failing CHECK that's calling it to allow for a bad element length.

[klausler]

Rework the fix.

[klausler]

One more tweak for clarity.

[PeteSteinfeld]

All builds, tests, and looks good.

flang/runtime/ISO_Fortran_binding.cpp

   std::size_t minElemLen{MinElemLen(type)};
   if (minElemLen > 0) {
     elem_len = minElemLen;
-  } else if (elem_len <= 0) {
-    return CFI_INVALID_ELEM_LEN;
   }
   descriptor->base_addr = base_addr;
   descriptor->elem_len = elem_len;