[Bug fortran/55978] New: [4.8 Regression] class_optional_2.f90 -Os fails

[Bug fortran/55978] New: [4.8 Regression] class_optional_2.f90 -Os fails

[tkoenig at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

             Bug #: 55978
           Summary: [4.8 Regression] class_optional_2.f90 -Os fails
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: tkoenig@gcc.gnu.org


This is for

Using built-in specs.
COLLECT_GCC=gfortran
COLLECT_LTO_WRAPPER=/home/ig25/lib/gcc/x86_64-unknown-linux-gnu/4.8.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ../trunk/configure --prefix=/home/ig25
--enable-languages=c,fortran --with-mpfr=/usr/local --with-gmp=/usr/local
--with-mpc=/usr/local
Thread model: posix
gcc version 4.8.0 20130113 (experimental) (GCC) 

FAIL: gfortran.dg/class_optional_2.f90  -Os  execution test

May be related to PR 55483.

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[tkoenig at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

--- Comment #1 from Thomas Koenig <tkoenig at gcc dot gnu.org> 2013-01-14 21:29:25 UTC ---
For -O0, valgrind complains about

==15263== Conditional jump or move depends on uninitialised value(s)
==15263==    at 0x4F26355: _gfortran_internal_pack (in_pack_generic.c:54)
==15263==    by 0x403762: a3a1.2119 (in /home/ig25/Krempel/Os/a.out)
==15263==    by 0x400B26: MAIN__ (in /home/ig25/Krempel/Os/a.out)
==15263==    by 0x408F0E: main (in /home/ig25/Krempel/Os/a.out)

and

==15263== 
==15263== Conditional jump or move depends on uninitialised value(s)
==15263==    at 0x4F26447: _gfortran_internal_pack (in_pack_generic.c:159)
==15263==    by 0x403762: a3a1.2119 (in /home/ig25/Krempel/Os/a.out)
==15263==    by 0x400B26: MAIN__ (in /home/ig25/Krempel/Os/a.out)
==15263==    by 0x408F0E: main (in /home/ig25/Krempel/Os/a.out)

which is

  size = GFC_DESCRIPTOR_SIZE (source);
  switch (type_size)

and


  dim = GFC_DESCRIPTOR_RANK (source);

respectively.

Reduced test case (run with -fcoarray=single):

! { dg-do run }
! { dg-options "-fcoarray=single" }
!
! PR fortran/50981
! PR fortran/54618
!

  implicit none
  type t
   integer, allocatable :: i
  end type t
  type, extends (t):: t2
   integer, allocatable :: j
  end type t2

  call a3a()

contains

 subroutine a3a(z, z2, z3)
   type(t2), optional :: z(4)
   type(t2), optional, pointer :: z2(:)
   type(t2), optional, allocatable :: z3(:)
   type(t2), allocatable :: x(:)
   type(t2), pointer :: y(:)
   y => null()
   call a4t2(y)
 end subroutine a3a

 subroutine a4t2(x)
   type(t2), intent(in), optional :: x(4)
   if (present (x)) call abort ()
   !print *, present(x)
 end subroutine a4t2

end

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-01-14
                 CC|                            |janus at gcc dot gnu.org
     Ever Confirmed|0                           |1

--- Comment #2 from Dominique d'Humieres <dominiq at lps dot ens.fr> 2013-01-14 22:32:35 UTC ---
This is caused by revision 195125, see 

http://gcc.gnu.org/ml/gcc-regression/2013-01/msg00119.html

Note that there is no regression on darwin, but I see the valgrind errors.

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[tkoenig at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

--- Comment #3 from Thomas Koenig <tkoenig at gcc dot gnu.org> 2013-01-14 23:03:04 UTC ---
A reduced test case which shows the problem in the dump:


! { dg-do run }
! { dg-options "-fcoarray=single" }
!
! PR fortran/50981
! PR fortran/54618
!
program main
  implicit none
  type t
   integer, allocatable :: i
  end type t
  type, extends (t):: t2
   integer, allocatable :: j
  end type t2

  call a3a()
contains

 subroutine a3a(z, z2, z3)
   type(t2), optional :: z(4)
   type(t2), optional, pointer :: z2(:)
   type(t2), optional, allocatable :: z3(:)
   type(t2), allocatable :: x(:)
   type(t2), pointer :: y(:)
   y => null()
   call a4t2(y)
 end subroutine a3a

 subroutine a4t2(x)
   type(t2), intent(in), optional :: x(4)
   if (present (x)) call abort ()
   !print *, present(x)
 end subroutine a4t2
end program
ig25@linux-fd1f:~/Krempel/Os> gfortran -fcoarray=single -fdump-tree-original
c.f90 
ig25@linux-fd1f:~/Krempel/Os> cat c.f90.003t.original
a4t2 (struct t2[4] * restrict x)
{
  if (x != 0B)
    {
      _gfortran_abort ();
    }
  L.1:;
}


a3a (struct t2[4] * restrict z, struct array1_t2 * z2, struct array1_t2 * z3)
{
  struct array1_t2 y;

  y.data = 0B;
  y.data = 0B;
  {
    void * D.1914;

    D.1914 = _gfortran_internal_pack (&y);
    a4t2 (D.1914);
    if ((struct t2[0:] *) y.data != (struct t2[0:] *) D.1914)
      {
        {
          void * D.1915;

          D.1915 = D.1914;
          if (D.1915 != 0B)
            {
              __builtin_free (D.1915);
            }
        }
      }
  }
}

_gfortran_internal_pack is called without setting up the array descriptor.

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

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

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

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |howarth at nitro dot
                   |                            |med.uc.edu

--- Comment #5 from Tobias Burnus <burnus at gcc dot gnu.org> 2013-01-15 11:26:12 UTC ---
*** Bug 55483 has been marked as a duplicate of this bug. ***

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

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

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

--- Comment #4 from Tobias Burnus <burnus at gcc dot gnu.org> 2013-01-15 11:26:00 UTC ---
*** Bug 55990 has been marked as a duplicate of this bug. ***

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code

--- Comment #6 from Tobias Burnus <burnus at gcc dot gnu.org> 2013-01-15 11:36:05 UTC ---
(In reply to comment #3)
>   y.data = 0B;
>     void * D.1914;
> 
>     D.1914 = _gfortran_internal_pack (&y);
>     a4t2 (D.1914);

Looks as if code of the form:

  if (y.data)
    D.1914 = _gfortran_internal_pack (&y);
  else
    D.1914 = NULL;

is missing - at least for "(gfc_option.allow_std & GFC_STD_F2008) != 0". I
assume that there is (implicitly?) a check for "y == NULL" for optional
dummies. (The latter is not applicable to the reduced example, but to line 39
of class_optional_2.f90.)

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

--- Comment #7 from Tobias Burnus <burnus at gcc dot gnu.org> 2013-01-15 14:47:31 UTC ---
(In reply to comment #6)
> Looks as if code of the form:
>   if (y.data)
>     D.1914 = _gfortran_internal_pack (&y);
>   else
>     D.1914 = NULL;
> 
> is missing

The bug is in trans-array.c's gfc_conv_array_parameter, which currently has:

  7156        ptr = build_call_expr_loc (input_location,
  7157                               gfor_fndecl_in_pack, 1, desc);
  7158  
  7159        if (fsym && fsym->attr.optional && sym && sym->attr.optional)
  7160          {
  7161            tmp = gfc_conv_expr_present (sym);


If the actual argument has sym->attr.optional, the code should work. Looking at
line 39 of class_optional_2.f90, there seems to be something not working, which
might be a bug in gfc_conv_expr_present.


If sym->attr.optional is not present, it is currently not handled by
gfc_conv_expr_present (assert).

Similarly, if the actual argument is an expression like "var%comp" - here, the
".data == NULL" check has to be done for "comp" and not for "var" alias "sym".
Similarly for expressions instead of variables. (There are presumably more
issues.)

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[hp at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

Hans-Peter Nilsson <hp at gcc dot gnu.org> changed:

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

--- Comment #8 from Hans-Peter Nilsson <hp at gcc dot gnu.org> 2013-01-19 07:40:22 UTC ---
For cris-elf, -O2 and above (not -Os).

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ubizjak at gmail dot com

--- Comment #9 from Dominique d'Humieres <dominiq at lps dot ens.fr> 2013-01-25 17:44:18 UTC ---
*** Bug 56107 has been marked as a duplicate of this bug. ***

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[ro at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

Rainer Orth <ro at gcc dot gnu.org> changed:

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

--- Comment #10 from Rainer Orth <ro at gcc dot gnu.org> 2013-02-04 13:10:56 UTC ---
On 32-bit sparc-sun-solaris2.* at -O2 and -O3.

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-02-04 13:44:21 UTC ---
It doesn't matter at which -O* levels it fails on what targets, the code uses
uninitialized fields and therefore it is pretty much random when it fails and
when it succeeds.  Worse yet is that this makes regression testing harder,
because this test behaves like the completely random mudflap tests.

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[janus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

--- Comment #12 from janus at gcc dot gnu.org 2013-02-05 21:25:09 UTC ---
Here is a further reduced test case, based on comment 1 and 3 (which are
basically identical):


program main
  implicit none

  integer, pointer :: y(:)

  y => null()
  call a4t2(y)

contains

 subroutine a4t2(x)
   integer, intent(in) :: x(4)
 end subroutine

end


It yields valgrind errors with all gfortran versions I tried (trunk, 4.7, 4.6
and 4.3):

==5664== Conditional jump or move depends on uninitialised value(s)
==5664==    at 0x4F25F05: _gfortran_internal_pack (in_pack_generic.c:53)
==5664==    by 0x40076E: MAIN__ (in /home/jweil/GSoC/PRs/55978/a.out)
==5664==    by 0x4007CA: main (in /home/jweil/GSoC/PRs/55978/a.out)


So it seems this is not an actual regression?!? class_optional_2.f90 was
introduced only in 4.8, and possibly r195125 only uncovered an existing bug
(e.g. by adding a call to  _gfortran_internal_pack).

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[janus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

--- Comment #13 from janus at gcc dot gnu.org 2013-02-05 22:26:55 UTC ---
(In reply to comment #12)
> program main
>   implicit none
> 
>   integer, pointer :: y(:)
> 
>   y => null()
>   call a4t2(y)
> 
> contains
> 
>  subroutine a4t2(x)
>    integer, intent(in) :: x(4)
>  end subroutine
> 
> end

Ok, sorry. This one is actually invalid, which is correctly caught by
-fcheck=all:

Fortran runtime error: Pointer actual argument 'y' is not associated

However, it becomes valid when making 'x' optional (but still gives the
valgrind error).

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[janus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

--- Comment #14 from janus at gcc dot gnu.org 2013-02-05 22:31:06 UTC ---
Draft patch:

Index: gcc/fortran/trans-array.c
===================================================================
--- gcc/fortran/trans-array.c    (revision 195644)
+++ gcc/fortran/trans-array.c    (working copy)
@@ -7164,6 +7164,17 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr *
             fold_convert (TREE_TYPE (se->expr), null_pointer_node));
     }

+      if (fsym && fsym->attr.optional && sym && sym->attr.pointer)
+    {
+      tmp = build_fold_indirect_ref_loc (input_location, desc);
+      tmp = gfc_conv_array_data (tmp);
+      tmp = fold_build2_loc (input_location, NE_EXPR, boolean_type_node,
+            tmp, fold_convert (TREE_TYPE (tmp), null_pointer_node));
+      ptr = build3_loc (input_location, COND_EXPR, TREE_TYPE (se->expr),
+            tmp, fold_convert (TREE_TYPE (se->expr), ptr),
+            fold_convert (TREE_TYPE (se->expr), null_pointer_node));
+    }
+
       ptr = gfc_evaluate_now (ptr, &se->pre);

       /* Use the packed data for the actual argument, except for contiguous
arrays,

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

--- Comment #15 from Tobias Burnus <burnus at gcc dot gnu.org> 2013-02-05 22:46:03 UTC ---
(In reply to comment #14)
> +      if (fsym && fsym->attr.optional && sym && sym->attr.pointer)

Shouldn't you use something like
  gfc_expr_attr(expr).pointer 
Otherwise, I fear it will fail for var%comp

Additionally, one might have to handle
- unallocated allocatables
- NULL()  [= EXPR_NULL]
- CLASS(), (pointer|allocatable)?

Furthermore, I think one can use:
  (gfc_option.allow_std & GFC_STD_F2008) != 0
as only since Fortran 2008, NULL pointers and deallocated allocatables count as
absent optionals (to nonpointer,nonallocatable actuals).

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[janus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

--- Comment #16 from janus at gcc dot gnu.org 2013-02-06 10:54:39 UTC ---
(In reply to comment #15)
> (In reply to comment #14)
> > +      if (fsym && fsym->attr.optional && sym && sym->attr.pointer)
> 
> Shouldn't you use something like
>   gfc_expr_attr(expr).pointer 
> Otherwise, I fear it will fail for var%comp

Agreed.


> Additionally, one might have to handle
> - unallocated allocatables
> - NULL()  [= EXPR_NULL]
> - CLASS(), (pointer|allocatable)?

Huh, good point. Here is an extended test case which tests some of these:

program main
  implicit none

  integer, pointer :: p(:)
  integer, allocatable :: a(:)

  type t
    integer, pointer :: p(:)
    integer, allocatable :: a(:)
  end type

  type(t) :: d

  ! (1) pointer
  p => null()
  call sub(p)

  ! (2) allocatable
  call sub(a)

  ! (3) pointer component
  d%p => null ()
  call sub(d%p)

  ! (4) NULL
  call sub(null())

contains

 subroutine sub(x)
   integer, intent(in), optional :: x(4)
 end subroutine

end



In fact the NULL case is rejected with (certainly a bug):

  call sub(null())
           1
Warning: Actual argument contains too few elements for dummy argument 'x' (1/4)
at (1)


For the allocatable case, no packing is done at all (I guess it's just not
needed there, since an allocatable array, contrary to a pointer, is always
contiguous, right?).


The pointer component case looks completely analogous to the simple pointer
case in the dump, but for some reason I do not see the valgrind errors there.

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

--- Comment #17 from Tobias Burnus <burnus at gcc dot gnu.org> 2013-02-06 11:16:38 UTC ---
(In reply to comment #16)
> For the allocatable case, no packing is done at all (I guess it's just not
> needed there, since an allocatable array, contrary to a pointer, is always
> contiguous, right?).

Good point. Also simply contiguous pointers shouldn't need packing, though, I
think that is checked before - hence, this code only reaches
non-simply-contiguous arrays.


Additionally, I think we currently add the wrong checks for (sym = DT with
pointer, optional attribute; fsym = optional):

  7159        if (fsym && fsym->attr.optional && sym && sym->attr.optional)
  7160          {
  7161            tmp = gfc_conv_expr_present (sym);

When one uses"sym%comp" as actual argument. In that case "sym" has to be
present and a non-null pointer. Thus, if "comp" is a pointer, only "sym%comp ==
NULL" needs to be checked, if "comp" isn't a pointer, no check at all is
required.


Maybe one should rename "gfc_conv_expr_present" to "gfc_conv_sym_present" - and
add a version for expressions, which can then be called for:

    if (fsym && fsym->attr.optional
        && (sym && sym->attr.optional)
           || expr->expr_type == EXPR_NULL
           || (expr->ts.type != BT_CLASS gfc_expr_attr(expr).pointer)
           || (expr->ts.type == BT_CLASS gfc_expr_attr(expr).class_pointer)))
      {
         tmp = gfc_conv_expr_present (sym);

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[janus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

--- Comment #18 from janus at gcc dot gnu.org 2013-02-06 11:44:46 UTC ---
Here is an updated patch, which works for pointers and pointer components, uses
GFC_STD_F2008 and does some minor refactoring:

Index: gcc/fortran/trans-array.c
===================================================================
--- gcc/fortran/trans-array.c    (revision 195800)
+++ gcc/fortran/trans-array.c    (working copy)
@@ -7156,12 +7156,26 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr *
       ptr = build_call_expr_loc (input_location,
                  gfor_fndecl_in_pack, 1, desc);

-      if (fsym && fsym->attr.optional && sym && sym->attr.optional)
+      if (fsym && fsym->attr.optional)
     {
-      tmp = gfc_conv_expr_present (sym);
-      ptr = build3_loc (input_location, COND_EXPR, TREE_TYPE (se->expr),
+      if (sym && sym->attr.optional)
+        {
+          tmp = gfc_conv_expr_present (sym);
+          ptr = build3_loc (input_location, COND_EXPR, TREE_TYPE (se->expr),
             tmp, fold_convert (TREE_TYPE (se->expr), ptr),
             fold_convert (TREE_TYPE (se->expr), null_pointer_node));
+        }
+      else if ((gfc_option.allow_std & GFC_STD_F2008)
+           && gfc_expr_attr (expr).pointer)
+        {
+          tmp = build_fold_indirect_ref_loc (input_location, desc);
+          tmp = gfc_conv_array_data (tmp);
+          tmp = fold_build2_loc (input_location, NE_EXPR, boolean_type_node,
+            tmp, fold_convert (TREE_TYPE (tmp), null_pointer_node));
+          ptr = build3_loc (input_location, COND_EXPR, TREE_TYPE (se->expr),
+            tmp, fold_convert (TREE_TYPE (se->expr), ptr),
+            fold_convert (TREE_TYPE (se->expr), null_pointer_node));
+        }
     }

       ptr = gfc_evaluate_now (ptr, &se->pre);


I think this should be sufficient to fix the regression in
class_optional_2.f90. I suggest everything else should wait for 4.9. (At this
point, producing a stable release should have priority over fixing more
non-regression bugs.)

Btw, as an alternative: Wouldn't it be more reasonable to move the above check
into _gfortran_internal_pack?

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

--- Comment #19 from Tobias Burnus <burnus at gcc dot gnu.org> 2013-02-06 12:17:14 UTC ---
(In reply to comment #18)
> Here is an updated patch, which works for pointers and pointer components,

Seems to work better than I expected. However, there is still an issue for:

integer, pointer :: p(:) => null()
call one(p) ! OK
call one()  ! Segfault
contains
subroutine one(x)
  integer, pointer, optional :: x(:)
  call two(x)
end subroutine one
subroutine two(x)
  integer, optional :: x(*)
  if (present(x)) print *, x(1)
end subroutine two
end


That generates the code:
    D.1896 = x != 0B
             ? (struct array1_integer(kind=4) *) _gfortran_internal_pack (x)
             : 0B;
    two (D.1896);
    if (x != 0B
        && (integer(kind=4)[0:] *) x->data != (integer(kind=4)[0:] *) D.1896)

I think it should be "x != 0B && x->data != 0B" in the first condition.

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[janus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

--- Comment #20 from janus at gcc dot gnu.org 2013-02-06 12:54:41 UTC ---
(In reply to comment #19)
> That generates the code:
>     D.1896 = x != 0B
>              ? (struct array1_integer(kind=4) *) _gfortran_internal_pack (x)
>              : 0B;
>     two (D.1896);
>     if (x != 0B
>         && (integer(kind=4)[0:] *) x->data != (integer(kind=4)[0:] *) D.1896)
> 
> I think it should be "x != 0B && x->data != 0B" in the first condition.

I don't see how this would help (if x is anyway OB, as it is the case for your
second call).

Moreover, it seems to fail only *after* the call to 'two', probably in the if
statement you show. The failure only happens at -O0 for me.

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[janus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

janus at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |janus at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #21 from janus at gcc dot gnu.org 2013-02-06 13:00:52 UTC ---
(In reply to comment #18)
> Btw, as an alternative: Wouldn't it be more reasonable to move the above check
> into _gfortran_internal_pack?

Here is the very simple patch which does this:


Index: libgfortran/runtime/in_pack_generic.c
===================================================================
--- libgfortran/runtime/in_pack_generic.c    (revision 195800)
+++ libgfortran/runtime/in_pack_generic.c    (working copy)
@@ -48,6 +48,9 @@ internal_pack (gfc_array_char * source)
   index_type size;
   index_type type_size;

+  if (source->base_addr == NULL)
+    return NULL;
+
   type_size = GFC_DTYPE_TYPE_SIZE(source);
   size = GFC_DESCRIPTOR_SIZE (source);
   switch (type_size)


It seems to work just as well as the patch in comment #18 and is much simpler.
Also it could catch this sort of thing in other situations as well. So I would
say that it is preferable over the previous patches.

Will test for regressions (but don't expect any).

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[janus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

--- Comment #22 from janus at gcc dot gnu.org 2013-02-06 13:56:02 UTC ---
(In reply to comment #21)
> It seems to work just as well as the patch in comment #18 and is much simpler.
> Also it could catch this sort of thing in other situations as well. So I would
> say that it is preferable over the previous patches.
> 
> Will test for regressions (but don't expect any).

The patch in comment 21, like the previous ones in comment 14 and 18, is free
of testsuite regressions.

[Bug fortran/55978] [4.8 Regression] class_optional_2.f90 -Os fails

[janus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

--- Comment #23 from janus at gcc dot gnu.org 2013-02-06 19:39:43 UTC ---
Author: janus
Date: Wed Feb  6 19:39:35 2013
New Revision: 195814

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195814
Log:
2013-02-06  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/55978
    * runtime/in_pack_generic.c (internal_pack): Return if base_addr is
    NULL.

2013-02-06  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/55978
    * gfortran.dg/class_optional_2.f90: Uncomment some cases which work now.

Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/class_optional_2.f90
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/runtime/in_pack_generic.c

[Bug fortran/55978] class_optional_2.f90 -Os fails

[janus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

janus at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW
         AssignedTo|janus at gcc dot gnu.org    |unassigned at gcc dot
                   |                            |gnu.org
            Summary|[4.8 Regression]            |class_optional_2.f90 -Os
                   |class_optional_2.f90 -Os    |fails
                   |fails                       |

--- Comment #24 from janus at gcc dot gnu.org 2013-02-06 19:51:08 UTC ---
The failures of class_optional_2.f90 should be fixed with r195814.

I'm removing the regression tag, but will leave open the PR, in order to track
the remaining issues:
 * check the things mentioned in comment 15
 * the NULL case in comment 16 is rejected
 * comment 17
 * comment 19

[Bug fortran/55978] class_optional_2.f90 -Os fails

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.0                       |4.8.1

--- Comment #25 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-03-22 14:44:19 UTC ---
GCC 4.8.0 is being released, adjusting target milestone.

[Bug fortran/55978] class_optional_2.f90 -Os fails

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.1                       |4.8.2

--- Comment #26 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.8.1 has been released.

[Bug fortran/55978] class_optional_2.f90 -Os fails

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55978

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.2                       |4.8.3

--- Comment #27 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.8.2 has been released.

[Bug fortran/55978] class_optional_2.f90 -Os fails

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.3                       |---

[Bug fortran/55978] class_optional_2.f90 -Os fails

[anlauf at gcc dot gnu.org]

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

anlauf at gcc dot gnu.org changed:

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

--- Comment #29 from anlauf at gcc dot gnu.org ---
(In reply to janus from comment #24)
>  * the NULL case in comment 16 is rejected

This is fixed by:

diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc
index fc4fe662eab..ad99285d513 100644
--- a/gcc/fortran/interface.cc
+++ b/gcc/fortran/interface.cc
@@ -3409,6 +3425,9 @@ gfc_compare_actual_formal (gfc_actual_arglist **ap,
gfc_formal_arglist *formal,
       if (f->sym->ts.type == BT_CLASS)
        goto skip_size_check;

+      if (a->expr->expr_type == EXPR_NULL || a->expr->ts.type == BT_UNKNOWN)
+       goto skip_size_check;
+
       actual_size = get_expr_storage_size (a->expr);
       formal_size = get_sym_storage_size (f->sym);
       if (actual_size != 0 && actual_size < formal_size

[Bug fortran/55978] class_optional_2.f90 -Os fails

[anlauf at gcc dot gnu.org]

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

--- Comment #30 from anlauf at gcc dot gnu.org ---
(In reply to anlauf from comment #29)
> +      if (a->expr->expr_type == EXPR_NULL || a->expr->ts.type == BT_UNKNOWN)
> +	goto skip_size_check;

Oops, that should read && instead of ||.

[Bug fortran/55978] class_optional_2.f90 -Os fails

[cvs-commit at gcc dot gnu.org]

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

--- Comment #31 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:c083a453dbe51853e26e02edd8b9346fb8618292

commit r14-9631-gc083a453dbe51853e26e02edd8b9346fb8618292
Author: Harald Anlauf <anlauf@gmx.de>
Date:   Fri Mar 22 18:17:15 2024 +0100

    Fortran: no size check passing NULL() without MOLD argument [PR55978]

    gcc/fortran/ChangeLog:

            PR fortran/55978
            * interface.cc (gfc_compare_actual_formal): Skip size check for
            NULL() actual without MOLD argument.

    gcc/testsuite/ChangeLog:

            PR fortran/55978
            * gfortran.dg/null_actual_5.f90: New test.

[Bug fortran/55978] class_optional_2.f90 -Os fails

[cvs-commit at gcc dot gnu.org]

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

--- Comment #32 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:57062bc355aece623c6a38c5e813ed24f8b775f1

commit r13-8491-g57062bc355aece623c6a38c5e813ed24f8b775f1
Author: Harald Anlauf <anlauf@gmx.de>
Date:   Fri Mar 22 18:17:15 2024 +0100

    Fortran: no size check passing NULL() without MOLD argument [PR55978]

    gcc/fortran/ChangeLog:

            PR fortran/55978
            * interface.cc (gfc_compare_actual_formal): Skip size check for
            NULL() actual without MOLD argument.

    gcc/testsuite/ChangeLog:

            PR fortran/55978
            * gfortran.dg/null_actual_5.f90: New test.

[Bug fortran/55978] class_optional_2.f90 -Os fails

[anlauf at gcc dot gnu.org]

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

--- Comment #33 from anlauf at gcc dot gnu.org ---
I've been repeatedly struggling with the testcase in comment#19.

Since the dump-tree did not reveal anything, I ran a reduced version
under gdb to see why the code crashes at -O0 and -Og but not at higher
optimization.  I got them impression that the dump-tree does not tell
the full truth: subroutine one has

  {
    struct array01_integer(kind=4) * D.4292;

    D.4292 = x != 0B ? (struct array01_integer(kind=4) *)
_gfortran_internal_pack (x) : 0B;
    two (D.4292);
    if (x != 0B && (integer(kind=4)[0:] *) x->data != (integer(kind=4)[0:] *)
D.4292)
      {
        _gfortran_internal_unpack (x, D.4292);
        __builtin_free ((void *) D.4292);
      }
  }

but to my untrained eye it looked like the test x->data != D.4292 was
executed before x != 0B.  This lead me to the following tentative patch:

diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index 19d69aec9c0..06bfea1f461 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -9025,7 +9100,7 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * expr,
bool g77,
                             fold_convert (TREE_TYPE (tmp), ptr), tmp);

       if (fsym && fsym->attr.optional && sym && sym->attr.optional)
-       tmp = fold_build2_loc (input_location, TRUTH_AND_EXPR,
+       tmp = fold_build2_loc (input_location, TRUTH_ANDIF_EXPR,
                               logical_type_node,
                               gfc_conv_expr_present (sym), tmp);

Strangely enough, this gives exactly the same dump-tree!
(If someone knows why, please enlighten me!)
But it fixed the issue.

The I looked at the optimized tree.  The reference version has:

  <bb 4> [local count: 1073741824]:
  # _5 = PHI <_19(3), y_17(D)(2)>
  two (_5);
  _1 = y_17(D) != 0B;
  _2 = y_17(D)->data;
  _3 = _2 != _5;
  _4 = _1 & _3;
  if (_4 != 0)
    goto <bb 5>; [33.00%]
  else
    goto <bb 6>; [67.00%]

  <bb 5> [local count: 354334800]:
  __builtin_free (_5);

This explains the "boom".

The version with the ANDIF has:

  <bb 4> [local count: 1073741824]:
  # _2 = PHI <_16(3), y_14(D)(2)>
  two (_2);
  if (y_14(D) != 0B)
    goto <bb 5>; [70.00%]
  else
    goto <bb 7>; [30.00%]

  <bb 5> [local count: 751619280]:
  _1 = y_14(D)->data;
  if (_1 != _2)
    goto <bb 6>; [53.47%]
  else
    goto <bb 7>; [46.53%]

  <bb 6> [local count: 401890828]:
  __builtin_free (_2);

This looks sane to me: the data component is not referenced if the dummy
is seen to be absent.

Regtesting...

[Bug fortran/55978] class_optional_2.f90 -Os fails

[cvs-commit at gcc dot gnu.org]

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

--- Comment #34 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:f02c70dafd384f0c44d7a0920f4a75a30e267045

commit r15-1585-gf02c70dafd384f0c44d7a0920f4a75a30e267045
Author: Harald Anlauf <anlauf@gmx.de>
Date:   Sun Jun 23 22:36:43 2024 +0200

    Fortran: fix passing of optional dummy as actual to optional argument
[PR55978]

    gcc/fortran/ChangeLog:

            PR fortran/55978
            * trans-array.cc (gfc_conv_array_parameter): Do not dereference
            data component of a missing allocatable dummy array argument for
            passing as actual to optional dummy.  Harden logic of presence
            check for optional pointer dummy by using TRUTH_ANDIF_EXPR instead
            of TRUTH_AND_EXPR.

    gcc/testsuite/ChangeLog:

            PR fortran/55978
            * gfortran.dg/optional_absent_12.f90: New test.