Messages in this thread | | | Date | Thu, 24 May 2018 11:19:55 -0700 | Subject | Re: [clang] stack protector and f1f029c7bf | From | hpa@zytor ... |
| |
On May 23, 2018 3:08:19 PM PDT, Nick Desaulniers <ndesaulniers@google.com> wrote: >H. Peter, > >It was reported [0] that compiling the Linux kernel with Clang + >CC_STACKPROTECTOR_STRONG was causing a crash in native_save_fl(), due >to >how GCC does not emit a stack guard for static inline functions (see >Alistair's excellent report in [1]) but Clang does. > >When working with the LLVM release maintainers, Tom had suggested [2] >changing the inline assembly constraint in native_save_fl() from '=rm' >to >'=r', and Alistair had verified the disassembly: > >(good) code generated w/o -fstack-protector-strong: > >native_save_fl: > pushfq > popq -8(%rsp) > movq -8(%rsp), %rax > retq > >(good) code generated w/ =r input constraint: > >native_save_fl: > pushfq > popq %rax > retq > >(bad) code generated with -fstack-protector-strong: > >native_save_fl: > subq $24, %rsp > movq %fs:40, %rax > movq %rax, 16(%rsp) > pushfq > popq 8(%rsp) > movq 8(%rsp), %rax > movq %fs:40, %rcx > cmpq 16(%rsp), %rcx > jne .LBB0_2 > addq $24, %rsp > retq >.LBB0_2: > callq __stack_chk_fail > >It looks like the sugguestion is actually a revert of your commit: >ab94fcf528d127fcb490175512a8910f37e5b346: >x86: allow "=rm" in native_save_fl() > >It seemed like there was a question internally about why worry about >pop >adjusting the stack if the stack could be avoided altogether. > >I think Sedat can retest this, but I was curious as well about the >commit >message in ab94fcf528d: "[ Impact: performance ]", but Alistair's >analysis >of the disassembly seems to indicate there is no performance impact (in >fact, looks better as there's one less mov). > >Is there a reason we should not revert ab94fcf528d12, or maybe a better >approach? > >[0] https://lkml.org/lkml/2018/5/7/534 >[1] https://bugs.llvm.org/show_bug.cgi?id=37512#c15 >[2] https://bugs.llvm.org/show_bug.cgi?id=37512#c22
A stack canary on an *inlined* function? That's bound to break things elsewhere too sooner or later.
It feels like *once again* clang is asking for the Linux kernel to change to paper over technical or compatibility problems in clang/LLVM. This is not exactly helping the feeling that we should just rip out any and all clang hacks and call it a loss. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
| |