On 04/05/2018 08:20 AM, Kyrill Tkachov wrote: > Hi all, > > In this PR the expansion code emits an invalid memory address for the > stack probe, which the backend fails to recognise. > The address is created explicitly in > anti_adjust_stack_and_probe_stack_clash in explow.c and passed down to > gen_probe_stack > without any validation in emit_stack_probe. > > This patch fixes the ICE by calling validize_mem on the memory location > before passing it down to the target. > Jakub pointed out that we also want to create valid addresses for the > probe_stack_address case, so this patch > creates an expand operand and legitimizes it before passing it down to > the probe_stack_address expander. > > This patch passes bootstrap and testing on arm-none-linux-gnueabihf and > aarch64-none-linux-gnu > and ppc64le-redhat-linux on gcc112 in the compile farm. > > Is this ok for trunk? > > Thanks, > Kyrill > > P.S. Uros, the alpha probe_stack expander in alpha.md seems incompatible > with the way the probe_stack name is > used in the midend. It accepts a const_int operand that is used as an > offset from the stack pointer, rather than accepting > a full memory operand like other targets. Do you think it's better to > rename the probe_stack pattern there to something > that doesn't conflict with the name the midend assumes? > > 2018-04-05 Kyrylo Tkachov <kyrylo.tkac...@arm.com> > > PR target/85173 > * explow.c (emit_stack_probe): Call validize_mem on memory location > before passing it to gen_probe_stack. Create address operand and > legitimize it for the probe_stack_address case. > > 2018-04-05 Kyrylo Tkachov <kyrylo.tkac...@arm.com> > > PR target/85173 > * gcc.target/arm/pr85173.c: New test. Alpha should be fixed -- the docs clearly state that the operand is "the memory reference in the stack that needs to be probed". Just passing in the offset seems wrong.
Note that -fstack-clash-protection on ARM (32bit) relies on the old -fstack-check code which has a variety of issues. It's better than nothing, but the real way to go here is to fix prologue generation as well as alloca/vla handling to have stack clash specific sequences. OK for the trunk. jeff