[Bug fortran/97592] New: Incorrectly set pointer remapping with array pointer argument to CONTIGUOUS dummy

[Bug fortran/97592] New: Incorrectly set pointer remapping with array pointer argument to CONTIGUOUS dummy

[reubendb at gmail dot com]

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

            Bug ID: 97592
           Summary: Incorrectly set pointer remapping with array pointer
                    argument to CONTIGUOUS dummy
           Product: gcc
           Version: og10 (devel/omp/gcc-10)
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: reubendb at gmail dot com
  Target Milestone: ---

Hi, 

The following code produces an incorrect results with GCC 10.1.0, 10.2.0.

The code works as expected with GCC/6.4.0,7.4.0,8.1.0,8.1.1 

program RemappingTest
  implicit none
  real, dimension ( :, : ), pointer :: & 
    B_2D
  real, dimension ( :, :, : ), pointer :: &
    B_3D

  !-- Prepare B_2D
  allocate ( B_2D ( 16, 3 ) )
  B_2D  =  - huge ( 1.0 )

  !-- Point B_3D to Storage
  call SetPointer ( B_2D, 4, 4, 3, B_3D )

  !-- Set B_3D
  B_3D  =  2.0

  !-- See if the result is reflected in Storage
  print*
  print*, "B_2D = ", B_2D  !-- expect 2.0 for all elements

contains

  subroutine SetPointer ( C_2D, n1, n2, n3, C_3D )
    real, dimension ( :, : ), intent ( inout ), target, contiguous :: &
      C_2D
    integer :: &
      n1, n2, n3
    real, dimension ( :, :, : ), intent ( inout ), pointer :: &
      C_3D
    C_3D ( 1 : n1, 1 : n2, 1 : n3 )  =>  C_2D
  end subroutine SetPointer

end program RemappingTest

]$ gfortran --version
GNU Fortran (GCC) 10.2.0
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

]$ gfortran -Wall -Wextra -fno-strict-aliasing -fwrapv -Warray-temporaries
PointerRemapTest.f90 
PointerRemapTest.f90:13:20:

   13 |   call SetPointer ( B_2D, 4, 4, 3, B_3D )
      |                    1
Warning: Creating array temporary at (1) [-Warray-temporaries]

]$ ./a.out 

 B_2D =   -3.40282347E+38  -3.40282347E+38  -3.40282347E+38  -3.40282347E+38 
-3.40282347E+38  -3.40282347E+38  -3.40282347E+38  -3.40282347E+38 
-3.40282347E+38  -3.40282347E+38  -3.40282347E+38  -3.40282347E+38 
-3.40282347E+38  -3.40282347E+38  -3.40282347E+38  -3.40282347E+38 
-3.40282347E+38  -3.40282347E+38  -3.40282347E+38  -3.40282347E+38 
-3.40282347E+38  -3.40282347E+38  -3.40282347E+38  -3.40282347E+38 
-3.40282347E+38  -3.40282347E+38  -3.40282347E+38  -3.40282347E+38 
-3.40282347E+38  -3.40282347E+38  -3.40282347E+38  -3.40282347E+38 
-3.40282347E+38  -3.40282347E+38  -3.40282347E+38  -3.40282347E+38 
-3.40282347E+38  -3.40282347E+38  -3.40282347E+38  -3.40282347E+38 
-3.40282347E+38  -3.40282347E+38  -3.40282347E+38  -3.40282347E+38 
-3.40282347E+38  -3.40282347E+38  -3.40282347E+38  -3.40282347E+38

It seems to be issue is because inside the subroutine SetPointer, a new array
`C_2D` is created. So `C_3D` points to this new array instead. Outside of
subroutine, this pointer (now `B_3D`) actually points to an invalid memory
location. In some cases I can trigger a segfault with the code with GCC-10.2.0

[Bug fortran/97592] Incorrectly set pointer remapping with array pointer argument to CONTIGUOUS dummy

