[Bug fortran/94958] New: gcc/fortran/trans-array.c:9797: possible typo ?

[Bug fortran/94958] New: gcc/fortran/trans-array.c:9797: possible typo ?

[dcb314 at hotmail dot com]

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

            Bug ID: 94958
           Summary: gcc/fortran/trans-array.c:9797: possible typo ?
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dcb314 at hotmail dot com
  Target Milestone: ---

gcc/fortran/trans-array.c:9797:15: style: Variable 'args.errmsg' is reassigned
a value before the old one has been used. [redundantAssignment]

Source code is

  args.errmsg = errmsg;
  args.errmsg = errmsg_len;

Maybe better code

  args.errmsg = errmsg;
  args.errmsg_len = errmsg_len;

[Bug fortran/94958] gcc/fortran/trans-array.c:9797: possible typo ?

[dcb314 at hotmail dot com]

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

David Binderman <dcb314 at hotmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fanfarillo.gcc at gmail dot com
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=89863

--- Comment #1 from David Binderman <dcb314 at hotmail dot com> ---
Adding original author from git blame and the static analyser bug report.

[Bug fortran/94958] gcc/fortran/trans-array.c:9797: possible typo ?

[dominiq at lps dot ens.fr]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2020-07-20
           Priority|P3                          |P4
     Ever confirmed|0                           |1

[Bug fortran/94958] gcc/fortran/trans-array.c:9797: possible typo ?

[dominiq at lps dot ens.fr]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |damian at sourceryinstitute dot or
                   |                            |g

--- Comment #2 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
I have tested

diff -up ../_clean/gcc/fortran/trans-array.c gcc/fortran/trans-array.c
--- ../_clean/gcc/fortran/trans-array.c 2020-06-13 03:11:46.000000000 +0200
+++ gcc/fortran/trans-array.c   2020-07-25 12:16:09.000000000 +0200
@@ -9724,7 +9724,8 @@ gfc_bcast_alloc_comp (gfc_symbol *derive
   args.image_index = image_index;
   args.stat = stat;
   args.errmsg = errmsg;
-  args.errmsg = errmsg_len;
+  args.errmsg_len = errmsg_len;
+  gcc_unreachable ();

   if (rank == 0)
     {

without regression, so the use of gfc_bcast_alloc_comp is not covered by the
testsuite.

However building OpenCoarrays-2.9.0 failed with

/opt/OpenCoarrays-2.9.0/src/tests/unit/collectives/co_broadcast_allocatable_components.f90:42:0:

   42 |    call co_broadcast(alloc_message,source_image=sender)
      | 
internal compiler error: in gfc_bcast_alloc_comp, at fortran/trans-array.c:9728

After removing the gcc_unreachable, I have been able to build and test
successfully OpenCoarrays-2.9.0 (100% tests passed, 0 tests failed out of 79).

I don't know if all the tests are OK without the patch. If so there may be a
coverage problem.

I don't know either if Alessandro Fanfarillo is still around, CCING Damian
Rouson.

[Bug fortran/94958] gcc/fortran/trans-array.c:9797: possible typo ?

[vehre at gcc dot gnu.org]

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

vehre at gcc dot gnu.org changed:

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

--- Comment #3 from vehre at gcc dot gnu.org ---
That should clearly be:

  args.errmsg = errmsg;
  args.errmsg_len = errmsg_len;

I have to see, how this can be tested best in the gfortran testsuite, if
possible at all.

[Bug fortran/94958] gcc/fortran/trans-array.c:9797: possible typo ?

[vehre at gcc dot gnu.org]

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

vehre at gcc dot gnu.org changed:

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

--- Comment #4 from vehre at gcc dot gnu.org ---
Further research showed, that the conseQuences of the mistake are not impacting
gfortran in any kind, because the errmsg and errmsg_len are never used. The
code here is jumped to only when a derived type containing an allocatable
component is broadcasted and then only in caf-lib-mode, e.g.:

! { dg-do compile }
!
! PR fortran/94958
!

implicit none

type struct
  integer, allocatable :: comp
end type

type(struct) :: a
character(kind=1, len=20) :: err
integer :: stat


call co_broadcast(a, this_image(), stat, err)
end

gfortran -fcoarray=lib bcast_1.f90 -lcaf_single

During traversal of the derived type for bcast, the function call to
CO_BROADCAST() for the type 'a' and it's allocatable component 'comp' is never
augmented with the nodes for stat, errmsg and errmsg_len. For this small
example the code already has two calls to co_broadcast. Which indicates the
necessity of having some way to aggregate the stat and errmsg over all these
calls. Unfortunately does the Fortran-standard (2015, 2018) not state how to
aggregate the stat and/or errmsg for multiple broadcasts.

I therefore propose a fix:
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 7a1b2fc74c9..73a45cd2dcf 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -9732,7 +9732,7 @@ gfc_bcast_alloc_comp (gfc_symbol *derived, gfc_expr
*expr, int rank,
   args.image_index = image_index;
   args.stat = stat;
   args.errmsg = errmsg;
-  args.errmsg = errmsg_len;
+  args.errmsg_len = errmsg_len;

   if (rank == 0)
     {

as obvious and get done with this pr.

[Bug fortran/94958] gcc/fortran/trans-array.c:9797: possible typo ?

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andre Vehreschild <vehre@gcc.gnu.org>:

https://gcc.gnu.org/g:05814dde7024a8fa05a735cafcda72b5eb5ec0c0

commit r11-2783-g05814dde7024a8fa05a735cafcda72b5eb5ec0c0
Author: Andre Vehreschild <vehre@gcc.gnu.org>
Date:   Thu Aug 20 17:50:16 2020 +0200

    Fix obvious typo were errmsg_len was assigned to errmsg.

    gcc/fortran/ChangeLog:

    2020-08-20  Andre Vehreschild  <vehre@gcc.gnu.org>

            PR fortran/94958
            * trans-array.c (gfc_bcast_alloc_comp): Use the correct variable.

[Bug fortran/94958] gcc/fortran/trans-array.c:9797: possible typo ?

[vehre at gcc dot gnu.org]

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

Andre Vehreschild <vehre at gcc dot gnu.org> changed:

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

--- Comment #6 from Andre Vehreschild <vehre at gcc dot gnu.org> ---
Waiting a week for regression before closing as fixed.

[Bug fortran/94958] gcc/fortran/trans-array.c:9797: possible typo ?

[vehre at gcc dot gnu.org]

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

Andre Vehreschild <vehre at gcc dot gnu.org> changed:

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