Re: [PATCH] Fix PR target/90458
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 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 >
Re: [PATCH] Fix PR target/90458
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
Re: [PATCH] Fix PR target/90458
On 2/17/23 01:56, NightStrike via Gcc-patches wrote: On Thu, Feb 16, 2023 at 3:16 AM Eric Botcazou 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.
Re: [PATCH] Fix PR target/90458
> 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. -- Eric Botcazou
Re: [PATCH] Fix PR target/90458
On Thu, Feb 16, 2023 at 3:16 AM Eric Botcazou 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)?
Re: [PATCH] Fix PR target/90458
> This fixes dg.exp/stack-check-2.c, -7, 8, and -16.c, which is great!
Try the attached patch.
--
Eric Botcazoudiff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 227e3004077..d4f036a3f1e 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -11655,6 +11655,11 @@ proc check_effective_target_autoincdec { } {
#
proc check_effective_target_supports_stack_clash_protection { } {
+# Stack probing is done unconditionally out-of-line on Windows
+if { [istarget *-*-cygwin*] || [istarget *-*-mingw*] } {
+ return 0
+}
+
if { [istarget x86_64-*-*] || [istarget i?86-*-*]
|| [istarget powerpc*-*-*] || [istarget rs6000*-*-*]
|| [istarget aarch64*-**] || [istarget s390*-*-*]
Re: [PATCH] Fix PR target/90458
On Wed, Feb 15, 2023 at 10:24 AM 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 > > * 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:
Re: [PATCH] Fix PR target/90458
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 * 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
[PATCH] Fix PR target/90458
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
* config/i386/i386.cc (ix86_compute_frame_layout): Disable the
effects of -fstack-clash-protection for TARGET_STACK_PROBE.
(ix86_expand_prologue): Likewise.
--
Eric Botcazoudiff --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;
