Re: [PATCH][AARCH64] PR target/84521 Fix frame pointer corruption with -fomit-frame-pointer with __builtin_setjmp

[James Greenhalgh <james dot greenhalgh at arm dot com>]

On Mon, Jun 25, 2018 at 04:24:14AM -0500, Sudakshina Das wrote:
> PING!
> 
> On 14/06/18 12:10, Sudakshina Das wrote:
> > Hi Eric
> > 
> > On 07/06/18 16:33, Eric Botcazou wrote:
> >>> Sorry this fell off my radar. I have reg-tested it on x86 and tried it
> >>> on the sparc machine from the gcc farm but I think I couldn't finished
> >>> the run and now its showing to he unreachable.
> >>
> >> The patch is a no-op for SPARC because it defines the nonlocal_goto 
> >> pattern.
> >>
> >> But I would nevertheless strongly suggest _not_ fiddling with the 
> >> generic code
> >> like that and just defining the nonlocal_goto pattern for Aarch64 
> >> instead.
> >>
> > 
> > Thank you for the suggestion, I have edited the patch accordingly and
> > defined the nonlocal_goto pattern for AArch64. This has also helped take
> > care of the issue with __builtin_longjmp that Wilco had mentioned in his
> > comment on the PR (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84521#c19).
> > 
> > I have also modified the test case according to Wilco's comment to add 
> > an extra jump buffer. This test case passes with AArch64 but fails on
> > x86 trunk as follows (It may fail on other targets as well):
> > 
> > FAIL: gcc.c-torture/execute/pr84521.c   -O1  execution test
> > FAIL: gcc.c-torture/execute/pr84521.c   -O2  execution test
> > FAIL: gcc.c-torture/execute/pr84521.c   -O3 -fomit-frame-pointer
> > -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
> > FAIL: gcc.c-torture/execute/pr84521.c   -O3 -g  execution test
> > FAIL: gcc.c-torture/execute/pr84521.c   -Os  execution test
> > FAIL: gcc.c-torture/execute/pr84521.c   -O2 -flto -fno-use-linker-plugin
> > -flto-partition=none  execution test
> > FAIL: gcc.c-torture/execute/pr84521.c   -O2 -flto -fuse-linker-plugin
> > -fno-fat-lto-objects  execution test
> > 
> > Testing: Bootstrapped and regtested on aarch64-none-linux-gnu.
> > 
> > Is this ok for trunk?

The AArch64 parts are OK. I've been holding off approving the patch while
I wait for Eric to reply on the x86_64 fails with your new testcase.

Thanks,
James



> > 
> > Sudi
> > 
> > *** gcc/ChangeLog ***
> > 
> > 2018-06-14  Sudakshina Das  <sudi.das@arm.com>
> > 
> >      PR target/84521
> >      * config/aarch64/aarch64.h (DONT_USE_BUILTIN_SETJMP): Update comment.
> >      * config/aarch64/aarch64.c (aarch64_needs_frame_chain): Add
> >      cfun->has_nonlocal_label to force frame chain.
> >      (aarch64_builtin_setjmp_frame_value): New.
> >      (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Define.
> >      * config/aarch64/aarch64.md (nonlocal_goto): New.
> > 
> > *** gcc/testsuite/ChangeLog ***
> > 
> > 2018-06-14  Sudakshina Das  <sudi.das@arm.com>
> > 
> >      PR target/84521
> >      * gcc.c-torture/execute/pr84521.c: New test.
>