[Bug rtl-optimization/108132] New: Wrong instruction scheduling around function call with longjmp on aarch64 at O2

[Bug rtl-optimization/108132] New: Wrong instruction scheduling around function call with longjmp on aarch64 at O2

[qinzhao at gcc dot gnu.org]

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

            Bug ID: 108132
           Summary: Wrong instruction scheduling around function call with
                    longjmp on aarch64 at O2
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: qinzhao at gcc dot gnu.org
  Target Milestone: ---

The error only happens on aarch64 (fine on X86). 

The following testing case:
[opc@qinzhao-aarch64-ol8]$ cat t.c
#include <stdio.h> 
#include <setjmp.h> 
#include <stdbool.h> 

jmp_buf ex_buf__; 

int f(int x) 
{ 
  int arr[] = {1,2,6,8,9,10}; 
  int lo=0; 
  int hi=5; 

  while(lo<=hi) { 
        int mid=(lo+hi)/2; 

        if(arr[mid]==x) { 
          longjmp(ex_buf__, 1); 
        } else if(arr[mid]<x) { 
          lo=mid+1; 
        } else if(arr[mid]>x) { 
          hi=mid-1; 
        } 
  } 

  return -1; 
} 

int 
main(int argc, char** argv) 
{ 
  int a=2; 
  bool b=false; 

  do {
    if( !setjmp(ex_buf__) ){
        a=f(a); 
        b=true; 
    } else { 
      printf("a : %d\n",a); 
      printf("Got Exception!\n"); 
    } 
  } while(0);

  if(b) { 
        printf("b is true!\n"); 
  } 
  return 0; 
}

when compiled with the latest upstream gcc13 on aarch64 with -O2, the variable
"b" was set to true, which is wrong. 
when disable insn scheduling by adding -fno-schedule-insns, the issue gone:
[opc@qinzhao-aarch64-ol8]$ /home/opc/Install/latest/bin/gcc -v
Using built-in specs.
COLLECT_GCC=/home/opc/Install/latest/bin/gcc
COLLECT_LTO_WRAPPER=/home/opc/Install/latest/libexec/gcc/aarch64-unknown-linux-gnu/13.0.0/lto-wrapper
Target: aarch64-unknown-linux-gnu
Configured with: ../latest_gcc/configure --prefix=/home/opc/Install/latest
--enable-languages=c,c++ --disable-bootstrap
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 13.0.0 20221215 (experimental) (GCC) 
[opc@qinzhao-aarch64-ol8]$ /home/opc/Install/latest/bin/gcc -O2 t.c
[opc@qinzhao-aarch64-ol8]$ ./a.out
a : 2
Got Exception!
b is true!
[opc@qinzhao-aarch64-ol8]$ /home/opc/Install/latest/bin/gcc -O2
-fno-schedule-insns t.c
[opc@qinzhao-aarch64-ol8]$ ./a.out
a : 2
Got Exception!
[opc@qinzhao-aarch64-ol8]$

[Bug rtl-optimization/108132] Wrong instruction scheduling around function call with longjmp on aarch64 at O2

[qinzhao at gcc dot gnu.org]

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

--- Comment #1 from qinzhao at gcc dot gnu.org ---
As we checked the assembly and the IR, the wrong transformation is something
like the following at source code level: (inside function "main")

from : 
        a=f(a); 
        b=true; 
to: 
        b=true; 
        a=f(a) 

since there is a "longjmp" inside the function "f", I think that it's not 
correct  to move b=true before the function call. 

I think that the function "f" which has a "longjmp" should be treated as a
memory barrier, any insn movement should not across it. 

This problem exists back to gcc8.

[Bug rtl-optimization/108132] Wrong instruction scheduling around function call with longjmp on aarch64 at O2

[qinzhao at gcc dot gnu.org]

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

qinzhao at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

--- Comment #2 from qinzhao at gcc dot gnu.org ---
I am changing the priority of this one to P2 since we have a lot of instances
in our important application on aarch64.

[Bug rtl-optimization/108132] Wrong instruction scheduling around function call with longjmp on aarch64 at O2

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |DUPLICATE

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Dup of bug 57067.

*** This bug has been marked as a duplicate of bug 57067 ***

[Bug rtl-optimization/108132] Wrong instruction scheduling around function call with longjmp on aarch64 at O2

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
From the duplicated bug: "In this case the RTL scheduler pass generates broken
code due to the missing control flow info."

[Bug rtl-optimization/108132] Wrong instruction scheduling around function call with longjmp on aarch64 at O2

[qinzhao at gcc dot gnu.org]

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

qinzhao at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-12-15
     Ever confirmed|0                           |1
         Resolution|DUPLICATE                   |---
             Status|RESOLVED                    |REOPENED

--- Comment #5 from qinzhao at gcc dot gnu.org ---
To me, this is an important bug that GCC should fix. making it as a duplication
of an Old bug and closing it seems not a good idea. 

I prefer to reopen this one as a higher priority bug in order to push a good
fix for it.

[Bug rtl-optimization/108132] Wrong instruction scheduling around function call with longjmp on aarch64 at O2

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to qinzhao from comment #5)
> To me, this is an important bug that GCC should fix. making it as a
> duplication of an Old bug and closing it seems not a good idea. 

Well it is an old bug inside GCC. I am not saying it is not an important bug to
fix but nobody has fixed it as setjmp/longjmp is less wildly used these days
and most programmers who use it usually just add volatile to fix issues with
their code and work around bugs in the compiler.

There is a patch in PR 57067 even which you could try to port to a newer
compiler version and fix up.

*** This bug has been marked as a duplicate of bug 57067 ***

[Bug rtl-optimization/108132] Wrong instruction scheduling around function call with longjmp on aarch64 at O2

[qing.zhao at oracle dot com]

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

--- Comment #7 from Qing Zhao <qing.zhao at oracle dot com> ---
> On Dec 15, 2022, at 2:33 PM, pinskia at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org> wrote:
> 
> 
> There is a patch in PR 57067 even which you could try to port to a newer
> compiler version and fix up.

Okay, will check that patch.
thanks.
>

[Bug rtl-optimization/108132] Wrong instruction scheduling around function call with longjmp on aarch64 at O2

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Alexander Monakov <amonakov@gcc.gnu.org>:

https://gcc.gnu.org/g:733a1b777f16cd397b43a242d9c31761f66d3da8

commit r13-5154-g733a1b777f16cd397b43a242d9c31761f66d3da8
Author: Alexander Monakov <amonakov@ispras.ru>
Date:   Fri Jan 13 21:04:02 2023 +0300

    sched-deps: do not schedule pseudos across calls [PR108117]

    Scheduling across calls in the pre-RA scheduler is problematic: we do
    not take liveness info into account, and are thus prone to extending
    lifetime of a pseudo over the loop, requiring a callee-saved hardreg
    or causing a spill.

    If current function called a setjmp, lifting an assignment over a call
    may be incorrect if a longjmp would happen before the assignment.

    Thanks to Jose Marchesi for testing on AArch64.

    gcc/ChangeLog:

            PR rtl-optimization/108117
            PR rtl-optimization/108132
            * sched-deps.cc (deps_analyze_insn): Do not schedule across
            calls before reload.

    gcc/testsuite/ChangeLog:

            PR rtl-optimization/108117
            PR rtl-optimization/108132
            * gcc.dg/pr108117.c: New test.