[Bug fortran/103039] New: Segfault with openmp block within a select type block

[Bug fortran/103039] New: Segfault with openmp block within a select type block

[mscfd at gmx dot net]

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

            Bug ID: 103039
           Summary: Segfault with openmp block within a select type block
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: mscfd at gmx dot net
  Target Milestone: ---

Created attachment 51720
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51720&action=edit
seltype.f90

The attached code compiles and executes fine with gfortran-11.1, but it first
shows warnings during compilation and then fails with a segfault with the
current master branch.
Compilation is done with: gfortran-12 -fopenmp -Wall seltype.f90
Output should be just the number 110.

In our code I have also seen lots of similar warnings with openmp blocks within
associate(.. => ...) blocks and related segfaults, but I could not come up with
a simple testcase.

[Bug fortran/103039] Segfault with openmp block within a select type block

[mscfd at gmx dot net]

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

--- Comment #1 from martin <mscfd at gmx dot net> ---
The warning which is shown during compilation is:
seltype.f90:16:59:

   16 | !$omp parallel do default(shared) private(k) reduction(+:s)
      |                                                           ^
Warning: ‘__tmp_class_a’ is used uninitialized [-Wuninitialized]
seltype.f90:15:12:

   15 | class is (a)
      |            ^
note: ‘__tmp_class_a’ declared here

[Bug fortran/103039] Segfault with openmp block within a select type block

[marxin at gcc dot gnu.org]

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

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r12-1016-g0e3b3b77e13cac76.

[Bug fortran/103039] Segfault with openmp block within a select type block since r12-1016-g0e3b3b77e13cac76

[mscfd at gmx dot net]

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

--- Comment #3 from martin <mscfd at gmx dot net> ---
Created attachment 51722
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51722&action=edit
seemingly same problem but with associate instead of select type

A similar problem with associate shows the same problem and probably has the
seem root cause. It is slightly more complicated than the variant with select
type, as it requires a class(..) component provided in the target of the
associate statement.
Otherwise warnings and subsequent segfault are similar.

[Bug fortran/103039] Segfault with openmp block within a select type block since r12-1016-g0e3b3b77e13cac76

[mscfd at gmx dot net]

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

--- Comment #4 from martin <mscfd at gmx dot net> ---
Is there any chance of fixing this regression (in particular the associate
block variant) till gfortran-12 release? Having an openmp block within an
associate block should not be that uncommon in modern code, right?

[Bug fortran/103039] [12 Regression] Segfault with openmp block within a select type block since r12-1016-g0e3b3b77e13cac76

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Segfault with openmp block  |[12 Regression] Segfault
                   |within a select type block  |with openmp block within a
                   |since                       |select type block since
                   |r12-1016-g0e3b3b77e13cac76  |r12-1016-g0e3b3b77e13cac76
   Target Milestone|---                         |12.0

[Bug fortran/103039] [12 Regression] Segfault with openmp block within a select type block since r12-1016-g0e3b3b77e13cac76

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-03-09
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |wrong-code
           Priority|P3                          |P1

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
in .ompexp I see

