From fc4494dffa5422b2be5442c235554e76bed79c8a Mon Sep 17 00:00:00 2001 From: Nick Desaulniers Date: Thu, 13 Apr 2023 09:30:19 -0700 Subject: [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). https://github.com/ClangBuiltLinux/linux/issues/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 d656ae280957 and undid it globally: https://android-review.googlesource.com/c/platform/build/soong/+/2524336 Reviewed By: xiangzhangllvm Differential Revision: https://reviews.llvm.org/D147975 --- llvm/lib/CodeGen/StackProtector.cpp | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) (limited to 'llvm/lib/CodeGen/StackProtector.cpp') diff --git a/llvm/lib/CodeGen/StackProtector.cpp b/llvm/lib/CodeGen/StackProtector.cpp index 05ac176..387b653 100644 --- a/llvm/lib/CodeGen/StackProtector.cpp +++ b/llvm/lib/CodeGen/StackProtector.cpp @@ -482,18 +482,15 @@ bool StackProtector::InsertStackProtectors() { if (&BB == FailBB) continue; Instruction *CheckLoc = dyn_cast(BB.getTerminator()); - if (!CheckLoc && !DisableCheckNoReturn) { - for (auto &Inst : BB) { - auto *CB = dyn_cast(&Inst); - if (!CB) - continue; - if (!CB->doesNotReturn()) - continue; - // Do stack check before non-return calls (e.g: __cxa_throw) - CheckLoc = CB; - break; - } - } + if (!CheckLoc && !DisableCheckNoReturn) + for (auto &Inst : BB) + if (auto *CB = dyn_cast(&Inst)) + // Do stack check before noreturn calls that aren't nounwind (e.g: + // __cxa_throw). + if (CB->doesNotReturn() && !CB->doesNotThrow()) { + CheckLoc = CB; + break; + } if (!CheckLoc) continue; -- cgit v1.1