On Thu, May 23, 2019 at 12:41 AM Jakub Jelinek <ja...@redhat.com> wrote:
>
> Hi!
>
> For stack_protect_test_[sd]i we don't use comparison, so that we clear the
> content of the register and don't keep the stack canary in any register for
> security reasons, but as mentioned in the PR, using sub instead of xor
> achieves the same effect in the likely case (no failure), in the failure
> case neither xor nor sub wipes it completely but in that case the program
> terminates anyway through __stack_chk_fail, and, on some CPUs sub can be
> fused with the conditional branch, while xor can't.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Incrementally, we should consider handling e.g. *subsi_2{,_zext} and similar
> patterns and also this stack_protect_test_[sd]i in ix86_macro_fusion_pair_p,
> not sure if unconditionally, or only when tuning for skylake+ / generic.
>
> 2019-05-22 Jakub Jelinek <ja...@redhat.com>
>
> PR target/90568
> * config/i386/i386.md (stack_protect_test_<mode>): Use sub instead
> of xor.
LGTM.
Thanks,
Uros.
> --- gcc/config/i386/i386.md.jj 2019-05-15 23:36:47.829062261 +0200
> +++ gcc/config/i386/i386.md 2019-05-22 18:40:04.360359867 +0200
> @@ -19513,7 +19513,7 @@ (define_insn "stack_protect_test_<mode>"
> UNSPEC_SP_TEST))
> (clobber (match_scratch:PTR 3 "=&r"))]
> ""
> - "mov{<imodesuffix>}\t{%1, %3|%3, %1}\;xor{<imodesuffix>}\t{%2, %3|%3, %2}"
> + "mov{<imodesuffix>}\t{%1, %3|%3, %1}\;sub{<imodesuffix>}\t{%2, %3|%3, %2}"
> [(set_attr "type" "multi")])
>
> (define_insn "sse4_2_crc32<mode>"
>
> Jakub