void MAIN__._omp_fn.0 (struct .omp_data_s.9 & restrict .omp_data_i)
{
...
  <bb 6> :
  s = 0;
  D.4352 = .omp_data_i->__tmp_class_a;
  __tmp_class_a.10 = __tmp_class_a;
  D.4355 = D.4352->_vptr;
  __tmp_class_a.10->_vptr = D.4355;
  D.4356 = .omp_data_i->__tmp_class_a;
  D.4357 = D.4356->_vptr;
  D.4358 = D.4357->_size;
  D.4359 = (unsigned long) D.4358;
  __tmp_class_a.11 = __tmp_class_a;
  D.4361 = __builtin_malloc (D.4359);
  __tmp_class_a.11->_data = D.4361;
  D.4362 = .omp_data_i->__tmp_class_a;
  D.4363 = D.4362->_vptr;
  D.4364 = D.4363->_copy;
  __tmp_class_a.12 = __tmp_class_a;
  D.4366 = .omp_data_i->__tmp_class_a;
  D.4367 = D.4366->_data;
  D.4364 (D.4367, __tmp_class_a.12);

so .omp_data_i->__tmp_class_a is accessed but then the local __tmp_class_a
is used in the end.  So sth is amiss here.

I suspect the unshare_expr is playing havoc with some later lookups.

[Bug fortran/103039] [12 Regression] Segfault with openmp block within a select type block since r12-1016-g0e3b3b77e13cac76

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
unshare_expr doesn't do anything wrong.
The problem is that because of the select we have firstprivate(__tmp_class_a)
clause where __tmp_class_a has type which has struct __class_seltype_A_p *
type.  Before the r12-1016 change, we clearly firstprivatized that var
because gfc_omp_clause_copy_ctor just did:
return build2_v (MODIFY_EXPR, dest, src);
for that case, so emitted:
                    D.4011 = .omp_data_i->__tmp_class_a;
                    __tmp_class_a = D.4011;
which essentially means that the __tmp_class_a var (the pointer) was
privatized, but what it points to was shared.
Now, since r12-1016 it copies various elements the pointer points to
from what the source pointer points to to what the destination pointer points
to.
But we don't really initialize what dest points to first.
I bet we want to privatize what the pointer points to, so we need to emit
something like:
  __class_seltype_A_p temp;
  __tmp_class_a = &temp;
  __tmp_class_a->vptr = .omp_data_i->__tmp_class_a->vptr;
etc. (not gimplified form).
I think the way to achieve this would be make sure to return true from the
gfc_omp_privatize_by_reference hook.

Tobias, can you please have a look?

[Bug fortran/103039] [12 Regression] Segfault with openmp block within a select type block since r12-1016-g0e3b3b77e13cac76

[burnus at gcc dot gnu.org]

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

--- Comment #7 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #6)
> I think the way to achieve this would be make sure to return true from the
> gfc_omp_privatize_by_reference hook.

What puzzles me is actually the reverse:

  class(a), pointer :: x
  ...
  select type (x)
  class is (a)
  !$omp parallel do default(shared) private(k) reduction(+:s)

Looking at the dump, one sees in the gimple dump:

  #pragma omp parallel private(k) reduction(+:s) default(shared)
              firstprivate(__tmp_class_a)

I think it is okay that __tmp_class_a is firstprivate – but in any case, no
deep copy should then be done in gfc_omp_clause_copy_ctor.

 * * *

Regarding why the firstprivate shows up:
gfc_omp_predetermined_sharing returns OMP_CLAUSE_DEFAULT_FIRSTPRIVATE because
of:

  /* Associate names preserve the association established during ASSOCIATE.
     As they are implemented either as pointers to the selector or array
     descriptor and shouldn't really change in the ASSOCIATE region,
     this decl can be either shared or firstprivate.  If it is a pointer,
     use firstprivate, as it is cheaper that way, otherwise make it shared.  */
  if (GFC_DECL_ASSOCIATE_VAR_P (decl))
    {
      if (TREE_CODE (TREE_TYPE (decl)) == POINTER_TYPE)
        return OMP_CLAUSE_DEFAULT_FIRSTPRIVATE;
      else
        return OMP_CLAUSE_DEFAULT_SHARED;
    }

Added in commit r5-1190-g92d28cbb59cc5a611af41342c5b224fbf779a44d

 * * *

BTW, using:   firstprivate(x)   fails with
  Error: ASSOCIATE name ‘__tmp_class_a’ in FIRSTPRIVATE clause at (1)

While

  select type (y => x)
  class is (a)
  !$omp parallel do default(shared) private(k) reduction(+:s) firstprivate(x)

is accepted but while the original dump has 'firstprivate(x)', it is removed
(as unused) during gimplify, resulting the output above.

Note: Associate names can appear for 'associate', 'select type', 'select rank'
and 'change team'. — For 'select type/rank', the selector name is the same as
the selector (wihch must be a variable) when left out.

 * * *

OpenMP has for "predetermined data-sharing attributes":
"An associate name that may appear in a variable definition context is shared
if its association occurs outside of the construct and otherwise it has the
same data-sharing attribute as the selector with which it is associated."

[Bug fortran/103039] [12 Regression] Segfault with openmp block within a select type block since r12-1016-g0e3b3b77e13cac76

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So it means the firstprivate clause is added implicitly during gimplification
(with OMP_CLAUSE_FIRSTPRIVATE_IMPLICIT?) for it.
If we want to firstprivatize just the pointer and not what it points to,
perhaps we should arrange for OMP_CLAUSE_FIRSTPRIVATE_NO_REFERENCE to be set on
it and then take it into account in gfc_omp_clause_copy_ctor etc. and just do
what we used to do before (i.e. copy the pointer and nothing else).

[Bug fortran/103039] [12 Regression] Segfault with openmp block within a select type block since r12-1016-g0e3b3b77e13cac76

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 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:6393122d271a92d5d9d8656a57ea167e92498871

commit r12-7701-g6393122d271a92d5d9d8656a57ea167e92498871
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Fri Mar 18 14:50:36 2022 +0100

    Fortran/OpenMP: Improve associate-name diagnostic [PR103039]

    gcc/fortran/ChangeLog:

            PR fortran/103039
            * openmp.cc (resolve_omp_clauses): Improve associate-name
diagnostic
            for select type/rank.

    gcc/testsuite/ChangeLog:

            PR fortran/103039
            * gfortran.dg/gomp/associate1.f90: Update dg-error.
            * gfortran.dg/gomp/associate2.f90: New test.

[Bug fortran/103039] [12 Regression] Segfault with openmp block within a select type block since r12-1016-g0e3b3b77e13cac76

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 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:c133bdfa9e7d9225510d00dbb7270cc052e4e4ee

commit r12-7709-gc133bdfa9e7d9225510d00dbb7270cc052e4e4ee
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Fri Mar 18 17:40:22 2022 +0100

    Fortran/OpenMP: Fix privatization of associated names

    gfc_omp_predetermined_sharing cases the associate-name pointer variable
    to be OMP_CLAUSE_DEFAULT_FIRSTPRIVATE, which is fine. However, the
associated
    selector is shared. Thus, the target of associate-name pointer should not
get
    copied. (It was before but because of gfc_omp_privatize_by_reference
returning
    false, the selector was not only wrongly copied but this was also not done
    properly.)

    gcc/fortran/ChangeLog:

            PR fortran/103039
            * trans-openmp.cc (gfc_omp_clause_copy_ctor, gfc_omp_clause_dtor):
            Only privatize pointer for associate names.

    libgomp/ChangeLog:

            PR fortran/103039
            * testsuite/libgomp.fortran/associate4.f90: New test.

[Bug fortran/103039] [12 Regression] Segfault with openmp block within a select type block since r12-1016-g0e3b3b77e13cac76

[burnus at gcc dot gnu.org]

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

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

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

--- Comment #11 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Should be FIXED (on mainline, which will become 12.1)

Thanks for the report, Martin – sorry for fixing it not more quickly.
Thanks Richard & Jakub for the debugging!

[Bug fortran/103039] [12 Regression] Segfault with openmp block within a select type block since r12-1016-g0e3b3b77e13cac76

[mscfd at gmx dot net]

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

--- Comment #12 from martin <mscfd at gmx dot net> ---
Thanks for fixing! I just checked with the real code and there it also looks
good.