[Bug libgomp/105745] New: Conditional OpenMP directive fails with GCC 12

[Bug libgomp/105745] New: Conditional OpenMP directive fails with GCC 12

[reiter.christoph at gmail dot com]

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

            Bug ID: 105745
           Summary: Conditional OpenMP directive fails with GCC 12
           Product: gcc
           Version: 12.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libgomp
          Assignee: unassigned at gcc dot gnu.org
          Reporter: reiter.christoph at gmail dot com
                CC: jakub at gcc dot gnu.org
  Target Milestone: ---

Downstream issue: https://github.com/msys2/MINGW-packages/issues/11729

The following example crashes with GCC 12.1.0, but works fine with GCC 11.3.0:

// g++ -fopenmp -o broken.exe broken.c; ./broken.exe; echo $?
int main() {
    #pragma omp parallel for if (0)
    for (int idx = 0; idx < 0; ++idx) { }
    return 0;
}

* Removing the "if (0)" condition makes it work again
* Replacing libgomp-1.dll with the one from GCC 11.3 also makes things work
again.
* Another example that crashes is "#pragma omp for schedule(dynamic)"

This is on 32/64 bit Windows.

[Bug target/105745] Conditional OpenMP directive fails with GCC 12

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Can you run this under the debugger to see where the crash is?
Because there have been almost no changes to the libgomp sources that would
effect this.

[Bug target/105745] [12/13 Regression] Conditional OpenMP directive fails with GCC 12

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.2
      Known to work|                            |11.3.0
            Summary|Conditional OpenMP          |[12/13 Regression]
                   |directive fails with GCC 12 |Conditional OpenMP
                   |                            |directive fails with GCC 12

[Bug target/105745] [12/13 Regression] Conditional OpenMP directive fails with GCC 12

[reiter.christoph at gmail dot com]

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

--- Comment #2 from Christoph Reiter <reiter.christoph at gmail dot com> ---
For "#pragma omp parallel for if (0)":

#0  0x00007ffacdeac6b3 in ntdll!RtlIsZeroMemory ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
#1  0x00007ffacdeb5512 in ntdll!.misaligned_access ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
#2  0x00007ffacdeb57fa in ntdll!.misaligned_access ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
#3  0x00007ffacdebfce5 in ntdll!.misaligned_access ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
#4  0x00007ffacddc8715 in ntdll!RtlGetCurrentServiceSessionId ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
#5  0x00007ffacddc79e1 in ntdll!RtlFreeHeap ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
#6  0x00007ffacb4e218b in ucrtbase!_free_base ()
   from C:\WINDOWS\System32\ucrtbase.dll
#7  0x00007ffa7fd3eab6 in free_team (team=team@entry=0x25d609d0fc0)
    at ../../../gcc-12.1.0/libgomp/team.c:237
#8  0x00007ffa7fd3ff8a in gomp_team_end ()
    at ../../../gcc-12.1.0/libgomp/team.c:1005
#9  0x00007ffa7fd3899e in GOMP_parallel_end ()
    at ../../../gcc-12.1.0/libgomp/parallel.c:167
#10 0x00007ffa7fd38a73 in GOMP_parallel (
    fn=fn@entry=0x7ff707941530 <main._omp_fn.0(void)>, data=data@entry=0x0,
    num_threads=num_threads@entry=1, flags=flags@entry=0)
    at ../../../gcc-12.1.0/libgomp/parallel.c:179
#11 0x00007ff707941584 in main () at myTest.cpp:2

For "#pragma omp for schedule(dynamic)"

#0  0x00007ffacdeac6b3 in ntdll!RtlIsZeroMemory ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
#1  0x00007ffacdeb5512 in ntdll!.misaligned_access ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
#2  0x00007ffacdeb57fa in ntdll!.misaligned_access ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
#3  0x00007ffacdebfce5 in ntdll!.misaligned_access ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
#4  0x00007ffacddc8715 in ntdll!RtlGetCurrentServiceSessionId ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
#5  0x00007ffacddc79e1 in ntdll!RtlFreeHeap ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
#6  0x00007ffacb4e218b in ucrtbase!_free_base ()
   from C:\WINDOWS\System32\ucrtbase.dll
#7  0x00007ffa7eae050e in free_work_share (team=team@entry=0x0,
    ws=0x2b6040a8480) at ../../../gcc-12.1.0/libgomp/work.c:159
#8  0x00007ffa7eae0682 in gomp_work_share_end ()
    at ../../../gcc-12.1.0/libgomp/work.c:238
#9  0x00007ffa7ead68b4 in GOMP_loop_end ()
    at ../../../gcc-12.1.0/libgomp/loop.c:958
#10 0x00007ff6b18f1593 in main () at myTest.cpp:3

[Bug target/105745] [12/13 Regression] Conditional OpenMP directive fails with GCC 12

[reiter.christoph at gmail dot com]

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

--- Comment #3 from Christoph Reiter <reiter.christoph at gmail dot com> ---
Wild guess: I see _aligned_malloc() being used in libgomp, but not
_aligned_free().

[Bug target/105745] [12/13 Regression] Conditional OpenMP directive fails with GCC 12

[reiter.christoph at gmail dot com]

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

--- Comment #4 from Christoph Reiter <reiter.christoph at gmail dot com> ---
Yes, that's it I think. libgomp is mixing aligned malloc with unaligned free in
various places.

[Bug target/105745] [12/13 Regression] Conditional OpenMP directive fails with GCC 12

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Ugh, that is a lame limitation.
Does mingw support any of the other aligned allocators (memalign,
posix_memalign or aligned_alloc) that can be freed just with free?
If not, I guess easiest fix would be to move the HAVE__ALIGNED_MALLOC support
as last option before fallback instead of first, make sure
GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC is not defined for HAVE__ALIGNED_MALLOC if no
other aligned allocator works and use _aligned_free in gomp_aligned_free if
HAVE__ALIGNED_MALLOC and not GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC.

