From 667de83339f62973258cfcc520242d237b2dcf9c Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Fri, 29 Aug 2025 11:18:04 +0200 Subject: [PATCH 1/5] Optimizer: move DiagnoseStaticExclusivity after MandatoryAllocBoxToStack This is needed because MandatoryAllocBoxToStack can convert dynamic accesses to static accesses. Also, it improves diagnostics for closure captures. --- lib/SILOptimizer/PassManager/PassPipeline.cpp | 5 ++++- test/SILGen/moveonly_escaping_closure.swift | 4 ++-- test/SILOptimizer/exclusivity_static_diagnostics.swift | 7 ++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/SILOptimizer/PassManager/PassPipeline.cpp b/lib/SILOptimizer/PassManager/PassPipeline.cpp index 8448ad1e217a8..4bf10b19e401b 100644 --- a/lib/SILOptimizer/PassManager/PassPipeline.cpp +++ b/lib/SILOptimizer/PassManager/PassPipeline.cpp @@ -117,7 +117,6 @@ static void addMandatoryDiagnosticOptPipeline(SILPassPipelinePlan &P) { P.startPipeline("Mandatory Diagnostic Passes + Enabling Optimization Passes"); P.addDiagnoseInvalidEscapingCaptures(); P.addReferenceBindingTransform(); - P.addDiagnoseStaticExclusivity(); P.addNestedSemanticFunctionCheck(); P.addCapturePromotion(); @@ -130,6 +129,10 @@ static void addMandatoryDiagnosticOptPipeline(SILPassPipelinePlan &P) { #else P.addLegacyAllocBoxToStack(); #endif + // Needs to run after MandatoryAllocBoxToStack, because MandatoryAllocBoxToStack + // can convert dynamic accesses to static accesses. + P.addDiagnoseStaticExclusivity(); + P.addNoReturnFolding(); P.addBooleanLiteralFolding(); addDefiniteInitialization(P); diff --git a/test/SILGen/moveonly_escaping_closure.swift b/test/SILGen/moveonly_escaping_closure.swift index 864107017265b..9fc5def229be9 100644 --- a/test/SILGen/moveonly_escaping_closure.swift +++ b/test/SILGen/moveonly_escaping_closure.swift @@ -143,7 +143,7 @@ func testLocalLetClosureCaptureVar() { consumeVal(x) // expected-note {{consumed here}} // expected-note @-1 {{consumed again here}} borrowConsumeVal(x, x) - // expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access}} + // expected-error @-1 {{overlapping accesses to 'x', but deinitialization requires exclusive access}} // expected-note @-2 {{conflicting access is here}} // expected-note @-3 {{used here}} // expected-note @-4 {{used here}} @@ -975,7 +975,7 @@ func testLocalLetClosureCaptureConsuming(_ x: consuming SingleElt) { consumeVal(x) // expected-note {{consumed here}} // expected-note @-1 {{consumed again here}} borrowConsumeVal(x, x) // expected-note {{used here}} - // expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access}} + // expected-error @-1 {{overlapping accesses to 'x', but deinitialization requires exclusive access}} // expected-note @-2 {{conflicting access is here}} // expected-note @-3 {{consumed here}} // expected-note @-4 {{used here}} diff --git a/test/SILOptimizer/exclusivity_static_diagnostics.swift b/test/SILOptimizer/exclusivity_static_diagnostics.swift index 646858a122e00..0fc1f2c9ee46c 100644 --- a/test/SILOptimizer/exclusivity_static_diagnostics.swift +++ b/test/SILOptimizer/exclusivity_static_diagnostics.swift @@ -263,12 +263,9 @@ func callsClosureLiteralImmediately() { func callsStoredClosureLiteral() { var i = 7; - let c = { (p: inout Int) in i} + let c = { (p: inout Int) in i} // expected-note {{conflicting access is here}} - // Closure literals that are stored and later called are treated as escaping - // We don't expect a static exclusivity diagnostic here, but the issue - // will be caught at run time - _ = c(&i) // no-error + _ = c(&i) // expected-error {{overlapping accesses to 'i', but modification requires exclusive access; consider copying to a local variable}} } From 86076e2ada6b28fe0ae5614d822e98407f71328b Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Fri, 29 Aug 2025 11:27:34 +0200 Subject: [PATCH 2/5] RawSILInstLowering: Don't insert an access scope for `assign_or_init` if there is already one. This avoids inserting a dynamic access check when the parent is static (and therefore can be statically enforced). --- .../Mandatory/RawSILInstLowering.cpp | 9 ++++++++- test/SILOptimizer/init_accessors.sil | 4 ++-- test/SILOptimizer/init_accessors.swift | 19 ++++++++++++------- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/lib/SILOptimizer/Mandatory/RawSILInstLowering.cpp b/lib/SILOptimizer/Mandatory/RawSILInstLowering.cpp index 24ccbb0842762..c24bf26979509 100644 --- a/lib/SILOptimizer/Mandatory/RawSILInstLowering.cpp +++ b/lib/SILOptimizer/Mandatory/RawSILInstLowering.cpp @@ -290,13 +290,20 @@ lowerAssignOrInitInstruction(SILBuilderWithScope &b, auto isRefSelf = selfValue->getType().getASTType()->mayHaveSuperclass(); SILValue selfRef; + bool needInsertEndAccess = false; if (isRefSelf) { selfRef = b.emitBeginBorrowOperation(loc, selfValue); + } else if (isa(selfValue)) { + // Don't insert an access scope if there is already one. This avoids + // inserting a dynamic access check when the parent is static (and therefore + // can be statically enforced). + selfRef = selfValue; } else { selfRef = b.createBeginAccess(loc, selfValue, SILAccessKind::Modify, SILAccessEnforcement::Dynamic, /*noNestedConflict=*/false, /*fromBuiltin=*/false); + needInsertEndAccess = true; } auto emitFieldReference = [&](VarDecl *field, @@ -341,7 +348,7 @@ lowerAssignOrInitInstruction(SILBuilderWithScope &b, if (isRefSelf) { if (selfRef != selfValue) b.emitEndBorrowOperation(loc, selfRef); - } else { + } else if (needInsertEndAccess) { b.createEndAccess(loc, selfRef, /*aborted=*/false); } diff --git a/test/SILOptimizer/init_accessors.sil b/test/SILOptimizer/init_accessors.sil index c7df2e3e4c99e..cab1e0b31ead6 100644 --- a/test/SILOptimizer/init_accessors.sil +++ b/test/SILOptimizer/init_accessors.sil @@ -39,9 +39,9 @@ bb0(%0 : $Int, %1 : $*Test): } // CHECK-LABEL: sil hidden [ossa] @$s14init_accessors4TestV1vACSi_tcfC : $@convention(method) (Int, @thin Test.Type) -> Test +// CHECK: [[SELF_REF:%.*]] = begin_access [modify] [unknown] [[SELF:%.*]] : $*Test // CHECK: [[INIT_REF:%.*]] = function_ref @$s14init_accessors4TestV1xSivi : $@convention(thin) (Int) -> @out Int -// CHECK: [[SELF_REF:%.*]] = begin_access [modify] [dynamic] [[SELF:%.*]] : $*Test -// CHECK-NEXT: [[FIELD_REF:%.*]] = struct_element_addr [[SELF_REF]] : $*Test, #Test._x +// CHECK: [[FIELD_REF:%.*]] = struct_element_addr [[SELF_REF]] : $*Test, #Test._x // CHECK-NEXT: {{.*}} = apply [[INIT_REF]]([[FIELD_REF]], %0) : $@convention(thin) (Int) -> @out Int sil hidden [ossa] @$s14init_accessors4TestV1vACSi_tcfC : $@convention(method) (Int, @thin Test.Type) -> Test { bb0(%0 : $Int, %1 : $@thin Test.Type): diff --git a/test/SILOptimizer/init_accessors.swift b/test/SILOptimizer/init_accessors.swift index e4481f6d5fa80..d601cf7f2b032 100644 --- a/test/SILOptimizer/init_accessors.swift +++ b/test/SILOptimizer/init_accessors.swift @@ -41,10 +41,11 @@ struct TestInit { } // CHECK-LABEL: sil hidden [ossa] @$s14init_accessors8TestInitV1x1yACSi_SitcfC : $@convention(method) (Int, Int, @thin TestInit.Type) -> TestInit + // CHECK: end_access + // CHECK: [[SELF_VALUE:%.*]] = begin_access [modify] [static] {{.*}} : $*TestInit // CHECK: // function_ref TestInit.point.init // CHECK-NEXT: [[INIT_ACCESSOR_FN:%.*]] = function_ref @$s14init_accessors8TestInitV5pointSi_Sitvi : $@convention(thin) (Int, Int, @inout Int, @thin TestInit.Type) -> (@out Int, @out (Int, Int)) // CHECK-NEXT: [[INIT_ACCESSOR:%.*]] = partial_apply [callee_guaranteed] [on_stack] [[INIT_ACCESSOR_FN]](%2) : $@convention(thin) (Int, Int, @inout Int, @thin TestInit.Type) -> (@out Int, @out (Int, Int)) - // CHECK: [[SELF_VALUE:%.*]] = begin_access [modify] [dynamic] {{.*}} : $*TestInit // CHECK: [[Y_REF:%.*]] = struct_element_addr [[SELF_VALUE]] : $*TestInit, #TestInit.y // CHECK-NEXT: [[FULL_REF:%.*]] = struct_element_addr [[SELF_VALUE]] : $*TestInit, #TestInit.full // CHECK-NEXT: ([[X_VAL:%.*]], [[Y_VAL:%.*]]) = destructure_tuple {{.*}} : $(Int, Int) @@ -71,10 +72,13 @@ struct TestSetter { } // CHECK-LABEL: sil hidden [ossa] @$s14init_accessors10TestSetterV1x1yACSi_SitcfC : $@convention(method) (Int, Int, @thin TestSetter.Type) -> TestSetter + // CHECK: [[AS:%.*]] = alloc_stack + // CHECK: end_access + // CHECK: end_access + // CHECK: [[SELF:%.*]] = begin_access [modify] [static] [[AS]] : $*TestSetter // CHECK: [[INIT_ACCESSOR_FN:%.*]] = function_ref @$s14init_accessors10TestSetterV5pointSi_Sitvi : $@convention(thin) (Int, Int, @inout Int, @inout Int, @thin TestSetter.Type) -> () // CHECK: [[INIT_ACCESSOR:%.*]] = partial_apply [callee_guaranteed] [on_stack] [[INIT_ACCESSOR_FN]](%2) : $@convention(thin) (Int, Int, @inout Int, @inout Int, @thin TestSetter.Type) -> () - // CHECK: [[SELF:%.*]] = begin_access [modify] [dynamic] %14 : $*TestSetter - // CHECK-NEXT: ([[X:%.*]], [[Y:%.*]]) = destructure_tuple {{.*}} : $(Int, Int) + // CHECK: ([[X:%.*]], [[Y:%.*]]) = destructure_tuple {{.*}} : $(Int, Int) // CHECK-NEXT: [[X_REF:%.*]] = struct_element_addr [[SELF]] : $*TestSetter, #TestSetter.x // CHECK-NEXT: [[Y_REF:%.*]] = struct_element_addr [[SELF]] : $*TestSetter, #TestSetter.y // CHECK-NEXT: {{.*}} = apply [[INIT_ACCESSOR]]([[X]], [[Y]], [[X_REF]], [[Y_REF]]) : $@noescape @callee_guaranteed (Int, Int, @inout Int, @inout Int) -> () @@ -204,17 +208,18 @@ struct TestNoInitAndInit { // CHECK-LABEL: sil hidden [ossa] @$s14init_accessors013TestNoInitAndE0V1x1yACSi_SitcfC : $@convention(method) (Int, Int, @thin TestNoInitAndInit.Type) -> TestNoInitAndInit // + // CHECK: end_access + // CHECK: [[SELF_REF:%.*]] = begin_access [modify] [static] {{.*}} : $*TestNoInitAndInit // CHECK: [[INIT_REF_FN:%.*]] = function_ref @$s14init_accessors013TestNoInitAndE0V6pointXSivi : $@convention(thin) (Int, @inout Int, @thin TestNoInitAndInit.Type) -> () // CHECK: [[INIT_REF:%.*]] = partial_apply [callee_guaranteed] [on_stack] [[INIT_REF_FN]](%2) : $@convention(thin) (Int, @inout Int, @thin TestNoInitAndInit.Type) -> () - // CHECK: [[SELF_REF:%.*]] = begin_access [modify] [dynamic] {{.*}} : $*TestNoInitAndInit - // CHECK-NEXT: [[X_REF:%.*]] = struct_element_addr [[SELF_REF]] : $*TestNoInitAndInit, #TestNoInitAndInit.x + // CHECK: [[X_REF:%.*]] = struct_element_addr [[SELF_REF]] : $*TestNoInitAndInit, #TestNoInitAndInit.x // CHECK-NEXT: {{.*}} = apply [[INIT_REF]](%0, [[X_REF]]) : $@noescape @callee_guaranteed (Int, @inout Int) -> () // CHECK-NEXT: end_access [[SELF_REF]] : $*TestNoInitAndInit // + // CHECK: [[SELF_REF:%.*]] = begin_access [modify] [static] {{.*}} : $*TestNoInitAndInit // CHECK: [[INIT_REF_FN:%.*]] = function_ref @$s14init_accessors013TestNoInitAndE0V6pointYSivi : $@convention(thin) (Int, @thin TestNoInitAndInit.Type) -> @out Int // CHECK: [[INIT_REF:%.*]] = partial_apply [callee_guaranteed] [on_stack] [[INIT_REF_FN]](%2) : $@convention(thin) (Int, @thin TestNoInitAndInit.Type) -> @out Int - // CHECK: [[SELF_REF:%.*]] = begin_access [modify] [dynamic] {{.*}} : $*TestNoInitAndInit - // CHECK-NEXT: [[Y_REF:%.*]] = struct_element_addr [[SELF_REF]] : $*TestNoInitAndInit, #TestNoInitAndInit.y + // CHECK: [[Y_REF:%.*]] = struct_element_addr [[SELF_REF]] : $*TestNoInitAndInit, #TestNoInitAndInit.y // CHECK-NEXT: {{.*}} = apply [[INIT_REF]]([[Y_REF]], %1) : $@noescape @callee_guaranteed (Int) -> @out Int // CHECK-NEXT: end_access [[SELF_REF]] : $*TestNoInitAndInit init(x: Int, y: Int) { From b6e9c4c5e4643e39e9b8c3ae409d8f689079843d Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Fri, 29 Aug 2025 12:01:39 +0200 Subject: [PATCH 3/5] TempLValueElimination: don't propagate alloc_stack which have access scopes If exclusivity is checked for the alloc_stack we must not replace it with the copy-destination. If the copy-destination is also in an access-scope this would result in an exclusivity violation which was not there before. Fixes a miscompile which results in a wrong exclusivity violation error at runtime. https://github.com/swiftlang/swift/issues/83924 rdar://159220436 --- .../TempLValueElimination.swift | 7 ++ .../templvalueopt_and_exclusivity.swift | 68 +++++++++++++++++++ test/SILOptimizer/templvalueopt_ossa.sil | 17 +++++ 3 files changed, 92 insertions(+) create mode 100644 test/SILOptimizer/templvalueopt_and_exclusivity.swift diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/TempLValueElimination.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/TempLValueElimination.swift index 12ae4d08998a3..83bfdb1411371 100644 --- a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/TempLValueElimination.swift +++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/TempLValueElimination.swift @@ -113,6 +113,13 @@ private func tryEliminate(copy: CopyLikeInstruction, _ context: FunctionPassCont return } + // If exclusivity is checked for the alloc_stack we must not replace it with the copy-destination. + // If the copy-destination is also in an access-scope this would result in an exclusivity violation + // which was not there before. + guard allocStack.uses.users(ofType: BeginAccessInst.self).isEmpty else { + return + } + var worklist = InstructionWorklist(context) defer { worklist.deinitialize() } worklist.pushIfNotVisited(firstUseOfAllocStack) diff --git a/test/SILOptimizer/templvalueopt_and_exclusivity.swift b/test/SILOptimizer/templvalueopt_and_exclusivity.swift new file mode 100644 index 0000000000000..d3847a512f569 --- /dev/null +++ b/test/SILOptimizer/templvalueopt_and_exclusivity.swift @@ -0,0 +1,68 @@ +// RUN: %empty-directory(%t) +// RUN: split-file %s %t + +// RUN: %target-build-swift %t/module.swift -parse-as-library -O -enable-library-evolution -module-name=Module -emit-module -emit-module-path %t/Module.swiftmodule +// RUN: %target-build-swift %t/module.swift -parse-as-library -O -enable-library-evolution -module-name=Module -c -o %t/module.o +// RUN: %target-build-swift -parse-as-library -O %t/main.swift -I%t %t/module.o -o %t/a.out +// RUN: %target-codesign %t/a.out +// RUN: %target-run %t/a.out | %FileCheck %s + +// REQUIRES: executable_test + + +// Check that TempLValueElimination does not cause an exclusivity violation at runtime. + +//--- module.swift + +public struct ResilientStruct { + public init() {} +} + +//--- main.swift + +import Module + +struct MyState { + var _prop: String + var prop: String + { + @storageRestrictions(initializes: _prop) + init(initialValue) { + _prop = initialValue + } + get { + return _prop + } + } + + let resilient = ResilientStruct() + + init(prop: String) { + self.prop = prop + } +} + +class Node { + var state = MyState(prop: "initial value") + + func update(_ body: (inout MyState) -> Void) { + body(&state) + } +} + +@inline(never) +func test_exclusivity() { + let node = Node() + node.update { state in + state = MyState(prop: "new value") + } +} + +@main +enum App { + static func main() { + test_exclusivity() + // CHECK: success + print("success") + } +} diff --git a/test/SILOptimizer/templvalueopt_ossa.sil b/test/SILOptimizer/templvalueopt_ossa.sil index 863227b2ba85c..17474d3aeb877 100644 --- a/test/SILOptimizer/templvalueopt_ossa.sil +++ b/test/SILOptimizer/templvalueopt_ossa.sil @@ -685,3 +685,20 @@ bb2: %r = tuple () return %r } + +// CHECK-LABEL: sil [ossa] @dont_propagate_with_access_scope : +// CHECK: alloc_stack +// CHECK: copy_addr +// CHECK-LABEL: } // end sil function 'dont_propagate_with_access_scope' +sil [ossa] @dont_propagate_with_access_scope : $@convention(thin) (@inout Int, Int) -> () { +bb0(%0 : $*Int, %1 : $Int): + %2 = alloc_stack $Int + %3 = begin_access [modify] [dynamic] %2 + store %1 to [trivial] %3 + end_access %3 + copy_addr %2 to %0 + dealloc_stack %2 + %8 = tuple () + return %8 +} + From 3de5d6a13e0bd25a84c2e0928904a265c7419e8f Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Fri, 29 Aug 2025 12:02:48 +0200 Subject: [PATCH 4/5] SIL: add get+set for `enforcement` in `BeginAccessInst` --- .../Sources/SIL/Instruction.swift | 29 ++++++++++++++++++- include/swift/SIL/SILBridging.h | 2 ++ include/swift/SIL/SILBridgingImpl.h | 8 +++++ include/swift/SIL/SILInstruction.h | 2 ++ 4 files changed, 40 insertions(+), 1 deletion(-) diff --git a/SwiftCompilerSources/Sources/SIL/Instruction.swift b/SwiftCompilerSources/Sources/SIL/Instruction.swift index 0bc516cb8663b..098462668650b 100644 --- a/SwiftCompilerSources/Sources/SIL/Instruction.swift +++ b/SwiftCompilerSources/Sources/SIL/Instruction.swift @@ -1629,12 +1629,39 @@ final public class BeginAccessInst : SingleValueInstruction, UnaryInstruction { public var accessKind: AccessKind { AccessKind(rawValue: bridged.BeginAccessInst_getAccessKind())! } - public func set(accessKind: BeginAccessInst.AccessKind, context: some MutatingContext) { + public func set(accessKind: AccessKind, context: some MutatingContext) { context.notifyInstructionsChanged() bridged.BeginAccess_setAccessKind(accessKind.rawValue) context.notifyInstructionChanged(self) } + // The raw values must match SILAccessEnforcement. + public enum Enforcement: Int { + /// The access's enforcement has not yet been determined. + case unknown = 0 + + /// The access is statically known to not conflict with other accesses. + case `static` = 1 + + /// The access is not statically known to not conflict with anything and must be dynamically checked. + case dynamic = 2 + + /// The access is not statically known to not conflict with anything but dynamic checking should + /// be suppressed, leaving it undefined behavior. + case unsafe = 3 + + /// Access to pointers that are signed via pointer authentication. + case signed = 4 + } + public var enforcement: Enforcement { + Enforcement(rawValue: bridged.BeginAccessInst_getEnforcement())! + } + public func set(enforcement: Enforcement, context: some MutatingContext) { + context.notifyInstructionsChanged() + bridged.BeginAccess_setEnforcement(enforcement.rawValue) + context.notifyInstructionChanged(self) + } + public var isStatic: Bool { bridged.BeginAccessInst_isStatic() } public var isUnsafe: Bool { bridged.BeginAccessInst_isUnsafe() } diff --git a/include/swift/SIL/SILBridging.h b/include/swift/SIL/SILBridging.h index 589112b768f13..95375d5e691bf 100644 --- a/include/swift/SIL/SILBridging.h +++ b/include/swift/SIL/SILBridging.h @@ -824,6 +824,8 @@ struct BridgedInstruction { BRIDGED_INLINE bool BeginAccessInst_isStatic() const; BRIDGED_INLINE bool BeginAccessInst_isUnsafe() const; BRIDGED_INLINE void BeginAccess_setAccessKind(SwiftInt accessKind) const; + BRIDGED_INLINE SwiftInt BeginAccessInst_getEnforcement() const; + BRIDGED_INLINE void BeginAccess_setEnforcement(SwiftInt accessKind) const; BRIDGED_INLINE bool CopyAddrInst_isTakeOfSrc() const; BRIDGED_INLINE bool CopyAddrInst_isInitializationOfDest() const; BRIDGED_INLINE void CopyAddrInst_setIsTakeOfSrc(bool isTakeOfSrc) const; diff --git a/include/swift/SIL/SILBridgingImpl.h b/include/swift/SIL/SILBridgingImpl.h index 5182f83034dd9..49b9e003dfe01 100644 --- a/include/swift/SIL/SILBridgingImpl.h +++ b/include/swift/SIL/SILBridgingImpl.h @@ -1494,6 +1494,14 @@ void BridgedInstruction::BeginAccess_setAccessKind(SwiftInt accessKind) const { getAs()->setAccessKind((swift::SILAccessKind)accessKind); } +SwiftInt BridgedInstruction::BeginAccessInst_getEnforcement() const { + return (SwiftInt)getAs()->getEnforcement(); +} + +void BridgedInstruction::BeginAccess_setEnforcement(SwiftInt accessKind) const { + getAs()->setEnforcement((swift::SILAccessEnforcement)accessKind); +} + bool BridgedInstruction::CopyAddrInst_isTakeOfSrc() const { return getAs()->isTakeOfSrc(); } diff --git a/include/swift/SIL/SILInstruction.h b/include/swift/SIL/SILInstruction.h index 9f3bfc7bf5dd6..ac3d59f3a4aac 100644 --- a/include/swift/SIL/SILInstruction.h +++ b/include/swift/SIL/SILInstruction.h @@ -4885,6 +4885,7 @@ class EndBorrowInst }; /// Different kinds of access. +/// This enum must stay in sync with `BeginAccessInst.AccessKind` in SwiftCompilerSources. enum class SILAccessKind : uint8_t { /// An access which takes uninitialized memory and initializes it. Init, @@ -4907,6 +4908,7 @@ enum { NumSILAccessKindBits = 2 }; StringRef getSILAccessKindName(SILAccessKind kind); /// Different kinds of exclusivity enforcement for accesses. +/// This enum must stay in sync with `BeginAccessInst.Enforcement` in SwiftCompilerSources. enum class SILAccessEnforcement : uint8_t { /// The access's enforcement has not yet been determined. Unknown, From c7900525907e724bccde4bb3cff895544b4bc0c6 Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Fri, 29 Aug 2025 13:38:58 +0200 Subject: [PATCH 5/5] AllocBoxToStack: convert access checks from "dynamic" to "static" Once we have promoted the box to stack, access violations can be detected statically by the DiagnoseStaticExclusivity pass (which runs after MandatoryAllocBoxToStack). Therefore we can convert dynamic accesses to static accesses. rdar://157458037 --- .../FunctionPasses/AllocBoxToStack.swift | 27 ++++++++++++++++--- .../allocbox_to_stack_ownership.sil | 16 +++++++++++ .../allocboxtostack_localapply.swift | 4 +++ 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/AllocBoxToStack.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/AllocBoxToStack.swift index f56dd90f20a4b..7caf2b992d41f 100644 --- a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/AllocBoxToStack.swift +++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/AllocBoxToStack.swift @@ -54,7 +54,7 @@ import SIL let allocBoxToStack = FunctionPass(name: "allocbox-to-stack") { (function: Function, context: FunctionPassContext) in - _ = tryConvertBoxesToStack(in: function, context) + _ = tryConvertBoxesToStack(in: function, isMandatory: false, context) } /// The "mandatory" version of the pass, which runs in the mandatory pipeline. @@ -72,7 +72,7 @@ let mandatoryAllocBoxToStack = ModulePass(name: "mandatory-allocbox-to-stack") { while let function = worklist.pop() { moduleContext.transform(function: function) { context in - let specFns = tryConvertBoxesToStack(in: function, context) + let specFns = tryConvertBoxesToStack(in: function, isMandatory: true, context) worklist.pushIfNotVisited(contentsOf: specFns.specializedFunctions) originalsOfSpecializedFunctions.pushIfNotVisited(contentsOf: specFns.originalFunctions) } @@ -86,9 +86,11 @@ let mandatoryAllocBoxToStack = ModulePass(name: "mandatory-allocbox-to-stack") { /// Converts all non-escaping `alloc_box` to `alloc_stack` and specializes called functions if a /// box is passed to a function. /// Returns the list of original functions for which a specialization has been created. -private func tryConvertBoxesToStack(in function: Function, _ context: FunctionPassContext) -> FunctionSpecializations { +private func tryConvertBoxesToStack(in function: Function, isMandatory: Bool, + _ context: FunctionPassContext +) -> FunctionSpecializations { var promotableBoxes = Array<(AllocBoxInst, Flags)>() - var functionsToSpecialize = FunctionSpecializations() + var functionsToSpecialize = FunctionSpecializations(isMandatory: isMandatory) findPromotableBoxes(in: function, &promotableBoxes, &functionsToSpecialize) @@ -189,6 +191,9 @@ private struct FunctionSpecializations { private var promotableArguments = CrossFunctionValueWorklist() private var originals = FunctionWorklist() private var originalToSpecialized = Dictionary() + private let isMandatory: Bool + + init(isMandatory: Bool) { self.isMandatory = isMandatory } var originalFunctions: [Function] { originals.functions } var specializedFunctions: [Function] { originals.functions.lazy.map { originalToSpecialized[$0]! } } @@ -221,6 +226,12 @@ private struct FunctionSpecializations { context.erase(instruction: user) case let projectBox as ProjectBoxInst: assert(projectBox.fieldIndex == 0, "only single-field boxes are handled") + if isMandatory { + // Once we have promoted the box to stack, access violations can be detected statically by the + // DiagnoseStaticExclusivity pass (which runs after MandatoryAllocBoxToStack). + // Therefore we can convert dynamic accesses to static accesses. + makeAccessesStatic(of: projectBox, context) + } projectBox.replace(with: stack, context) case is MarkUninitializedInst, is CopyValueInst, is BeginBorrowInst, is MoveValueInst: // First, replace the instruction with the original `box`, which adds more uses to `box`. @@ -478,6 +489,14 @@ private func hoistMarkUnresolvedInsts(stackAddress: Value, .replaceAll(with: mu, context) } +private func makeAccessesStatic(of address: Value, _ context: FunctionPassContext) { + for beginAccess in address.uses.users(ofType: BeginAccessInst.self) { + if beginAccess.enforcement == .dynamic { + beginAccess.set(enforcement: .static, context: context) + } + } +} + private extension ApplySite { func getSpecializableCallee() -> Function? { if let callee = referencedFunction, diff --git a/test/SILOptimizer/allocbox_to_stack_ownership.sil b/test/SILOptimizer/allocbox_to_stack_ownership.sil index 3aa046df52537..06659d2f26ca9 100644 --- a/test/SILOptimizer/allocbox_to_stack_ownership.sil +++ b/test/SILOptimizer/allocbox_to_stack_ownership.sil @@ -1435,3 +1435,19 @@ die: destroy_value %copy unreachable } + +// CHECK-LABEL: sil [ossa] @test_access_enforcement : +// OPT: begin_access [modify] [dynamic] +// MANDATORY: begin_access [modify] [static] +// CHECK-LABEL: } // end sil function 'test_access_enforcement' +sil [ossa] @test_access_enforcement : $(Int) -> Int { +bb0(%0 : $Int): + %1 = alloc_box ${ var Int } + %2 = project_box %1 : ${ var Int }, 0 + %3 = begin_access [modify] [dynamic] %2 + store %0 to [trivial] %3 + end_access %3 + %4 = load [trivial] %2 + destroy_value %1 + return %4 : $Int +} diff --git a/test/SILOptimizer/allocboxtostack_localapply.swift b/test/SILOptimizer/allocboxtostack_localapply.swift index 06f375b9bbd7a..188e7eef18afa 100644 --- a/test/SILOptimizer/allocboxtostack_localapply.swift +++ b/test/SILOptimizer/allocboxtostack_localapply.swift @@ -179,6 +179,7 @@ public func testdfs2() -> Int { // CHECK-LABEL: sil @$s26allocboxtostack_localapply15call2localfuncsSiyF : // CHECK-NOT: alloc_box +// CHECK-NOT: begin_access // CHECK-LABEL:} // end sil function '$s26allocboxtostack_localapply15call2localfuncsSiyF' public func call2localfuncs() -> Int { var a1 = 1 @@ -194,3 +195,6 @@ public func call2localfuncs() -> Int { return a1 } +// CHECK-LABEL: sil {{.*}} @$s26allocboxtostack_localapply15call2localfuncsSiyF13innerFunctionL_yyFTf0s_n : +// CHECK-NOT: begin_access +// CHECK: } // end sil function '$s26allocboxtostack_localapply15call2localfuncsSiyF13innerFunctionL_yyFTf0s_n'