[Bug fortran/108621] New: [12 regression]: bind(c) pointer array spurious maybe-uninitialized warning

[Bug fortran/108621] New: [12 regression]: bind(c) pointer array spurious maybe-uninitialized warning

[michael at scivision dot dev]

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

            Bug ID: 108621
           Summary: [12 regression]: bind(c) pointer array spurious
                    maybe-uninitialized warning
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: michael at scivision dot dev
  Target Milestone: ---

GCC 12 introduced a new regression--spurious Wmaybe-initialized warnings from
example code compiled with:

gfortran -Wall -c main.f90

```
program demo
use, intrinsic :: iso_c_binding, only : c_int
implicit none

interface
subroutine fun(f_p) bind(c)
import c_int
integer(c_int), pointer, intent(out) :: f_p(:)
end subroutine
end interface

integer(c_int), pointer :: f(:)

nullify(f)
call fun(f)

end program
```

emits spurious warnings not seen in GCC < 12:

```
   18 | call fcpoint(f)
      |                 ^
Warning: 'f.dim[idx.1_32].lbound' may be used uninitialized
[-Wmaybe-uninitialized]

   14 | integer(c_int), pointer :: f(:)
      |                                 ^
note: 'f' declared here

   18 | call fcpoint(f)
      |                 ^
Warning: 'f.dim[idx.1_32].ubound' may be used uninitialized
[-Wmaybe-uninitialized]

   14 | integer(c_int), pointer :: f(:)
      |                                 ^
note: 'f' declared here

   18 | call fcpoint(f)
      |                 ^
Warning: 'f.dim[idx.1_32].lbound' may be used uninitialized
[-Wmaybe-uninitialized]

   14 | integer(c_int), pointer :: f(:)
      |                                 ^
note: 'f' declared here

   18 | call fcpoint(f)
      |                 ^
Warning: 'f.dim[idx.1_32].stride' may be used uninitialized
[-Wmaybe-uninitialized]

   14 | integer(c_int), pointer :: f(:)
      |                                 ^
note: 'f' declared her
```

[Bug fortran/108621] [12 regression]: bind(c) pointer array spurious maybe-uninitialized warning

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.3
           Priority|P3                          |P4

[Bug fortran/108621] [12 regression]: bind(c) pointer array spurious maybe-uninitialized warning

[kargl at gcc dot gnu.org]

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

kargl at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-02-01
                 CC|                            |kargl at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #1 from kargl at gcc dot gnu.org ---
This appears to be related to Sandra and Tobias's work on CFI.  In particular,
trans-expr.cc(gfc_conv_gfc_desc_to_cfi_desc) appear to not handle an
intent(out) deferred entity.  gfortran eventually gets to line 5818 and
following where the `tree gfx` and `tree idx` are getting manipulated, but
`fsym->as` shows

(gdb) b trans-expr.cc:5818
Breakpoint 1 at 0x9c6412: file ../../gccx/gcc/fortran/trans-expr.cc, line 5818.
(gdb) run -Wall a.f90
Breakpoint 1, gfc_conv_gfc_desc_to_cfi_desc (parmse=0x7fffffffdba0,
e=0x203c24fc0, fsym=0x203c54900) at ../../gccx/gcc/fortran/trans-expr.cc:5818
5818          if (fsym->attr.pointer || fsym->attr.allocatable)
(gdb) p *fsym->as
$1 = {rank = 1, corank = 0, type = AS_DEFERRED, cotype = 0, lower = {0x0
<repeats 15 times>}, upper = {
    0x0 <repeats 15 times>}, cray_pointee = false, cp_was_assumed = false,
resolved = true}
(gdb) 

Here, lower == NULL and upper == NULL, and I suspect the gfc is not set up
correctly.

[Bug fortran/108621] [12 regression]: bind(c) pointer array spurious maybe-uninitialized warning

[sgk at troutmask dot apl.washington.edu]

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

--- Comment #2 from Steve Kargl <sgk at troutmask dot apl.washington.edu> ---
On Wed, Feb 01, 2023 at 06:13:33PM +0000, kargl at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108621
> 
> This appears to be related to Sandra and Tobias's work on CFI.  In particular,
> trans-expr.cc(gfc_conv_gfc_desc_to_cfi_desc) appear to not handle an
> intent(out) deferred entity.  gfortran eventually gets to line 5818 and
> following where the `tree gfx` and `tree idx` are getting manipulated, but
> `fsym->as` shows
> 

