On 6/13/21 12:36 PM, José Rui Faustino de Sousa via Gcc-patches wrote:
Hi All!
Proposed patch to:
Bug 100906 - Bind(c): failure handling character with len/=1
Bug 100907 - Bind(c): failure handling wide character
Bug 100911 - Bind(c): failure handling C_PTR
Bug 100914 - Bind(c): errors handling complex
Bug 100915 - Bind(c): failure handling C_FUNPTR
Bug 100916 - Bind(c): CFI_type_other unimplemented
I've been playing a bit with this patch in conjunction with my TS29113
testsuite. It seems to help some things, but it causes a regression in
library/section-1.f90 that I don't really understand. :-S I'm also
still running up against the long double bug in PR100917; after doing
some experiments on top of this patch, I suggested some possible
solutions/workarounds in that issue.
Patch tested only on x86_64-pc-linux-gnu.
In general, I think it is a good idea to test interoperability features
on a 32-bit host as well. I've been building i686-pc-linux-gnu and
testing both -m32 and -m64 multilibs. ISO_Fortran_binding.h is pretty
broken on 32-bit host but I'll have a patch for that in the next day or so.
Anyway, I have a few comments on the libgfortran changes, mostly
cosmetic things. I'm not at all familiar with the workings of the
Fortran front end code that produces descriptors, so I don't think I
have anything useful to say about that part of the patch.
diff --git a/libgfortran/runtime/ISO_Fortran_binding.c
b/libgfortran/runtime/ISO_Fortran_binding.c
index 20833ad..3a269d7 100644
--- a/libgfortran/runtime/ISO_Fortran_binding.c
+++ b/libgfortran/runtime/ISO_Fortran_binding.c
@@ -36,31 +36,81 @@ export_proto(cfi_desc_to_gfc_desc);
void
cfi_desc_to_gfc_desc (gfc_array_void *d, CFI_cdesc_t **s_ptr)
{
+ signed char type, kind;
+ size_t size;
int n;
- index_type kind;
CFI_cdesc_t *s = *s_ptr;
if (!s)
return;
+ /* Verify descriptor. */
+ switch(s->attribute)
GNU coding standards: space before the open paren.
+ {
+ case CFI_attribute_pointer:
+ case CFI_attribute_allocatable:
+ break;
+ case CFI_attribute_other:
+ if (s->base_addr)
+ break;
+ /* FALL THROUGH */
+ default:
+ internal_error (NULL, "INVALID CFI DESCRIPTOR");
We could have a better message here that doesn't SHOUT. Maybe "Invalid
attribute in CFI descriptor"?
+ kind = _CFI_DECODE_KIND (s->type);
+ switch(type)
Space before the paren again.
+ {
+ case BT_INTEGER:
+ case BT_LOGICAL:
+ case BT_REAL:
+ size = (size_t)kind;
+ break;
+ case BT_COMPLEX:
+ size = (size_t)(kind << 1);
+ break;
+ case BT_DERIVED:
+ case BT_CHARACTER:
+ case BT_VOID:
+ size = s->elem_len;
+ break;
+ default:
+ if (type != CFI_type_other)
+ internal_error(NULL, "TYPE ERROR");
Space before the paren and better error message again.
Section 18.5.4 of the Fortran 2018 standard says: "If a C type is not
interoperable with a Fortran type and kind supported by the Fortran
processor, its macro shall evaluate to a negative value." And in fact
I'll have a patch for PR101305 soon that will use a negative value
distinct from CFI_type_other for that. If you construct a descriptor
for such an object in C and try passing it to Fortran, I think that's a
user error, not an internal error.
+ if (size <= 0)
+ internal_error(NULL, "SIZE ERROR");
Same issues here with fixing formatting and better message.
+ GFC_DESCRIPTOR_SIZE (d) = size;
+
d->dtype.version = s->version;
+
+ if ((s->rank < 0) || (s->rank > CFI_MAX_RANK))
You could delete the inner parentheses here.
+ internal_error(NULL, "Rank out of range.");
Space before the paren again. It also looks like other error messages
in libgfortran don't add punctuation at the end.
There are several more instances of space-before-paren problems and
fixing the errpr messages, I'm not going to point the rest of them out
individually.
@@ -74,14 +124,19 @@ cfi_desc_to_gfc_desc (gfc_array_void *d, CFI_cdesc_t
**s_ptr)
}
d->offset = 0;
- for (n = 0; n < GFC_DESCRIPTOR_RANK (d); n++)
- {
- GFC_DESCRIPTOR_LBOUND(d, n) = (index_type)s->dim[n].lower_bound;
- GFC_DESCRIPTOR_UBOUND(d, n) = (index_type)(s->dim[n].extent
- + s->dim[n].lower_bound - 1);
- GFC_DESCRIPTOR_STRIDE(d, n) = (index_type)(s->dim[n].sm / s->elem_len);
- d->offset -= GFC_DESCRIPTOR_STRIDE(d, n) * GFC_DESCRIPTOR_LBOUND(d, n);
- }
+ if (GFC_DESCRIPTOR_DATA (d))
+ for (n = 0; n < GFC_DESCRIPTOR_RANK (d); n++)
+ {
+ CFI_index_t lb = 1;
+
+ if (s->attribute != CFI_attribute_other)
+ lb = s->dim[n].lower_bound;
+
+ GFC_DESCRIPTOR_LBOUND(d, n) = (index_type)lb;
+ GFC_DESCRIPTOR_UBOUND(d, n) = (index_type)(s->dim[n].extent + lb - 1);
+ GFC_DESCRIPTOR_STRIDE(d, n) = (index_type)(s->dim[n].sm / s->elem_len);
+ d->offset -= GFC_DESCRIPTOR_STRIDE(d, n) * GFC_DESCRIPTOR_LBOUND(d, n);
+ }
}
You might as well fix the space-before-paren issues in the code block
you reindented as well.
@@ -92,32 +147,87 @@ gfc_desc_to_cfi_desc (CFI_cdesc_t **d_ptr, const
gfc_array_void *s)
{
int n;
CFI_cdesc_t *d;
+ signed char type, kind;
/* Play it safe with allocation of the flexible array member 'dim'
by setting the length to CFI_MAX_RANK. This should not be necessary
but valgrind complains accesses after the allocated block. */
if (*d_ptr == NULL)
- d = malloc (sizeof (CFI_cdesc_t)
+ d = calloc (1, sizeof (CFI_cdesc_t)
+ (CFI_type_t)(CFI_MAX_RANK * sizeof (CFI_dim_t)));
This change screwed up the indentation of that last line; the "+" should
line up with "sizeof". IIRC the GNU coding standards also recommend
parenthesizing that binary expression so Emacs will indent it properly
automatically.
+
d->rank = (CFI_rank_t)GFC_DESCRIPTOR_RANK (s);
+ if ((d->rank < 0) || (d->rank > CFI_MAX_RANK))
+ internal_error(NULL, "Rank out of range.");
Another too-many-parentheses instance.
+ switch (d->type)
+ {
+ case CFI_type_Integer:
+ case CFI_type_Logical:
+ case CFI_type_Real:
+ kind = (signed char)d->elem_len;
+ break;
+ case CFI_type_Complex:
+ kind = (signed char)(d->elem_len >> 1);
+ break;
+ case CFI_type_Character:
+ kind = 1;
+ break;
+ case CFI_type_struct:
+ case CFI_type_cptr:
+ case CFI_type_other:
+ kind = 0;
+ break;
+ default:
+ internal_error(NULL, "TYPE ERROR");
+ }
case CFI_type_cfunptr: ???
-Sandra
