| Message ID | 20190522224051.GI19695@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | Improve stack_protect_test_<mode> on x86 (PR target/90568) | expand |
On Thu, May 23, 2019 at 12:41 AM Jakub Jelinek <jakub@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 <jakub@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
--- 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>"
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 <jakub@redhat.com> PR target/90568 * config/i386/i386.md (stack_protect_test_<mode>): Use sub instead of xor. Jakub