It seems that bugzilla does not recognize the email address
that Tobias's uses in ChangeLong.  Try to cc him here to see
if he gets added to the audit trail.

[Bug fortran/108621] [12 regression]: bind(c) pointer array spurious maybe-uninitialized warning

[jvdelisle at gcc dot gnu.org]

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

--- Comment #3 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
(In reply to Steve Kargl from comment #2)
> On Wed, Feb 01, 2023 at 06:13:33PM +0000, kargl at gcc dot gnu.org wrote:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108621
> > 
> > This appears to be related to Sandra and Tobias's work on CFI.  In particular,
> > trans-expr.cc(gfc_conv_gfc_desc_to_cfi_desc) appear to not handle an
> > intent(out) deferred entity.  gfortran eventually gets to line 5818 and
> > following where the `tree gfx` and `tree idx` are getting manipulated, but
> > `fsym->as` shows
> > 
> 
> It seems that bugzilla does not recognize the email address
> that Tobias's uses in ChangeLong.  Try to cc him here to see
> if he gets added to the audit trail.

Got it

[Bug fortran/108621] [12 regression]: bind(c) pointer array spurious maybe-uninitialized warning

[burnus at gcc dot gnu.org]

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

--- Comment #4 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Steve Kargl from comment #2)
> It seems that bugzilla does not recognize the email address
> that Tobias's uses in ChangeLong.  Try to cc him here to see
> if he gets added to the audit trail.

That's pretty common - but if you type the name in the CC field and wait a
while, it suggests emails matching this pattern. Using the @gcc.gnu.org name
makes most sense, if it exists. (Caveat: if someone has an odd user name and
does not provide a full name, it might be more difficult.)


> $1 = {rank = 1, corank = 0, type = AS_DEFERRED, cotype = 0, lower = {0x0 <repeats 15 times>}, upper = { 0x0

that looks fine. For a deferred shape, there is no lower/upper bound except in
the array descriptor.

* * *

Regarding the bug, it looks as if

* some 'if (cfi->base_addr == NULL)' or 'if(gfc->base_addr == NULL)'
  check is missing,  either at the beginning (gfc->cfi)
  and/or at the end (cfi->gfc).

* Possibly, some 'intent == OUT' check might be missing as well
  maybe only for 'pointer'.

* Additionally, for variables not used in the procedure, we do not do anything
  here, except for 'allocatable, intent(out)' - implying an early return.

[Bug fortran/108621] [12/13 regression]: bind(c) pointer array spurious maybe-uninitialized warning

[burnus at gcc dot gnu.org]

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

--- Comment #5 from Tobias Burnus <burnus at gcc dot gnu.org> ---
The warning itself is bogus (false positive in the middle end).
I get:

Warning: ‘f.dim[idx.1_32].lbound’ may be used uninitialized
[-Wmaybe-uninitialized]


If I now look at the 021t.ssa dump, I see:

  f.data = 0B;
...
  _1 = f.data;
  cfi.0.base_addr = _1;
...
  _2 = cfi.0.base_addr;
  if (_2 != 0B)
    goto <bb 3>; [INV]
  else
    goto <bb 6>; [INV]
...
  <bb 4> :
  _3 = f.dim[idx.1_32].lbound;
...
  <bb 6> :
  fun (&cfi.0);

The basic block <bb 4> is the only place where idx.1_32 gets used - but as
f.data == NULL, we directly jump to <bb 6>. Seemingly, the range/value
propagation from f.data to cfi.0.base_addr did not work. — Thus and probably
unsurprisingly, the warning is gone once optimization has been turned on (-Og,
-Os or -O1 are sufficient).

→ This is now tracked in the new PR middle-end/108906

* * *

Still, we can do better in the FE by producing less code when we know that the
dummy argument is 'intent(out)'. That's what the lightly tested patch below
does.

(Note: It fixes the testcase of comment 0, but when changing the intent, e.g., 
to 'intent(inout)' the bogus warning will re-appear.)

TODO: I think something similar needs to be done for allocatable +
'intent(out)', except that we still need to handle the DEALLOCATE in the
caller.


--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -5675,3 +5675,3 @@ gfc_conv_gfc_desc_to_cfi_desc (gfc_se *parmse, gfc_expr
*e, gfc_symbol *fsym)

-  if (e->rank == 0)
+  if (e->rank == 0 && (!fsym->attr.pointer || fsym->attr.intent !=
INTENT_OUT))
     {
@@ -5680,3 +5680,3 @@ gfc_conv_gfc_desc_to_cfi_desc (gfc_se *parmse, gfc_expr
*e, gfc_symbol *fsym)
     }
-  else
+  else if (!fsym->attr.pointer || fsym->attr.intent != INTENT_OUT)
     {
@@ -5697,2 +5697,5 @@ gfc_conv_gfc_desc_to_cfi_desc (gfc_se *parmse, gfc_expr
*e, gfc_symbol *fsym)

+  if (fsym->attr.pointer && fsym->attr.intent == INTENT_OUT)
+    goto done;
+
   /* When allocatable + intent out, free the cfi descriptor.  */

[Bug fortran/108621] [12/13 regression]: bind(c) pointer array spurious maybe-uninitialized warning

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 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:d3e427f684b0cd7cedbe7b93a06f455e562c5901

commit r13-6341-gd3e427f684b0cd7cedbe7b93a06f455e562c5901
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Sat Feb 25 11:55:08 2023 +0100

    Fortran: Skip bound conv in gfc_conv_gfc_desc_to_cfi_desc with intent(out)
ptr [PR108621]

    When the dummy argument of the bind(C) proc is 'pointer, intent(out)', the
conversion
    of the GFC to the CFI bounds can be skipped: it is not needed and avoids
issues with
    noninit memory.

    Note that the 'cfi->base_addr = gfc->addr' assignment is kept as the C code
of a user
    might assume that a nullified pointer arrives as NULL (or even a specific
value).
    For instance, gfortran.dg/c-interop/section-{1,2}.f90 assumes the value
NULL.

    Note 2: The PR is about a may-be-uninitialized warning with intent(out). In
the PR's
    testcase, the pointer was nullified and should not have produced that
warning.
    That is a diagnostic issue, now tracked as PR middle-end/108906 as the
issue in principle
    still exists (e.g. with 'intent(inout)'). [But no longer for intent(out).]

    Note 3: With undefined pointers and no 'intent', accessing uninit memory is
unavoidable
    on the caller side as the compiler cannot know what the C function does
(but this usage
    determines whether the pointer is permitted be undefined or whether the
bounds must be
    gfc-to-cfi converted).

    gcc/fortran/ChangeLog:

            PR fortran/108621
            * trans-expr.cc (gfc_conv_gfc_desc_to_cfi_desc): Skip setting of
            bounds of CFI desc for 'pointer,intent(out)'.

    gcc/testsuite/ChangeLog:

            PR fortran/108621
            * gfortran.dg/c-interop/fc-descriptor-pr108621.f90: New test.

[Bug fortran/108621] [12/13 regression]: bind(c) pointer array spurious maybe-uninitialized warning

[burnus at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #7 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Close as FIXED - and thanks to Michael for the report!

On GCC 13 (mainline), only:
* For intent(out), the pointless code is no longer produced
  and, thus, also the warning is gone.

→ No backport to GCC 12 planned as this was only a bogus warning
  that ignored the "nullify()" - contrary to the generated code.


* PR middle-end/108906 – tracks the bogus warning with -O0 (gone with
  optimization). Note: this message contains "may", i.e. the message
  is correct (even if the "may" never occurs).
  The issue still occurs with any intent but 'intent(out)'


* The generated code is/was fine – except possibly for the following,
  but we cannot do anything about this with the current descriptors:

– For undefined pointers, which is valid* if no 'intent' is known
  at the caller side (and the pointer is not read in the called proc):
  Unless the pointer happens to point to NULL, the code will access unit
  memory and might even overflowing integer arithmetic.
→ Should be usually still fine as long nothing is trapping or overflows
  are checked for with -fsanitize=undefined. (And cannot be avoided.)

[(*) it is also valid with 'intent(out)' but since the just committed patch,
GCC 13 no longer generates affected code.]