Skip to content

Commit bf80902

Browse files
nickdesaulnierstru
authored andcommitted
[StackProtector] don't check stack protector before calling nounwind functions
https://reviews.llvm.org/rGd656ae28095726830f9beb8dbd4d69f5144ef821 introduced a additional checks before calling noreturn functions in response to this security paper related to Catch Handler Oriented Programming (CHOP): https://download.vusec.net/papers/chop_ndss23.pdf See also: https://bugs.chromium.org/p/llvm/issues/detail?id=30 This causes stack canaries to be inserted in C code which was unexpected; we noticed certain Linux kernel trees stopped booting after this (in functions trying to initialize the stack canary itself). ClangBuiltLinux/linux#1815 There is no point checking the stack canary like this when exceptions are disabled (-fno-exceptions or function is marked noexcept) or for C code. The GCC patch for this issue does something similar: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=a25982ada523689c8745d7fb4b1b93c8f5dab2e7 Android measured a 2% regression in RSS as a result of d656ae2 and undid it globally: https://android-review.googlesource.com/c/platform/build/soong/+/2524336 Reviewed By: xiangzhangllvm Differential Revision: https://reviews.llvm.org/D147975 (cherry picked from commit fc4494d)
1 parent 82432ac commit bf80902

File tree

3 files changed

+23
-17
lines changed

3 files changed

+23
-17
lines changed

llvm/lib/CodeGen/StackProtector.cpp

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -455,18 +455,15 @@ bool StackProtector::InsertStackProtectors() {
455455
if (&BB == FailBB)
456456
continue;
457457
Instruction *CheckLoc = dyn_cast<ReturnInst>(BB.getTerminator());
458-
if (!CheckLoc && !DisableCheckNoReturn) {
459-
for (auto &Inst : BB) {
460-
auto *CB = dyn_cast<CallBase>(&Inst);
461-
if (!CB)
462-
continue;
463-
if (!CB->doesNotReturn())
464-
continue;
465-
// Do stack check before non-return calls (e.g: __cxa_throw)
466-
CheckLoc = CB;
467-
break;
468-
}
469-
}
458+
if (!CheckLoc && !DisableCheckNoReturn)
459+
for (auto &Inst : BB)
460+
if (auto *CB = dyn_cast<CallBase>(&Inst))
461+
// Do stack check before noreturn calls that aren't nounwind (e.g:
462+
// __cxa_throw).
463+
if (CB->doesNotReturn() && !CB->doesNotThrow()) {
464+
CheckLoc = CB;
465+
break;
466+
}
470467

471468
if (!CheckLoc)
472469
continue;

llvm/test/CodeGen/X86/stack-protector-2.ll

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
; RUN: llc -mtriple=x86_64-pc-linux-gnu -start-before=stack-protector -stop-after=stack-protector -o - < %s | FileCheck %s
1+
; RUN: llc -mtriple=x86_64-pc-linux-gnu -start-before=stack-protector \
2+
; RUN: -stop-after=stack-protector -o - < %s | FileCheck %s
23
; Bugs 42238/43308: Test some additional situations not caught previously.
34

45
define void @store_captures() #0 {
@@ -219,5 +220,14 @@ return: ; preds = %if.end, %if.then
219220
ret i32 0
220221
}
221222

223+
declare void @callee() noreturn nounwind
224+
define void @caller() sspstrong {
225+
; Test that a stack protector is NOT inserted when we call nounwind functions.
226+
; CHECK-LABEL: @caller
227+
; CHECK-NEXT: call void @callee
228+
call void @callee() noreturn nounwind
229+
ret void
230+
}
231+
222232
attributes #0 = { sspstrong }
223233
attributes #1 = { noreturn sspreq}

llvm/test/CodeGen/X86/stack-protector-recursively.ll

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,14 @@ define dso_local void @__stack_chk_fail() local_unnamed_addr #0 {
1212
; CHECK-NEXT: cmpq (%rsp), %rax
1313
; CHECK-NEXT: jne .LBB0_2
1414
; CHECK-NEXT: # %bb.1: # %SP_return
15-
; CHECK-NEXT: ud2
15+
; CHECK-NEXT: callq foo@PLT
1616
; CHECK-NEXT: .LBB0_2: # %CallStackCheckFailBlk
1717
; CHECK-NEXT: callq __stack_chk_fail
1818
entry:
19-
tail call void @llvm.trap()
19+
tail call void @foo() noreturn
2020
unreachable
2121
}
2222

23-
declare void @llvm.trap() #1
23+
declare void @foo() noreturn
2424

2525
attributes #0 = { noreturn nounwind sspreq }
26-
attributes #1 = { noreturn nounwind }

0 commit comments

Comments
 (0)