[Bug fortran/94331] New: Bind(C) corrupts array descriptors

[Bug fortran/94331] New: Bind(C) corrupts array descriptors

[jrfsousa at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94331

            Bug ID: 94331
           Summary: Bind(C) corrupts array descriptors
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jrfsousa at gmail dot com
  Target Milestone: ---

Created attachment 48119
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48119&action=edit
Fortran code demonstrating problems.

Hi all!

Call to bind(c) procedure corrupts array descriptor overwriting array bounds.

in:

GNU Fortran (GCC) 10.0.1 20200324 (experimental)

Also in 9 but it's too buggy there to be relevant.

Thank you very much.

Best regards,
José Rui

[Bug fortran/94331] Bind(C) corrupts array descriptors

[jrfsousa at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94331

--- Comment #1 from José Rui Faustino de Sousa <jrfsousa at gmail dot com> ---
Created attachment 48120
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48120&action=edit
C code demonstrating problems.

[Bug fortran/94331] Bind(C) corrupts array descriptors

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94331

Dominique d'Humieres <dominiq at lps dot ens.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2020-07-23
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |WAITING

--- Comment #2 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
I got the impression that it is a duplicate, but I cannot find it.

[Bug fortran/94331] Bind(C) corrupts array descriptors

[sandra at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94331

sandra at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sandra at gcc dot gnu.org

--- Comment #3 from sandra at gcc dot gnu.org ---
I think this is probably a duplicate of PR93308, but there are so many things
going on in the test case I can't be sure there aren't other problems lurking
as well.

[Bug fortran/94331] Bind(C) corrupts array descriptors

[sandra at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94331

--- Comment #4 from sandra at gcc dot gnu.org ---
OK, there are two flavors of failures being diagnosed by the test case.

The first flavor involves the lower bounds of an array passed into a bind(c)
procedure being set to zero in the callee when the test case thinks they should
be inherited from the array in the caller.  I'm still trying to grok what the
standard actually says about these cases.

The second flavor looks like the same bug as PR93308, involving the lower
bounds of an array passed to a bind(c) procedure appearing to be correct in the
callee, but set to zero in the caller after return.

[Bug fortran/94331] Bind(C) corrupts array descriptors

[jrfsousa at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94331

--- Comment #5 from José Rui Faustino de Sousa <jrfsousa at gcc dot gnu.org> ---
Hi Sandra!

I have a patch for this problem, which also seems to work for PR93308, I expect
to post it in the next week, if all goes well.

Best regards,
José Rui

[Bug fortran/94331] Bind(C) corrupts array descriptors

[jrfsousa at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94331

--- Comment #6 from José Rui Faustino de Sousa <jrfsousa at gcc dot gnu.org> ---
Patch posted:

https://gcc.gnu.org/pipermail/fortran/2021-May/056054.html

[Bug fortran/94331] Bind(C) corrupts array descriptors

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94331

Dominique d'Humieres <dominiq at lps dot ens.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |ASSIGNED

--- Comment #7 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
> Patch posted:
>
> https://gcc.gnu.org/pipermail/fortran/2021-May/056054.html

With the patch the test PR94331.f90 fails with -flto:

% gfc /opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.f90
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.c -flto
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.f90:121:10: warning: type of
'checkb_o_ar' does not match original declaration [-Wlto-type-mismatch]
  121 |   if(.not.checkb_o_ar(a, 0, ex-1))stop 28
      |          ^
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.c:38:1: note: type mismatch in
parameter 2
   38 | check_bounds (const CFI_cdesc_t *restrict auxp, const CFI_index_t lb,
const CFI_index_t ub)
      | ^
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.c:38:1: note: type
'CFI_index_t' should match type 'int'
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.f90:111:10: warning: type of
'checkb_o_as' does not match original declaration [-Wlto-type-mismatch]
  111 |   if(.not.checkb_o_as(a, 0, ex-1))stop 20
      |          ^
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.c:38:1: note: type mismatch in
parameter 2
   38 | check_bounds (const CFI_cdesc_t *restrict auxp, const CFI_index_t lb,
const CFI_index_t ub)
      | ^
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.c:38:1: note: type
'CFI_index_t' should match type 'int'
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.f90:101:10: warning: type of
'checkb_p_ar' does not match original declaration [-Wlto-type-mismatch]
  101 |   if(.not.checkb_p_ar(a, lb, ub)) stop 12
      |          ^
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.c:38:1: note: type mismatch in
parameter 2
   38 | check_bounds (const CFI_cdesc_t *restrict auxp, const CFI_index_t lb,
const CFI_index_t ub)
      | ^
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.c:38:1: note: type
'CFI_index_t' should match type 'int'
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.f90:91:10: warning: type of
'checkb_p_as' does not match original declaration [-Wlto-type-mismatch]
   91 |   if(.not.checkb_p_as(a, lb, ub)) stop 4
      |          ^
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.c:38:1: note: type mismatch in
parameter 2
   38 | check_bounds (const CFI_cdesc_t *restrict auxp, const CFI_index_t lb,
const CFI_index_t ub)
      | ^
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.c:38:1: note: type
'CFI_index_t' should match type 'int'
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.f90:167:10: warning: type of
'checkb_a_ar' does not match original declaration [-Wlto-type-mismatch]
  167 |   if(.not.checkb_a_ar(b, lb, ub)) stop 63
      |          ^
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.c:38:1: note: type mismatch in
parameter 2
   38 | check_bounds (const CFI_cdesc_t *restrict auxp, const CFI_index_t lb,
const CFI_index_t ub)
      | ^
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.c:38:1: note: type
'CFI_index_t' should match type 'int'
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.f90:155:10: warning: type of
'checkb_a_as' does not match original declaration [-Wlto-type-mismatch]
  155 |   if(.not.checkb_a_as(b, lb, ub)) stop 54
      |          ^
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.c:38:1: note: type mismatch in
parameter 2
   38 | check_bounds (const CFI_cdesc_t *restrict auxp, const CFI_index_t lb,
const CFI_index_t ub)
      | ^
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.c:38:1: note: type
'CFI_index_t' should match type 'int'
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.c:38:1: note: 'check_bounds'
was previously declared here
/opt/gcc/work/gcc/testsuite/gfortran.dg/PR94331.c:38:1: note: code may be
misoptimized unless '-fno-strict-aliasing' is used

Note that the original test seems fixed even with -flto.

[Bug fortran/94331] Bind(C) corrupts array descriptors

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94331

--- Comment #8 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
Easier to read warnings:

pr94331_1.f90:121:10: warning: type of 'checkb_o_ar' does not match original
declaration [-Wlto-type-mismatch]
  121 |   if(.not.checkb_o_ar(a, 0, ex-1))stop 28
      |          ^
pr94331_1.c:38:1: note: type mismatch in parameter 2
   38 | check_bounds (const CFI_cdesc_t *restrict auxp, const CFI_index_t lb,
const CFI_index_t ub)
      | ^
pr94331_1.c:38:1: note: type 'CFI_index_t' should match type 'int'
pr94331_1.f90:111:10: warning: type of 'checkb_o_as' does not match original
declaration [-Wlto-type-mismatch]
  111 |   if(.not.checkb_o_as(a, 0, ex-1))stop 20
      |          ^
pr94331_1.c:38:1: note: type mismatch in parameter 2
   38 | check_bounds (const CFI_cdesc_t *restrict auxp, const CFI_index_t lb,
const CFI_index_t ub)
      | ^
pr94331_1.c:38:1: note: type 'CFI_index_t' should match type 'int'
pr94331_1.f90:101:10: warning: type of 'checkb_p_ar' does not match original
declaration [-Wlto-type-mismatch]
  101 |   if(.not.checkb_p_ar(a, lb, ub)) stop 12
      |          ^
pr94331_1.c:38:1: note: type mismatch in parameter 2
   38 | check_bounds (const CFI_cdesc_t *restrict auxp, const CFI_index_t lb,
const CFI_index_t ub)
      | ^
pr94331_1.c:38:1: note: type 'CFI_index_t' should match type 'int'
pr94331_1.f90:91:10: warning: type of 'checkb_p_as' does not match original
declaration [-Wlto-type-mismatch]
   91 |   if(.not.checkb_p_as(a, lb, ub)) stop 4
      |          ^
pr94331_1.c:38:1: note: type mismatch in parameter 2
   38 | check_bounds (const CFI_cdesc_t *restrict auxp, const CFI_index_t lb,
const CFI_index_t ub)
      | ^
pr94331_1.c:38:1: note: type 'CFI_index_t' should match type 'int'
pr94331_1.f90:167:10: warning: type of 'checkb_a_ar' does not match original
declaration [-Wlto-type-mismatch]
  167 |   if(.not.checkb_a_ar(b, lb, ub)) stop 63
      |          ^
pr94331_1.c:38:1: note: type mismatch in parameter 2
   38 | check_bounds (const CFI_cdesc_t *restrict auxp, const CFI_index_t lb,
const CFI_index_t ub)
      | ^
pr94331_1.c:38:1: note: type 'CFI_index_t' should match type 'int'
pr94331_1.f90:155:10: warning: type of 'checkb_a_as' does not match original
declaration [-Wlto-type-mismatch]
  155 |   if(.not.checkb_a_as(b, lb, ub)) stop 54
      |          ^
pr94331_1.c:38:1: note: type mismatch in parameter 2
   38 | check_bounds (const CFI_cdesc_t *restrict auxp, const CFI_index_t lb,
const CFI_index_t ub)
      | ^
pr94331_1.c:38:1: note: type 'CFI_index_t' should match type 'int'
pr94331_1.c:38:1: note: 'check_bounds' was previously declared here
pr94331_1.c:38:1: note: code may be misoptimized unless '-fno-strict-aliasing'
is used

Note that the test passes with -Wno-lto-type-mismatch.

[Bug fortran/94331] Bind(C) corrupts array descriptors

[jrfsousa at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94331

--- Comment #9 from José Rui Faustino de Sousa <jrfsousa at gcc dot gnu.org> ---
Created attachment 50890
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50890&action=edit
Updated PR94331.c test file

[Bug fortran/94331] Bind(C) corrupts array descriptors

[jrfsousa at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94331

--- Comment #10 from José Rui Faustino de Sousa <jrfsousa at gcc dot gnu.org> ---
Hi Dominique,

Thank you vary much for taking the time to test the patch.

If I understand you correctly the problem is on the test not on the patch
itself, right?

I have fixed the glaring mistake in PR94331.c, could you be so gentle as to
test it to verify that it does indeed solve the problems you found?

Thank you very much.

Best regards,
José Rui

[Bug fortran/94331] Bind(C) corrupts array descriptors

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94331

--- Comment #11 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
> I have fixed the glaring mistake in PR94331.c, could you be so gentle
> as to test it to verify that it does indeed solve the problems you found?

The problem seems solved with the updated PR94331.c. Thanks.

[Bug fortran/94331] Bind(C) corrupts array descriptors

[jrfsousa at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94331

--- Comment #12 from José Rui Faustino de Sousa <jrfsousa at gcc dot gnu.org> ---
Updated patch posted:

https://gcc.gnu.org/pipermail/fortran/2021-June/056163.html

[Bug fortran/94331] Bind(C) corrupts array descriptors

[jrfsousa at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94331

--- Comment #13 from José Rui Faustino de Sousa <jrfsousa at gcc dot gnu.org> ---
Created attachment 51017
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51017&action=edit
Updated patch.

[Bug fortran/94331] Bind(C) corrupts array descriptors

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94331

--- Comment #14 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tobias Burnus <burnus@gcc.gnu.org>:

https://gcc.gnu.org/g:0cbf03689e3e7d9d6002b8e5d159ef3716d0404c

commit r12-2511-g0cbf03689e3e7d9d6002b8e5d159ef3716d0404c
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Mon Jul 26 14:20:46 2021 +0200

    PR fortran/93308/93963/94327/94331/97046 problems raised by descriptor
handling

    Fortran: Fix attributes and bounds in ISO_Fortran_binding.

    2021-07-26  José Rui Faustino de Sousa  <jrfsousa@gmail.com>
                Tobias Burnus  <tobias@codesourcery.com>

            PR fortran/93308
            PR fortran/93963
            PR fortran/94327
            PR fortran/94331
            PR fortran/97046

    gcc/fortran/ChangeLog:

            * trans-decl.c (convert_CFI_desc): Only copy out the descriptor
            if necessary.
            * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Updated attribute
            handling which reflect a previous intermediate version of the
            standard. Only copy out the descriptor if necessary.

    libgfortran/ChangeLog:

            * runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Add code
            to verify the descriptor. Correct bounds calculation.
            (gfc_desc_to_cfi_desc): Add code to verify the descriptor.

    gcc/testsuite/ChangeLog:

            * gfortran.dg/ISO_Fortran_binding_1.f90: Add pointer attribute,
            this test is still erroneous but now it compiles.
            * gfortran.dg/bind_c_array_params_2.f90: Update regex to match
            code changes.
            * gfortran.dg/PR93308.f90: New test.
            * gfortran.dg/PR93963.f90: New test.
            * gfortran.dg/PR94327.c: New test.
            * gfortran.dg/PR94327.f90: New test.
            * gfortran.dg/PR94331.c: New test.
            * gfortran.dg/PR94331.f90: New test.
            * gfortran.dg/PR97046.f90: New test.

[Bug fortran/94331] Bind(C) corrupts array descriptors

[burnus at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94331

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |burnus at gcc dot gnu.org
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #15 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Close as FIXED.

  * * *

Now that my big patch has been applied, I tried the example from comment 0
(Fortran part) and comment 1 (C part) and the stand-alone testcases PR93308.f90
 PR93963.f90  PR94327.c  PR94327.f90  PR94331.c  PR94331.f90  PR97046.f90 of
the patch in comment 12.

All pass with -O0 and -Ofast.  (Well, I had to change the expected version
number to 1 (not 0) of comment 1 - as GCC uses "#define CFI_VERSION 1".)


I am not sure whether it makes sense to add any of those testcases or not.
Given that there is already quite some coverage, I have not – but can be easily
convinced to do so.

Thanks, José for the testcases - and the patches, which weren't reviewed. (Tip:
please ping them after a while; otherwise, they might end up unreviewed.)

(I think at the end doing the full inlining of the conversion code made more
sense, but was also a huge effort.)