[Bug target/84521] aarch64: Frame-pointer corruption with __builtin_setjmp/__builtin_longjmp and -fomit-frame-pointer

2019-06-19 Thread wilco at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84521

--- Comment #29 from Wilco  ---
Author: wilco
Date: Wed Jun 19 12:52:43 2019
New Revision: 272473

URL: https://gcc.gnu.org/viewcvs?rev=272473=gcc=rev
Log:
Simplify setjmp and non-local goto implementation (PR84521)

This fixes and simplifies the setjmp and non-local goto implementation.
Currently the virtual frame pointer is saved when using __builtin_setjmp or
a non-local goto.  Depending on whether a frame pointer is used, this may
either save SP or FP with an immediate offset.  However the goto or longjmp
always updates the hard frame pointer.

A receiver veneer in the original function then assigns the hard frame pointer
to the virtual frame pointer, which should, if it works correctly, again assign
SP or FP.  However the special elimination code in eliminate_regs_in_insn
doesn't do this correctly unless the frame pointer is used, and even if it
worked by writing SP, the frame pointer would still be corrupted.

A much simpler implementation is to always save and restore the hard frame
pointer.  This avoids 2 redundant instructions which add/subtract the virtual
frame offset.  A large amount of code can be removed as a result, including all
implementations of TARGET_BUILTIN_SETJMP_FRAME_VALUE (all of which already use
the hard frame pointer).  The expansion of nonlocal_goto on PA can be simplied
to just restore the hard frame pointer. 

This fixes the most obvious issues, however there are still issues on targets
which define HARD_FRAME_POINTER_IS_FRAME_POINTER (arm, mips).
Each function could have a different hard frame pointer, so a non-local goto
may restore the wrong frame pointer (TARGET_BUILTIN_SETJMP_FRAME_VALUE could
be useful for this).

The i386 TARGET_BUILTIN_SETJMP_FRAME_VALUE was incorrect: if stack_realign_fp
is true, it would save the hard frame pointer value but restore the virtual
frame pointer which according to ix86_initial_elimination_offset can have a
non-zero offset from the hard frame pointer.

The ia64 implementation of nonlocal_goto seems incorrect since the helper
function moves the the frame pointer value into the static chain register
(so this patch does nothing to make it better or worse).

AArch64 + x86-64 bootstrap OK, new test passes on AArch64, x86-64 and Arm.

gcc/
PR middle-end/84521
* builtins.c (expand_builtin_setjmp_setup): Save
hard_frame_pointer_rtx.
(expand_builtin_setjmp_receiver): Do not emit sfp = fp move since we
restore fp.
* function.c (expand_function_start): Save hard_frame_pointer_rtx for
non-local goto.
* lra-eliminations.c (eliminate_regs_in_insn): Remove sfp = fp
elimination code.
(remove_reg_equal_offset_note): Remove unused function.
* reload1.c (eliminate_regs_in_insn): Remove sfp = hfp elimination
code.
* config/arc/arc.c (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
(arc_builtin_setjmp_frame_value): Remove function.
* config/avr/avr.c  (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
(avr_builtin_setjmp_frame_value): Remove function.
* config/i386/i386.c (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
(ix86_builtin_setjmp_frame_value): Remove function.
* config/pa/pa.md (nonlocal_goto): Remove FP adjustment.
* config/sparc/sparc.c (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
(sparc_builtin_setjmp_frame_value): Remove function.
* config/vax/vax.c (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
(vax_builtin_setjmp_frame_value): Remove function.
* config/xtensa/xtensa.c (xtensa_frame_pointer_required): Force frame
pointer if has_nonlocal_label.

testsuite/
PR middle-end/84521
* gcc.c-torture/execute/pr84521.c: New test.

Added:
trunk/gcc/testsuite/gcc.c-torture/execute/pr84521.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/builtins.c
trunk/gcc/config/arc/arc.c
trunk/gcc/config/avr/avr.c
trunk/gcc/config/i386/i386.c
trunk/gcc/config/pa/pa.md
trunk/gcc/config/sparc/sparc.c
trunk/gcc/config/vax/vax.c
trunk/gcc/config/xtensa/xtensa.c
trunk/gcc/function.c
trunk/gcc/lra-eliminations.c
trunk/gcc/reload1.c
trunk/gcc/testsuite/ChangeLog

[Bug target/84521] aarch64: Frame-pointer corruption with __builtin_setjmp/__builtin_longjmp and -fomit-frame-pointer

2018-08-01 Thread sudi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84521

--- Comment #28 from sudi at gcc dot gnu.org ---
Created attachment 44478
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=44478=edit
Failing test case

As advised by James on the mailing list, I am adding the test case that is
failing on at least AAcrh64 and x86. My proposed patch fixes this on AArch64,
but I would like to add this test in the general gcc.c-torture/execute/ folder
so that other targets can also check.

[Bug target/84521] aarch64: Frame-pointer corruption with __builtin_setjmp/__builtin_longjmp and -fomit-frame-pointer

2018-07-26 Thread pinskia at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84521

Andrew Pinski  changed:

   What|Removed |Added

   Target Milestone|8.3 |---

[Bug target/84521] aarch64: Frame-pointer corruption with __builtin_setjmp/__builtin_longjmp and -fomit-frame-pointer

2018-07-26 Thread jakub at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84521

Jakub Jelinek  changed:

   What|Removed |Added

   Target Milestone|8.2 |8.3

--- Comment #27 from Jakub Jelinek  ---
GCC 8.2 has been released.

[Bug target/84521] aarch64: Frame-pointer corruption with __builtin_setjmp/__builtin_longjmp and -fomit-frame-pointer

2018-05-02 Thread jakub at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84521

Jakub Jelinek  changed:

   What|Removed |Added

   Target Milestone|8.0 |8.2

--- Comment #26 from Jakub Jelinek  ---
GCC 8.1 has been released.

[Bug target/84521] aarch64: Frame-pointer corruption with __builtin_setjmp/__builtin_longjmp and -fomit-frame-pointer

2018-03-14 Thread sudi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84521

--- Comment #25 from sudi at gcc dot gnu.org ---
Proposed patch. This obviously does not solve all the issues

https://gcc.gnu.org/ml/gcc-patches/2018-03/msg00668.html

[Bug target/84521] aarch64: Frame-pointer corruption with __builtin_setjmp/__builtin_longjmp and -fomit-frame-pointer

2018-03-13 Thread jakub at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84521

Jakub Jelinek  changed:

   What|Removed |Added

   Priority|P1  |P3
Summary|[8 Regression] aarch64: |aarch64: Frame-pointer
   |Frame-pointer corruption|corruption with
   |with|__builtin_setjmp/__builtin_
   |__builtin_setjmp/__builtin_ |longjmp and
   |longjmp and |-fomit-frame-pointer
   |-fomit-frame-pointer|

--- Comment #24 from Jakub Jelinek  ---
With the change of the default this actually isn't a regression anymore, so not
a release blocker.  That doesn't mean a simple fix, e.g. __builtin_setjmp
forcing frame pointer even if not otherwise needed, can't be accepted for GCC8
(as an exception), but it is unlikely right time for finding all issues with
the builtin and trying to fix all of them.  That can be done in GCC9.