lkml.org 
[lkml]   [2018]   [May]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
Subject[clang] stack protector and f1f029c7bf
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
--
Thanks,
~Nick Desaulniers

\
 
 \ /
  Last update: 2018-05-24 00:08    [W:0.172 / U:0.856 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site