| Message ID | 8221231.NyiUUSuA9g@fomalhaut |
|---|---|
| State | New |
| Headers | show |
| Series | Fix PR target/90458 | expand |
On 2/15/23 08:24, Eric Botcazou via Gcc-patches wrote: > Hi, > > this is the incompatibility of -fstack-clash-protection with Windows SEH. Now > the Windows ports always enable TARGET_STACK_PROBE, which means that the stack > is always probed (out of line) so -fstack-clash-protection does nothing more. > > Tested on x86-64/Windows and Linux, OK for all active branches? > > > 2023-02-15 Eric Botcazou <ebotcazou@adacore.com> > > * config/i386/i386.cc (ix86_compute_frame_layout): Disable the > effects of -fstack-clash-protection for TARGET_STACK_PROBE. > (ix86_expand_prologue): Likewise. > OK. THanks for taking care of this. I let it languish far too long. jeff
On Wed, Feb 15, 2023 at 10:24 AM Eric Botcazou via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Hi, > > this is the incompatibility of -fstack-clash-protection with Windows SEH. Now > the Windows ports always enable TARGET_STACK_PROBE, which means that the stack > is always probed (out of line) so -fstack-clash-protection does nothing more. > > Tested on x86-64/Windows and Linux, OK for all active branches? > > > 2023-02-15 Eric Botcazou <ebotcazou@adacore.com> > > * config/i386/i386.cc (ix86_compute_frame_layout): Disable the > effects of -fstack-clash-protection for TARGET_STACK_PROBE. > (ix86_expand_prologue): Likewise. This fixes dg.exp/stack-check-2.c, -7, 8, and -16.c, which is great! -6 no longer ICEs, but still fails. -3, -4, -5, and -9 didn't ICE, and still fail: gcc.dg/stack-check-10.c: pattern found 0 times FAIL: gcc.dg/stack-check-10.c scan-rtl-dump-times pro_and_epilogue "Stack clash no probe" 2 gcc.dg/stack-check-10.c: pattern found 0 times FAIL: gcc.dg/stack-check-10.c scan-rtl-dump-times pro_and_epilogue "Stack clash not noreturn" 2 gcc.dg/stack-check-10.c: pattern found 0 times FAIL: gcc.dg/stack-check-10.c scan-rtl-dump-times pro_and_epilogue "Stack clash residual allocation in prologue" 2 gcc.dg/stack-check-10.c: pattern found 0 times FAIL: gcc.dg/stack-check-10.c scan-rtl-dump-times pro_and_epilogue "Stack clash no frame pointer needed" 2 gcc.dg/stack-check-3.c: pattern found 0 times FAIL: gcc.dg/stack-check-3.c scan-rtl-dump-times expand "allocation and probing residuals" 7 gcc.dg/stack-check-3.c: pattern found 0 times FAIL: gcc.dg/stack-check-3.c scan-rtl-dump-times expand "allocation and probing in loop" 7 gcc.dg/stack-check-4.c: pattern found 0 times FAIL: gcc.dg/stack-check-4.c scan-rtl-dump-times pro_and_epilogue "Stack clash noreturn" 1 gcc.dg/stack-check-4.c: pattern found 0 times FAIL: gcc.dg/stack-check-4.c scan-rtl-dump-times pro_and_epilogue "Stack clash not noreturn" 1 gcc.dg/stack-check-5.c: pattern found 0 times FAIL: gcc.dg/stack-check-5.c scan-rtl-dump-times pro_and_epilogue "Stack clash no probe" 4 gcc.dg/stack-check-5.c: pattern found 0 times FAIL: gcc.dg/stack-check-5.c scan-rtl-dump-times pro_and_epilogue "Stack clash not noreturn" 4 gcc.dg/stack-check-5.c: pattern found 0 times FAIL: gcc.dg/stack-check-5.c scan-rtl-dump-times pro_and_epilogue "Stack clash no frame pointer needed" 4 gcc.dg/stack-check-5.c: pattern found 0 times FAIL: gcc.dg/stack-check-5.c scan-rtl-dump-times pro_and_epilogue "Stack clash no residual allocation in prologue" 1 gcc.dg/stack-check-5.c: pattern found 0 times FAIL: gcc.dg/stack-check-5.c scan-rtl-dump-times pro_and_epilogue "Stack clash residual allocation in prologue" 3 gcc.dg/stack-check-6.c: pattern found 0 times FAIL: gcc.dg/stack-check-6.c scan-rtl-dump-times pro_and_epilogue "Stack clash inline probes" 2 gcc.dg/stack-check-6.c: pattern found 0 times FAIL: gcc.dg/stack-check-6.c scan-rtl-dump-times pro_and_epilogue "Stack clash probe loop" 2 gcc.dg/stack-check-6.c: pattern found 0 times FAIL: gcc.dg/stack-check-6.c scan-rtl-dump-times pro_and_epilogue "Stack clash residual allocation in prologue" 4 gcc.dg/stack-check-6.c: pattern found 0 times FAIL: gcc.dg/stack-check-6.c scan-rtl-dump-times pro_and_epilogue "Stack clash not noreturn" 4 gcc.dg/stack-check-6.c: pattern found 0 times FAIL: gcc.dg/stack-check-6.c scan-rtl-dump-times pro_and_epilogue "Stack clash no frame pointer needed" 4 gcc.dg/stack-check-6a.c: pattern found 0 times FAIL: gcc.dg/stack-check-6a.c scan-rtl-dump-times pro_and_epilogue "Stack clash residual allocation in prologue" 4 gcc.dg/stack-check-6a.c: pattern found 0 times FAIL: gcc.dg/stack-check-6a.c scan-rtl-dump-times pro_and_epilogue "Stack clash not noreturn" 4 gcc.dg/stack-check-6a.c: pattern found 0 times FAIL: gcc.dg/stack-check-6a.c scan-rtl-dump-times pro_and_epilogue "Stack clash no frame pointer needed" 4 gcc.dg/stack-check-9.c: pattern found 0 times FAIL: gcc.dg/stack-check-9.c scan-rtl-dump-times pro_and_epilogue "Stack clash inline probes" 1 gcc.dg/stack-check-9.c: pattern found 0 times FAIL: gcc.dg/stack-check-9.c scan-rtl-dump-times pro_and_epilogue "Stack clash residual allocation in prologue" 1 gcc.dg/stack-check-9.c: pattern found 0 times FAIL: gcc.dg/stack-check-9.c scan-rtl-dump-times pro_and_epilogue "Stack clash not noreturn" 1 gcc.dg/stack-check-9.c: pattern found 0 times FAIL: gcc.dg/stack-check-9.c scan-rtl-dump-times pro_and_epilogue "Stack clash no frame pointer needed" 1 target/i386.exp/stack-check-12.c no longer ICEs but still fails. -11, -18 and -19 still fail: gcc.target/i386/stack-check-11.c: sub[ql] found 1 times FAIL: gcc.target/i386/stack-check-11.c scan-assembler-times sub[ql] 4 gcc.target/i386/stack-check-11.c: or[ql] found 0 times FAIL: gcc.target/i386/stack-check-11.c scan-assembler-times or[ql] 3 ia32882847.c:2:13: error: size of array 'dummy' is negative^M ia32882847.c:4:55: error: '__i386__' undeclared here (not in a function)^M compiler exited with status 1 FAIL: gcc.target/i386/stack-check-12.c scan-assembler pushq\t%rax FAIL: gcc.target/i386/stack-check-12.c scan-assembler popq\t%rax gcc.target/i386/stack-check-18.c: pattern found 0 times FAIL: gcc.target/i386/stack-check-18.c scan-rtl-dump-times expand "allocation and probing in loop" 1 gcc.target/i386/stack-check-18.c: pattern found 0 times FAIL: gcc.target/i386/stack-check-18.c scan-rtl-dump-times expand "allocation and probing residuals" 1 gcc.target/i386/stack-check-18.c: or[ql] found 0 times FAIL: gcc.target/i386/stack-check-18.c scan-assembler-times or[ql] 1 gcc.target/i386/stack-check-19.c: pattern found 0 times FAIL: gcc.target/i386/stack-check-19.c scan-rtl-dump-times expand "allocation and probing in loop" 1 gcc.target/i386/stack-check-19.c: pattern found 0 times FAIL: gcc.target/i386/stack-check-19.c scan-rtl-dump-times expand "allocation and probing residuals" 1 gcc.target/i386/stack-check-19.c: or[ql] found 0 times FAIL: gcc.target/i386/stack-check-19.c scan-assembler-times or[ql] 2 gcc.target/i386/stack-check-19.c: (?:je|jne) found 0 times FAIL: gcc.target/i386/stack-check-19.c scan-assembler-times (?:je|jne) 3
> This fixes dg.exp/stack-check-2.c, -7, 8, and -16.c, which is great!
Try the attached patch.
On Thu, Feb 16, 2023 at 3:16 AM Eric Botcazou <botcazou@adacore.com> wrote: > > > This fixes dg.exp/stack-check-2.c, -7, 8, and -16.c, which is great! > > Try the attached patch. Well... that patch just marks all of the tests as unsupported. But for example, the ones quoted above run, work, and pass. And when they didn't pass, they highlighted the ICE that you fixed. So running the test despite the nature of stack protection on Windows still has value. It is useful to ensure for example that stack protection continues to work even if options are passed to disable it. But the tests that remain are looking for rtl patterns that (I assume) shouldn't exist. So it's perhaps useful to modify the test to say that on windows, the scan-rtl-dump-times check should have zero hits. If it found a match, that would be an error. Is there a way to say that the test results should be inverted on a particular platform (instead of purely unsupported)?
> Is there a way to say that the test results should be inverted on a > particular platform (instead of purely unsupported)? Yes, you can do pretty much what you want with the testsuite harness.
On 2/17/23 01:56, NightStrike via Gcc-patches wrote: > On Thu, Feb 16, 2023 at 3:16 AM Eric Botcazou <botcazou@adacore.com> wrote: >> >>> This fixes dg.exp/stack-check-2.c, -7, 8, and -16.c, which is great! >> >> Try the attached patch. > > Well... that patch just marks all of the tests as unsupported. But > for example, the ones quoted above run, work, and pass. And when they > didn't pass, they highlighted the ICE that you fixed. So running the > test despite the nature of stack protection on Windows still has > value. It is useful to ensure for example that stack protection > continues to work even if options are passed to disable it. But the > tests that remain are looking for rtl patterns that (I assume) > shouldn't exist. So it's perhaps useful to modify the test to say > that on windows, the scan-rtl-dump-times check should have zero hits. > If it found a match, that would be an error. Those tests may compile, work and pass, but they're really there to check the probing decisions. Windows has a totally different model and none of those tests really make sense in that model. > > Is there a way to say that the test results should be inverted on a > particular platform (instead of purely unsupported)? You can express all kinds of things, I'm just not sure it's worth the effort for these tests.
On 2/16/23 01:16, Eric Botcazou via Gcc-patches wrote: >> This fixes dg.exp/stack-check-2.c, -7, 8, and -16.c, which is great! > > Try the attached patch. I'd suggest going ahead and applying the fix to check_effective_target_supports_stack_clash_protection. There's really not a strong reason to run those tests on a cygwin or mingw target given how probing is done (out of line). Jeff
The reason would be to show that they continue to not ICE as they used to. No go paths are just as useful as go paths. On Sat, Mar 11, 2023, 10:57 Jeff Law <jeffreyalaw@gmail.com> wrote: > > > On 2/16/23 01:16, Eric Botcazou via Gcc-patches wrote: > >> This fixes dg.exp/stack-check-2.c, -7, 8, and -16.c, which is great! > > > > Try the attached patch. > I'd suggest going ahead and applying the fix to > check_effective_target_supports_stack_clash_protection. > > There's really not a strong reason to run those tests on a cygwin or > mingw target given how probing is done (out of line). > > Jeff >
diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc index 3cacf738c4a..22f444be23c 100644 --- a/gcc/config/i386/i386.cc +++ b/gcc/config/i386/i386.cc @@ -6876,7 +6876,9 @@ ix86_compute_frame_layout (void) stack clash protections are enabled and the allocated frame is larger than the probe interval, then use pushes to save callee saved registers. */ - || (flag_stack_clash_protection && to_allocate > get_probe_interval ())) + || (flag_stack_clash_protection + && !ix86_target_stack_probe () + && to_allocate > get_probe_interval ())) frame->save_regs_using_mov = false; if (ix86_using_red_zone () @@ -8761,8 +8763,11 @@ ix86_expand_prologue (void) sse_registers_saved = true; } - /* If stack clash protection is requested, then probe the stack. */ - if (allocate >= 0 && flag_stack_clash_protection) + /* If stack clash protection is requested, then probe the stack, unless it + is already probed on the target. */ + if (allocate >= 0 + && flag_stack_clash_protection + && !ix86_target_stack_probe ()) { ix86_adjust_stack_and_probe (allocate, int_registers_saved, false); allocate = 0;