[clang] stack protector and f1f029c7bf

* [clang] stack protector and f1f029c7bf
@ 2018-05-23 22:08 Nick Desaulniers
  2018-05-24 10:33 ` Sedat Dilek
                   ` (2 more replies)
  0 siblings, 3 replies; 36+ messages in thread
From: Nick Desaulniers @ 2018-05-23 22:08 UTC (permalink / raw)
  To: hpa
  Cc: Alistair Strachan, Manoj Gupta, Matthias Kaehlcke, Greg Hackmann,
	sedat.dilek, tstellar, LKML

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

^ permalink raw reply	[flat|nested] 36+ messages in thread