Re: [PATCH, Fortran] [PR libfortran/101317] Bind(c): Improve error checking in CFI_* functions

[Tobias Burnus <tobias@codesourcery.com>]

Hi Sandra,

On 21.07.21 20:01, Sandra Loosemore wrote:
> Hmmm. CFI_establish explicitly says that the elem_len has to be
> greater than zero.  It seems somewhat confusing that it's inconsistent
> with the other functions that take an elem_len argument.

Congratulation – we have found a bug in the spec, which is also
present in the current draft (21-007). I have now written to J3:
https://mailman.j3-fortran.org/pipermail/j3/2021-July/013189.html

> Ha! I noticed the same thing and already posted a separate patch for
> that.  :-P
> https://gcc.gnu.org/pipermail/fortran/2021-July/056243.html
:-)
>> How about PRIiPTR + ptrdiff_t instead of %d + (int) cast? At least as
>> positive value, extent may exceed INT_MAX.
> Hmmm, there are similar problems in existing code in other functions
> in this file (e.g., CFI_section).

I think that you could fix as well. At least for size(array), it is not
uncommon that this exceeds MAX_INT.

On the other hand, I think it is unlikely to occur for a single
dimension (→ extent). In particular, the most likely way to get a
negative value is doing 'int' calculations with an overflow – and then
assigning the result "array->dim[i].extent". But in that case, that
(possibly negative) value fits into an int by construction.

>>> +      if (source->attribute == CFI_attribute_other
>>> +          && source->rank > 0
>>> +          && source->dim[source->rank - 1].extent == -1)
>>> +        {
>>> +          fprintf (stderr, "CFI_setpointer: The source is a "
>>> +               "nonallocatable nonpointer object that is an "
>>> +               "assumed-size array.\n");
>>
>> I think you could just check for assumed rank – without
>> CFI_attribute_other in the 'if' and 'nonallocatable nonpointer' in
>> the error message. Only nonallocatable nonpointer variables can be of
>> assumed size (in Fortran); I think that makes the message simpler
>> (focusing on the issue) and if the C user passes an
>> allocatable/pointer, which is assumed rank, it is also a bug.
>
> The wording of the message reflects the language of the standard:
> "source shall be a null pointer or the address of a C descriptor for
> an allocated allocatable object, a data pointer object, or a
> nonallocatable nonpointer data object that is not an assumed-size array.

I know – but the wording is such that it permits all 'nonallocatable
nonpointer data object' – with one exception.

This does not mean that 'assumed-size array' is only invalid for
'nonallocatable nonpointer' – it is also invalid for
allocatables/pointers. The latter cannot occur for Fortran code as only
deferred-shape arrays are permitted in that case, but from the C side,
you can easily set it to the wrong value.

Thus, by simplifying the wording, the error message is clearer (directly
pointing to the issue) and it additionally catches another wrong use of
the array descriptor, which can be (only) triggered from C.

Tobias

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955