This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][explow] PR target/85173: validize memory before passing it on to target probe_stack
- From: Jeff Law <law at redhat dot com>
- To: Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Cc: Jakub Jelinek <jakub at redhat dot com>, Uros Bizjak <ubizjak at gmail dot com>
- Date: Mon, 9 Apr 2018 16:26:01 -0600
- Subject: Re: [PATCH][explow] PR target/85173: validize memory before passing it on to target probe_stack
- Autocrypt: addr=law at redhat dot com; prefer-encrypt=mutual; keydata= xsBNBFkbIO8BCACVIqDhDVh9ur8C+zNV1J/cXfwvVDAUcphDEFl4jyHqZORK4Pd3Db8oWqLm Q8lOCr/VOS7lrCtdpVMQkLGOGA16oJ8g7hzhnojpjY09UjsoUiG7oKacuxj8skfp6SIx93Zl +iNYPRa4S+za6nY8qiVjyUuiyX04ZPZMrKp2c2sGi+HnBKUZXGhrz/Jdzdox3tjajWZnObyy nhEN6hn9L3KawTtGPE/R6A/1RhHTD9FQmIWIeucpaY5c6GNKXTFpj2VYx57LY5hve1R5vhrJ IZcgwZAiOtmik5lVi96glY5h6bugRwpexjhwORTLPBCkwiYotSxX99mWd6EHL576i5CNABEB AAHNGUplZmYgTGF3IDxsYXdAcmVkaGF0LmNvbT7CwI4EEwEIADgWIQR+niGjtnP5P/8PpRq8 fP682pgzWwUCWRsg7wIbAwULCQgHAgYVCAkKCwIEFgIDAQIeAQIXgAAKCRC8fP682pgzW5QG B/9VATJmx5235RB+8jiDYGXQf3vd9gBfPy/l1tsaK400eFAevDzfGvKmeCKe+uGnlrH3vyT8 rg9zqH+s5a1Y+lDXPOpJAFmmzbOLU4FW4ucbawmtYvBL65PqpQneCTYnC802/OAcxjm/Onem HlgeK6WicNsBTPwYN/0araDFUejyYBIFi9CNqqflwk5Z3brKbQ9bAYIkysVLC/c3njKPmM0c WPFHG91ubLbWCHwTIK0+mAL714eTD74dXzOjO2ZDBPLGlFN/kO3+YjaO6UOD2O8acvAMCivT kWLr7JwRgLIQDN2DkhQDd3LTPqQE/yOcMcXBTO+fxm8KG0iKQBqWMyGJzsBNBFkbIO8BCACy qbOsv7XegSeea8XORt5zMaBVWKoSyhmmcCmlxZFS2cuYOBt79MO13lZE2DlO3Lv5IKikj/D4 ketGVO4+h5psEMH5Yz5P8bx0TmgwbK1GxPZrzeXozUFJDvvCDbIlT0v0pwUXuK3hg8Ieo2h5 uTed/cn1OjySXW5BqLxN0cyr5hL+J6dcsHvKLT/N3nTgCQhoJXK2MrEMhAGgF3jKpMn3CoS4 i/ZbNI2MQR6LWHwdZ95f0fI8NzHSfVzeLtzCKQec7nr9fgd6Ylk1ZpGWQUPlQmKjzYgeCeTK NO04cwt20WIrQWeWiZFPA0U86NDBdSBrYp4kG3dfIXE+wSSvE7qPABEBAAHCwHYEGAEIACAW IQR+niGjtnP5P/8PpRq8fP682pgzWwUCWRsg7wIbDAAKCRC8fP682pgzW3REB/9cT7iKRPg/ OK9bpLlllIEDM90IaKC79DQrv+fRudOR78cdV4XUwPSFnyHUsP3VJ4lDy5FhiKCwGie0BK53 EsxgMrLy1L8hboFdTE4Vi0xzCheMaMVp4hATDU29k1cuxu1VPpCa8E3mYeHjNV7ip0HN5L4D rfs8lRPJE/oM1vGs9DgQFZrCPPNRNGKC97BH+DHccesEJr7tSsQrkPkt0z/FTKr5wIM02vSx OJjgmcVbGB7dc2j/Sx8loXmuKnuKtM35668kUG8jeJvSQk3o/VHpD27bhl0rR68R2jN6G6kQ egMVb6dPu1Ius8rBE5rFw88J4JEb5q4hMNClWWUFHIdP
- Openpgp: preference=signencrypt
- References: <5AC630B9.1060504@foss.arm.com>
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.tkachov@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.tkachov@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