On 2/2/19 3:22 AM, Jakub Jelinek wrote:
> On Fri, Feb 01, 2019 at 11:52:04PM +0100, Eric Botcazou wrote:
>>> So, can we e.g. keep emitting the epilogue where it is now for
>>> naked_return_label != NULL_RTX and move it otherwise?
>>> For __builtin_return the setter and use of the hard register won't be
>>> adjacent in any case.
>>
>> See my comment in the audit trail of the PR; I'd suspend it and go to bed.
>> ;-)
>
> While the set of -fno- and -f options in some PRs are unlikely to be used by
> people in the wild, often those PRs uncover latent bugs that could cause
> serious wrong-code or ICEs, of course not always. So IMHO we shouldn't
> ignore those PRs, especially if they are regressions.
>
> In the meantime, I've bootstrapped/regtested successfully following version
> of the patch that should fix the builtin return case.
> In addition to normal {x86_64,i686}-linux bootstrap/regtest I've done a
> distro build where we 1) build the compiler itself with
> -fstack-protector-strong 2) run testsuite with
> --tool-test=\{,-fstack-protector-strong\}, so far on
> {x86_64,i686,powerpc64le}-linux, other targets still pending.
>
> The only "regression" was gcc.target/i386/call-1.c with
> -fstack-protector-strong, but that is because the test is invalid:
> void set_eax(int val)
> {
> __asm__ __volatile__ ("mov %0, %%eax" : : "m" (val));
> }
> - missing "eax" clobber or "=a" (dummy) output and when set_eax is inlined
> that can break optimizations badly. Of course, in addition to fixing that,
> I'd expect if the tests wants to test what it originally wanted to test, it
> needs to disable inlining or perhaps all IPA opts, not sure if just for
> set_eax or also for foo/bar. Perhaps we can keep the testcase as is with
> the "eax" clobber and add another one with __attribute__((noipa)) on
> set_eax/foo/bar.
>
> 2019-02-01 Jakub Jelinek <ja...@redhat.com>
>
> PR rtl-optimization/87485
> * function.c (expand_function_end): Move stack_protect_epilogue
> before loading of return value into hard register(s).
>
> * gcc.dg/pr87485.c: New test.
OK. Though you do need to twiddle the call-1.c test in some manner.
I've got no strong opinions on setting noipa on everything in the test
vs twiddling the clobbers list. Your call on best approach for
adjusting the call-1 test.
jeff
