From f0b7bdb38222db423a5aa79c611a36b522311cd2 Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Wed, 19 Feb 2025 10:33:18 +0100 Subject: [PATCH] TempLValueOpt: avoid creating invalid apply argument aliasing. An indirect argument (except `@inout_aliasable`) must not alias with another indirect argument. Now, if we would replace tempAddr with destAddr in ``` apply %f(%tempAddr, %destAddr) : (@in T) -> @out T ``` we would invalidate this rule. This is even true if the called function does not read from destAddr. Fixes a SIL verification error. rdar://145090659 --- lib/SILOptimizer/Transforms/TempLValueOpt.cpp | 46 +++++++++++++ test/SILOptimizer/templvalueopt_crash.swift | 24 +++++++ test/SILOptimizer/templvalueopt_ossa.sil | 69 +++++++++++++++++++ 3 files changed, 139 insertions(+) create mode 100644 test/SILOptimizer/templvalueopt_crash.swift diff --git a/lib/SILOptimizer/Transforms/TempLValueOpt.cpp b/lib/SILOptimizer/Transforms/TempLValueOpt.cpp index 65371eb2e61f0..9d5378c87047d 100644 --- a/lib/SILOptimizer/Transforms/TempLValueOpt.cpp +++ b/lib/SILOptimizer/Transforms/TempLValueOpt.cpp @@ -83,6 +83,8 @@ class TempLValueOptPass : public SILFunctionTransform { private: void tempLValueOpt(CopyAddrInst *copyInst); void combineCopyAndDestroy(CopyAddrInst *copyInst); + bool hasInvalidApplyArgumentAliasing(SILInstruction *inst, SILValue tempAddr, + SILValue destAddr, AliasAnalysis *aa); }; void TempLValueOptPass::run() { @@ -230,6 +232,11 @@ void TempLValueOptPass::tempLValueOpt(CopyAddrInst *copyInst) { projections.contains(inst) == 0) return; + // Check if replacing the temporary with destination would invalidate an applied + // function's alias rules of indirect arguments. + if (hasInvalidApplyArgumentAliasing(inst, temporary, destination, AA)) + return; + if (needDestroyEarly && isDeinitBarrier(inst, bca)) return; } @@ -269,6 +276,45 @@ void TempLValueOptPass::tempLValueOpt(CopyAddrInst *copyInst) { invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions); } +/// Returns true if after replacing tempAddr with destAddr an apply instruction +/// would have invalid aliasing of indirect arguments. +/// +/// An indirect argument (except `@inout_aliasable`) must not alias with another +/// indirect argument. Now, if we would replace tempAddr with destAddr in +/// +/// apply %f(%tempAddr, %destAddr) : (@in T) -> @out T +/// +/// we would invalidate this rule. +/// This is even true if the called function does not read from destAddr. +/// +bool TempLValueOptPass::hasInvalidApplyArgumentAliasing(SILInstruction *inst, + SILValue tempAddr, + SILValue destAddr, + AliasAnalysis *aa) { + auto as = FullApplySite::isa(inst); + if (!as) + return false; + + bool tempAccessed = false; + bool destAccessed = false; + bool mutatingAccess = false; + for (Operand &argOp : as.getArgumentOperands()) { + auto conv = as.getArgumentConvention(argOp); + if (conv.isExclusiveIndirectParameter()) { + if (aa->mayAlias(tempAddr, argOp.get())) { + tempAccessed = true; + if (!conv.isGuaranteedConventionInCaller()) + mutatingAccess = true; + } else if (aa->mayAlias(destAddr, argOp.get())) { + destAccessed = true; + if (!conv.isGuaranteedConventionInCaller()) + mutatingAccess = true; + } + } + } + return mutatingAccess && tempAccessed && destAccessed; +} + void TempLValueOptPass::combineCopyAndDestroy(CopyAddrInst *copyInst) { if (copyInst->isTakeOfSrc()) return; diff --git a/test/SILOptimizer/templvalueopt_crash.swift b/test/SILOptimizer/templvalueopt_crash.swift new file mode 100644 index 0000000000000..0693c27f702ea --- /dev/null +++ b/test/SILOptimizer/templvalueopt_crash.swift @@ -0,0 +1,24 @@ +// RUN: %target-swift-frontend %s -O -sil-verify-all -emit-sil -o /dev/null + + +// Check that TempLValueOpt does not create invalid SIL which causes a verification error. + +public struct S { + internal var a: T? = nil + public var b: T? {a} + public var c: T? = nil + + public mutating func set() { + c = b + } +} + +public class Klass{} + +public func test() -> S { + var s = S() + s.set() + return s +} + + diff --git a/test/SILOptimizer/templvalueopt_ossa.sil b/test/SILOptimizer/templvalueopt_ossa.sil index c223c746a78a8..93eb755808445 100644 --- a/test/SILOptimizer/templvalueopt_ossa.sil +++ b/test/SILOptimizer/templvalueopt_ossa.sil @@ -334,3 +334,72 @@ bb0(%0 : @guaranteed $Any, %1 : $*Any): %11 = tuple () return %11 : $() } + +struct TwoFields { + var a: Child + var b: Child +} + +sil @no_read : $@convention(thin) (@in_guaranteed TwoFields) -> @out Child { +[%0: write v**] +[global: ] +} +sil @no_reads : $@convention(thin) (@in_guaranteed Child, @in_guaranteed TwoFields) -> () { +[global: ] +} + +// CHECK-LABEL: sil [ossa] @argument_aliasing : +// CHECK: %1 = alloc_stack +// CHECK: apply %{{[0-9]+}}(%1, %0) +// CHECK-LABEL: } // end sil function 'argument_aliasing' +sil [ossa] @argument_aliasing : $@convention(thin) (@inout TwoFields) -> () { +bb0(%0 : $*TwoFields): + %1 = alloc_stack $Child + %2 = function_ref @no_read : $@convention(thin) (@in_guaranteed TwoFields) -> @out Child + %3 = apply %2(%1, %0) : $@convention(thin) (@in_guaranteed TwoFields) -> @out Child + %4 = struct_element_addr %0, #TwoFields.a + copy_addr [take] %1 to %4 + dealloc_stack %1 + %7 = tuple () + return %7 +} + +// CHECK-LABEL: sil [ossa] @no_argument_aliasing : +// CHECK-NOT: alloc_stack +// CHECK: [[A:%.*]] = struct_element_addr %0 : $*TwoFields, #TwoFields.a +// CHECK-NEXT: destroy_addr [[A]] +// CHECK: apply %{{[0-9]+}}([[A]], %1) +// CHECK-NOT: copy_addr +// CHECK-LABEL: } // end sil function 'no_argument_aliasing' +sil [ossa] @no_argument_aliasing : $@convention(thin) (@inout TwoFields, @inout TwoFields) -> () { +bb0(%0 : $*TwoFields, %1 : $*TwoFields): + %2 = alloc_stack $Child + %3 = function_ref @no_read : $@convention(thin) (@in_guaranteed TwoFields) -> @out Child + %4 = apply %3(%2, %1) : $@convention(thin) (@in_guaranteed TwoFields) -> @out Child + %5 = struct_element_addr %0, #TwoFields.a + copy_addr [take] %2 to %5 + dealloc_stack %2 + %8 = tuple () + return %8 +} + +// CHECK-LABEL: sil [ossa] @guaranteed_argument_aliasing : +// CHECK-NOT: alloc_stack +// CHECK: [[A:%.*]] = struct_element_addr %0 : $*TwoFields, #TwoFields.a +// CHECK-NEXT: destroy_addr [[A]] +// CHECK: apply %{{[0-9]+}}([[A]], %0) +// CHECK-NOT: copy_addr +// CHECK-LABEL: } // end sil function 'guaranteed_argument_aliasing' +sil [ossa] @guaranteed_argument_aliasing : $@convention(thin) (@inout TwoFields, @owned Child) -> () { +bb0(%0 : $*TwoFields, %1 : @owned $Child): + %2 = alloc_stack $Child + store %1 to [init] %2 + %3 = function_ref @no_reads : $@convention(thin) (@in_guaranteed Child, @in_guaranteed TwoFields) -> () + %4 = apply %3(%2, %0) : $@convention(thin) (@in_guaranteed Child, @in_guaranteed TwoFields) -> () + %5 = struct_element_addr %0, #TwoFields.a + copy_addr [take] %2 to %5 + dealloc_stack %2 + %8 = tuple () + return %8 +} +