On 7/24/24 12:00 PM, Raphael Moreira Zinsly wrote:
This implements stack-clash protection for riscv, with
riscv_allocate_and_probe_stack_space being based of
aarch64_allocate_and_probe_stack_space from aarch64's implementation.
We enforce the probing interval and the guard size to always be equal, their
default value is 4Kb which is riscv page size.
We also probe up by 1024 bytes in the general case when a probe is required.
gcc/ChangeLog:
* config/riscv/riscv.cc
(riscv_option_override): Enforce that interval is the same size as
guard size.
(riscv_allocate_and_probe_stack_space): New function.
(riscv_expand_prologue): Call riscv_allocate_and_probe_stack_space
to the final allocation of the stack and add stack-clash dump
information.
* config/riscv/riscv.h: Define STACK_CLASH_CALLER_GUARD and
STACK_CLASH_MAX_UNROLL_PAGES.
gcc/testsuite/ChangeLog:
* gcc.dg/params/blocksort-part.c: Skip riscv for
stack-clash protection intervals.
* gcc.dg/pr82788.c: Skip riscv.
* gcc.dg/stack-check-6.c: Skip residual check for riscv.
* gcc.dg/stack-check-6a.c: Skip riscv.
* gcc.target/riscv/stack-check-12.c: New test.
* gcc.target/riscv/stack-check-13.c: New test.
* gcc.target/riscv/stack-check-cfa-1.c: New test.
* gcc.target/riscv/stack-check-cfa-2.c: New test.
* gcc.target/riscv/stack-check-prologue-1.c: New test.
* gcc.target/riscv/stack-check-prologue-10.c: New test.
* gcc.target/riscv/stack-check-prologue-11.c: New test.
* gcc.target/riscv/stack-check-prologue-12.c: New test.
* gcc.target/riscv/stack-check-prologue-13.c: New test.
* gcc.target/riscv/stack-check-prologue-14.c: New test.
* gcc.target/riscv/stack-check-prologue-15.c: New test.
* gcc.target/riscv/stack-check-prologue-2.c: New test.
* gcc.target/riscv/stack-check-prologue-3.c: New test.
* gcc.target/riscv/stack-check-prologue-4.c: New test.
* gcc.target/riscv/stack-check-prologue-5.c: New test.
* gcc.target/riscv/stack-check-prologue-6.c: New test.
* gcc.target/riscv/stack-check-prologue-7.c: New test.
* gcc.target/riscv/stack-check-prologue-8.c: New test.
* gcc.target/riscv/stack-check-prologue-9.c: New test.
* gcc.target/riscv/stack-check-prologue.h: New file.
* lib/target-supports.exp
(check_effective_target_supports_stack_clash_protection):
Add riscv.
(check_effective_target_caller_implicit_probes): Likewise.
Guessing you've got a mixture of tabs and spaces in the ChangeLog entry.
I suspect the pre-commit hooks will complain about them.
This all looks really good. It follows the aarch64 implementation
reasonably closely with the notable exception of the RTL probe loop
rather than using probe_stack_range, but with the ability to have
multiple blocks in the prologue (that we didn't have in 2017/2018),
yours is the current preferred method.
I did reasonably closely check the bits you moved from
riscv_expand_prologue given the difficulties we had with them recently.
That all looked good as well.
I initially expected more changes to be necessary in
target_supports.exp, but reviewing aarch64's handling in there, I think
you got it right. For aarch64 & riscv, we support stack clash
protection and have limited implicit probes due to saving $ra. The
other properties don't apply to aarch64/riscv.
So again, overall it looks really good.
And to get on the record testing-wise. Raphael and I had just started
doing large scale testing of Fedora packages about a month ago. We
identified ~6k binary packages that looked potentially vulnerable given
the scanner work from Red Hat & Samsung. That narrowed down to around
4k source packages that we'd need to test with before/after builds.
We were just starting the rebuild & rescan process and were seeing good
results and our milkv pioneer system completely scrambled its disk (for
the 2nd time :( That naturally brought testing to a halt. Just
recovering data off the data drive is proving somewhat painful. We're
still committed to doing that testing as it proved quite valuable on x86
years ago when I did that implementation. I fully expect it'll find
minor glitches either in the scanner or the compiler bits.
Raphael has done bootstraps and regression testing for rv32 and rv64
with stack clash enabled (which was definitely useful in exposing
additional issues).
Your call whether or not to include it now or wait for review on 4/5 and
5/5.
Jeff
