[Bug target/92841] Optimize -fstack-protector-strong code generation a bit
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92841
--- Comment #12 from Jakub Jelinek ---
Author: jakub
Date: Fri Dec 20 08:23:42 2019
New Revision: 279633
URL: https://gcc.gnu.org/viewcvs?rev=279633=gcc=rev
Log:
PR target/92841
* config/i386/i386.md (*stack_protect_set_3): For pic_32bit_operand
always use lea{q}, no matter what value which_alternative has.
* gcc.target/i386/pr92841-2.c: New test.
Added:
trunk/gcc/testsuite/gcc.target/i386/pr92841-2.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/i386/i386.md
trunk/gcc/testsuite/ChangeLog
[Bug target/92841] Optimize -fstack-protector-strong code generation a bit
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92841 --- Comment #11 from Jakub Jelinek --- Author: jakub Date: Tue Dec 17 20:40:01 2019 New Revision: 279468 URL: https://gcc.gnu.org/viewcvs?rev=279468=gcc=rev Log: PR target/92841 * config/i386/i386.md (@stack_protect_set_1_, @stack_protect_test_1_): Use output_asm_insn. (*stack_protect_set_2_, *stack_protect_set_3): New define_insns and corresponding define_peephole2s. * gcc.target/i386/pr92841.c: New test. Added: trunk/gcc/testsuite/gcc.target/i386/pr92841.c Modified: trunk/gcc/ChangeLog trunk/gcc/config/i386/i386.md trunk/gcc/testsuite/ChangeLog
[Bug target/92841] Optimize -fstack-protector-strong code generation a bit
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92841 --- Comment #10 from Boris --- Ok, fair enough. After all, security is not free. :) If you need me to test anything else, lemme know. Thx guys.
[Bug target/92841] Optimize -fstack-protector-strong code generation a bit
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92841 --- Comment #9 from Jakub Jelinek --- There is a scheduling pass in between the peephole2 which this patch uses and the final pass, so it is possible there was some other instruction in between before scheduling, thus the peephole2 didn't trigger, later sched2 changes the order and now you see it. If we don't want to split the pattern and thus risk scheduling moving it away significantly etc., that will simply always happen, what matters is that the patch handles the common cases. And no, we aren't going to do anything about the cases where the register is set after several instructions, the clearing of the register is a security feature and we simply don't want to extend the exposure of the security sensitive data in any register.
[Bug target/92841] Optimize -fstack-protector-strong code generation a bit
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92841 --- Comment #8 from Jakub Jelinek --- In the patch attached in this PR, yes, but I think it is fixed in the one I've posted to gcc-patches - https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00664.html
[Bug target/92841] Optimize -fstack-protector-strong code generation a bit
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92841 --- Comment #7 from Boris --- Created attachment 47465 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=47465=edit Micha's patterns fix Fix for mix-up between patterns with and without multi-nodes.
[Bug target/92841] Optimize -fstack-protector-strong code generation a bit
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92841 --- Comment #6 from Boris --- Ok, so there was a mix-up between patterns with and without multi-nodes in your untested fix, which Micha found and fixed, see attached patch. (otherwise it wouldn't even build a whole kernel). With it, it fixed the pattern which I reported to properly overwrite RAX: mov%gs:0x28,%rax mov%rax,0x10(%rsp) mov$0x6,%eax Other places look good too: mov%gs:0x28,%rax mov%rax,0xf0(%rsp) mov0x120e664(%rip),%rax# 8220f020 But browsing the code it generated, there are still suboptimal patterns like: mov%gs:0x28,%rax mov%rax,0x8(%rsp) xor%eax,%eax mov0x98(%rdi),%rax or mov%gs:0x28,%rax mov%rax,0x80(%rsp) xor%eax,%eax mov%rbx,%rax I'm guessing this has to do with the patterns which probably should also match mem and reg source operand. Or so, just guessing here. Also, there's more complex stuff like: mov%gs:0x28,%rax mov%rax,-0x28(%rbp) xor%eax,%eax add%gs:0x7f00ffc4(%rip),%r13# 12360 mov0x80(%rdi),%rax test %rax,%rax and if that ADD can land under the following MOV, then, one could remove the XOR too. But maybe that's too complicated to do or it'll become too ugly... Oh, and there actually are cases where the XOR *is* needed: mov%gs:0x28,%rax mov%rax,0x68(%rsp) xor%eax,%eax rep stos %rax,%es:(%rdi) Fun. Thanks!
[Bug target/92841] Optimize -fstack-protector-strong code generation a bit
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92841
--- Comment #5 from Boris ---
Hohumm, looks good - this is the same site it generated with your patch:
# arch/x86/kernel/cpu/scattered.c:48: {
movq%gs:40, %rax# MEM[( long unsigned int
*)40B], prephitmp_18
movq%rax, 16(%rsp) # prephitmp_18, D.21446
movl$6, %eax#, prephitmp_18
jmp .L6 #
I haven't checked any other sites whether they are still fine but I'll do that
soon.
Thx.
[Bug target/92841] Optimize -fstack-protector-strong code generation a bit
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92841 Jakub Jelinek changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org --- Comment #4 from Jakub Jelinek --- Created attachment 47452 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=47452=edit gcc10-pr92841.patch Untested fix.
[Bug target/92841] Optimize -fstack-protector-strong code generation a bit
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92841
Jakub Jelinek changed:
What|Removed |Added
Status|UNCONFIRMED |NEW
Last reconfirmed||2019-12-09
Ever confirmed|0 |1
--- Comment #3 from Jakub Jelinek ---
Reduced testcase that reproduces this with -O2 -fstack-protector-strong on
x86_64-linux -m64:
const struct S { int b; } c[] = {30, 12, 20, 0, 11};
void bar (int *);
void
foo (void)
{
int e[4];
const struct S *a;
for (a = c; a < c + sizeof (c); a++)
if (a->b)
bar (e);
}
