nickdesaulniers created this revision.
Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, steven_wu,
hiraditya.
Herald added a reviewer: jdoerfert.
Herald added projects: clang, LLVM.
nickdesaulniers requested review of this revision.
It's currently ambiguous in IR whether the source language explicitly
did not want a stack a stack protector (in C, via function attribute
no_stack_protector) or doesn't care for any given function.
Prior to this, developers only had translation unit granularity to
enable or disable stack protectors via command line flags. Now we can do
so on a per function level granularity.
It's common for code that manipulates the stack via inline assembly or
that has to set up its own stack canary (such as the Linux kernel) would
like to avoid stack protectors in certain functions. In this case, we've
been bitten by numerous bugs where a callee with a stack protector is
inlined into an __attribute__((__no_stack_protector__)) caller, which
generally breaks the caller's assumptions about not having a stack
protector. LTO can exacerbate the issue.
While developers can avoid this by putting all no_stack_protector
functions in one translation unit together, it's generally not very
ergonomic or as ergonomic as a function attribute, and still doesn't
work for LTO. See also:
https://lore.kernel.org/linux-pm/20200915172658.1432732-1-r...@google.com/
Typically, when inlining a callee into a caller, the caller will be
upgraded in its level of stack protection (see adjustCallerSSPLevel()).
By adding an explicit attribute in the IR when the function attribute is
used in the source language, we can now identify such cases and prevent
inlining.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D87956
Files:
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGen/stack-protector.c
llvm/include/llvm/Bitcode/LLVMBitCodes.h
llvm/include/llvm/IR/Attributes.td
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/AsmParser/LLToken.h
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/CodeGen/SafeStack.cpp
llvm/lib/CodeGen/StackProtector.cpp
llvm/lib/IR/Attributes.cpp
llvm/lib/IR/Verifier.cpp
llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
llvm/lib/Transforms/Utils/CodeExtractor.cpp
llvm/lib/Transforms/Utils/InlineFunction.cpp
llvm/test/Transforms/CodeExtractor/PartialInlineAttributes.ll
llvm/test/Transforms/Inline/inline_ssp.ll
Index: llvm/test/Transforms/Inline/inline_ssp.ll
===================================================================
--- llvm/test/Transforms/Inline/inline_ssp.ll
+++ llvm/test/Transforms/Inline/inline_ssp.ll
@@ -2,6 +2,8 @@
; RUN: opt -passes='cgscc(inline)' %s -S | FileCheck %s
; Ensure SSP attributes are propagated correctly when inlining.
+; TODO: ensure no_stack_protector are not inlined.
+
@.str = private unnamed_addr constant [11 x i8] c"fun_nossp\0A\00", align 1
@.str1 = private unnamed_addr constant [9 x i8] c"fun_ssp\0A\00", align 1
@.str2 = private unnamed_addr constant [15 x i8] c"fun_sspstrong\0A\00", align 1
Index: llvm/test/Transforms/CodeExtractor/PartialInlineAttributes.ll
===================================================================
--- llvm/test/Transforms/CodeExtractor/PartialInlineAttributes.ll
+++ llvm/test/Transforms/CodeExtractor/PartialInlineAttributes.ll
@@ -73,10 +73,10 @@
attributes #0 = {
inlinehint minsize noduplicate noimplicitfloat norecurse noredzone nounwind
nonlazybind optsize safestack sanitize_address sanitize_hwaddress sanitize_memory
- sanitize_thread ssp sspreq sspstrong strictfp uwtable "foo"="bar"
+ sanitize_thread no-ssp ssp sspreq sspstrong strictfp uwtable "foo"="bar"
"patchable-function"="prologue-short-redirect" "probe-stack"="_foo_guard" "stack-probe-size"="4096" }
-; CHECK: attributes [[FN_ATTRS]] = { inlinehint minsize noduplicate noimplicitfloat norecurse noredzone nounwind nonlazybind optsize safestack sanitize_address sanitize_hwaddress sanitize_memory sanitize_thread ssp sspreq sspstrong strictfp uwtable "foo"="bar" "patchable-function"="prologue-short-redirect" "probe-stack"="_foo_guard" "stack-probe-size"="4096" }
+; CHECK: attributes [[FN_ATTRS]] = { inlinehint minsize noduplicate noimplicitfloat norecurse noredzone nounwind nonlazybind optsize safestack sanitize_address sanitize_hwaddress sanitize_memory sanitize_thread no-ssp ssp sspreq sspstrong strictfp uwtable "foo"="bar" "patchable-function"="prologue-short-redirect" "probe-stack"="_foo_guard" "stack-probe-size"="4096" }
; attributes to drop
attributes #1 = {
Index: llvm/lib/Transforms/Utils/InlineFunction.cpp
===================================================================
--- llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -1676,6 +1676,16 @@
return InlineResult::failure("incompatible GC");
}
+ // Inlining a function the explicitly should not have a stack protector may
+ // break the code if inlined into a function that does have a stack
+ // protector.
+ if (Caller->hasFnAttribute(Attribute::NoStackProtect))
+ if (CalledFunc->hasFnAttribute(Attribute::StackProtect) ||
+ CalledFunc->hasFnAttribute(Attribute::StackProtectStrong) ||
+ CalledFunc->hasFnAttribute(Attribute::StackProtectReq))
+ return InlineResult::failure(
+ "no_stack_protected caller would inline stack protected callee");
+
// Get the personality function from the callee if it contains a landing pad.
Constant *CalledPersonality =
CalledFunc->hasPersonalityFn()
Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===================================================================
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -925,6 +925,7 @@
case Attribute::SanitizeHWAddress:
case Attribute::SanitizeMemTag:
case Attribute::SpeculativeLoadHardening:
+ case Attribute::NoStackProtect:
case Attribute::StackProtect:
case Attribute::StackProtectReq:
case Attribute::StackProtectStrong:
Index: llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
===================================================================
--- llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
+++ llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
@@ -68,6 +68,7 @@
.Case("sanitize_thread", Attribute::SanitizeThread)
.Case("sanitize_memtag", Attribute::SanitizeMemTag)
.Case("speculative_load_hardening", Attribute::SpeculativeLoadHardening)
+ .Case("no-ssp", Attribute::NoStackProtect)
.Case("ssp", Attribute::StackProtect)
.Case("sspreq", Attribute::StackProtectReq)
.Case("sspstrong", Attribute::StackProtectStrong)
Index: llvm/lib/IR/Verifier.cpp
===================================================================
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1572,6 +1572,7 @@
case Attribute::NoInline:
case Attribute::AlwaysInline:
case Attribute::OptimizeForSize:
+ case Attribute::NoStackProtect:
case Attribute::StackProtect:
case Attribute::StackProtectReq:
case Attribute::StackProtectStrong:
Index: llvm/lib/IR/Attributes.cpp
===================================================================
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -421,6 +421,8 @@
return "speculative_load_hardening";
if (hasAttribute(Attribute::Speculatable))
return "speculatable";
+ if (hasAttribute(Attribute::NoStackProtect))
+ return "no-ssp";
if (hasAttribute(Attribute::StackProtect))
return "ssp";
if (hasAttribute(Attribute::StackProtectReq))
@@ -1894,6 +1896,7 @@
/// If the inlined function had a higher stack protection level than the
/// calling function, then bump up the caller's stack protection level.
static void adjustCallerSSPLevel(Function &Caller, const Function &Callee) {
+ // TODO: what if Callee has NoStackProtect?
// If upgrading the SSP attribute, clear out the old SSP Attributes first.
// Having multiple SSP attributes doesn't actually hurt, but it adds useless
// clutter to the IR.
Index: llvm/lib/CodeGen/StackProtector.cpp
===================================================================
--- llvm/lib/CodeGen/StackProtector.cpp
+++ llvm/lib/CodeGen/StackProtector.cpp
@@ -284,6 +284,7 @@
// are not available this late in the IR pipeline.
OptimizationRemarkEmitter ORE(F);
+ // TODO: no stack protector?
if (F->hasFnAttribute(Attribute::StackProtectReq)) {
ORE.emit([&]() {
return OptimizationRemark(DEBUG_TYPE, "StackProtectorRequested", F)
Index: llvm/lib/CodeGen/SafeStack.cpp
===================================================================
--- llvm/lib/CodeGen/SafeStack.cpp
+++ llvm/lib/CodeGen/SafeStack.cpp
@@ -805,6 +805,7 @@
AllocaInst *StackGuardSlot = nullptr;
// FIXME: implement weaker forms of stack protector.
+ // TODO: NoStackProtect?
if (F.hasFnAttribute(Attribute::StackProtect) ||
F.hasFnAttribute(Attribute::StackProtectStrong) ||
F.hasFnAttribute(Attribute::StackProtectReq)) {
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===================================================================
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -697,6 +697,8 @@
return bitc::ATTR_KIND_SPECULATABLE;
case Attribute::StackAlignment:
return bitc::ATTR_KIND_STACK_ALIGNMENT;
+ case Attribute::NoStackProtect:
+ return bitc::ATTR_KIND_NO_STACK_PROTECT;
case Attribute::StackProtect:
return bitc::ATTR_KIND_STACK_PROTECT;
case Attribute::StackProtectReq:
Index: llvm/lib/AsmParser/LLToken.h
===================================================================
--- llvm/lib/AsmParser/LLToken.h
+++ llvm/lib/AsmParser/LLToken.h
@@ -223,6 +223,7 @@
kw_returns_twice,
kw_signext,
kw_speculatable,
+ kw_nossp,
kw_ssp,
kw_sspreq,
kw_sspstrong,
Index: llvm/lib/AsmParser/LLParser.cpp
===================================================================
--- llvm/lib/AsmParser/LLParser.cpp
+++ llvm/lib/AsmParser/LLParser.cpp
@@ -1335,6 +1335,9 @@
case lltok::kw_returns_twice:
B.addAttribute(Attribute::ReturnsTwice); break;
case lltok::kw_speculatable: B.addAttribute(Attribute::Speculatable); break;
+ case lltok::kw_nossp:
+ B.addAttribute(Attribute::NoStackProtect);
+ break;
case lltok::kw_ssp: B.addAttribute(Attribute::StackProtect); break;
case lltok::kw_sspreq: B.addAttribute(Attribute::StackProtectReq); break;
case lltok::kw_sspstrong:
@@ -1739,6 +1742,7 @@
case lltok::kw_sanitize_memory:
case lltok::kw_sanitize_thread:
case lltok::kw_speculative_load_hardening:
+ case lltok::kw_nossp:
case lltok::kw_ssp:
case lltok::kw_sspreq:
case lltok::kw_sspstrong:
@@ -1843,6 +1847,7 @@
case lltok::kw_sanitize_memory:
case lltok::kw_sanitize_thread:
case lltok::kw_speculative_load_hardening:
+ case lltok::kw_nossp:
case lltok::kw_ssp:
case lltok::kw_sspreq:
case lltok::kw_sspstrong:
Index: llvm/include/llvm/IR/Attributes.td
===================================================================
--- llvm/include/llvm/IR/Attributes.td
+++ llvm/include/llvm/IR/Attributes.td
@@ -179,6 +179,9 @@
/// Function can be speculated.
def Speculatable : EnumAttr<"speculatable">;
+/// Stack protection explicitly disabled.
+def NoStackProtect : EnumAttr<"no-ssp">;
+
/// Stack protection.
def StackProtect : EnumAttr<"ssp">;
Index: llvm/include/llvm/Bitcode/LLVMBitCodes.h
===================================================================
--- llvm/include/llvm/Bitcode/LLVMBitCodes.h
+++ llvm/include/llvm/Bitcode/LLVMBitCodes.h
@@ -606,6 +606,7 @@
ATTR_KIND_RETURNS_TWICE = 23,
ATTR_KIND_S_EXT = 24,
ATTR_KIND_STACK_ALIGNMENT = 25,
+ ATTR_KIND_NO_STACK_PROTECT = 70,
ATTR_KIND_STACK_PROTECT = 26,
ATTR_KIND_STACK_PROTECT_REQ = 27,
ATTR_KIND_STACK_PROTECT_STRONG = 28,
Index: clang/test/CodeGen/stack-protector.c
===================================================================
--- clang/test/CodeGen/stack-protector.c
+++ clang/test/CodeGen/stack-protector.c
@@ -36,7 +36,7 @@
// SSPREQ: attributes #[[A]] = {{.*}} sspreq
// SAFESTACK-NOSSP: attributes #[[A]] = {{.*}} safestack
-// SAFESTACK-NOSSP-NOT: ssp
+// SAFESTACK-NOSSP-NOT: attribute #[[A]] = {{.*}} ssp
// SAFESTACK-SSP: attributes #[[A]] = {{.*}} safestack ssp{{ }}
// SAFESTACK-SSPSTRONG: attributes #[[A]] = {{.*}} safestack sspstrong
@@ -47,6 +47,11 @@
// SSPSTRONG-NOT: attributes #[[B]] = {{.*}} sspstrong
// SSPREQ-NOT: attributes #[[B]] = {{.*}} sspreq
+// NOSSP: attributes #[[B]] = {{.*}} no-ssp
+// SSP: attributes #[[B]] = {{.*}} no-ssp
+// SSPSTRONG: attributes #[[B]] = {{.*}} no-ssp
+// SSPREQ: attributes #[[B]] = {{.*}} no-ssp
+
// SAFESTACK-SSP: attributes #[[B]] = {{.*}} safestack
// SAFESTACK-SSP-NOT: attributes #[[B]] = {{.*}} safestack ssp{{ }}
// SAFESTACK-SSPSTRONG: attributes #[[B]] = {{.*}} safestack
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1577,14 +1577,14 @@
if (!hasUnwindExceptions(LangOpts))
B.addAttribute(llvm::Attribute::NoUnwind);
- if (!D || !D->hasAttr<NoStackProtectorAttr>()) {
- if (LangOpts.getStackProtector() == LangOptions::SSPOn)
- B.addAttribute(llvm::Attribute::StackProtect);
- else if (LangOpts.getStackProtector() == LangOptions::SSPStrong)
- B.addAttribute(llvm::Attribute::StackProtectStrong);
- else if (LangOpts.getStackProtector() == LangOptions::SSPReq)
- B.addAttribute(llvm::Attribute::StackProtectReq);
- }
+ if (D && D->hasAttr<NoStackProtectorAttr>())
+ B.addAttribute(llvm::Attribute::NoStackProtect);
+ else if (LangOpts.getStackProtector() == LangOptions::SSPOn)
+ B.addAttribute(llvm::Attribute::StackProtect);
+ else if (LangOpts.getStackProtector() == LangOptions::SSPStrong)
+ B.addAttribute(llvm::Attribute::StackProtectStrong);
+ else if (LangOpts.getStackProtector() == LangOptions::SSPReq)
+ B.addAttribute(llvm::Attribute::StackProtectReq);
if (!D) {
// If we don't have a declaration to control inlining, the function isn't
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
