Skip to content

Commit a14128a

Browse files
committed
TempLValue opt: don't insert destroy_addr too early due to ignoring deinit barriers
This can happen for inout arguments. It fixes a potential miscompile, e.g. observable by a weak reference being nil where it shouldn't. rdar://116335089
1 parent a8296e1 commit a14128a

File tree

2 files changed

+45
-0
lines changed

2 files changed

+45
-0
lines changed

lib/SILOptimizer/Transforms/TempLValueOpt.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
#include "swift/SILOptimizer/PassManager/Transforms.h"
2020
#include "swift/SILOptimizer/Analysis/AliasAnalysis.h"
21+
#include "swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h"
2122
#include "swift/SIL/NodeBits.h"
2223
#include "swift/SIL/SILFunction.h"
2324
#include "swift/SIL/SILBasicBlock.h"
@@ -176,6 +177,13 @@ void TempLValueOptPass::tempLValueOpt(CopyAddrInst *copyInst) {
176177
// but a block argument.
177178
SILInstruction *destRootInst = destRootAddr->getDefiningInstruction();
178179

180+
bool needDestroyEarly = false;
181+
BasicCalleeAnalysis *bca = nullptr;
182+
if (!copyInst->isInitializationOfDest()) {
183+
needDestroyEarly = true;
184+
bca = PM->getAnalysis<BasicCalleeAnalysis>();
185+
}
186+
179187
// Iterate over the liferange of the temporary and make some validity checks.
180188
AliasAnalysis *AA = nullptr;
181189
SILInstruction *beginOfLiferange = nullptr;
@@ -220,6 +228,9 @@ void TempLValueOptPass::tempLValueOpt(CopyAddrInst *copyInst) {
220228
// Needed to treat init_existential_addr as not-writing projection.
221229
projections.contains(inst) == 0)
222230
return;
231+
232+
if (needDestroyEarly && isDeinitBarrier(inst, bca))
233+
return;
223234
}
224235
}
225236
assert(endOfLiferangeReached);

test/SILOptimizer/templvalueopt_ossa.sil

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// RUN: %target-sil-opt -enable-sil-verify-all %s -temp-lvalue-opt | %FileCheck %s
22

3+
// REQUIRES: swift_in_compiler
4+
35
import Swift
46
import Builtin
57

@@ -300,3 +302,35 @@ bb0(%0 : $*T):
300302
return %78 : $()
301303
}
302304

305+
sil @createAny : $@convention(thin) () -> @out Any
306+
sil @createAny_no_barrier : $@convention(thin) () -> @out Any {
307+
[global:]
308+
}
309+
310+
// CHECK-LABEL: sil [ossa] @test_deinit_barrier :
311+
// CHECK: copy_addr [take]
312+
// CHECK-LABEL: } // end sil function 'test_deinit_barrier'
313+
sil [ossa] @test_deinit_barrier : $@convention(thin) (@guaranteed Any, @inout Any) -> () {
314+
bb0(%0 : @guaranteed $Any, %1 : $*Any):
315+
%2 = alloc_stack $Any
316+
%4 = function_ref @createAny : $@convention(thin) () -> @out Any
317+
%5 = apply %4(%2) : $@convention(thin) () -> @out Any
318+
copy_addr [take] %2 to %1 : $*Any
319+
dealloc_stack %2 : $*Any
320+
%11 = tuple ()
321+
return %11 : $()
322+
}
323+
324+
// CHECK-LABEL: sil [ossa] @test_no_deinit_barrier :
325+
// CHECK-NOT: copy_addr
326+
// CHECK-LABEL: } // end sil function 'test_no_deinit_barrier'
327+
sil [ossa] @test_no_deinit_barrier : $@convention(thin) (@guaranteed Any, @inout Any) -> () {
328+
bb0(%0 : @guaranteed $Any, %1 : $*Any):
329+
%2 = alloc_stack $Any
330+
%4 = function_ref @createAny_no_barrier : $@convention(thin) () -> @out Any
331+
%5 = apply %4(%2) : $@convention(thin) () -> @out Any
332+
copy_addr [take] %2 to %1 : $*Any
333+
dealloc_stack %2 : $*Any
334+
%11 = tuple ()
335+
return %11 : $()
336+
}

0 commit comments

Comments
 (0)