[Bug fortran/107874] New: merge not using all its arguments

[Bug fortran/107874] New: merge not using all its arguments

[john.harper at vuw dot ac.nz]

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

            Bug ID: 107874
           Summary: merge not using all its arguments
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: john.harper at vuw dot ac.nz
  Target Milestone: ---

The F2018 standard 15.5.3 requires all actual arguments in a function reference
to be evaluated.

In the program below, merge has three scalar arguments; ifort evaluates
all three, gfortran evaluates only the useful two, which violates 15.5.3.

program testmerge9
  implicit none
  integer i
  logical:: x(2)=(/.true., .false./), y ! avoiding [ ] allows f95 option
  do i = 1,2
     y = merge(tstuff(),fstuff(),x(i))
     print *,y
  end do
contains
  logical function tstuff()
    print *,'tstuff'
    tstuff = .true.
  end function tstuff

  logical function fstuff()
    print *,'fstuff'
    fstuff = .false.
  end function fstuff
end program testmerge9

Steve Kargl said "Looks like a bug. Feel free to send a bug report with this
patch."

diff --git a/gcc/fortran/trans-intrinsic.cc b/gcc/fortran/trans-intrinsic.cc
index bb938026828..c5b63d356e1 100644
--- a/gcc/fortran/trans-intrinsic.cc
+++ b/gcc/fortran/trans-intrinsic.cc
@@ -7538,9 +7538,9 @@ gfc_conv_intrinsic_merge (gfc_se * se, gfc_expr * expr)
   gfc_conv_intrinsic_function_args (se, expr, args, num_args);
   if (expr->ts.type != BT_CHARACTER)
     {
-      tsource = args[0];
-      fsource = args[1];
-      mask = args[2];
+      tsource = gfc_evaluate_now (args[0], &se->pre);
+      fsource = gfc_evaluate_now (args[1], &se->pre);
+      mask = gfc_evaluate_now (args[2], &se->pre);
     }
   else
     {

[Bug fortran/107874] merge not using all its arguments

[anlauf at gcc dot gnu.org]

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

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
                 CC|                            |anlauf at gcc dot gnu.org
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=105371
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-11-26
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from anlauf at gcc dot gnu.org ---
Confirmed.

The patch by Steve looks good, but I think it needs to be moved down slightly
so that it also handles character.

With the patch, a few cases that are discussed in pr105371 are also fixed.
Still missing is a fix for constant mask argument that we mishandle during
simplification.

[Bug fortran/107874] merge not using all its arguments

[kargl at gcc dot gnu.org]

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

kargl at gcc dot gnu.org changed:

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

--- Comment #2 from kargl at gcc dot gnu.org ---
John thanks for posting the bug.

Harald, you are likely right the patch can be moved down.  I'll programmed up
the example from the Fortran 2018 standard, which works as expected.  So, there
is definitely something about a scalar mask choosing the actual argument before
both are evaluated.

   program foo
   call bah
   contains
      subroutine bah
         logical, parameter :: t = .true., f = .false.
         logical, parameter ::  mask(2,3)= reshape([t, f, f, f, t, t], [2,3])
         integer tsrc(2,3), fsrc(2,3), res(2,3)
         !
         ! Direct reference to merge
         !
         tsrc = reshape([1, 2, 6, 4, 5, 6], [2,3])
         fsrc = reshape([0, 7, 3, 4, 2, 8], [2,3])
         res = 42
         res = merge(tsrc, fsrc, mask)

         write(*,'(*(I0,1X))') res(1,:)  ! Should be 1 3 5
         write(*,'(*(I0,1X))') res(2,:)  ! Should be 7 4 6
         write(*,*)
         !
         ! Load matrices via a function.
         !
         res = 0 ! Clear
         res = merge(load('t'), load('f'), mask) ! Clear
         write(*,'(*(I0,1X))') res(1,:)  ! Should be 1 3 5
         write(*,'(*(I0,1X))') res(2,:)  ! Should be 7 4 6    
      end subroutine bah
      function load(c) result(r)
         integer r(2,3)
         character, intent(in) :: c
         if (c == 't') then
            r = reshape([1, 2, 6, 4, 5, 6], [2,3])
            print *, 'loaded t'
         else if (c == 'f') then
            r = reshape([0, 7, 3, 4, 2, 8], [2,3])
            print *, 'loaded f'
         else
            stop 'whoops'
         end if
      end function load
end program foo

[Bug fortran/107874] merge not using all its arguments

[anlauf at gcc dot gnu.org]

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

--- Comment #3 from anlauf at gcc dot gnu.org ---
(In reply to kargl from comment #2)
> Harald, you are likely right the patch can be moved down.  I'll programmed
> up the example from the Fortran 2018 standard, which works as expected.  So,
> there is definitely something about a scalar mask choosing the actual
> argument before both are evaluated.
> 
>    program foo

Steve,

this example from the standard seems to be working down to 7.5 for me.
Am I missing something?  Do we need this in the testsuite?

I'd say it's rather the following two lines replacing the loop in the
reproducer in comment#0:

  print *, merge(tstuff(),fstuff(),.true.)
  print *, merge(tstuff(),fstuff(),.false.)

This is mis-simplified in simplify.cc:4909

gfc_expr *
gfc_simplify_merge (gfc_expr *tsource, gfc_expr *fsource, gfc_expr *mask)
{
  gfc_expr * result;
  gfc_constructor *tsource_ctor, *fsource_ctor, *mask_ctor;

  if (mask->expr_type == EXPR_CONSTANT)
    {
      result = gfc_copy_expr (mask->value.logical ? tsource : fsource);
      /* Parenthesis is needed to get lower bounds of 1.  */
      result = gfc_get_parentheses (result);
      gfc_simplify_expr (result, 1);
      return result;
    }

So unless tsource and fsource are both constant, we have to give up here.

[Bug fortran/107874] merge not using all its arguments

[anlauf at gcc dot gnu.org]

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

--- Comment #4 from anlauf at gcc dot gnu.org ---
The following patch fixes comment#3:

diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc
index 9c2fea8c5f2..2f69c4369ab 100644
--- a/gcc/fortran/simplify.cc
+++ b/gcc/fortran/simplify.cc
@@ -4913,6 +4914,11 @@ gfc_simplify_merge (gfc_expr *tsource, gfc_expr
*fsource, gfc_expr *mask)

   if (mask->expr_type == EXPR_CONSTANT)
     {
+      /* The standard requires evaluation of all function arguments.
+        Simplify only when TSOURCE, FSOURCE are constant expressions.  */
+      if (!gfc_is_constant_expr (tsource) || !gfc_is_constant_expr (fsource))
+       return NULL;
+
       result = gfc_copy_expr (mask->value.logical ? tsource : fsource);
       /* Parenthesis is needed to get lower bounds of 1.  */
       result = gfc_get_parentheses (result);

This leads to a "regression" for gfortran.dg/merge_init_expr_2.f90,
which is due to the pattern matching the old, faulty simplification result.
That's trivial to fix, though.

[Bug fortran/107874] merge not using all its arguments

[sgk at troutmask dot apl.washington.edu]

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

--- Comment #5 from Steve Kargl <sgk at troutmask dot apl.washington.edu> ---
On Sun, Nov 27, 2022 at 08:00:35PM +0000, anlauf at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874
> 
> --- Comment #3 from anlauf at gcc dot gnu.org ---
> (In reply to kargl from comment #2)
> > Harald, you are likely right the patch can be moved down.  I'll programmed
> > up the example from the Fortran 2018 standard, which works as expected.  So,
> > there is definitely something about a scalar mask choosing the actual
> > argument before both are evaluated.
> > 
> >    program foo
> 
> Steve,
> 
> this example from the standard seems to be working down to 7.5 for me.
> Am I missing something?  Do we need this in the testsuite?

You are not missing anything.  I wanted an example that works
with or without the patch John included, so that we don't 
accidently introduce a regression.

> I'd say it's rather the following two lines replacing the loop in the
> reproducer in comment#0:
> 
>   print *, merge(tstuff(),fstuff(),.true.)
>   print *, merge(tstuff(),fstuff(),.false.)
> 
> This is mis-simplified in simplify.cc:4909

Good find!  This may indeed be a source of the issue.

[Bug fortran/107874] merge not using all its arguments

[anlauf at gcc dot gnu.org]

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

anlauf at gcc dot gnu.org changed:

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

--- Comment #6 from anlauf at gcc dot gnu.org ---
Submitted: https://gcc.gnu.org/pipermail/fortran/2022-November/058558.html

Taking.

[Bug fortran/107874] merge not using all its arguments

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS 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:3832c6f7e672e76bba74a508bf3a49740ea38046

commit r13-4394-g3832c6f7e672e76bba74a508bf3a49740ea38046
Author: Harald Anlauf <anlauf@gmx.de>
Date:   Mon Nov 28 20:43:02 2022 +0100

    Fortran: intrinsic MERGE shall use all its arguments [PR107874]

    gcc/fortran/ChangeLog:

            PR fortran/107874
            * simplify.cc (gfc_simplify_merge): When simplifying MERGE with a
            constant scalar MASK, ensure that arguments TSOURCE and FSOURCE are
            either constant or will be evaluated.
            * trans-intrinsic.cc (gfc_conv_intrinsic_merge): Evaluate arguments
            before generating conditional expression.

    gcc/testsuite/ChangeLog:

            PR fortran/107874
            * gfortran.dg/merge_init_expr_2.f90: Adjust code to the corrected
            simplification.
            * gfortran.dg/merge_1.f90: New test.

    Co-authored-by: Steven G. Kargl <kargl@gcc.gnu.org>

[Bug fortran/107874] merge not using all its arguments

[anlauf at gcc dot gnu.org]

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

anlauf at gcc dot gnu.org changed:

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

--- Comment #8 from anlauf at gcc dot gnu.org ---
Fixed on mainline for gcc-13.  Closing.

Thanks for the report!

[Bug fortran/107874] merge not using all its arguments

[jvdelisle2 at gmail dot com]

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

--- Comment #9 from Jerry DeLisle <jvdelisle2 at gmail dot com> ---
I am seeing timeout failures with merge_1.f90. Can you look into this?

Thanks,

Jerry

On Thu, Dec 1, 2022, 12:28 PM anlauf at gcc dot gnu.org via Gcc-bugs <
gcc-bugs@gcc.gnu.org> wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874
>
> anlauf at gcc dot gnu.org changed:
>
>            What    |Removed                     |Added
>
> ----------------------------------------------------------------------------
>    Target Milestone|---                         |13.0
>          Resolution|---                         |FIXED
>              Status|ASSIGNED                    |RESOLVED
>
> --- Comment #8 from anlauf at gcc dot gnu.org ---
> Fixed on mainline for gcc-13.  Closing.
>
> Thanks for the report!

[Bug fortran/107874] merge not using all its arguments

[anlauf at gcc dot gnu.org]

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

anlauf at gcc dot gnu.org changed:

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

--- Comment #10 from anlauf at gcc dot gnu.org ---
Hmmm, I do not see the failures here, but gcc-testresults also shows them.

Can you please attach a tree-dump?

[Bug fortran/107874] merge not using all its arguments

[anlauf at gcc dot gnu.org]

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

--- Comment #11 from anlauf at gcc dot gnu.org ---
And this is the output I'm getting on stdout:

 tstuff
 fstuff
 T
 tstuff
 fstuff
 F
 tstuff
 fstuff
 T
 tstuff
 fstuff
 F
 tstuff
 T
 tstuff
 F
 fstuff
 T
 fstuff
 F

Can you compare your output with the above to narrow down what might be wrong?

[Bug fortran/107874] merge not using all its arguments

[jvdelisle at gcc dot gnu.org]

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

Jerry DeLisle <jvdelisle at gcc dot gnu.org> changed:

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

--- Comment #12 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
Created attachment 54003
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54003&action=edit
-fdump-tree-original output

I get no output at runtime. The program simply hangs and does nothing.

This is on:

$ uname -a
Linux r7laptop 6.0.10-300.fc37.x86_64 #1 SMP PREEMPT_DYNAMIC Sat Nov 26
16:55:13 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

[Bug fortran/107874] merge not using all its arguments

[jvdelisle at gcc dot gnu.org]

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

--- Comment #13 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
A debug session gives:

(gdb) c
Continuing.
^C
Program received signal SIGINT, Interrupt.
futex_wait (private=0, expected=2, futex_word=0x405950) at
../sysdeps/nptl/futex-internal.h:146
146       int err = lll_futex_timed_wait (futex_word, expected, NULL, private);

(gdb) bt
#0  futex_wait (private=0, expected=2, futex_word=0x405950)
    at ../sysdeps/nptl/futex-internal.h:146
#1  __GI___lll_lock_wait (futex=futex@entry=0x405950, private=0)
    at lowlevellock.c:49
#2  0x00007ffff79d1432 in lll_mutex_lock_optimized (mutex=0x405950)
    at pthread_mutex_lock.c:48
#3  ___pthread_mutex_lock (mutex=0x405950) at pthread_mutex_lock.c:93
#4  0x00007ffff7e73503 in __gthread_mutex_lock (__mutex=0x405950)
    at ../libgcc/gthr-default.h:749
#5  get_gfc_unit (n=6, do_create=1) at ../../../trunk/libgfortran/io/unit.c:395
#6  0x00007ffff7e71b85 in data_transfer_init (dtp=0x7fffffffd9d0, read_flag=0)
    at ../../../trunk/libgfortran/io/transfer.c:3007
#7  0x00000000004017dd in testmerge9::tstuff () at merge_1.f90:39
#8  0x000000000040128c in testmerge9 () at merge_1.f90:14
#9  0x000000000040184f in main (argc=1, argv=0x7fffffffe2c4) at merge_1.f90:36
#10 0x00007ffff796a510 in __libc_start_call_main (
    main=main@entry=0x40181b <main>, argc=argc@entry=1, 
    argv=argv@entry=0x7fffffffdfa8)
    at ../sysdeps/nptl/libc_start_call_main.h:58
#11 0x00007ffff796a5c9 in __libc_start_main_impl (main=0x40181b <main>, 
    argc=1, argv=0x7fffffffdfa8, init=<optimized out>, fini=<optimized out>, 
    rtld_fini=<optimized out>, stack_end=0x7fffffffdf98)
    at ../csu/libc-start.c:381

I would speculate we are trashing some memory since there is no reason to hang
in there.

[Bug fortran/107874] merge not using all its arguments

[jvdelisle at gcc dot gnu.org]

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

--- Comment #14 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
Now this is interesting.

Compling with -static I get:

$ gfc -static merge_1.f90 
[jerry@r7laptop merge]$ ./a.out 
 tstuff
 fstuff
 T
 tstuff
 fstuff
 F
 tstuff
 fstuff
 T
 tstuff
 fstuff
 F
 tstuff
 T
 tstuff
 F
 fstuff
 T
 fstuff
 F


So there is something wrong with the linking as it hangs without -static

$ echo $LD_LIBRARY_PATH
/home/jerry/dev/usr/lib64/:/home/jerry/dev/usr/lib/

$ gfc -v
Using built-in specs.
COLLECT_GCC=gfc
COLLECT_LTO_WRAPPER=/home/jerry/dev/usr/libexec/gcc/x86_64-pc-linux-gnu/13.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../trunk/configure --prefix=/home/jerry/dev/usr
--enable-languages=c,c++,fortran --enable-libgomp --disable-bootstrap
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 13.0.0 20221201 (experimental) (GCC) 

[jerry@r7laptop bin]$ ls -l gfc
lrwxrwxrwx. 1 jerry jerry 32 Nov 20 15:50 gfc ->
/home/jerry/dev/usr/bin/gfortran

Am I doing something wrong?

Also Valgrind does show some issues, but one can not be sure.:

$ gfc -static merge_1.f90 
[jerry@r7laptop merge]$ valgrind --leak-check=full --show-leak-kinds=all
./a.out
==17321== Memcheck, a memory error detector
==17321== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==17321== Using Valgrind-3.20.0 and LibVEX; rerun with -h for copyright info
==17321== Command: ./a.out
==17321== 
==17321== Syscall param set_robust_list(head) points to uninitialised byte(s)
==17321==    at 0x48848A: __tls_init_tp (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x439361: __libc_setup_tls (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x438630: (below main) (in /home/jerry/dev/test/merge/a.out)
==17321==  Address 0x40006b0 is in the brk data segment 0x4000000-0x4000d3f
==17321== 
==17321== Conditional jump or move depends on uninitialised value(s)
==17321==    at 0x460387: malloc (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x4B3B9A: _dl_get_origin (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x489416: _dl_non_dynamic_init (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x48ABE8: __libc_init_first (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x4386A8: (below main) (in /home/jerry/dev/test/merge/a.out)
==17321== 
==17321== Conditional jump or move depends on uninitialised value(s)
==17321==    at 0x460459: malloc (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x4B3B9A: _dl_get_origin (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x489416: _dl_non_dynamic_init (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x48ABE8: __libc_init_first (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x4386A8: (below main) (in /home/jerry/dev/test/merge/a.out)
==17321== 
==17321== Conditional jump or move depends on uninitialised value(s)
==17321==    at 0x45EFBB: _int_malloc (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x45FC34: tcache_init.part.0 (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x460463: malloc (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x4B3B9A: _dl_get_origin (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x489416: _dl_non_dynamic_init (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x48ABE8: __libc_init_first (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x4386A8: (below main) (in /home/jerry/dev/test/merge/a.out)
==17321== 
==17321== Conditional jump or move depends on uninitialised value(s)
==17321==    at 0x4757D4: __strcspn_sse42 (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x4B8CFC: strsep (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x4AEE1E: fillin_rpath.isra.0 (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x4AF52C: _dl_init_paths (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x489A3B: _dl_non_dynamic_init (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x48ABE8: __libc_init_first (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x4386A8: (below main) (in /home/jerry/dev/test/merge/a.out)
==17321== 
==17321== Conditional jump or move depends on uninitialised value(s)
==17321==    at 0x4757B2: __strcspn_sse42 (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x4B8CFC: strsep (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x4AEE1E: fillin_rpath.isra.0 (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x4AF52C: _dl_init_paths (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x489A3B: _dl_non_dynamic_init (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x48ABE8: __libc_init_first (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x4386A8: (below main) (in /home/jerry/dev/test/merge/a.out)
==17321== 
 tstuff
 fstuff
 T
 tstuff
 fstuff
 F
 tstuff
 fstuff
 T
 tstuff
 fstuff
 F
 tstuff
 T
 tstuff
 F
 fstuff
 T
 fstuff
 F
==17321== Conditional jump or move depends on uninitialised value(s)
==17321==    at 0x45B3D9: __libc_cleanup_push_defer (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x459EA9: _IO_flush_all_lockp (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x45A122: _IO_cleanup (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x43F40A: __run_exit_handlers (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x43F4AF: exit (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x4370C0: (below main) (in /home/jerry/dev/test/merge/a.out)
==17321== 
==17321== Conditional jump or move depends on uninitialised value(s)
==17321==    at 0x45B425: __libc_cleanup_pop_restore (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x45A070: _IO_flush_all_lockp (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x45A122: _IO_cleanup (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x43F40A: __run_exit_handlers (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x43F4AF: exit (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x4370C0: (below main) (in /home/jerry/dev/test/merge/a.out)
==17321== 
==17321== Conditional jump or move depends on uninitialised value(s)
==17321==    at 0x45B3D9: __libc_cleanup_push_defer (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x45A141: _IO_cleanup (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x43F40A: __run_exit_handlers (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x43F4AF: exit (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x4370C0: (below main) (in /home/jerry/dev/test/merge/a.out)
==17321== 
==17321== Conditional jump or move depends on uninitialised value(s)
==17321==    at 0x45B425: __libc_cleanup_pop_restore (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x45A2D0: _IO_cleanup (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x43F40A: __run_exit_handlers (in
/home/jerry/dev/test/merge/a.out)
==17321==    by 0x43F4AF: exit (in /home/jerry/dev/test/merge/a.out)
==17321==    by 0x4370C0: (below main) (in /home/jerry/dev/test/merge/a.out)
==17321== 
==17321== 
==17321== HEAP SUMMARY:
==17321==     in use at exit: 0 bytes in 0 blocks
==17321==   total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==17321== 
==17321== All heap blocks were freed -- no leaks are possible
==17321== 
==17321== Use --track-origins=yes to see where uninitialised values come from
==17321== For lists of detected and suppressed errors, rerun with: -s
==17321== ERROR SUMMARY: 10 errors from 10 contexts (suppressed: 0 from 0)

[Bug fortran/107874] merge not using all its arguments

[anlauf at gcc dot gnu.org]

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

--- Comment #15 from anlauf at gcc dot gnu.org ---
Created attachment 54006
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54006&action=edit
Modified testcase

I found that I get a hang on my system when I specify -fopenmp.

It appears that there is a deadlock due to the evaluation of the merge
arguments during the print.  So changing the testcase as attached might
fix the problem.

Can you please verify?

[Bug fortran/107874] merge not using all its arguments

[jvdelisle at gcc dot gnu.org]

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

--- Comment #16 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
(In reply to anlauf from comment #15)
--- snip ---
> Can you please verify?

Yes, this fixes the test case.  However if the orginal test case is valid
fortran we probably need to fix something else. Paul Thomas was noticing a
similar problem with his Finalization patches.  He was doing the finalization
inside trans_transfer or similar so it was blocking on a mutex trying to
finalize in the middle of an I/O operation.

So in this case, my guess is the merge expression needs to be resolved before
the translation phase.

[Bug fortran/107874] merge not using all its arguments

[anlauf at gcc dot gnu.org]

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

--- Comment #17 from anlauf at gcc dot gnu.org ---
(In reply to Jerry DeLisle from comment #16)
> (In reply to anlauf from comment #15)
> --- snip ---
> > Can you please verify?
> 
> Yes, this fixes the test case.

OK, thanks for confirming.

> However if the orginal test case is valid
> fortran we probably need to fix something else. Paul Thomas was noticing a
> similar problem with his Finalization patches.  He was doing the
> finalization inside trans_transfer or similar so it was blocking on a mutex
> trying to finalize in the middle of an I/O operation.
> 
> So in this case, my guess is the merge expression needs to be resolved
> before the translation phase.

If I interprete the tree-dump correctly, we have e.g.:

          _gfortran_st_write (&dt_parm.2);
          {
            logical(kind=4) D.4279;
            logical(kind=4) D.4280;
            logical(kind=4) D.4281;
            logical(kind=4) D.4282;

            D.4279 = tstuff ();
            D.4280 = fstuff ();
            D.4281 = x[(integer(kind=8)) i + -1];
            D.4282 = D.4281 ? D.4279 : D.4280;
            _gfortran_transfer_logical_write (&dt_parm.2, &D.4282, 4);
          }
          _gfortran_st_write_done (&dt_parm.2);

so we start the write, then evaluate the merge(), which is tstuff()/fstuff(),
and which does its own I/O, and then return to continue with the transfer.

So this might be non-conforming code, see

F2018:12.12 Restrictions on input/output statements

(2) An input/output statement that is executed while another input/output
statement is being executed is a recursive input/output statement.
A recursive input/output statement shall not identify an external unit that
is identified by another input/output statement being executed except that
a child data transfer statement may identify its parent data transfer
statement external unit.

I am not sure I fully understand the last sentence in this paragraph,
but I think the pushed testcase might be invalid and should be replaced.

If you agree, I'll simply do this.

[Bug fortran/107874] merge not using all its arguments

[john.harper at vuw dot ac.nz]

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

--- Comment #18 from john.harper at vuw dot ac.nz ---
An interesting problem! But I thought my original test case did not have 
recursive I/O because tstuff and fstuff each print something in the 
statement
    y = merge(tstuff(),fstuff(),x(i))
but y itself is printed only in the next statement,
    print *,y

Or does evaluating merge allow each of tstuff and fstuff to be evaluated
at the same time? I was thinking of tstuff and fstuff being evaluated
in succession but could there be systems in which they are evaluated 
simultaneously? If so, whether the program is valid Fortran depends on the
kind of system on which it is being executed.

John Harper

On Fri, 2 Dec 2022, anlauf at gcc dot gnu.org wrote:

> Date: Fri, 2 Dec 2022 20:22:23 +0000
> From: anlauf at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org>
> To: John Harper <john.harper@vuw.ac.nz>
> Subject: [Bug fortran/107874] merge not using all its arguments
> Resent-Date: Sat, 3 Dec 2022 09:22:36 +1300 (NZDT)
> Resent-From: <john.harper@vuw.ac.nz>
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874
>
> --- Comment #17 from anlauf at gcc dot gnu.org ---
> (In reply to Jerry DeLisle from comment #16)
>> (In reply to anlauf from comment #15)
>> --- snip ---
>>> Can you please verify?
>>
>> Yes, this fixes the test case.
>
> OK, thanks for confirming.
>
>> However if the orginal test case is valid
>> fortran we probably need to fix something else. Paul Thomas was noticing a
>> similar problem with his Finalization patches.  He was doing the
>> finalization inside trans_transfer or similar so it was blocking on a mutex
>> trying to finalize in the middle of an I/O operation.
>>
>> So in this case, my guess is the merge expression needs to be resolved
>> before the translation phase.
>
> If I interprete the tree-dump correctly, we have e.g.:
>
>          _gfortran_st_write (&dt_parm.2);
>          {
>            logical(kind=4) D.4279;
>            logical(kind=4) D.4280;
>            logical(kind=4) D.4281;
>            logical(kind=4) D.4282;
>
>            D.4279 = tstuff ();
>            D.4280 = fstuff ();
>            D.4281 = x[(integer(kind=8)) i + -1];
>            D.4282 = D.4281 ? D.4279 : D.4280;
>            _gfortran_transfer_logical_write (&dt_parm.2, &D.4282, 4);
>          }
>          _gfortran_st_write_done (&dt_parm.2);
>
> so we start the write, then evaluate the merge(), which is tstuff()/fstuff(),
> and which does its own I/O, and then return to continue with the transfer.
>
> So this might be non-conforming code, see
>
> F2018:12.12 Restrictions on input/output statements
>
> (2) An input/output statement that is executed while another input/output
> statement is being executed is a recursive input/output statement.
> A recursive input/output statement shall not identify an external unit that
> is identified by another input/output statement being executed except that
> a child data transfer statement may identify its parent data transfer
> statement external unit.
>
> I am not sure I fully understand the last sentence in this paragraph,
> but I think the pushed testcase might be invalid and should be replaced.
>
> If you agree, I'll simply do this.
>
> -- 
> You are receiving this mail because:
> You reported the bug.
>


-- John Harper, School of Mathematics and Statistics
Victoria Univ. of Wellington, PO Box 600, Wellington 6140, New Zealand.
e-mail john.harper@vuw.ac.nz phone +64(0) 4 463 5276

[Bug fortran/107874] merge not using all its arguments

[anlauf at gcc dot gnu.org]

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

--- Comment #19 from anlauf at gcc dot gnu.org ---
(In reply to john.harper from comment #18)
> An interesting problem! But I thought my original test case did not have 
> recursive I/O because tstuff and fstuff each print something in the 
> statement
>     y = merge(tstuff(),fstuff(),x(i))
> but y itself is printed only in the next statement,
>     print *,y

John, your original testcase in comment#0 was fine.
I tried to extend it to check for constant as well as non-constant mask,
and as you see I made a mistake by trying to make it smaller.  Bad idea.

> Or does evaluating merge allow each of tstuff and fstuff to be evaluated
> at the same time? I was thinking of tstuff and fstuff being evaluated
> in succession but could there be systems in which they are evaluated 
> simultaneously?

I don't recall having seen a mentioning in the standard of the order of
evaluation of different function (or subroutine) arguments.  Do you?

(Of course, if side-effects happen during that evaluation, such as I/O,
unexpected things may happen.)

> If so, whether the program is valid Fortran depends on the
> kind of system on which it is being executed.

Well, even if the print in tstuff/fstuff were a problem, one could construct
other testcases with side-effects that might be conforming.

[Bug fortran/107874] merge not using all its arguments

[anlauf at gcc dot gnu.org]

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

--- Comment #20 from anlauf at gcc dot gnu.org ---
(In reply to anlauf from comment #19)
> I don't recall having seen a mentioning in the standard of the order of
> evaluation of different function (or subroutine) arguments.  Do you?

The closest I can find is the following in F2018:

C.5.1  Evaluation of function references (10.1.7)

(1) If more than one function reference appears in a statement, they can be
executed in any order (subject to a function result being evaluated after the
evaluation of its arguments) and their values cannot depend on the order of
execution. This lack of dependence on order of evaluation enables parallel
execution of the function references.


And:

A.2 Processor dependencies

According to this document, the following are processor dependent:
...
• the order of finalization of components of objects of derived type (7.5.6.2)
• the order of finalization when several objects are finalized as the
  consequence of a single event (7.5.6.2);

[Bug fortran/107874] merge not using all its arguments

[john.harper at vuw dot ac.nz]

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

--- Comment #21 from john.harper at vuw dot ac.nz ---
I now have a new test case that avoids the possibility of recursive I/O
by tstuff and fstuff doing internal writes to two different character 
variables. It still reveals the merge problem. It compiles and runs with
gfortran-12 and ifort, giving different outputs.

! Must merge evaluate all 3 arguments?
program testmerge10
   implicit none
   character(7):: string(2) = ' '
   logical:: x(2) = [.true., .false.], y
   integer i
   do i = 1,2
      y = merge(tstuff(),fstuff(),x(i))
      print *,y,string
      string = ' '
   end do
contains

logical function tstuff()
     write(string(1),"(A)") ' tstuff'
     tstuff = .true.
   end function tstuff

   logical function fstuff()
     write(string(2),"(A)") ' fstuff'
     fstuff = .false.
   end function fstuff
end program testmerge10

Good luck with your bughunt!
John H

On Fri, 2 Dec 2022, anlauf at gcc dot gnu.org wrote:

> Date: Fri, 2 Dec 2022 21:05:43 +0000
> From: anlauf at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org>
> To: John Harper <john.harper@vuw.ac.nz>
> Subject: [Bug fortran/107874] merge not using all its arguments
> Resent-Date: Sat, 3 Dec 2022 10:05:53 +1300 (NZDT)
> Resent-From: <john.harper@vuw.ac.nz>
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874
>
> --- Comment #19 from anlauf at gcc dot gnu.org ---
> (In reply to john.harper from comment #18)
>> An interesting problem! But I thought my original test case did not have
>> recursive I/O because tstuff and fstuff each print something in the
>> statement
>>     y = merge(tstuff(),fstuff(),x(i))
>> but y itself is printed only in the next statement,
>>     print *,y
>
> John, your original testcase in comment#0 was fine.
> I tried to extend it to check for constant as well as non-constant mask,
> and as you see I made a mistake by trying to make it smaller.  Bad idea.
>
>> Or does evaluating merge allow each of tstuff and fstuff to be evaluated
>> at the same time? I was thinking of tstuff and fstuff being evaluated
>> in succession but could there be systems in which they are evaluated
>> simultaneously?
>
> I don't recall having seen a mentioning in the standard of the order of
> evaluation of different function (or subroutine) arguments.  Do you?
>
> (Of course, if side-effects happen during that evaluation, such as I/O,
> unexpected things may happen.)
>
>> If so, whether the program is valid Fortran depends on the
>> kind of system on which it is being executed.
>
> Well, even if the print in tstuff/fstuff were a problem, one could construct
> other testcases with side-effects that might be conforming.
>
> -- 
> You are receiving this mail because:
> You reported the bug.
>


-- John Harper, School of Mathematics and Statistics
Victoria Univ. of Wellington, PO Box 600, Wellington 6140, New Zealand.
e-mail john.harper@vuw.ac.nz phone +64(0) 4 463 5276

[Bug fortran/107874] merge not using all its arguments

[cvs-commit at gcc dot gnu.org]

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

--- Comment #22 from CVS 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:36a4ee406b95ae24a59b8b3f8ebe29af6fd5261e

commit r13-4473-g36a4ee406b95ae24a59b8b3f8ebe29af6fd5261e
Author: Harald Anlauf <anlauf@gmx.de>
Date:   Fri Dec 2 22:30:16 2022 +0100

    Fortran: intrinsic MERGE shall use all its arguments [PR107874]

    gcc/testsuite/ChangeLog:

            PR fortran/107874
            * gfortran.dg/merge_1.f90: Avoid recursive I/O.

[Bug fortran/107874] merge not using all its arguments

[jvdelisle at gcc dot gnu.org]

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

--- Comment #23 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
John,

Your original case in Comment 1 now gives:

$ gfc original.f90 
$ ./a.out 
 tstuff
 fstuff
 T
 tstuff
 fstuff
 F

So I think it is OK,  Harald's test case has function calls embedded in the
print statements within the main program and those functions have embedded
print statements themselves to the same output device which is stdout and this
is not allowed per the Fortran standard.

So the issue is only in the test case he previously committed and it is now
fixed.

[Bug fortran/107874] merge not using all its arguments

[anlauf at gcc dot gnu.org]

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

anlauf at gcc dot gnu.org changed:

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

--- Comment #24 from anlauf at gcc dot gnu.org ---
I didn't see any testsuite failures left.  Closing again.  (Fingers crossed.)