[Bug c/108691] New: ICE when compiling for AArch64 with BTI protection at -O1

[Bug c/108691] New: ICE when compiling for AArch64 with BTI protection at -O1

[david.spickett at linaro dot org]

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

            Bug ID: 108691
           Summary: ICE when compiling for AArch64 with BTI protection at
                    -O1
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: david.spickett at linaro dot org
  Target Milestone: ---

Created attachment 54416
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54416&action=edit
Output from -freport-bug

This code:
```
extern int __attribute__((returns_twice)) setjmp(void*);

void bbb(void) {
  int (*fnptr)(void*) = setjmp;
  fnptr(0);
}
```
Causes an ICE when compiled with gcc trunk.
```
during GIMPLE pass: cddce
<source>: In function 'bbb':
<source>:6:1: internal compiler error: in eliminate_unnecessary_stmts, at
tree-ssa-dce.cc:1512
    6 | }
      | ^
Please submit a full bug report, with preprocessed source (by using
-freport-bug).
See <https://gcc.gnu.org/bugs/> for instructions.
Compiler returned: 1
```

https://godbolt.org/z/6dEMEbEG8

Compiling at -O0, or calling setjmp directly avoids the crash. There is no
crash with gcc 12.2.

Also calling setjmp directly after using the function pointer avoids the crash.
One thing that is happening here is that gcc is converting the indirect call
into a direct one. Perhaps when that is the only call in the function, that's
the problematic situation.

For context, this is not from real code. It's a clang test case for bti
protection of returns twice calls. Either way, the compiler shouldn't crash and
if there's something wrong with the source it should say so.

[Bug c/108691] ICE when compiling for AArch64 with BTI protection at -O1

[david.spickett at linaro dot org]

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

--- Comment #1 from David Spickett <david.spickett at linaro dot org> ---
Output from freport-bug attached.

[Bug tree-optimization/108691] ICE with function ptr and setjmp/returns twice at -O1

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|ICE when compiling for      |ICE with function ptr and
                   |AArch64 with BTI protection |setjmp/returns twice at -O1
                   |at -O1                      |

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This is unrelated to BTI. It ICEs for x86_64 too.q

[Bug tree-optimization/108691] [13 Regression] ICE with function ptr and setjmp/returns twice at -O1

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0
            Summary|ICE with function ptr and   |[13 Regression] ICE with
                   |setjmp/returns twice at -O1 |function ptr and
                   |                            |setjmp/returns twice at -O1
           Keywords|                            |needs-bisection

[Bug tree-optimization/108691] [13 Regression] ICE with function ptr and setjmp/returns twice at -O1

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2023-02-07
             Status|UNCONFIRMED                 |NEW

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed.

[Bug tree-optimization/108691] [13 Regression] ICE with function ptr and setjmp/returns twice at -O1

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Most likely caused by the patch which inserted the assert
r13-2518-ga262f969d6fd93

[Bug tree-optimization/108691] [13 Regression] ICE with function ptr and setjmp/returns twice at -O1

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
I will have a look.

[Bug tree-optimization/108691] [13 Regression] ICE with function ptr and setjmp/returns twice at -O1

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
The reason is that gimple_call_flags returns a different answer from the
indirect function call vs. the direct function call after propagating of
the &setjmp constant.  First of all 'returns_twice' is an attribute that
cannot be set on a function type:

int __attribute__((returns_twice)) (*foo) (void);
> gcc-12 -S t.c -Wall
t.c:1:1: warning: 'returns_twice' attribute ignored [-Wattributes]
    1 | int __attribute__((returns_twice)) (*foo) (void);
      | ^~~

so one cannot have an indirect call annotated.  But even then we for
example treat indirect missing 'noreturn' conservatively, not requiring
it to end a BB and only enforce that in the fixup-CFG pass.

Now, for return_twice we could similarly check gimple_call_ctrl_altering.

That still leaves us with missing abnormal edges - David, was this reduced
from an actual program?

When fixing up DCE to not set cfun->calls_setjmp it will eventually be
set late in emit_call_1 via

  if (ecf_flags & ECF_RETURNS_TWICE)
    {
      add_reg_note (call_insn, REG_SETJMP, const0_rtx);
      cfun->calls_setjmp = 1;
    }

a "fix" here would be to _check_ for cfun->calls_setjmp instead of setting it
(or assert it is set).  The control-altering flag has no representation
on GENERIC, and we're still expanding calls via intermediate GENERIC ...

[Bug tree-optimization/108691] [13 Regression] ICE with function ptr and setjmp/returns twice at -O1

[david.spickett at linaro dot org]

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

--- Comment #7 from David Spickett <david.spickett at linaro dot org> ---
> That still leaves us with missing abnormal edges - David, was this reduced from an actual program?

No, it's a simplified version of the C used to generate an LLVM IR test case:
extern int setjmp(ptr);
extern void notsetjmp(void);

void bbb(void) {
  setjmp(0);
  int (*fnptr)(ptr) = setjmp;
  fnptr(0);
  notsetjmp();
}

