Skip to content

SILOptimizer: avoid stack protection in two cases where it's not needed #63676

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -486,13 +486,23 @@ private extension Instruction {
if !atp.needsStackProtection {
return nil
}
var hasNoStores = NoStores()
if hasNoStores.walkDownUses(ofValue: atp, path: SmallProjectionPath()) == .continueWalk {
return nil
}

// The result of an `address_to_pointer` may be used in any unsafe way, e.g.
// passed to a C function.
baseAddr = atp.operand
case let ia as IndexAddrInst:
if !ia.needsStackProtection {
return nil
}
var hasNoStores = NoStores()
if hasNoStores.walkDownUses(ofAddress: ia, path: SmallProjectionPath()) == .continueWalk {
return nil
}

// `index_addr` is unsafe if not used for tail-allocated elements (e.g. in Array).
baseAddr = ia.base
default:
Expand All @@ -509,6 +519,29 @@ private extension Instruction {
}
}

/// Checks if there are no stores to an address or raw pointer.
private struct NoStores : ValueDefUseWalker, AddressDefUseWalker {
var walkDownCache = WalkerCache<SmallProjectionPath>()

mutating func leafUse(value: Operand, path: SmallProjectionPath) -> WalkResult {
if let ptai = value.instruction as? PointerToAddressInst {
return walkDownUses(ofAddress: ptai, path: path)
}
return .abortWalk
}

mutating func leafUse(address: Operand, path: SmallProjectionPath) -> WalkResult {
switch address.instruction {
case is LoadInst:
return .continueWalk
case let cai as CopyAddrInst:
return address == cai.sourceOperand ? .continueWalk : .abortWalk
default:
return .abortWalk
}
}
}

