Skip to content

Commit bc044a8

Browse files
nickdesaulniersnickdesaulniers
committed
[Inline] prevent inlining on stack protector mismatch
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 exacerbates the issue. While developers can avoid this by putting all no_stack_protector functions in one translation unit together and compiling those with -fno-stack-protector, 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/[email protected]/ https://lore.kernel.org/lkml/[email protected]/T/#u SSP attributes can be ordered by strength. Weakest to strongest, they are: ssp, sspstrong, sspreq. Callees with differing SSP attributes may be inlined into each other, and the strongest attribute will be applied to the caller. (No change) After this change: * A callee with no SSP attributes will no longer be inlined into a caller with SSP attributes. * The reverse is also true: a callee with an SSP attribute will not be inlined into a caller with no SSP attributes. * The alwaysinline attribute overrides these rules. Functions that get synthesized by the compiler may not get inlined as a result if they are not created with the same stack protector function attribute as their callers. Alternative approach to https://reviews.llvm.org/D87956. Fixes pr/47479. Signed-off-by: Nick Desaulniers <[email protected]> Reviewed By: rnk, MaskRay Differential Revision: https://reviews.llvm.org/D91816
1 parent e5085a6 commit bc044a8

File tree

13 files changed

+283
-28
lines changed

13 files changed

+283
-28
lines changed

