[Bug c/54017] New: Incorrect implementation of infinite loops in OpenMP sections leads to SIGILL

[Bug c/54017] New: Incorrect implementation of infinite loops in OpenMP sections leads to SIGILL

[iliev at rz dot rwth-aachen.de]

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

             Bug #: 54017
           Summary: Incorrect implementation of infinite loops in OpenMP
                    sections leads to SIGILL
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: iliev@rz.rwth-aachen.de


Consider this very simple OpenMP sections code:

int main()
{
    #pragma omp parallel sections
    {
        #pragma omp section
        {
            for(;1;) {}
        }
    }
    return 0;
}

Since there is only one OpenMP section, if this code is run with more than one
thread, GOMP_sections_start() would return 0 in all but the first thread to
call it. Unfortunately the compiler generates the following assembly code to
test the return value:

        movl    $1, %edi
        call    GOMP_sections_start
        cmpl    $1, %eax
        je      .L4
                .value  0x0b0f
.L4:
        jmp     .L4

In all threads but the first one the JE instruction would not branch and the
UD2 instruction emitted by __builtin_trap() would get hit and the program will
abort with SIGILL. Tested also with "for(;;) {}" and "while(1) {}" - same
assembly code emitted.

If instead the code of the section is modified to read:

    #pragma omp section
    {
        int i = 1;
        for(;i;) {}
    }

the produced assembly code correctly handles 0 as the possible return value of 
GOMP_sections_next():

        call    GOMP_sections_next
.L7:
        testl   %eax, %eax
        je      .L4
        cmpl    $1, %eax
        je      .L5
                .value  0x0b0f
.L4:
        call    GOMP_sections_end_nowait
        jmp     .L8
.L5:
        movl    $1, -4(%rbp)
.L6:
        cmpl    $0, -4(%rbp)
        jne     .L6
        call    GOMP_sections_next
        jmp     .L7
.L8:
        leave

Also if at least one another section without infinite loop is added, e.g.:

    #pragma omp section
    {
        for(;1;) {}
    }
    #pragma omp section
    {
        int k = 5;
    }

return value from GOMP_sections_next() is processed correctly:

        call    GOMP_sections_next
.L8:
        cmpl    $1, %eax
        je      .L6
        cmpl    $1, %eax
        jb      .L5
        cmpl    $2, %eax
        je      .L7
                .value  0x0b0f
.L5:
        call    GOMP_sections_end_nowait
        jmp     .L9
.L7:
        movl    $5, -4(%rbp)
        call    GOMP_sections_next
        jmp     .L8
.L6:
        jmp     .L6
.L9:
        leave

The correct behaviour in the first case should be that other threads would wait
infinitely at the barrier in GOMP_parallel_end() and not crash with SIGILL as
in the first case.

Unfortunately I don't have access to GCC versions higher than 4.7.0 but I've
tested it with previous versions down to 4.3.4 with the same result.
Optimisation level has no influence on the problem. It also fails in 32-bit
mode.

[Bug middle-end/54017] Incorrect implementation of infinite loops in OpenMP sections leads to SIGILL

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2012-07-19
                 CC|                            |jakub at gcc dot gnu.org
          Component|c                           |middle-end
         AssignedTo|unassigned at gcc dot       |jakub at gcc dot gnu.org
                   |gnu.org                     |
   Target Milestone|---                         |4.7.2
     Ever Confirmed|0                           |1

[Bug middle-end/54017] Incorrect implementation of infinite loops in OpenMP sections leads to SIGILL

[jakub at gcc dot gnu.org]

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

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-07-19 11:29:27 UTC ---
Created attachment 27832
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27832
gcc48-pr54017.patch

Untested fix.  Though, of course, the testcase is very questionable when omp
section doesn't exit, then the region is not single entry single exit.

[Bug middle-end/54017] Incorrect implementation of infinite loops in OpenMP sections leads to SIGILL

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-07-19 14:02:37 UTC ---
Author: jakub
Date: Thu Jul 19 14:02:32 2012
New Revision: 189658

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=189658
Log:
    PR middle-end/54017
    * tree-cfgcleanup.c (cleanup_omp_return): Remove.
    (cleanup_tree_cfg_bb): Don't call it.
    * omp-low.c (expand_omp_sections): Fix up the !exit_reachable case
    handling.

    * c-c++-common/gomp/pr54017.c: New test.

Added:
    trunk/gcc/testsuite/c-c++-common/gomp/pr54017.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/omp-low.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-cfgcleanup.c

[Bug middle-end/54017] Incorrect implementation of infinite loops in OpenMP sections leads to SIGILL

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-07-19 14:05:59 UTC ---
Author: jakub
Date: Thu Jul 19 14:05:54 2012
New Revision: 189659

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=189659
Log:
    PR middle-end/54017
    * tree-cfgcleanup.c (cleanup_omp_return): Remove.
    (cleanup_tree_cfg_bb): Don't call it.
    * omp-low.c (expand_omp_sections): Fix up the !exit_reachable case
    handling.

    * c-c++-common/gomp/pr54017.c: New test.

Added:
    branches/gcc-4_7-branch/gcc/testsuite/c-c++-common/gomp/pr54017.c
Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/omp-low.c
    branches/gcc-4_7-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_7-branch/gcc/tree-cfgcleanup.c

[Bug middle-end/54017] Incorrect implementation of infinite loops in OpenMP sections leads to SIGILL

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.7.2                       |4.7.3

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-09-20 10:14:11 UTC ---
GCC 4.7.2 has been released.

[Bug middle-end/54017] Incorrect implementation of infinite loops in OpenMP sections leads to SIGILL

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.7.3                       |4.7.4

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> 2013-04-11 07:59:04 UTC ---
GCC 4.7.3 is being released, adjusting target milestone.

[Bug middle-end/54017] Incorrect implementation of infinite loops in OpenMP sections leads to SIGILL

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.7.4                       |---

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Unsetting target milestone of open non-regression bug from version of branch
being closed.