The test is checking that llvm follows each call to a returns twice function
with a branch target identifier instruction. In case the function returns using
a jump instead of the normal ret. That's probably beside the point for this bug
though.

The "real code" that motivated the feature was not doing indirect calls. I just
tested that because there's no reason it shouldn't also work.

I don't know of a use case for code like this. Like you said, if the compiler
doesn't know what the indirect call target is, it won't know it's return twice.
If it does know what you're calling, you could probably just call it directly.

So I was just shuffling statements around to test clang and compare against gcc
and found the ICE that way.

[Bug tree-optimization/108691] [13 Regression] ICE with function ptr and setjmp/returns twice at -O1

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|needs-bisection             |

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
Thanks.  I'm thinking what to most sensibly do given that we do not support
annotating an indirect call as returns_twice (that's something easy to add
though, but won't help legacy code)

[Bug tree-optimization/108691] [13 Regression] ICE with function ptr and setjmp/returns twice at -O1

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:6436add49ed9ecf74f1f01b4fe18961df4f3e83d

commit r13-5965-g6436add49ed9ecf74f1f01b4fe18961df4f3e83d
Author: Richard Biener <rguenther@suse.de>
Date:   Mon Feb 13 10:41:51 2023 +0100

    tree-optimization/108691 - indirect calls to setjmp

    DCE now chokes on indirect setjmp calls becoming direct because
    that exposes them too late to be subject to abnormal edge creation.
    The following patch honors gimple_call_ctrl_altering for those and
    _not_ treat formerly indirect calls to setjmp as calls to setjmp
    in notice_special_calls.

    Unfortunately there's no way to have an indirect call to setjmp
    properly annotated (the returns_twice attribute is ignored on types).

    RTL expansion late discovers returns-twice for the purpose of
    adding REG_SETJMP notes and also sets ->calls_setjmp
    (instead of asserting it is set).  There's no good way to
    transfer proper knowledge around here so I'm using ->calls_setjmp
    as a flag to indicate whether gimple_call_ctrl_altering_p was set.

            PR tree-optimization/108691
            * tree-cfg.cc (notice_special_calls): When the CFG is built
            honor gimple_call_ctrl_altering_p.
            * cfgexpand.cc (expand_call_stmt): Clear cfun->calls_setjmp
            temporarily if the call is not control-altering.
            * calls.cc (emit_call_1): Do not add REG_SETJMP if
            cfun->calls_setjmp is not set.  Do not alter cfun->calls_setjmp.

            * gcc.dg/pr108691.c: New testcase.

[Bug tree-optimization/108691] [13 Regression] ICE with function ptr and setjmp/returns twice at -O1

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.

[Bug tree-optimization/108691] [13 Regression] ICE with function ptr and setjmp/returns twice at -O1

[david.spickett at linaro dot org]

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

--- Comment #11 from David Spickett <david.spickett at linaro dot org> ---
Thanks for the quick fix!

> That still leaves us with missing abnormal edges - David, was this reduced from an actual program?

Further to this, someone pointed out to me that an indirect call to setjmp is
undefined behaviour:
https://pubs.opengroup.org/onlinepubs/9699919799/functions/setjmp.html

So even if this was real code, it's undefined behaviour.

(it just so happens that llvm's IR is able to represent that situation)

[Bug tree-optimization/108691] [13 Regression] ICE with function ptr and setjmp/returns twice at -O1

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
The fix was reverted.

[Bug tree-optimization/108691] [13 Regression] ICE with function ptr and setjmp/returns twice at -O1

[sam at gentoo dot org]

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

--- Comment #13 from Sam James <sam at gentoo dot org> ---
(Revert was for pr108868).

[Bug tree-optimization/108691] [13 Regression] ICE with function ptr and setjmp/returns twice at -O1

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
and PR108855.

[Bug tree-optimization/108691] [13 Regression] ICE with function ptr and setjmp/returns twice at -O1

[cvs-commit at gcc dot gnu.org]

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

--- Comment #15 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:1dd033b8bcb0c50ad80084a3a13a391808b2deb6

commit r13-6259-g1dd033b8bcb0c50ad80084a3a13a391808b2deb6
Author: Richard Biener <rguenther@suse.de>
Date:   Tue Feb 21 13:02:31 2023 +0100

    tree-optimization/108691 - remove trigger-happy assert

    The following gets rid of the idea that we should prevent setjmp
    like calls from popping up in uncontrolled way in the IL for now.
    The solution is probably to handle it similar as noreturn has
    the ctrl-altering flag on stmts, but use another flag, for example
    ctrl-receiving which would also make sure the stmt is first.

            PR tree-optimization/108691
            * tree-ssa-dce.cc (eliminate_unnecessary_stmts): Remove
            assert about calls_setjmp not becoming true when it was false.

            * gcc.dg/pr108691.c: New testcase.

[Bug tree-optimization/108691] [13 Regression] ICE with function ptr and setjmp/returns twice at -O1

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.