[Bug target/105745] [12/13 Regression] Conditional OpenMP directive fails with GCC 12

[reiter.christoph at gmail dot com]

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

--- Comment #6 from Christoph Reiter <reiter.christoph at gmail dot com> ---
Created attachment 53042
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53042&action=edit
libgomp: use _aligned_free in gomp_aligned_free() if  needed

[Bug target/105745] [12/13 Regression] Conditional OpenMP directive fails with GCC 12

[reiter.christoph at gmail dot com]

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

--- Comment #7 from Christoph Reiter <reiter.christoph at gmail dot com> ---
Created attachment 53043
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53043&action=edit
libgomp: don't use GOMP_USE_ALIGNED_WORK_SHARES on Windows

[Bug target/105745] [12/13 Regression] Conditional OpenMP directive fails with GCC 12

[reiter.christoph at gmail dot com]

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

--- Comment #8 from Christoph Reiter <reiter.christoph at gmail dot com> ---
(In reply to Jakub Jelinek from comment #5)
> Ugh, that is a lame limitation.
> Does mingw support any of the other aligned allocators (memalign,
> posix_memalign or aligned_alloc) that can be freed just with free?

I don't think so. There is one internal one in
https://github.com/mingw-w64/mingw-w64/blob/master/mingw-w64-crt/misc/mingw-aligned-malloc.c
but that has the same limitation afaics.

> If not, I guess easiest fix would be to move the HAVE__ALIGNED_MALLOC
> support as last option before fallback instead of first, make sure
> GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC is not defined for HAVE__ALIGNED_MALLOC if
> no other aligned allocator works and use _aligned_free in gomp_aligned_free
> if
> HAVE__ALIGNED_MALLOC and not GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC.

I went with fixing gomp_aligned_free() and disabling
GOMP_USE_ALIGNED_WORK_SHARES on Windows instead (hoping that in other cases
gomp_aligned_free() is called properly.. (??)). With the goal in mind that this
could be fixed if the alloc/free calls are paired properly in the future.

Feedback welcome, or feel free to take over :)

[Bug libgomp/105745] [12/13 Regression] Conditional OpenMP directive fails with GCC 12

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-05-27
             Status|UNCONFIRMED                 |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 53045
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53045&action=edit
gcc13-pr105745.patch

GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC is documented as:
/* Defined if gomp_aligned_alloc doesn't use fallback version
   and free can be used instead of gomp_aligned_free.  */
so the _aligned_malloc/_aligned_free pair doesn't satisfy that.

So my preference is this patch instead.

[Bug libgomp/105745] [12/13 Regression] Conditional OpenMP directive fails with GCC 12

[reiter.christoph at gmail dot com]

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

--- Comment #10 from Christoph Reiter <reiter.christoph at gmail dot com> ---
lgtm

[Bug libgomp/105745] [12/13 Regression] Conditional OpenMP directive fails with GCC 12

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:42fd2cd932384288914174f4af7974a060972bff

commit r13-808-g42fd2cd932384288914174f4af7974a060972bff
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat May 28 08:30:47 2022 +0200

    libgomp: Don't define GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC for _aligned_malloc
[PR105745]

    since apparently _aligned_malloc requires freeing with _aligned_free and:
     /* Defined if gomp_aligned_alloc doesn't use fallback version
        and free can be used instead of gomp_aligned_free.  */
     #define GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC 1
    so the second condition isn't satisfied.  For uses inside of the OpenMP
    allocators we can still use _aligned_malloc but we need to call
_aligned_free
    in gomp_aligned_free.

    2022-05-28  Jakub Jelinek  <jakub@redhat.com>

            PR libgomp/105745
            * libgomp.h (GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC): Don't define for
            defined(HAVE__ALIGNED_MALLOC) case.
            * alloc.c (gomp_aligned_alloc): Move defined(HAVE__ALIGNED_MALLOC)
            handling as last option before fallback instead of first.
            (gomp_aligned_free): For defined(HAVE__ALIGNED_MALLOC) call
            _aligned_free.

[Bug libgomp/105745] [12/13 Regression] Conditional OpenMP directive fails with GCC 12

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:93e60642891abc85af7a2efb2b7095062e10719f

commit r12-8435-g93e60642891abc85af7a2efb2b7095062e10719f
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat May 28 08:30:47 2022 +0200

    libgomp: Don't define GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC for _aligned_malloc
[PR105745]

    since apparently _aligned_malloc requires freeing with _aligned_free and:
     /* Defined if gomp_aligned_alloc doesn't use fallback version
        and free can be used instead of gomp_aligned_free.  */
     #define GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC 1
    so the second condition isn't satisfied.  For uses inside of the OpenMP
    allocators we can still use _aligned_malloc but we need to call
_aligned_free
    in gomp_aligned_free.

    2022-05-28  Jakub Jelinek  <jakub@redhat.com>

            PR libgomp/105745
            * libgomp.h (GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC): Don't define for
            defined(HAVE__ALIGNED_MALLOC) case.
            * alloc.c (gomp_aligned_alloc): Move defined(HAVE__ALIGNED_MALLOC)
            handling as last option before fallback instead of first.
            (gomp_aligned_free): For defined(HAVE__ALIGNED_MALLOC) call
            _aligned_free.

    (cherry picked from commit 42fd2cd932384288914174f4af7974a060972bff)

[Bug libgomp/105745] [12/13 Regression] Conditional OpenMP directive fails with GCC 12

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Hopefully fixed.