[Patch] PR fortran/92470 Fixes for CFI_address

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[Patch] PR fortran/92470 Fixes for CFI_address

Tobias Burnus-3
Regarding the uncontroversial part: CFI_address. This has been reported
by Vipul Parekh a few hours ago and the problem is: The lower bounds
stored in a bind(C) descriptor are either 0 – or, for
pointer/allocatable arrays, the value used during allocation/pointer
association (cf. F2018, 18.5.3, para 3, quoted in the PR).

But CFI_address was always assuming 0.

When fixing it, ISO_Fortran_binding_1.f90 started to fail – and looking
through the code, I run in two problems related to the "lower_bound"s:

(1) CFI_section: Nothing in the standard states, which 'lower_bound's
shall be used for  'result'. Creating a section in Fortran always gives
.true. for "any(lbound(array(<section>)) == 1)" – and the CFI array
descriptors often uses '0' when Fortran has '1'. Another option would be
to propagate the specified array section on to the CFI descriptor (i.e.
the specified lower_bounds if not NULL or the "source"'s lower bounds
(if lower_bound is NULL) – gfortran does the latter.

(2) CFI_establish: For allocatables, it is clear – base_addr == NULL.
For pointers, it is clear as well – it has to be '0' according to the
standard. But for CFI_attribute_other …

I have now asked at
https://mailman.j3-fortran.org/pipermail/j3/2019-November/thread.html#11740 
– Bob thinks there might be an issue for (2) but both Bob and Bill claim
that it is well-defined for (1). But I am not convinced. However, as it
is unclear, I have now reverted my local changes and only kept the non
lower_bound changes for CFI_establish/CFI_section.

Additionally, the 'dv' value of CFI_establish is some pointer to memory
which can hold an array descriptor. This memory can contain any garbage
(e.g. via dv = malloc(…) with glibc's MALLOC_PERTURB_ set). Hence, it
does not make sense to check 'dv' for a certain value.

Build + regtested on x86_64-gnu-linux.
OK for the trunk? Should it be backported to GCC 9?

Cheers,

Tobias


bind-c-fix.diff (13K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] PR fortran/92470 Fixes for CFI_address

Paul Richard Thomas
Hi Tobias,

Thanks for taking care of this so rapidly :-)

OK for trunk and for 9-branch.

Cheers

Paul

On Tue, 12 Nov 2019 at 14:42, Tobias Burnus <[hidden email]> wrote:

>
> Regarding the uncontroversial part: CFI_address. This has been reported
> by Vipul Parekh a few hours ago and the problem is: The lower bounds
> stored in a bind(C) descriptor are either 0 – or, for
> pointer/allocatable arrays, the value used during allocation/pointer
> association (cf. F2018, 18.5.3, para 3, quoted in the PR).
>
> But CFI_address was always assuming 0.
>
> When fixing it, ISO_Fortran_binding_1.f90 started to fail – and looking
> through the code, I run in two problems related to the "lower_bound"s:
>
> (1) CFI_section: Nothing in the standard states, which 'lower_bound's
> shall be used for  'result'. Creating a section in Fortran always gives
> .true. for "any(lbound(array(<section>)) == 1)" – and the CFI array
> descriptors often uses '0' when Fortran has '1'. Another option would be
> to propagate the specified array section on to the CFI descriptor (i.e.
> the specified lower_bounds if not NULL or the "source"'s lower bounds
> (if lower_bound is NULL) – gfortran does the latter.
>
> (2) CFI_establish: For allocatables, it is clear – base_addr == NULL.
> For pointers, it is clear as well – it has to be '0' according to the
> standard. But for CFI_attribute_other …
>
> I have now asked at
> https://mailman.j3-fortran.org/pipermail/j3/2019-November/thread.html#11740
> – Bob thinks there might be an issue for (2) but both Bob and Bill claim
> that it is well-defined for (1). But I am not convinced. However, as it
> is unclear, I have now reverted my local changes and only kept the non
> lower_bound changes for CFI_establish/CFI_section.
>
> Additionally, the 'dv' value of CFI_establish is some pointer to memory
> which can hold an array descriptor. This memory can contain any garbage
> (e.g. via dv = malloc(…) with glibc's MALLOC_PERTURB_ set). Hence, it
> does not make sense to check 'dv' for a certain value.
>
> Build + regtested on x86_64-gnu-linux.
> OK for the trunk? Should it be backported to GCC 9?
>
> Cheers,
>
> Tobias
>


--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] PR fortran/92470 Fixes for CFI_address

Paul Richard Thomas
In reply to this post by Tobias Burnus-3
I too had some considerable difficulty on this point. I wasn't at all
sure that the C world view was relevant here since the API includes
CFI_address and, in principle, one could reference directly the
elements pointed to by base_addr. However, I bow to the wisdom of your
correspondents on the j3 list :-)

OK for both trunk and 9-branch.

Thanks for the patch.

Paul

On Tue, 12 Nov 2019 at 20:25, Tobias Burnus <[hidden email]> wrote:

>
> Hi all,
>
> On 11/12/19 3:42 PM, Tobias Burnus wrote:
> > (2) CFI_establish: For allocatables, it is clear – base_addr == NULL.
> > For pointers, it is clear as well – it has to be '0' according to the
> > standard. But for CFI_attribute_other … I have now asked at
> > https://mailman.j3-fortran.org/pipermail/j3/2019-November/thread.html#11740
> >
>
> While I still have problems to decipher the standard, regarding
> CFI_establish, Steve L wrote:
>
> "In the C descriptor world, arrays start at zero as they do in C. The
> only way they can become non-zero is through argument association,
> allocation or pointer association as specified in 18.5.3p3. For
> non-pointer, not-allocatable objects (this means "other"), the lower
> bounds are supposed to be always zero."
>
> Hence, I now also set it for CFI_attribute_other to 0 – and check it in
> a test case (most users there have NULL as base_addr, hence, only a
> single assert is in that file).
>
> Build on x86-64_gnu-linux.
> OK for the trunk and GCC-9?
>
> Tobias
>


--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] PR fortran/92470 Fixes for CFI_address

Andreas Schwab
In reply to this post by Tobias Burnus-3
On Nov 14 2019, Jakub Jelinek wrote:

> --- gcc/testsuite/gfortran.dg/ISO_Fortran_binding_17.c.jj 2019-11-13 10:54:37.081172852 +0100
> +++ gcc/testsuite/gfortran.dg/ISO_Fortran_binding_17.c 2019-11-14 01:19:36.704285484 +0100
> @@ -2,7 +2,7 @@
>  
>  #include <stdio.h>
>  #include <assert.h>
> -#include "ISO_Fortran_binding.h"
> +#include "../../../libgfortran/ISO_Fortran_binding.h"

Shoudn't that be fixed generically, by directing the compiler to use the
uninstalled headers?

Andreas.

--
Andreas Schwab, SUSE Labs, [hidden email]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."