From a0eb58fa073b4745725a835685019ce12afe1e59 Mon Sep 17 00:00:00 2001 From: Meghana Gupta Date: Tue, 28 Oct 2025 05:10:57 -0700 Subject: [PATCH 1/8] Handle return_borrow in a few more places in SwiftCompilerSources --- .../Optimizer/Analysis/AliasAnalysis.swift | 6 ++-- .../FunctionPasses/ComputeEscapeEffects.swift | 8 ++--- .../FunctionPasses/ComputeSideEffects.swift | 6 ++-- .../LifetimeDependenceScopeFixup.swift | 2 +- .../FunctionPasses/PackSpecialization.swift | 2 +- .../TestPasses/EscapeInfoDumper.swift | 4 +-- .../Optimizer/Utilities/EscapeUtils.swift | 4 +-- .../Utilities/LifetimeDependenceUtils.swift | 2 +- .../Optimizer/Utilities/OptUtils.swift | 2 +- .../Sources/SIL/Argument.swift | 2 +- .../Sources/SIL/Function.swift | 4 +-- test/SILOptimizer/escape_effects.sil | 30 +++++++++++++++++++ 12 files changed, 51 insertions(+), 21 deletions(-) diff --git a/SwiftCompilerSources/Sources/Optimizer/Analysis/AliasAnalysis.swift b/SwiftCompilerSources/Sources/Optimizer/Analysis/AliasAnalysis.swift index 62b433d473fcc..c5f84ced7e707 100644 --- a/SwiftCompilerSources/Sources/Optimizer/Analysis/AliasAnalysis.swift +++ b/SwiftCompilerSources/Sources/Optimizer/Analysis/AliasAnalysis.swift @@ -775,7 +775,7 @@ private struct FullApplyEffectsVisitor : EscapeVisitorWithResult { mutating func visitUse(operand: Operand, path: EscapePath) -> UseResult { let user = operand.instruction - if user is ReturnInst { + if user is ReturnInstruction { // Anything which is returned cannot escape to an instruction inside the function. return .ignore } @@ -805,7 +805,7 @@ private struct PartialApplyEffectsVisitor : EscapeVisitorWithResult { mutating func visitUse(operand: Operand, path: EscapePath) -> UseResult { let user = operand.instruction - if user is ReturnInst { + if user is ReturnInstruction { // Anything which is returned cannot escape to an instruction inside the function. return .ignore } @@ -848,7 +848,7 @@ private struct EscapesToInstructionVisitor : EscapeVisitor { if user == target { return .abort } - if user is ReturnInst { + if user is ReturnInstruction { // Anything which is returned cannot escape to an instruction inside the function. return .ignore } diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeEscapeEffects.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeEscapeEffects.swift index 3b2ab2c5e242c..4693454da27d6 100644 --- a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeEscapeEffects.swift +++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeEscapeEffects.swift @@ -81,7 +81,7 @@ let computeEscapeEffects = FunctionPass(name: "compute-escape-effects") { private func addArgEffects(_ arg: FunctionArgument, argPath ap: SmallProjectionPath, to newEffects: inout [EscapeEffects.ArgumentEffect], - _ returnInst: ReturnInst?, _ context: FunctionPassContext) -> Bool { + _ returnInst: ReturnInstruction?, _ context: FunctionPassContext) -> Bool { // Correct the path if the argument is not a class reference itself, but a value type // containing one or more references. let argPath = arg.type.isClass ? ap : ap.push(.anyValueFields) @@ -157,7 +157,7 @@ private struct ArgEffectsVisitor : EscapeVisitorWithResult { var result = EscapeDestination.notSet mutating func visitUse(operand: Operand, path: EscapePath) -> UseResult { - if operand.instruction is ReturnInst { + if operand.instruction is ReturnInstruction { // The argument escapes to the function return if path.followStores { // The escaping path must not introduce a followStores. @@ -209,13 +209,13 @@ private struct IsExclusiveReturnEscapeVisitor : EscapeVisitorWithResult { let returnPath: SmallProjectionPath var result = false - func isExclusiveEscape(returnInst: ReturnInst, _ context: FunctionPassContext) -> Bool { + func isExclusiveEscape(returnInst: ReturnInstruction, _ context: FunctionPassContext) -> Bool { return returnInst.returnedValue.at(returnPath).visit(using: self, context) ?? false } mutating func visitUse(operand: Operand, path: EscapePath) -> UseResult { switch operand.instruction { - case is ReturnInst: + case is ReturnInstruction: if path.followStores { return .abort } if path.projectionPath.matches(pattern: returnPath) { return .ignore diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeSideEffects.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeSideEffects.swift index 7c5d8bee4b635..1c4a22fd41a42 100644 --- a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeSideEffects.swift +++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeSideEffects.swift @@ -254,7 +254,7 @@ private struct CollectedEffects { // effect, it would not give any significant benefit in any of our current optimizations. addEffects(.destroy, to: inst.operands[0].value, fromInitialPath: SmallProjectionPath(.anyValueFields)) - case is ReturnInst: + case is ReturnInstruction: if inst.parentFunction.convention.hasAddressResult { addEffects(.read, to: inst.operands[0].value) } @@ -532,7 +532,7 @@ private struct ArgumentEscapingWalker : ValueDefUseWalker, AddressDefUseWalker { case is CopyValueInst, is RetainValueInst, is StrongRetainInst, is DestroyValueInst, is ReleaseValueInst, is StrongReleaseInst, is DebugValueInst, is UnconditionalCheckedCastInst, - is ReturnInst: + is ReturnInstruction: return .continueWalk case let apply as ApplySite: @@ -620,7 +620,7 @@ private extension PartialApplyInst { // Any escape to apply - regardless if it's an argument or the callee operand - might cause // the closure to be called. return .abort - case is ReturnInst: + case is ReturnInstruction: return .ignore default: return .continueWalk diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift index f90815bbee2d9..0ae577a587127 100644 --- a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift +++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift @@ -894,7 +894,7 @@ extension ExtendableScope { assert(end.parentBlock.singleSuccessor!.terminator is ReturnInst, "a phi only ends a use range if it is a returned value") fallthrough - case is ReturnInst: + case is ReturnInstruction: // End this inner scope just before the return. The mark_dependence base operand will be redirected to a // function argument. let builder = Builder(before: end, location: location, context) diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/PackSpecialization.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/PackSpecialization.swift index 820f36e421c3b..a77335dae994d 100644 --- a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/PackSpecialization.swift +++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/PackSpecialization.swift @@ -682,7 +682,7 @@ private struct PackExplodedFunction { if resultMap.contains(where: { $0.expandedElements.contains(where: { !$0.isSILIndirect }) }) { - if let originalReturnStatement = specialized.returnInstruction { + if let originalReturnStatement = specialized.returnInstruction as? ReturnInst { self.createExplodedReturn( replacing: originalReturnStatement, argumentMap: argumentMap, specContext) } diff --git a/SwiftCompilerSources/Sources/Optimizer/TestPasses/EscapeInfoDumper.swift b/SwiftCompilerSources/Sources/Optimizer/TestPasses/EscapeInfoDumper.swift index d13427a94c5c9..f3ab94d275fe8 100644 --- a/SwiftCompilerSources/Sources/Optimizer/TestPasses/EscapeInfoDumper.swift +++ b/SwiftCompilerSources/Sources/Optimizer/TestPasses/EscapeInfoDumper.swift @@ -26,7 +26,7 @@ let escapeInfoDumper = FunctionPass(name: "dump-escape-info") { var result: Set = Set() mutating func visitUse(operand: Operand, path: EscapePath) -> UseResult { - if operand.instruction is ReturnInst { + if operand.instruction is ReturnInstruction { result.insert("return[\(path.projectionPath)]") return .ignore } @@ -94,7 +94,7 @@ let addressEscapeInfoDumper = FunctionPass(name: "dump-addr-escape-info") { if user == apply { return .abort } - if user is ReturnInst { + if user is ReturnInstruction { // Anything which is returned cannot escape to an instruction inside the function. return .ignore } diff --git a/SwiftCompilerSources/Sources/Optimizer/Utilities/EscapeUtils.swift b/SwiftCompilerSources/Sources/Optimizer/Utilities/EscapeUtils.swift index 278c26c12cd4b..536f93ff3808f 100644 --- a/SwiftCompilerSources/Sources/Optimizer/Utilities/EscapeUtils.swift +++ b/SwiftCompilerSources/Sources/Optimizer/Utilities/EscapeUtils.swift @@ -387,7 +387,7 @@ fileprivate struct EscapeWalker : ValueDefUseWalker, if handleDestroy(of: operand.value, path: path) == .abortWalk { return .abortWalk } - case is ReturnInst: + case is ReturnInstruction: return isEscaping case is ApplyInst, is TryApplyInst, is BeginApplyInst: return walkDownCallee(argOp: operand, apply: instruction as! FullApplySite, path: path) @@ -534,7 +534,7 @@ fileprivate struct EscapeWalker : ValueDefUseWalker, if handleDestroy(of: operand.value, path: path) == .abortWalk { return .abortWalk } - case is ReturnInst: + case is ReturnInstruction: return isEscaping case is ApplyInst, is TryApplyInst, is BeginApplyInst: return walkDownCallee(argOp: operand, apply: instruction as! FullApplySite, path: path) diff --git a/SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift b/SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift index bf7e8e68bf6dd..ac38efd7036d5 100644 --- a/SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift +++ b/SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift @@ -730,7 +730,7 @@ extension LifetimeDependenceDefUseWalker { if let apply = operand.instruction as? FullApplySite { return visitAppliedUse(of: operand, by: apply) } - if operand.instruction is ReturnInst, !operand.value.isEscapable { + if operand.instruction is ReturnInstruction, !operand.value.isEscapable { return returnedDependence(result: operand) } if operand.instruction is YieldInst, !operand.value.isEscapable { diff --git a/SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift b/SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift index c5d256f221aa2..443a44eeb07d2 100644 --- a/SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift +++ b/SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift @@ -880,7 +880,7 @@ private struct EscapesToValueVisitor : EscapeVisitor { if operand.value == target.value && path.projectionPath.mayOverlap(with: target.path) { return .abort } - if operand.instruction is ReturnInst { + if operand.instruction is ReturnInstruction { // Anything which is returned cannot escape to an instruction inside the function. return .ignore } diff --git a/SwiftCompilerSources/Sources/SIL/Argument.swift b/SwiftCompilerSources/Sources/SIL/Argument.swift index 837489c4b3a5e..88676db46a375 100644 --- a/SwiftCompilerSources/Sources/SIL/Argument.swift +++ b/SwiftCompilerSources/Sources/SIL/Argument.swift @@ -212,7 +212,7 @@ public struct Phi { extension Phi { /// Return true of this phi is directly returned with no side effects between the phi and the return. public var isReturnValue: Bool { - if let singleUse = value.uses.singleUse, let ret = singleUse.instruction as? ReturnInst, + if let singleUse = value.uses.singleUse, let ret = singleUse.instruction as? ReturnInstruction, ret.parentBlock == successor { for inst in successor.instructions { if inst.mayHaveSideEffects { diff --git a/SwiftCompilerSources/Sources/SIL/Function.swift b/SwiftCompilerSources/Sources/SIL/Function.swift index e96b44229b5ca..a500cb63befaf 100644 --- a/SwiftCompilerSources/Sources/SIL/Function.swift +++ b/SwiftCompilerSources/Sources/SIL/Function.swift @@ -121,9 +121,9 @@ final public class Function : CustomStringConvertible, HasShortDescription, Hash blocks.reversed().lazy.flatMap { $0.instructions.reversed() } } - public var returnInstruction: ReturnInst? { + public var returnInstruction: ReturnInstruction? { for block in blocks.reversed() { - if let retInst = block.terminator as? ReturnInst { return retInst } + if let retInst = block.terminator as? ReturnInstruction { return retInst } } return nil } diff --git a/test/SILOptimizer/escape_effects.sil b/test/SILOptimizer/escape_effects.sil index e4bd20b653db9..485b0e8ba7b69 100644 --- a/test/SILOptimizer/escape_effects.sil +++ b/test/SILOptimizer/escape_effects.sil @@ -384,3 +384,33 @@ bb0(%0 : $*Str): return %1 : $Y } +class Klass {} + +struct Wrapper { + let _k: Klass +} + +// CHECK: sil [ossa] @borrow_accessor1 : $@convention(thin) (@guaranteed Wrapper) -> @guaranteed Klass { +// CHECK: [%0: escape v** => %r.v**, escape v**.c*.v** => %r.v**.c*.v**] +sil [ossa] @borrow_accessor1 : $@convention(thin) (@guaranteed Wrapper) -> @guaranteed Klass { +bb0(%0 : @guaranteed $Wrapper): + %1 = struct_extract %0, #Wrapper._k + return %1 +} + +// CHECK: sil [ossa] @borrow_accessor2 : $@convention(thin) (@in_guaranteed Wrapper) -> @guaranteed Klass { +// CHECK: [%0: escape v** -> %r.v**, escape v**.c*.v** -> %r.v**.c*.v**] +sil [ossa] @borrow_accessor2 : $@convention(thin) (@in_guaranteed Wrapper) -> @guaranteed Klass { +bb0(%0 : $*Wrapper): + %1 = load_borrow %0 + %2 = struct_extract %1, #Wrapper._k + return_borrow %1 from_scopes (%1) +} + +// CHECK: sil [ossa] @mutate_accessor : $@convention(thin) (@inout Wrapper) -> @inout Klass { +// CHECK: [%0: escape v** => %r.v**, escape v**.c*.v** => %r.v**.c*.v**] +sil [ossa] @mutate_accessor : $@convention(thin) (@inout Wrapper) -> @inout Klass { +bb0(%0 : $*Wrapper): + %2 = struct_element_addr %0, #Wrapper._k + return %2 +} From 32cd86f719dcedeac03dfcb785e2d8ac6d902a00 Mon Sep 17 00:00:00 2001 From: Meghana Gupta Date: Tue, 28 Oct 2025 11:49:02 -0700 Subject: [PATCH 2/8] Update access base for borrow/mutate accessors in SwiftCompilerSources --- .../Sources/SIL/Utilities/WalkUtils.swift | 3 ++ test/SILOptimizer/accessbase_unit.sil | 30 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/SwiftCompilerSources/Sources/SIL/Utilities/WalkUtils.swift b/SwiftCompilerSources/Sources/SIL/Utilities/WalkUtils.swift index 59c27a7cf291a..e887cc27b13f9 100644 --- a/SwiftCompilerSources/Sources/SIL/Utilities/WalkUtils.swift +++ b/SwiftCompilerSources/Sources/SIL/Utilities/WalkUtils.swift @@ -838,6 +838,9 @@ extension AddressUseDefWalker { } else { return walkUp(address: ia.base, path: path.push(.anyIndexedElement, index: 0)) } + case let apply as ApplyInst: + let selfArgument = apply.arguments.last! + return walkUp(address: selfArgument, path: path.push(.anyValueFields, index: 0)) case is BeginAccessInst, is MarkDependenceInst, is MarkUninitializedInst, diff --git a/test/SILOptimizer/accessbase_unit.sil b/test/SILOptimizer/accessbase_unit.sil index 3c6af4bdad822..eb42e01b48bf1 100644 --- a/test/SILOptimizer/accessbase_unit.sil +++ b/test/SILOptimizer/accessbase_unit.sil @@ -86,3 +86,33 @@ bb0(%0 : @owned $C): %9999 = tuple() return %9999 : $() } + +public struct GenWrapper { + @_hasStorage var _prop: T { get set } + public var prop: T +} + +sil @borrow_addressonly_prop : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0 +// CHECK: begin running test 1 of 2 on test_borrow_base: get_access_base with: %2 +// CHECK: sil [ossa] @test_borrow_base : $@convention(thin) (@in_guaranteed GenWrapper) -> @guaranteed_address T { +// CHECK: bb0(%0 : $*GenWrapper): +// CHECK: %1 = function_ref @borrow_addressonly_prop : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0 +// CHECK: %2 = apply %1(%0) : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0 +// CHECK: return %2 +// CHECK: } +// CHECK: Address: %2 = apply %1(%0) : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0 +// CHECK: Base: %0 = argument of bb0 : $*GenWrapper +// CHECK: end running test 1 of 2 on test_borrow_base: get_access_base with: %2 +// CHECK: begin running test 2 of 2 on test_borrow_base: swift_get_access_base with: %2 +// CHECK: Address: %2 = apply %1(%0) : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0 +// CHECK: Base: argument - %0 = argument of bb0 : $*GenWrapper + +sil [ossa] @test_borrow_base : $@convention(thin) (@in_guaranteed GenWrapper) -> @guaranteed_address T { +bb0(%0 : $*GenWrapper): + %1 = function_ref @borrow_addressonly_prop : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0 + %2 = apply %1(%0) : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0 + specify_test "get_access_base %2" + specify_test "swift_get_access_base %2" + return %2 +} + From 778ad0dfb57d93822607401c6ce8fb36bbdc1de2 Mon Sep 17 00:00:00 2001 From: Meghana Gupta Date: Wed, 5 Nov 2025 12:42:23 -0800 Subject: [PATCH 3/8] Add SIL verification to ensure we don't return local addresses --- lib/SIL/Verifier/SILVerifier.cpp | 11 ++++++++ test/SIL/borrow_accessor_verify_errors.sil | 32 ++++++++++++++++++++++ test/SILGen/borrow_accessor_failues.swift | 2 +- 3 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 test/SIL/borrow_accessor_verify_errors.sil diff --git a/lib/SIL/Verifier/SILVerifier.cpp b/lib/SIL/Verifier/SILVerifier.cpp index 9ea91953ef8e5..8978a6c6c4e51 100644 --- a/lib/SIL/Verifier/SILVerifier.cpp +++ b/lib/SIL/Verifier/SILVerifier.cpp @@ -5359,6 +5359,17 @@ class SILVerifier : public SILVerifierBase { instResultType.dump();); requireSameType(functionResultType, instResultType, "return value type does not match return type of function"); + + // If the result type is an address, ensure it's base address is from a + // function argument. + if (F.getModule().getStage() >= SILStage::Canonical && + functionResultType.isAddress()) { + auto base = getAccessBase(RI->getOperand()); + require(!base->getType().isAddress() || isa(base) || + isa(base) && + cast(base)->hasAddressResult(), + "unidentified address return"); + } } void checkThrowInst(ThrowInst *TI) { diff --git a/test/SIL/borrow_accessor_verify_errors.sil b/test/SIL/borrow_accessor_verify_errors.sil new file mode 100644 index 0000000000000..a9e508c16734f --- /dev/null +++ b/test/SIL/borrow_accessor_verify_errors.sil @@ -0,0 +1,32 @@ +// RUN: %target-sil-opt -dont-abort-on-memory-lifetime-errors -verify-continue-on-failure -enable-sil-verify-all=0 %s 2>&1 | %FileCheck %s + +sil_stage canonical + +public struct GenWrapper { + @_hasStorage var _prop: T { get set } + public var prop: T +} + +sil [ossa] @borrow_addressonly_prop : $@convention(method) (@in_guaranteed GenWrapper) -> @guaranteed_address T { +bb0(%0 : $*GenWrapper): + %2 = struct_element_addr %0, #GenWrapper._prop + return %2 +} + +// CHECK: Begin Error in function test_address_of_temp +// CHECK: SIL verification failed: unidentified address return: !base->getType().isAddress() || isa(base) || isa(base) && cast(base)->hasAddressResult() +// CHECK: Verifying instruction: +// CHECK: %4 = apply %3(%1) : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0 +// CHECK: -> return %4 : $*T +// CHECK: End Error in function test_address_of_temp +sil [ossa] @test_address_of_temp : $@convention(thin) (@in_guaranteed GenWrapper) -> @guaranteed_address T { +bb0(%0 : $*GenWrapper): + %stk = alloc_stack $GenWrapper + copy_addr %0 to [init] %stk + %1 = function_ref @borrow_addressonly_prop : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0 + %2 = apply %1(%stk) : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0 + destroy_addr %stk + dealloc_stack %stk + return %2 +} + diff --git a/test/SILGen/borrow_accessor_failues.swift b/test/SILGen/borrow_accessor_failues.swift index 488665bacc6d3..5e1c300e876db 100644 --- a/test/SILGen/borrow_accessor_failues.swift +++ b/test/SILGen/borrow_accessor_failues.swift @@ -1,4 +1,4 @@ -// RUN:%target-swift-frontend -emit-silgen %s -verify -enable-experimental-feature BorrowAndMutateAccessors +// RUN:%target-swift-frontend -emit-silgen %s -enable-experimental-feature BorrowAndMutateAccessors -sil-verify-none -verify // REQUIRES: swift_feature_BorrowAndMutateAccessors From e48bdf7307eb17878b97057ff0a56ac77b55c1a6 Mon Sep 17 00:00:00 2001 From: Meghana Gupta Date: Tue, 28 Oct 2025 14:34:04 -0700 Subject: [PATCH 4/8] [NFC] Test rename --- ...rrow_accessor_failues.swift => borrow_accessor_failures.swift} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/SILGen/{borrow_accessor_failues.swift => borrow_accessor_failures.swift} (100%) diff --git a/test/SILGen/borrow_accessor_failues.swift b/test/SILGen/borrow_accessor_failures.swift similarity index 100% rename from test/SILGen/borrow_accessor_failues.swift rename to test/SILGen/borrow_accessor_failures.swift From d50aa8ad8757ba0a28e79d1f51db60c58ac0f3ec Mon Sep 17 00:00:00 2001 From: Meghana Gupta Date: Tue, 28 Oct 2025 14:55:59 -0700 Subject: [PATCH 5/8] [NFC] Comment on the use of unchecked_ownership --- lib/SILGen/SILGenApply.cpp | 3 +++ lib/SILGen/SILGenStmt.cpp | 3 +++ 2 files changed, 6 insertions(+) diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index 06070526901c9..8975c16d061e6 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -5483,6 +5483,9 @@ ManagedValue SILGenFunction::applyBorrowMutateAccessor( if (fn.getFunction()->getConventions().hasGuaranteedResult()) { auto selfArg = args.back().getValue(); if (isa(selfArg)) { + // unchecked_ownership is used to silence the ownership verifier for + // returning a value produced within a load_borrow scope. SILGenCleanup + // eliminates it and introduces return_borrow appropriately. rawResult = B.createUncheckedOwnership(loc, rawResult); } } diff --git a/lib/SILGen/SILGenStmt.cpp b/lib/SILGen/SILGenStmt.cpp index 095cc11e48bda..51233f2021861 100644 --- a/lib/SILGen/SILGenStmt.cpp +++ b/lib/SILGen/SILGenStmt.cpp @@ -720,6 +720,9 @@ bool SILGenFunction::emitBorrowOrMutateAccessorResult( assert(F.getConventions().hasGuaranteedResult()); auto regularLoc = RegularLocation::getAutoGeneratedLocation(); auto load = B.createLoadBorrow(regularLoc, guaranteedAddress).getValue(); + // unchecked_ownership is used to silence the ownership verifier for + // returning a value produced within a load_borrow scope. SILGenCleanup + // eliminates it and introduces return_borrow appropriately. return B.createUncheckedOwnership(regularLoc, load); }; From 21cc1a185b781072ab9b62c9da694c875d6d6e2c Mon Sep 17 00:00:00 2001 From: Meghana Gupta Date: Thu, 30 Oct 2025 12:47:10 -0700 Subject: [PATCH 6/8] Add memory lifetime verification support for borrow accessors --- include/swift/SIL/SILFunctionConventions.h | 13 ++- include/swift/SIL/SILInstruction.h | 4 + lib/SIL/Utils/MemoryLocations.cpp | 31 ++++- lib/SIL/Verifier/MemoryLifetimeVerifier.cpp | 2 +- .../SIL/OwnershipVerifier/borrow_accessor.sil | 24 ---- test/SIL/memory_lifetime.sil | 11 ++ test/SIL/memory_lifetime_failures.sil | 110 +++++++++++++++++- 7 files changed, 157 insertions(+), 38 deletions(-) diff --git a/include/swift/SIL/SILFunctionConventions.h b/include/swift/SIL/SILFunctionConventions.h index bae6d44124c5e..31b912625889b 100644 --- a/include/swift/SIL/SILFunctionConventions.h +++ b/include/swift/SIL/SILFunctionConventions.h @@ -338,15 +338,18 @@ class SILFunctionConventions { } bool hasAddressResult() const { + return hasGuaranteedAddressResult() || hasInoutResult(); + } + + bool hasGuaranteedAddressResult() const { if (funcTy->getNumResults() != 1) { return false; } - auto resultConvention = funcTy->getResults()[0].getConvention(); - if (silConv.loweredAddresses) { - return resultConvention == ResultConvention::GuaranteedAddress || - resultConvention == ResultConvention::Inout; + if (!silConv.loweredAddresses) { + return false; } - return resultConvention == ResultConvention::Inout; + auto resultConvention = funcTy->getResults()[0].getConvention(); + return resultConvention == ResultConvention::GuaranteedAddress; } struct SILResultTypeFunc; diff --git a/include/swift/SIL/SILInstruction.h b/include/swift/SIL/SILInstruction.h index efad3428a1a6c..56b4fbf61f6ad 100644 --- a/include/swift/SIL/SILInstruction.h +++ b/include/swift/SIL/SILInstruction.h @@ -3155,6 +3155,10 @@ class ApplyInst final return getSubstCalleeConv().hasGuaranteedResult(); } + bool hasGuaranteedAddressResult() const { + return getSubstCalleeConv().hasGuaranteedAddressResult(); + } + bool hasAddressResult() const { return getSubstCalleeConv().hasAddressResult(); } diff --git a/lib/SIL/Utils/MemoryLocations.cpp b/lib/SIL/Utils/MemoryLocations.cpp index a36e0163d779e..e68513b0049c3 100644 --- a/lib/SIL/Utils/MemoryLocations.cpp +++ b/lib/SIL/Utils/MemoryLocations.cpp @@ -66,10 +66,15 @@ MemoryLocations::Location::Location(SILValue val, unsigned index, int parentIdx) representativeValue(val), parentIdx(parentIdx) { assert(((parentIdx >= 0) == - (isa(val) || isa(val) || - isa(val) || isa(val) || - isa(val) || isa(val))) - && "sub-locations can only be introduced with struct/tuple/enum projections"); + (isa(val) || isa(val) || + isa(val) || + isa(val) || + isa(val) || + isa(val) || isa(val) || + isa(val))) && + "sub-locations can only be introduced with " + "struct/tuple/enum/store_borrow/borrow accessor " + "projections"); setBitAndResize(subLocations, index); setBitAndResize(selfAndParents, index); } @@ -349,6 +354,21 @@ bool MemoryLocations::analyzeLocationUsesRecursively(SILValue V, unsigned locIdx if (cast(user)->hasAddrVal()) break; return false; + case SILInstructionKind::ApplyInst: { + auto *apply = cast(user); + if (apply->hasAddressResult()) { + if (!analyzeAddrProjection(apply, locIdx, 0, collectedVals, + subLocationMap)) + return false; + } + break; + } + case SILInstructionKind::StoreBorrowInst: { + if (!analyzeAddrProjection(cast(user), locIdx, 0, + collectedVals, subLocationMap)) + return false; + break; + } case SILInstructionKind::InjectEnumAddrInst: case SILInstructionKind::SelectEnumAddrInst: case SILInstructionKind::ExistentialMetatypeInst: @@ -357,13 +377,11 @@ bool MemoryLocations::analyzeLocationUsesRecursively(SILValue V, unsigned locIdx case SILInstructionKind::FixLifetimeInst: case SILInstructionKind::LoadInst: case SILInstructionKind::StoreInst: - case SILInstructionKind::StoreBorrowInst: case SILInstructionKind::EndAccessInst: case SILInstructionKind::DestroyAddrInst: case SILInstructionKind::CheckedCastAddrBranchInst: case SILInstructionKind::UncheckedRefCastAddrInst: case SILInstructionKind::UnconditionalCheckedCastAddrInst: - case SILInstructionKind::ApplyInst: case SILInstructionKind::TryApplyInst: case SILInstructionKind::BeginApplyInst: case SILInstructionKind::CopyAddrInst: @@ -371,6 +389,7 @@ bool MemoryLocations::analyzeLocationUsesRecursively(SILValue V, unsigned locIdx case SILInstructionKind::DeallocStackInst: case SILInstructionKind::SwitchEnumAddrInst: case SILInstructionKind::WitnessMethodInst: + case SILInstructionKind::EndBorrowInst: break; case SILInstructionKind::MarkUnresolvedMoveAddrInst: // We do not want the memory lifetime verifier to verify move_addr inst diff --git a/lib/SIL/Verifier/MemoryLifetimeVerifier.cpp b/lib/SIL/Verifier/MemoryLifetimeVerifier.cpp index 0ee5cd00f3d5b..0c41d088d5110 100644 --- a/lib/SIL/Verifier/MemoryLifetimeVerifier.cpp +++ b/lib/SIL/Verifier/MemoryLifetimeVerifier.cpp @@ -335,7 +335,7 @@ bool MemoryLifetimeVerifier::applyMayRead(Operand *argOp, SILValue addr) { void MemoryLifetimeVerifier::requireNoStoreBorrowLocation( SILValue addr, SILInstruction *where) { - if (isa(addr)) { + if (isStoreBorrowLocation(addr)) { reportError("store-borrow location cannot be written", locations.getLocationIdx(addr), where); } diff --git a/test/SIL/OwnershipVerifier/borrow_accessor.sil b/test/SIL/OwnershipVerifier/borrow_accessor.sil index 7aab0c25969f9..e6a986a6c0220 100644 --- a/test/SIL/OwnershipVerifier/borrow_accessor.sil +++ b/test/SIL/OwnershipVerifier/borrow_accessor.sil @@ -8,26 +8,14 @@ public struct Wrapper { var k: Klass } -public struct GenWrapper { - @_hasStorage var _prop: T { get set } - public var prop: T -} - sil [ossa] @borrow_loadable_prop : $@convention(method) (@guaranteed Wrapper) -> @guaranteed Klass { bb0(%0 : @guaranteed $Wrapper): %2 = struct_extract %0, #Wrapper._k return %2 } -sil [ossa] @borrow_addressonly_prop : $@convention(method) (@in_guaranteed GenWrapper) -> @guaranteed_address T { -bb0(%0 : $*GenWrapper): - %2 = struct_element_addr %0, #GenWrapper._prop - return %2 -} - sil @get_wrapper : $@convention(thin) () -> @owned Klass sil @use_klass : $@convention(thin) (@guaranteed Klass) -> () -sil @use_T : $@convention(thin) (@in_guaranteed T) -> () // CHECK-LABEL: Error#: 0. Begin Error in Function: 'test_end_borrow_on_guaranteed_return_value' // CHECK: Invalid End Borrow! @@ -151,15 +139,3 @@ bb0(%0 : @owned $Wrapper): return %6 } -// TODO: Add verification support in MemoryLifetimeVerifier -sil [ossa] @test_use_after_free_address_only : $@convention(thin) (@in GenWrapper) -> () { -bb0(%0 : $*GenWrapper): - %1 = function_ref @borrow_addressonly_prop : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0 - %2 = apply %1(%0) : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0 - %3 = function_ref @use_T : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> () - destroy_addr %0 - %5 = apply %3(%2) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> () - %6 = tuple () - return %6 -} - diff --git a/test/SIL/memory_lifetime.sil b/test/SIL/memory_lifetime.sil index 17542002b8e92..f4ba812b65401 100644 --- a/test/SIL/memory_lifetime.sil +++ b/test/SIL/memory_lifetime.sil @@ -532,6 +532,17 @@ bb0(%0 : @guaranteed $T): return %res : $() } +sil [ossa] @test_store_borrow_nested : $@convention(thin) (@guaranteed Inner) -> () { +bb0(%0 : @guaranteed $Inner): + %s = alloc_stack $Inner + %sb = store_borrow %0 to %s + %elem = struct_element_addr %sb, #Inner.a + end_borrow %sb + dealloc_stack %s + %res = tuple () + return %res : $() +} + sil [ossa] @test_cast_br_take_always : $@convention(thin) (@in U) -> () { bb0(%0 : $*U): %s = alloc_stack $V diff --git a/test/SIL/memory_lifetime_failures.sil b/test/SIL/memory_lifetime_failures.sil index 728581fef0c0e..f49fa90da42af 100644 --- a/test/SIL/memory_lifetime_failures.sil +++ b/test/SIL/memory_lifetime_failures.sil @@ -29,9 +29,24 @@ struct Mixed { var i: Int } +public struct InnerWrapper { + @_hasStorage var _prop: T { get set } + public var prop: T +} + +public struct GenWrapper { + @_hasStorage var _prop: T { get set } + public var prop: T + + @_hasStorage var _nestedProp: InnerWrapper { get set } + public var nestedProp: InnerWrapper +} + sil @use_owned : $@convention(thin) (@owned T) -> () sil @use_guaranteed : $@convention(thin) (@guaranteed T) -> () sil @get_owned : $@convention(thin) () -> @owned T +sil @use_T : $@convention(thin) (@in_guaranteed T) -> () +sil @mutate_T : $@convention(thin) (@inout T) -> () // CHECK: SIL memory lifetime failure in @test_simple: indirect argument is not alive at function return sil [ossa] @test_simple : $@convention(thin) (@inout T) -> @owned T { @@ -276,8 +291,8 @@ bb0(%0 : $*T): return %res : $() } -// CHECK: SIL memory lifetime failure in @test_store_borrow_destroy: store-borrow location cannot be written -sil [ossa] @test_store_borrow_destroy : $@convention(thin) (@guaranteed T) -> () { +// CHECK: SIL memory lifetime failure in @test_store_borrow_destroy1: store-borrow location cannot be written +sil [ossa] @test_store_borrow_destroy1 : $@convention(thin) (@guaranteed T) -> () { bb0(%0 : @guaranteed $T): %s = alloc_stack $T %sb = store_borrow %0 to %s : $*T @@ -288,6 +303,19 @@ bb0(%0 : @guaranteed $T): return %res : $() } +// CHECK: SIL memory lifetime failure in @test_store_borrow_destroy2: store-borrow location cannot be written +sil [ossa] @test_store_borrow_destroy2 : $@convention(thin) (@guaranteed Inner) -> () { +bb0(%0 : @guaranteed $Inner): + %s = alloc_stack $Inner + %sb = store_borrow %0 to %s + %elem = struct_element_addr %sb, #Inner.a + destroy_addr %elem + end_borrow %sb + dealloc_stack %s + %res = tuple () + return %res : $() +} + sil [ossa] @func_with_inout_param : $@convention(thin) (@inout T) -> () // T-CHECK: SIL memory lifetime failure in @test_store_borrow_inout: store-borrow location cannot be written @@ -842,3 +870,81 @@ bb0(%0 : @owned $T, %1 : @owned $Inner): dealloc_stack %2 return %8 } + +sil [ossa] @borrow_addressonly_prop : $@convention(method) (@in_guaranteed GenWrapper) -> @guaranteed_address T { +bb0(%0 : $*GenWrapper): + %2 = struct_element_addr %0, #GenWrapper._prop + return %2 +} + +sil [ossa] @mutate_addressonly_prop : $@convention(method) (@inout GenWrapper) -> @inout T { +bb0(%0 : $*GenWrapper): + %2 = struct_element_addr %0, #GenWrapper._prop + return %2 +} + +sil [ossa] @borrow_addressonly_nested_prop : $@convention(method) (@in_guaranteed GenWrapper) -> @guaranteed_address InnerWrapper { +bb0(%0 : $*GenWrapper): + %2 = struct_element_addr %0, #GenWrapper._nestedProp + return %2 +} + +sil [ossa] @mutate_addressonly_nested_prop : $@convention(method) (@inout GenWrapper) -> @inout InnerWrapper { +bb0(%0 : $*GenWrapper): + %2 = struct_element_addr %0, #GenWrapper._nestedProp + return %2 +} + +// CHECK: SIL memory lifetime failure in @test_use_after_free_address_only1: memory is not initialized, but should be +// CHECK: memory location: %2 = apply %1(%0) : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0 +// CHECK: at instruction: %5 = apply %3(%2) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> () +sil [ossa] @test_use_after_free_address_only1 : $@convention(thin) (@in GenWrapper) -> () { +bb0(%0 : $*GenWrapper): + %1 = function_ref @borrow_addressonly_prop : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0 + %2 = apply %1(%0) : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0 + %3 = function_ref @use_T : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> () + destroy_addr %0 + %5 = apply %3(%2) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> () + %6 = tuple () + return %6 +} + +// CHECK: SIL memory lifetime failure in @test_use_after_free_address_only2: memory is not initialized, but should be +// CHECK: memory location: %2 = apply %1(%0) : $@convention(method) <τ_0_0> (@inout GenWrapper<τ_0_0>) -> @inout τ_0_0 +// CHECK: at instruction: %5 = apply %3(%2) : $@convention(thin) <τ_0_0> (@inout τ_0_0) -> () +sil [ossa] @test_use_after_free_address_only2 : $@convention(thin) (@in GenWrapper) -> () { +bb0(%0 : $*GenWrapper): + %1 = function_ref @mutate_addressonly_prop : $@convention(method) <τ_0_0> (@inout GenWrapper<τ_0_0>) -> @inout τ_0_0 + %2 = apply %1(%0) : $@convention(method) <τ_0_0> (@inout GenWrapper<τ_0_0>) -> @inout τ_0_0 + %3 = function_ref @mutate_T : $@convention(thin) <τ_0_0> (@inout τ_0_0) -> () + destroy_addr %0 + %5 = apply %3(%2) : $@convention(thin) <τ_0_0> (@inout τ_0_0) -> () + %6 = tuple () + return %6 +} + +// TODO-CHECK: SIL memory lifetime failure in @test_guaranteed_address_consume: store-borrow location cannot be written +// TODO-CHECK: memory location: %2 = apply %1(%0) : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0 +// TODO-CHECK: at instruction: destroy_addr %2 : $*T +sil [ossa] @test_guaranteed_address_consume : $@convention(thin) (@in GenWrapper) -> () { +bb0(%0 : $*GenWrapper): + %1 = function_ref @borrow_addressonly_prop : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0 + %2 = apply %1(%0) : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0 + destroy_addr %2 + %4 = tuple () + return %4 +} + +// TODO-CHECK: SIL memory lifetime failure in @test_guaranteed_nested_address_consume: store-borrow location cannot be written +// TODO-CHECK: memory location: %3 = struct_element_addr %2 : $*InnerWrapper, #InnerWrapper._prop +// TODO-CHECK: at instruction: destroy_addr %3 : $*T +sil [ossa] @test_guaranteed_nested_address_consume : $@convention(thin) (@in GenWrapper) -> () { +bb0(%0 : $*GenWrapper): + %1 = function_ref @borrow_addressonly_nested_prop : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address InnerWrapper<τ_0_0> + %2 = apply %1(%0) : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address InnerWrapper<τ_0_0> + %3 = struct_element_addr %2, #InnerWrapper._prop + destroy_addr %3 + %4 = tuple () + return %4 +} + From 290d2f00a3be746d75eed01fc17dab81c515c237 Mon Sep 17 00:00:00 2001 From: Meghana Gupta Date: Thu, 30 Oct 2025 14:55:46 -0700 Subject: [PATCH 7/8] Fix mark_unresolved_noncopyable_value instruction introduced for mutate accessor result --- include/swift/SIL/SILFunctionConventions.h | 8 ++++++++ lib/SILGen/SILGenApply.cpp | 7 ++++++- test/SILGen/borrow_accessor.swift | 12 ++++++------ 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/include/swift/SIL/SILFunctionConventions.h b/include/swift/SIL/SILFunctionConventions.h index 31b912625889b..d8c105f122b58 100644 --- a/include/swift/SIL/SILFunctionConventions.h +++ b/include/swift/SIL/SILFunctionConventions.h @@ -352,6 +352,14 @@ class SILFunctionConventions { return resultConvention == ResultConvention::GuaranteedAddress; } + bool hasInoutResult() const { + if (funcTy->getNumResults() != 1) { + return false; + } + auto resultConvention = funcTy->getResults()[0].getConvention(); + return resultConvention == ResultConvention::Inout; + } + struct SILResultTypeFunc; // Gratuitous template parameter is to delay instantiating `mapped_iterator` diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index 8975c16d061e6..32862d21c1f0b 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -5492,9 +5492,14 @@ ManagedValue SILGenFunction::applyBorrowMutateAccessor( if (rawResult->getType().isMoveOnly()) { if (rawResult->getType().isAddress()) { + SILFunctionConventions substFnConv(substFnType, SGM.M); auto result = B.createMarkUnresolvedNonCopyableValueInst( loc, rawResult, - MarkUnresolvedNonCopyableValueInst::CheckKind::NoConsumeOrAssign); + substFnConv.hasInoutResult() + ? MarkUnresolvedNonCopyableValueInst::CheckKind:: + AssignableButNotConsumable + : MarkUnresolvedNonCopyableValueInst::CheckKind:: + NoConsumeOrAssign); return ManagedValue::forRValueWithoutOwnership(result); } auto result = emitManagedCopy(loc, rawResult); diff --git a/test/SILGen/borrow_accessor.swift b/test/SILGen/borrow_accessor.swift index 94cc4ca1bd1c4..f615f33e49b60 100644 --- a/test/SILGen/borrow_accessor.swift +++ b/test/SILGen/borrow_accessor.swift @@ -699,7 +699,7 @@ public struct GenNCWrapper : ~Copyable { // CHECK: [[REG3:%.*]] = struct_element_addr [[REG2]], #GenNCWrapper._w // CHECK: [[REG4:%.*]] = function_ref @$s15borrow_accessor15SimpleNCWrapperVAARi_zrlE4propxvz : $@convention(method) <τ_0_0 where τ_0_0 : ~Copyable> (@inout SimpleNCWrapper<τ_0_0>) -> @inout τ_0_0 // CHECK: [[REG5:%.*]] = apply [[REG4]]([[REG3]]) : $@convention(method) <τ_0_0 where τ_0_0 : ~Copyable> (@inout SimpleNCWrapper<τ_0_0>) -> @inout τ_0_0 -// CHECK: [[REG6:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG5]] +// CHECK: [[REG6:%.*]] = mark_unresolved_non_copyable_value [assignable_but_not_consumable] [[REG5]] // CHECK: return [[REG6]] // CHECK: } @@ -719,7 +719,7 @@ public struct GenNCWrapper : ~Copyable { // CHECK: [[REG2:%.*]] = mark_unresolved_non_copyable_value [consumable_and_assignable] [[REG0]] // CHECK: [[REG3:%.*]] = function_ref @$s15borrow_accessor12GenNCWrapperVAARi_zrlE4propxvz : $@convention(method) <τ_0_0 where τ_0_0 : ~Copyable> (@inout GenNCWrapper<τ_0_0>) -> @inout τ_0_0 // CHECK: [[REG4:%.*]] = apply [[REG3]]([[REG2]]) : $@convention(method) <τ_0_0 where τ_0_0 : ~Copyable> (@inout GenNCWrapper<τ_0_0>) -> @inout τ_0_0 -// CHECK: [[REG5:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG4]] +// CHECK: [[REG5:%.*]] = mark_unresolved_non_copyable_value [assignable_but_not_consumable] [[REG4]] // CHECK: return [[REG5]] // CHECK: } @@ -748,7 +748,7 @@ public struct GenNCWrapper : ~Copyable { // CHECK: [[REG3:%.*]] = struct_element_addr [[REG2]], #GenNCWrapper._ncw // CHECK: [[REG4:%.*]] = function_ref @$s15borrow_accessor9NCWrapperV2ncAA2NCVvz : $@convention(method) (@inout NCWrapper) -> @inout NC // CHECK: [[REG5:%.*]] = apply [[REG4]]([[REG3]]) : $@convention(method) (@inout NCWrapper) -> @inout NC -// CHECK: [[REG6:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG5]] +// CHECK: [[REG6:%.*]] = mark_unresolved_non_copyable_value [assignable_but_not_consumable] [[REG5]] // CHECK: return [[REG6]] // CHECK: } @@ -779,10 +779,10 @@ public struct GenNCWrapper : ~Copyable { // CHECK: [[REG2:%.*]] = mark_unresolved_non_copyable_value [consumable_and_assignable] [[REG0]] // CHECK: [[REG3:%.*]] = function_ref @$s15borrow_accessor12GenNCWrapperVAARi_zrlE3ncwAA0D0Vvz : $@convention(method) <τ_0_0 where τ_0_0 : ~Copyable> (@inout GenNCWrapper<τ_0_0>) -> @inout NCWrapper // CHECK: [[REG4:%.*]] = apply [[REG3]]([[REG2]]) : $@convention(method) <τ_0_0 where τ_0_0 : ~Copyable> (@inout GenNCWrapper<τ_0_0>) -> @inout NCWrapper -// CHECK: [[REG5:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG4]] +// CHECK: [[REG5:%.*]] = mark_unresolved_non_copyable_value [assignable_but_not_consumable] [[REG4]] // CHECK: [[REG6:%.*]] = function_ref @$s15borrow_accessor9NCWrapperV2ncAA2NCVvz : $@convention(method) (@inout NCWrapper) -> @inout NC // CHECK: [[REG7:%.*]] = apply [[REG6]]([[REG5]]) : $@convention(method) (@inout NCWrapper) -> @inout NC -// CHECK: [[REG8:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG7]] +// CHECK: [[REG8:%.*]] = mark_unresolved_non_copyable_value [assignable_but_not_consumable] [[REG7]] // CHECK: return [[REG8]] // CHECK: } @@ -828,7 +828,7 @@ public struct GenNCWrapper : ~Copyable { // CHECK: [[REG6:%.*]] = apply [[REG5]]([[REG3]], [[REG4]]) : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int // CHECK: [[REG7:%.*]] = function_ref @$s15borrow_accessor12GenNCWrapperVAARi_zrlEyxSiciz : $@convention(method) <τ_0_0 where τ_0_0 : ~Copyable> (Int, @inout GenNCWrapper<τ_0_0>) -> @inout τ_0_0 // CHECK: [[REG8:%.*]] = apply [[REG7]]([[REG6]], [[REG2]]) : $@convention(method) <τ_0_0 where τ_0_0 : ~Copyable> (Int, @inout GenNCWrapper<τ_0_0>) -> @inout τ_0_0 -// CHECK: [[REG9:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG8]] +// CHECK: [[REG9:%.*]] = mark_unresolved_non_copyable_value [assignable_but_not_consumable] [[REG8]] // CHECK: return [[REG9]] // CHECK: } From ff486b742e3ff8d457472afb61e82f869160cc0e Mon Sep 17 00:00:00 2001 From: Meghana Gupta Date: Mon, 3 Nov 2025 20:16:54 -0800 Subject: [PATCH 8/8] Update MoveOnlyAddressCheckerUtils for mutate accessors --- include/swift/SIL/SILInstruction.h | 2 ++ .../Mandatory/MoveOnlyAddressCheckerUtils.cpp | 16 ++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/include/swift/SIL/SILInstruction.h b/include/swift/SIL/SILInstruction.h index 56b4fbf61f6ad..ff26cc48a88c5 100644 --- a/include/swift/SIL/SILInstruction.h +++ b/include/swift/SIL/SILInstruction.h @@ -3162,6 +3162,8 @@ class ApplyInst final bool hasAddressResult() const { return getSubstCalleeConv().hasAddressResult(); } + + bool hasInoutResult() const { return getSubstCalleeConv().hasInoutResult(); } }; /// PartialApplyInst - Represents the creation of a closure object by partial diff --git a/lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp b/lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp index c3080a6e2195f..d863a9bf2b27d 100644 --- a/lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp +++ b/lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp @@ -472,6 +472,22 @@ static bool visitScopeEndsRequiringInit( return true; } + // Check for mutate accessor. + if (auto ai = + dyn_cast_or_null(operand->getDefiningInstruction())) { + if (ai->hasInoutResult()) { + auto *access = + dyn_cast(getAccessScope(ai->getSelfArgument())); + if (access) { + assert(access->getAccessKind() == SILAccessKind::Modify); + for (auto *inst : access->getEndAccesses()) { + visit(inst, ScopeRequiringFinalInit::ModifyMemoryAccess); + } + return true; + } + return false; + } + } return false; }