Re: [clang] stack protector and f1f029c7bf

From: hpa
Date: Fri May 25 2018 - 07:00:41 EST


On May 23, 2018 3:08:19 PM PDT, Nick Desaulniers <ndesaulniers@xxxxxxxxxx> 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.