‎llvm/include/llvm/IR/Function.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,9 @@ class Function : public GlobalObject, public ilist_node<Function> {
381381
void setGC(std::string Str);
382382
void clearGC();
383383

384+
/// Returns true if the function has ssp, sspstrong, or sspreq fn attrs.
385+
bool hasStackProtectorFnAttr() const;
386+
384387
/// adds the attribute to the list of attributes.
385388
void addAttribute(unsigned i, Attribute::AttrKind Kind);
386389

‎llvm/lib/Analysis/InlineCost.cpp

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2382,6 +2382,15 @@ Optional<InlineResult> llvm::getAttributeBasedInliningDecision(
23822382
if (Call.isNoInline())
23832383
return InlineResult::failure("noinline call site attribute");
23842384

2385+
// Don't inline functions if one does not have any stack protector attribute
2386+
// but the other does.
2387+
if (Caller->hasStackProtectorFnAttr() && !Callee->hasStackProtectorFnAttr())
2388+
return InlineResult::failure(
2389+
"stack protected caller but callee requested no stack protector");
2390+
if (Callee->hasStackProtectorFnAttr() && !Caller->hasStackProtectorFnAttr())
2391+
return InlineResult::failure(
2392+
"stack protected callee but caller requested no stack protector");
2393+
23852394
return None;
23862395
}
23872396

@@ -2441,7 +2450,8 @@ InlineResult llvm::isInlineViable(Function &F) {
24412450
continue;
24422451

24432452
// Disallow recursive calls.
2444-
if (&F == Call->getCalledFunction())
2453+
Function *Callee = Call->getCalledFunction();
2454+
if (&F == Callee)
24452455
return InlineResult::failure("recursive call");
24462456

24472457
// Disallow calls which expose returns-twice to a function not previously
@@ -2450,8 +2460,8 @@ InlineResult llvm::isInlineViable(Function &F) {
24502460
cast<CallInst>(Call)->canReturnTwice())
24512461
return InlineResult::failure("exposes returns-twice attribute");
24522462

2453-
if (Call->getCalledFunction())
2454-
switch (Call->getCalledFunction()->getIntrinsicID()) {
2463+
if (Callee)
2464+
switch (Callee->getIntrinsicID()) {
24552465
default:
24562466
break;
24572467
case llvm::Intrinsic::icall_branch_funnel:

‎llvm/lib/CodeGen/StackProtector.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,6 @@ static const CallInst *findStackProtectorIntrinsic(Function &F) {
274274
bool StackProtector::RequiresStackProtector() {
275275
bool Strong = false;
276276
bool NeedsProtector = false;
277-
HasPrologue = findStackProtectorIntrinsic(*F);
278277

279278
if (F->hasFnAttribute(Attribute::SafeStack))
280279
return false;
@@ -295,8 +294,6 @@ bool StackProtector::RequiresStackProtector() {
295294
Strong = true; // Use the same heuristic as strong to determine SSPLayout
296295
} else if (F->hasFnAttribute(Attribute::StackProtectStrong))
297296
Strong = true;
298-
else if (HasPrologue)
299-
NeedsProtector = true;
300297
else if (!F->hasFnAttribute(Attribute::StackProtect))
301298
return false;
302299

‎llvm/lib/IR/Attributes.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1939,6 +1939,16 @@ static void setOR(Function &Caller, const Function &Callee) {
19391939
/// If the inlined function had a higher stack protection level than the
19401940
/// calling function, then bump up the caller's stack protection level.
19411941
static void adjustCallerSSPLevel(Function &Caller, const Function &Callee) {
1942+
#ifndef NDEBUG
1943+
if (!Callee.hasFnAttribute(Attribute::AlwaysInline)) {
1944+
assert(!(!Callee.hasStackProtectorFnAttr() &&
1945+
Caller.hasStackProtectorFnAttr()) &&
1946+
"stack protected caller but callee requested no stack protector");
1947+
assert(!(!Caller.hasStackProtectorFnAttr() &&
1948+
Callee.hasStackProtectorFnAttr()) &&
1949+
"stack protected callee but caller requested no stack protector");
1950+
}
1951+
#endif
19421952
// If upgrading the SSP attribute, clear out the old SSP Attributes first.
19431953
// Having multiple SSP attributes doesn't actually hurt, but it adds useless
19441954
// clutter to the IR.

‎llvm/lib/IR/Function.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,12 @@ void Function::clearGC() {
610610
setValueSubclassDataBit(14, false);
611611
}
612612

613+
bool Function::hasStackProtectorFnAttr() const {
614+
return hasFnAttribute(Attribute::StackProtect) ||
615+
hasFnAttribute(Attribute::StackProtectStrong) ||
616+
hasFnAttribute(Attribute::StackProtectReq);
617+
}
618+
613619
/// Copy all additional attributes (those not needed to create a Function) from
614620
/// the Function Src to this one.
615621
void Function::copyAttributesFrom(const Function *Src) {

‎llvm/test/CodeGen/AArch64/stack-guard-remat-bitcast.ll

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,53 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
12
; RUN: llc < %s -mtriple=arm64-apple-ios -relocation-model=pic -frame-pointer=all | FileCheck %s
23

34
@__stack_chk_guard = external global i64*
45

56
; PR20558
67

7-
; CHECK: adrp [[R0:x[0-9]+]], ___stack_chk_guard@GOTPAGE
8-
; CHECK: ldr [[R1:x[0-9]+]], {{\[}}[[R0]], ___stack_chk_guard@GOTPAGEOFF{{\]}}
8+
define i32 @test_stack_guard_remat2() ssp {
9+
; CHECK-LABEL: test_stack_guard_remat2:
10+
; CHECK: ; %bb.0: ; %entry
11+
; CHECK-NEXT: sub sp, sp, #64 ; =64
12+
; CHECK-NEXT: stp x29, x30, [sp, #48] ; 16-byte Folded Spill
13+
; CHECK-NEXT: add x29, sp, #48 ; =48
14+
; CHECK-NEXT: .cfi_def_cfa w29, 16
15+
; CHECK-NEXT: .cfi_offset w30, -8
16+
; CHECK-NEXT: .cfi_offset w29, -16
17+
; CHECK-NEXT: Lloh0:
18+
; CHECK-NEXT: adrp x8, ___stack_chk_guard@GOTPAGE
19+
; CHECK-NEXT: Lloh1:
920
; Load the stack guard for the second time, just in case the previous value gets spilled.
10-
; CHECK: adrp [[GUARD_PAGE:x[0-9]+]], ___stack_chk_guard@GOTPAGE
11-
; CHECK: ldr [[R2:x[0-9]+]], {{\[}}[[R1]]{{\]}}
12-
; CHECK: stur [[R2]], {{\[}}x29, [[SLOT0:[0-9#\-]+]]{{\]}}
13-
; CHECK: ldur [[R3:x[0-9]+]], {{\[}}x29, [[SLOT0]]{{\]}}
14-
; CHECK: ldr [[GUARD_ADDR:x[0-9]+]], {{\[}}[[GUARD_PAGE]], ___stack_chk_guard@GOTPAGEOFF{{\]}}
15-
; CHECK: ldr [[GUARD:x[0-9]+]], {{\[}}[[GUARD_ADDR]]{{\]}}
16-
; CHECK: cmp [[GUARD]], [[R3]]
17-
; CHECK: b.ne LBB
18-
19-
define i32 @test_stack_guard_remat2() {
21+
; CHECK-NEXT: adrp x9, ___stack_chk_guard@GOTPAGE
22+
; CHECK-NEXT: Lloh2:
23+
; CHECK-NEXT: ldr x8, [x8, ___stack_chk_guard@GOTPAGEOFF]
24+
; CHECK-NEXT: Lloh3:
25+
; CHECK-NEXT: ldr x9, [x9, ___stack_chk_guard@GOTPAGEOFF]
26+
; CHECK-NEXT: Lloh4:
27+
; CHECK-NEXT: ldr x8, [x8]
28+
; CHECK-NEXT: Lloh5:
29+
; CHECK-NEXT: ldr x9, [x9]
30+
; CHECK-NEXT: str x8, [sp]
31+
; CHECK-NEXT: stur x9, [x29, #-8]
32+
; CHECK-NEXT: Lloh6:
33+
; CHECK-NEXT: adrp x9, ___stack_chk_guard@GOTPAGE
34+
; CHECK-NEXT: ldur x8, [x29, #-8]
35+
; CHECK-NEXT: Lloh7:
36+
; CHECK-NEXT: ldr x9, [x9, ___stack_chk_guard@GOTPAGEOFF]
37+
; CHECK-NEXT: Lloh8:
38+
; CHECK-NEXT: ldr x9, [x9]
39+
; CHECK-NEXT: cmp x9, x8
40+
; CHECK-NEXT: b.ne LBB0_2
41+
; CHECK-NEXT: ; %bb.1: ; %entry
42+
; CHECK-NEXT: ldp x29, x30, [sp, #48] ; 16-byte Folded Reload
43+
; CHECK-NEXT: mov w0, #-1
44+
; CHECK-NEXT: add sp, sp, #64 ; =64
45+
; CHECK-NEXT: ret
46+
; CHECK-NEXT: LBB0_2: ; %entry
47+
; CHECK-NEXT: bl ___stack_chk_fail
48+
; CHECK-NEXT: .loh AdrpLdrGotLdr Lloh6, Lloh7, Lloh8
49+
; CHECK-NEXT: .loh AdrpLdrGotLdr Lloh1, Lloh3, Lloh5
50+
; CHECK-NEXT: .loh AdrpLdrGotLdr Lloh0, Lloh2, Lloh4
2051
entry:
2152
%StackGuardSlot = alloca i8*
2253
%StackGuard = load i8*, i8** bitcast (i64** @__stack_chk_guard to i8**)
@@ -26,5 +57,5 @@ entry:
2657
ret i32 -1
2758
}
2859

29-
declare void @llvm.stackprotector(i8*, i8**)
30-
declare void @llvm.stackprotectorcheck(i8**)
60+
declare void @llvm.stackprotector(i8*, i8**) ssp
61+
declare void @llvm.stackprotectorcheck(i8**) ssp

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,4 +162,34 @@ entry:
162162

163163
declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg)
164164

165+
; Intentionally does not have any fn attrs.
166+
declare dso_local void @foo(i8*)
167+
168+
; @bar_sspstrong and @bar_nossp are the same function, but differ only in
169+
; function attributes. Test that a callee without stack protector function
170+
; attribute does not trigger a stack guard slot in a caller that also does not
171+
; have a stack protector slot.
172+
define dso_local void @bar_sspstrong(i64 %0) #0 {
173+
; CHECK-LABEL: @bar_sspstrong
174+
; CHECK-NEXT: %StackGuardSlot = alloca i8*
175+
%2 = alloca i64, align 8
176+
store i64 %0, i64* %2, align 8
177+
%3 = load i64, i64* %2, align 8
178+
%4 = alloca i8, i64 %3, align 16
179+
call void @foo(i8* %4)
180+
ret void
181+
}
182+
183+
; Intentionally does not have any fn attrs.
184+
define dso_local void @bar_nossp(i64 %0) {
185+
; CHECK-LABEL: @bar_nossp
186+
; CHECK-NEXT: %2 = alloca i64
187+
%2 = alloca i64, align 8
188+
store i64 %0, i64* %2, align 8
189+
%3 = load i64, i64* %2, align 8
190+
%4 = alloca i8, i64 %3, align 16
191+
call void @foo(i8* %4)
192+
ret void
193+
}
194+
165195
attributes #0 = { sspstrong }

‎llvm/test/ThinLTO/X86/nossp.ll

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
; RUN: split-file %s %t
2+
; RUN: opt -module-summary %t/a.ll -o %a.bc
3+
; RUN: opt -module-summary %t/b.ll -o %b.bc
4+
; RUN: llvm-lto2 run %a.bc %b.bc -o %c.bc -save-temps \
5+
; RUN: -r=%a.bc,nossp_caller,px \
6+
; RUN: -r=%a.bc,ssp_caller,px \
7+
; RUN: -r=%a.bc,nossp_caller2,px \
8+
; RUN: -r=%a.bc,ssp_caller2,px \
9+
; RUN: -r=%a.bc,nossp_callee,x \
10+
; RUN: -r=%a.bc,ssp_callee,x \
11+
; RUN: -r=%b.bc,nossp_callee,px \
12+
; RUN: -r=%b.bc,ssp_callee,px \
13+
; RUN: -r=%b.bc,foo
14+
; RUN: llvm-dis %c.bc.1.4.opt.bc -o - | FileCheck %s
15+
16+
;--- a.ll
17+
18+
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
19+
target triple = "x86_64-pc-linux-gnu"
20+
21+
declare void @nossp_callee()
22+
declare void @ssp_callee() ssp
23+
24+
; nossp caller should be able to inline nossp callee.
25+
define void @nossp_caller() {
26+
; CHECK-LABEL: @nossp_caller
27+
; CHECK-NEXT: tail call void @foo
28+
tail call void @nossp_callee()
29+
ret void
30+
}
31+
32+
; ssp caller should be able to inline ssp callee.
33+
define void @ssp_caller() ssp {
34+
; CHECK-LABEL: @ssp_caller
35+
; CHECK-NEXT: tail call void @foo
36+
tail call void @ssp_callee()
37+
ret void
38+
}
39+
40+
; nossp caller should *NOT* be able to inline ssp callee.
41+
define void @nossp_caller2() {
42+
; CHECK-LABEL: @nossp_caller2
43+
; CHECK-NEXT: tail call void @ssp_callee
44+
tail call void @ssp_callee()
45+
ret void
46+
}
47+
48+
; ssp caller should *NOT* be able to inline nossp callee.
49+
define void @ssp_caller2() ssp {
50+
; CHECK-LABEL: @ssp_caller2
51+
; CHECK-NEXT: tail call void @nossp_callee
52+
tail call void @nossp_callee()
53+
ret void
54+
}
55+
56+
;--- b.ll
57+
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
58+
target triple = "x86_64-pc-linux-gnu"
59+
60+
declare void @foo()
61+
62+
define void @nossp_callee() {
63+
call void @foo()
64+
ret void
65+
}
66+
67+
define void @ssp_callee() ssp {
68+
call void @foo()
69+
ret void
70+
}

‎llvm/test/Transforms/CodeExtractor/PartialInlineAttributes.ll

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ if.end:
3737
ret i32 %add
3838
}
3939

40-
define i32 @callee_writeonly(i32 %v) writeonly {
40+
define i32 @callee_writeonly(i32 %v) writeonly ssp {
4141
entry:
4242
%cmp = icmp sgt i32 %v, 2000
4343
br i1 %cmp, label %if.then, label %if.end
@@ -58,15 +58,15 @@ if.end:
5858
; CHECK: call void @callee_most.2.if.then(i32 %v
5959
; CHECK: call i32 @callee_noinline(i32 %v)
6060
; CHECK: call void @callee_writeonly.1.if.then(i32 %v
61-
define i32 @caller(i32 %v) {
61+
define i32 @caller(i32 %v) ssp {
6262
entry:
6363
%c1 = call i32 @callee_most(i32 %v)
6464
%c2 = call i32 @callee_noinline(i32 %v)
6565
%c3 = call i32 @callee_writeonly(i32 %v)
6666
ret i32 %c3
6767
}
6868

69-
; CHECK: define internal void @callee_writeonly.1.if.then(i32 %v, i32* %sub.out) {
69+
; CHECK: define internal void @callee_writeonly.1.if.then(i32 %v, i32* %sub.out) [[FN_ATTRS0:#[0-9]+]]
7070
; CHECK: define internal void @callee_most.2.if.then(i32 %v, i32* %sub.out) [[FN_ATTRS:#[0-9]+]]
7171

7272
; attributes to preserve
@@ -76,6 +76,7 @@ attributes #0 = {
7676
sanitize_thread ssp sspreq sspstrong strictfp uwtable "foo"="bar"
7777
"patchable-function"="prologue-short-redirect" "probe-stack"="_foo_guard" "stack-probe-size"="4096" }
7878

79+
; CHECK: attributes [[FN_ATTRS0]] = { ssp
7980
; 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" }
8081

8182
; attributes to drop

‎llvm/test/Transforms/Inline/devirtualize.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ entry:
9898
ret i32 4
9999
}
100100

101-
define linkonce_odr i32 @_ZThn8_N1D1fEv(%struct.C* %this) {
101+
define linkonce_odr i32 @_ZThn8_N1D1fEv(%struct.C* %this) ssp {
102102
entry:
103103
%0 = bitcast %struct.C* %this to i8* ; <i8*> [#uses=1]
104104
%1 = getelementptr inbounds i8, i8* %0, i64 -8 ; <i8*> [#uses=1]

‎llvm/test/Transforms/Inline/inline-byval-bonus.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ target triple = "x86_64-apple-macosx10.8.0"
1515
%struct.ray = type { %struct.vec3, %struct.vec3 }
1616
%struct.spoint = type { %struct.vec3, %struct.vec3, %struct.vec3, double }
1717

18-
define i32 @caller(%struct.sphere* %i) {
18+
define i32 @caller(%struct.sphere* %i) ssp {
1919
%shadow_ray = alloca %struct.ray, align 8
2020
call void @fix(%struct.ray* %shadow_ray)
2121

0 commit comments

Comments
 (0)