private extension Function {
func setNeedsStackProtection(_ context: FunctionPassContext) {
if !needsStackProtection {
Expand Down
4 changes: 4 additions & 0 deletions include/swift/AST/Builtins.def
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,10 @@ BUILTIN_MISC_OPERATION(DeallocRaw, "deallocRaw", "", Special)
/// is not known at compile time, MaximumAlignment is assumed.
BUILTIN_MISC_OPERATION(StackAlloc, "stackAlloc", "", Special)

/// Like `stackAlloc`, but doesn't set the `[stack_protection]` flag on its
/// containing function.
BUILTIN_MISC_OPERATION(UnprotectedStackAlloc, "unprotectedStackAlloc", "", Special)

/// StackDealloc has type (Builtin.RawPointer) -> ()
///
/// Parameters: address.
Expand Down
1 change: 1 addition & 0 deletions include/swift/Basic/Features.def
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ LANGUAGE_FEATURE(BuiltinBuildMainExecutor, 0, "MainActor executor building built
LANGUAGE_FEATURE(BuiltinCreateAsyncTaskInGroup, 0, "MainActor executor building builtin", true)
LANGUAGE_FEATURE(BuiltinCopy, 0, "Builtin.copy()", true)
LANGUAGE_FEATURE(BuiltinStackAlloc, 0, "Builtin.stackAlloc", true)
LANGUAGE_FEATURE(BuiltinUnprotectedStackAlloc, 0, "Builtin.unprotectedStackAlloc", true)
LANGUAGE_FEATURE(BuiltinTaskRunInline, 0, "Builtin.taskRunInline", true)
LANGUAGE_FEATURE(BuiltinUnprotectedAddressOf, 0, "Builtin.unprotectedAddressOf", true)
SUPPRESSIBLE_LANGUAGE_FEATURE(SpecializeAttributeWithAvailability, 0, "@_specialize attribute with availability", true)
Expand Down
4 changes: 4 additions & 0 deletions lib/AST/ASTPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3020,6 +3020,10 @@ static bool usesFeatureBuiltinStackAlloc(Decl *decl) {
return false;
}

static bool usesFeatureBuiltinUnprotectedStackAlloc(Decl *decl) {
return false;
}

static bool usesFeatureBuiltinAssumeAlignment(Decl *decl) {
return false;
}
Expand Down
1 change: 1 addition & 0 deletions lib/AST/Builtins.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2707,6 +2707,7 @@ ValueDecl *swift::getBuiltinValueDecl(ASTContext &Context, Identifier Id) {
return getDeallocOperation(Context, Id);

case BuiltinValueKind::StackAlloc:
case BuiltinValueKind::UnprotectedStackAlloc:
return getStackAllocOperation(Context, Id);
case BuiltinValueKind::StackDealloc:
return getStackDeallocOperation(Context, Id);
Expand Down
3 changes: 2 additions & 1 deletion lib/IRGen/IRGenSIL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3237,7 +3237,8 @@ static Alignment getStackAllocationAlignment(IRGenSILFunction &IGF,
/// some other builtin.)
static bool emitStackAllocBuiltinCall(IRGenSILFunction &IGF,
swift::BuiltinInst *i) {
if (i->getBuiltinKind() == BuiltinValueKind::StackAlloc) {
if (i->getBuiltinKind() == BuiltinValueKind::StackAlloc ||
i->getBuiltinKind() == BuiltinValueKind::UnprotectedStackAlloc) {
// Stack-allocate a buffer with the specified size/alignment.
auto loc = i->getLoc().getSourceLoc();
auto size = getStackAllocationSize(
Expand Down
1 change: 1 addition & 0 deletions lib/SIL/IR/OperandOwnership.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,7 @@ BUILTIN_OPERAND_OWNERSHIP(InstantaneousUse, SRem)
BUILTIN_OPERAND_OWNERSHIP(InstantaneousUse, GenericSRem)
BUILTIN_OPERAND_OWNERSHIP(InstantaneousUse, SSubOver)
BUILTIN_OPERAND_OWNERSHIP(InstantaneousUse, StackAlloc)
BUILTIN_OPERAND_OWNERSHIP(InstantaneousUse, UnprotectedStackAlloc)
BUILTIN_OPERAND_OWNERSHIP(InstantaneousUse, StackDealloc)
BUILTIN_OPERAND_OWNERSHIP(InstantaneousUse, SToSCheckedTrunc)
BUILTIN_OPERAND_OWNERSHIP(InstantaneousUse, SToUCheckedTrunc)
Expand Down
3 changes: 2 additions & 1 deletion lib/SIL/IR/SILInstruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1243,7 +1243,8 @@ bool SILInstruction::isAllocatingStack() const {
return PA->isOnStack();

if (auto *BI = dyn_cast<BuiltinInst>(this)) {
if (BI->getBuiltinKind() == BuiltinValueKind::StackAlloc) {
if (BI->getBuiltinKind() == BuiltinValueKind::StackAlloc ||
BI->getBuiltinKind() == BuiltinValueKind::UnprotectedStackAlloc) {
return true;
}
}
Expand Down
1 change: 1 addition & 0 deletions lib/SIL/IR/ValueOwnership.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ CONSTANT_OWNERSHIP_BUILTIN(None, AllocRaw)
CONSTANT_OWNERSHIP_BUILTIN(None, AssertConf)
CONSTANT_OWNERSHIP_BUILTIN(None, UToSCheckedTrunc)
CONSTANT_OWNERSHIP_BUILTIN(None, StackAlloc)
CONSTANT_OWNERSHIP_BUILTIN(None, UnprotectedStackAlloc)
CONSTANT_OWNERSHIP_BUILTIN(None, StackDealloc)
CONSTANT_OWNERSHIP_BUILTIN(None, SToSCheckedTrunc)
CONSTANT_OWNERSHIP_BUILTIN(None, SToUCheckedTrunc)
Expand Down
1 change: 1 addition & 0 deletions lib/SIL/Utils/MemAccessUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2519,6 +2519,7 @@ static void visitBuiltinAddress(BuiltinInst *builtin,
case BuiltinValueKind::AllocRaw:
case BuiltinValueKind::DeallocRaw:
case BuiltinValueKind::StackAlloc:
case BuiltinValueKind::UnprotectedStackAlloc:
case BuiltinValueKind::StackDealloc:
case BuiltinValueKind::Fence:
case BuiltinValueKind::StaticReport:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ static bool isBarrier(SILInstruction *inst) {
case BuiltinValueKind::CreateTaskGroupWithFlags:
case BuiltinValueKind::DestroyTaskGroup:
case BuiltinValueKind::StackAlloc:
case BuiltinValueKind::UnprotectedStackAlloc:
case BuiltinValueKind::StackDealloc:
case BuiltinValueKind::AssumeAlignment:
return false;
Expand Down
125 changes: 115 additions & 10 deletions stdlib/public/core/TemporaryAllocation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -129,16 +129,7 @@ internal func _withUnsafeTemporaryAllocation<T, R>(
let byteCount = _byteCountForTemporaryAllocation(of: type, capacity: capacity)

guard _isStackAllocationSafe(byteCount: byteCount, alignment: alignment) else {
// Fall back to the heap. This may still be optimizable if escape analysis
// shows that the allocated pointer does not escape.
let buffer = UnsafeMutableRawPointer.allocate(
byteCount: byteCount,
alignment: alignment
)
defer {
buffer.deallocate()
}
return try body(buffer._rawValue)
return try _fallBackToHeapAllocation(byteCount: byteCount, alignment: alignment, body)
}

// This declaration must come BEFORE Builtin.stackAlloc() or
Expand Down Expand Up @@ -170,6 +161,63 @@ internal func _withUnsafeTemporaryAllocation<T, R>(
#endif
}

#if $BuiltinUnprotectedStackAlloc
@_alwaysEmitIntoClient @_transparent
internal func _withUnprotectedUnsafeTemporaryAllocation<T, R>(
of type: T.Type,
capacity: Int,
alignment: Int,
_ body: (Builtin.RawPointer) throws -> R
) rethrows -> R {
// How many bytes do we need to allocate?
let byteCount = _byteCountForTemporaryAllocation(of: type, capacity: capacity)

guard _isStackAllocationSafe(byteCount: byteCount, alignment: alignment) else {
return try _fallBackToHeapAllocation(byteCount: byteCount, alignment: alignment, body)
}

// This declaration must come BEFORE Builtin.unprotectedStackAlloc() or
// Builtin.stackDealloc() will end up blowing it away (and the verifier will
// notice and complain.)
let result: R

let stackAddress = Builtin.unprotectedStackAlloc(
capacity._builtinWordValue,
MemoryLayout<T>.stride._builtinWordValue,
alignment._builtinWordValue
)

// The multiple calls to Builtin.stackDealloc() are because defer { } produces
// a child function at the SIL layer and that conflicts with the verifier's
// idea of a stack allocation's lifetime.
do {
result = try body(stackAddress)
Builtin.stackDealloc(stackAddress)
return result

} catch {
Builtin.stackDealloc(stackAddress)
throw error
}
}
#endif

@_alwaysEmitIntoClient @_transparent
internal func _fallBackToHeapAllocation<R>(
byteCount: Int,
alignment: Int,
_ body: (Builtin.RawPointer) throws -> R
) rethrows -> R {
let buffer = UnsafeMutableRawPointer.allocate(
byteCount: byteCount,
alignment: alignment
)
defer {
buffer.deallocate()
}
return try body(buffer._rawValue)
}

// MARK: - Public interface

/// Provides scoped access to a raw buffer pointer with the specified byte count
Expand Down Expand Up @@ -222,6 +270,34 @@ public func withUnsafeTemporaryAllocation<R>(
}
}

/// Provides scoped access to a raw buffer pointer with the specified byte count
/// and alignment.
///
/// This function is similar to `withUnsafeTemporaryAllocation`, except that it
/// doesn't trigger stack protection for the stack allocated memory.
@_alwaysEmitIntoClient @_transparent
public func _withUnprotectedUnsafeTemporaryAllocation<R>(
byteCount: Int,
alignment: Int,
_ body: (UnsafeMutableRawBufferPointer) throws -> R
) rethrows -> R {
return try _withUnsafeTemporaryAllocation(
of: Int8.self,
capacity: byteCount,
alignment: alignment
) { pointer in
#if $BuiltinUnprotectedStackAlloc
let buffer = UnsafeMutableRawBufferPointer(
start: .init(pointer),
count: byteCount
)
return try body(buffer)
#else
return try withUnsafeTemporaryAllocation(byteCount: byteCount, alignment: alignment, body)
#endif
}
}

/// Provides scoped access to a buffer pointer to memory of the specified type
/// and with the specified capacity.
///
Expand Down Expand Up @@ -272,3 +348,32 @@ public func withUnsafeTemporaryAllocation<T, R>(
return try body(buffer)
}
}

/// Provides scoped access to a buffer pointer to memory of the specified type
/// and with the specified capacity.
///
/// This function is similar to `withUnsafeTemporaryAllocation`, except that it
/// doesn't trigger stack protection for the stack allocated memory.
@_alwaysEmitIntoClient @_transparent
public func _withUnprotectedUnsafeTemporaryAllocation<T, R>(
of type: T.Type,
capacity: Int,
_ body: (UnsafeMutableBufferPointer<T>) throws -> R
) rethrows -> R {
return try _withUnprotectedUnsafeTemporaryAllocation(
of: type,
capacity: capacity,
alignment: MemoryLayout<T>.alignment
) { pointer in
#if $BuiltinUnprotectedStackAlloc
Builtin.bindMemory(pointer, capacity._builtinWordValue, type)
let buffer = UnsafeMutableBufferPointer<T>(
start: .init(pointer),
count: capacity
)
return try body(buffer)
#else
return try withUnsafeTemporaryAllocation(of: type, capacity: capacity, body)
#endif
}
}
4 changes: 2 additions & 2 deletions stdlib/public/core/UnsafeRawPointer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ public struct UnsafeRawPointer: _Pointer {
as type: T.Type
) -> T {
_debugPrecondition(_isPOD(T.self))
return withUnsafeTemporaryAllocation(of: T.self, capacity: 1) {
return _withUnprotectedUnsafeTemporaryAllocation(of: T.self, capacity: 1) {
let temporary = $0.baseAddress._unsafelyUnwrappedUnchecked
Builtin.int_memcpy_RawPointer_RawPointer_Int64(
temporary._rawValue,
Expand Down Expand Up @@ -1245,7 +1245,7 @@ public struct UnsafeMutableRawPointer: _Pointer {
as type: T.Type
) -> T {
_debugPrecondition(_isPOD(T.self))
return withUnsafeTemporaryAllocation(of: T.self, capacity: 1) {
return _withUnprotectedUnsafeTemporaryAllocation(of: T.self, capacity: 1) {
let temporary = $0.baseAddress._unsafelyUnwrappedUnchecked
Builtin.int_memcpy_RawPointer_RawPointer_Int64(
temporary._rawValue,
Expand Down
7 changes: 7 additions & 0 deletions test/IRGen/temporary_allocation/codegen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ withUnsafeTemporaryAllocation(of: Int32.self, capacity: 4) { buffer in
// CHECK: [[INT_PTR:%[0-9]+]] = ptrtoint [16 x i8]* [[INT_PTR_RAW]] to [[WORD]]
// CHECK: call swiftcc void @blackHole([[WORD]] [[INT_PTR]])

_withUnprotectedUnsafeTemporaryAllocation(of: Int32.self, capacity: 4) { buffer in
blackHole(buffer.baseAddress)
}
// CHECK: [[INT_PTR_RAW:%temp_alloc[0-9]*]] = alloca [16 x i8], align 4
// CHECK: [[INT_PTR:%[0-9]+]] = ptrtoint [16 x i8]* [[INT_PTR_RAW]] to [[WORD]]
// CHECK: call swiftcc void @blackHole([[WORD]] [[INT_PTR]])

withUnsafeTemporaryAllocation(of: Void.self, capacity: 2) { buffer in
blackHole(buffer.baseAddress)
}
Expand Down
Loading