From 89071447a2e44431f9985692679575cb12897bcb Mon Sep 17 00:00:00 2001 From: Leonard Chan Date: Thu, 12 Jun 2025 14:26:35 -0700 Subject: [PATCH] [llvm][StackProtector] Add noreturn to __stack_chk_fail aliassee It's possible for __stack_chk_fail to be an alias when using CrossDSOCFI since it will make a jump table entry for this function and replace it with an alias. StackProtector can crash since it always expects this to be a regular function. I think it should be safe to continue treating this as an alias since we only call into it, and we can apply the noreturn attr to the underlying function if it is an alias. --- llvm/lib/CodeGen/StackProtector.cpp | 4 +-- .../cross-dso-cfi-stack-chk-fail.ll | 33 +++++++++++++++++++ .../StackProtector/stack-chk-fail-alias.ll | 21 ++++++++++++ 3 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 llvm/test/Transforms/StackProtector/cross-dso-cfi-stack-chk-fail.ll create mode 100644 llvm/test/Transforms/StackProtector/stack-chk-fail-alias.ll diff --git a/llvm/lib/CodeGen/StackProtector.cpp b/llvm/lib/CodeGen/StackProtector.cpp index 5f866eea7d4e7..dda392d38b27a 100644 --- a/llvm/lib/CodeGen/StackProtector.cpp +++ b/llvm/lib/CodeGen/StackProtector.cpp @@ -725,8 +725,8 @@ BasicBlock *CreateFailBB(Function *F, const Triple &Trip) { StackChkFail = M->getOrInsertFunction("__stack_chk_fail", Type::getVoidTy(Context)); } - cast(StackChkFail.getCallee())->addFnAttr(Attribute::NoReturn); - B.CreateCall(StackChkFail, Args); + CallInst *Call = B.CreateCall(StackChkFail, Args); + Call->addFnAttr(Attribute::NoReturn); B.CreateUnreachable(); return FailBB; } diff --git a/llvm/test/Transforms/StackProtector/cross-dso-cfi-stack-chk-fail.ll b/llvm/test/Transforms/StackProtector/cross-dso-cfi-stack-chk-fail.ll new file mode 100644 index 0000000000000..af03039813a2e --- /dev/null +++ b/llvm/test/Transforms/StackProtector/cross-dso-cfi-stack-chk-fail.ll @@ -0,0 +1,33 @@ +;; This is a minimal reproducer that caused StackProtector to crash with a bad cast when +;; CrossDSOCFI is used. This test just needs to not crash. +; RUN: opt -mtriple=x86_64-pc-linux-gnu %s -passes=lowertypetests,cross-dso-cfi,stack-protector + +define hidden void @__stack_chk_fail() !type !1{ + unreachable +} + +define void @store_captures() sspstrong { +entry: + %a = alloca i32, align 4 + %j = alloca ptr, align 8 + store ptr %a, ptr %j, align 8 + ret void +} + +define void @func(ptr %0) { +entry: + %1 = call i1 @llvm.type.test(ptr %0, metadata !"typeid") + br i1 %1, label %cont, label %trap + +trap: ; preds = %entry + call void @llvm.trap() + unreachable + +cont: ; preds = %entry + call void %0() + ret void +} + +!llvm.module.flags = !{!0} +!0 = !{i32 4, !"Cross-DSO CFI", i32 1} +!1 = !{i64 0, !"typeid"} diff --git a/llvm/test/Transforms/StackProtector/stack-chk-fail-alias.ll b/llvm/test/Transforms/StackProtector/stack-chk-fail-alias.ll new file mode 100644 index 0000000000000..ab0a6e3f455e7 --- /dev/null +++ b/llvm/test/Transforms/StackProtector/stack-chk-fail-alias.ll @@ -0,0 +1,21 @@ +;; __stack_chk_fail should have the noreturn attr even if it is an alias +; RUN: opt -mtriple=x86_64-pc-linux-gnu %s -passes=stack-protector -S | FileCheck %s + +define hidden void @__stack_chk_fail_impl() { + unreachable +} + +@__stack_chk_fail = hidden alias void (), ptr @__stack_chk_fail_impl + +; CHECK-LABEL: @store_captures( +; CHECK: CallStackCheckFailBlk: +; CHECK-NEXT: call void @__stack_chk_fail() [[ATTRS:#.*]] +define void @store_captures() sspstrong { +entry: + %a = alloca i32, align 4 + %j = alloca ptr, align 8 + store ptr %a, ptr %j, align 8 + ret void +} + +; CHECK: attributes [[ATTRS]] = { noreturn }