[anlauf at gcc dot gnu.org]

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

anlauf at gcc dot gnu.org changed:

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

--- Comment #1 from anlauf at gcc dot gnu.org ---
gfortran seems to see the CONTIGUOUS attribute in the declaration of the
dummy argument C_2D of SetPointer, but no corresponding attribute to B_2D.

Workaround: add CONTIGUOUS to the declaration of B_2D.

[Bug fortran/97592] Incorrectly set pointer remapping with array pointer argument to CONTIGUOUS dummy

[dominiq at lps dot ens.fr]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-02-14

--- Comment #2 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
> Workaround: add CONTIGUOUS to the declaration of B_2D.

Confirmed for GCC9 up to GCC11. Note that GCC7 and GCC8 give the expected
result without the CONTIGUOUS attribute.

[Bug fortran/97592] Incorrectly set pointer remapping with array pointer argument to CONTIGUOUS dummy

[anlauf at gcc dot gnu.org]

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

--- Comment #3 from anlauf at gcc dot gnu.org ---
It looks like argument association is confused here.
(The F2018 reference is 15.5.2.3 and 15.5.2.4).

The following patch appears to fix the testcase:

diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 06713f24f95..c7fb4633ab1 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6854,7 +6854,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
                                             INTENT_IN, fsym->attr.pointer);
                }
              else if (fsym && fsym->attr.contiguous
-                      && !gfc_is_simply_contiguous (e, false, true)
+                      && gfc_is_not_contiguous (e)
                       && gfc_expr_is_variable (e))
                {
                  gfc_conv_subref_array_arg (&parmse, e, nodesc_arg,

but unfortunately regresses on gfortran.dg/bind-c-contiguous-3.f90 :-(

[Bug fortran/97592] [10/11/12/13 Regression] Incorrectly set pointer remapping with array pointer argument to CONTIGUOUS dummy

[anlauf at gcc dot gnu.org]

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

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |7.5.0, 8.5.0
           Keywords|                            |wrong-code
            Summary|Incorrectly set pointer     |[10/11/12/13 Regression]
                   |remapping with array        |Incorrectly set pointer
                   |pointer argument to         |remapping with array
                   |CONTIGUOUS dummy            |pointer argument to
                   |                            |CONTIGUOUS dummy
   Target Milestone|---                         |10.5
      Known to fail|                            |10.4.0, 11.3.0, 12.2.0,
                   |                            |13.0, 9.5.0

--- Comment #4 from anlauf at gcc dot gnu.org ---
Adding known-to-work, known-to-fail versions.

[Bug fortran/97592] [10/11/12/13 Regression] Incorrectly set pointer remapping with array pointer argument to CONTIGUOUS dummy

[anlauf at gcc dot gnu.org]

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

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4

[Bug fortran/97592] [11/12/13/14 Regression] Incorrectly set pointer remapping with array pointer argument to CONTIGUOUS dummy

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.5                        |11.5

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 10 branch is being closed.

[Bug fortran/97592] [11/12/13/14 Regression] Incorrectly set pointer remapping with array pointer argument to CONTIGUOUS dummy

[anlauf at gcc dot gnu.org]

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

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |anlauf at gcc dot gnu.org

--- Comment #6 from anlauf at gcc dot gnu.org ---
(In reply to anlauf from comment #3)
> It looks like argument association is confused here.
> (The F2018 reference is 15.5.2.3 and 15.5.2.4).
> 
> The following patch appears to fix the testcase:
> 
> diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
> index 06713f24f95..c7fb4633ab1 100644
> --- a/gcc/fortran/trans-expr.cc
> +++ b/gcc/fortran/trans-expr.cc
> @@ -6854,7 +6854,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
>                                              INTENT_IN, fsym->attr.pointer);
>                 }
>               else if (fsym && fsym->attr.contiguous
> -                      && !gfc_is_simply_contiguous (e, false, true)
> +                      && gfc_is_not_contiguous (e)
>                        && gfc_expr_is_variable (e))
>                 {
>                   gfc_conv_subref_array_arg (&parmse, e, nodesc_arg,
> 
> but unfortunately regresses on gfortran.dg/bind-c-contiguous-3.f90 :-(

This works:

diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index b2463a28748..7bc6d72decc 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -7124,7 +7125,9 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
                                             INTENT_IN, fsym->attr.pointer);
                }
              else if (fsym && fsym->attr.contiguous
-                      && !gfc_is_simply_contiguous (e, false, true)
+                      && (fsym->attr.target
+                          ? gfc_is_not_contiguous (e)
+                          : !gfc_is_simply_contiguous (e, false, true))
                       && gfc_expr_is_variable (e))
                {
                  gfc_conv_subref_array_arg (&parmse, e, nodesc_arg,

[Bug fortran/97592] [11/12/13/14 Regression] Incorrectly set pointer remapping with array pointer argument to CONTIGUOUS dummy

[anlauf at gcc dot gnu.org]

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

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #7 from anlauf at gcc dot gnu.org ---
Submitted: https://gcc.gnu.org/pipermail/fortran/2023-December/060042.html

[Bug fortran/97592] [11/12/13/14 Regression] Incorrectly set pointer remapping with array pointer argument to CONTIGUOUS dummy

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Harald Anlauf <anlauf@gcc.gnu.org>:

https://gcc.gnu.org/g:5060825aa78b3da036df6437390fd42d094d8f15

commit r14-6644-g5060825aa78b3da036df6437390fd42d094d8f15
Author: Harald Anlauf <anlauf@gmx.de>
Date:   Sat Dec 16 19:14:55 2023 +0100

    Fortran: fix argument passing to CONTIGUOUS,TARGET dummy [PR97592]

    gcc/fortran/ChangeLog:

            PR fortran/97592
            * trans-expr.cc (gfc_conv_procedure_call): For a contiguous dummy
            with the TARGET attribute, the effective argument may still be
            contiguous even if the actual argument is not simply-contiguous.
            Allow packing to be decided at runtime by _gfortran_internal_pack.

    gcc/testsuite/ChangeLog:

            PR fortran/97592
            * gfortran.dg/contiguous_15.f90: New test.

[Bug fortran/97592] [11/12/13/14 Regression] Incorrectly set pointer remapping with array pointer argument to CONTIGUOUS dummy

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Harald Anlauf
<anlauf@gcc.gnu.org>:

https://gcc.gnu.org/g:019abe7aa98feae1514c0e51c51fe424e28e2c21

commit r13-8181-g019abe7aa98feae1514c0e51c51fe424e28e2c21
Author: Harald Anlauf <anlauf@gmx.de>
Date:   Sat Dec 16 19:14:55 2023 +0100

    Fortran: fix argument passing to CONTIGUOUS,TARGET dummy [PR97592]

    gcc/fortran/ChangeLog:

            PR fortran/97592
            * trans-expr.cc (gfc_conv_procedure_call): For a contiguous dummy
            with the TARGET attribute, the effective argument may still be
            contiguous even if the actual argument is not simply-contiguous.
            Allow packing to be decided at runtime by _gfortran_internal_pack.

    gcc/testsuite/ChangeLog:

            PR fortran/97592
            * gfortran.dg/contiguous_15.f90: New test.

    (cherry picked from commit 5060825aa78b3da036df6437390fd42d094d8f15)

[Bug fortran/97592] [11/12/13/14 Regression] Incorrectly set pointer remapping with array pointer argument to CONTIGUOUS dummy

[anlauf at gcc dot gnu.org]

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

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
   Target Milestone|11.5                        |13.3
             Status|ASSIGNED                    |RESOLVED

--- Comment #10 from anlauf at gcc dot gnu.org ---
Fixed on mainline for gcc-14, and backported to 13-branch.  Closing.

The patch might be backportable to other branches.  If needed, please reopen.