Skip to content

Commit 988d409

Browse files
committed
[+0-all-args] When diagnosing closure escapes, look through sil borrows.
I also added an option called sil-assert-on-exclusivity-failure that causes the optimizer to assert if an exclusivity failure is hit. This enables quicker debugging of exclusivity violations since at the assertion point, you drop straight down into the debugger. This is only enabled with asserts. rdar://34222540
1 parent 3df5928 commit 988d409

File tree

3 files changed

+39
-2
lines changed

3 files changed

+39
-2
lines changed

lib/SIL/InstructionUtils.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,10 @@ static SILValue findClosureStoredIntoBlock(SILValue V) {
490490
/// Return the partial_apply and a flag set to true if the closure is
491491
/// indirectly captured by a reabstraction thunk.
492492
FindClosureResult swift::findClosureForAppliedArg(SILValue V) {
493+
// Look through borrows.
494+
if (auto *bbi = dyn_cast<BeginBorrowInst>(V))
495+
V = bbi->getOperand();
496+
493497
if (auto optionalObjTy = V->getType().getOptionalObjectType())
494498
V = cast<EnumInst>(V)->getOperand();
495499

lib/SILOptimizer/Mandatory/DiagnoseStaticExclusivity.cpp

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,9 @@
4141
#include "swift/SILOptimizer/PassManager/Passes.h"
4242
#include "swift/SILOptimizer/PassManager/Transforms.h"
4343
#include "llvm/ADT/PostOrderIterator.h"
44+
#include "llvm/Support/CommandLine.h"
4445
#include "llvm/Support/Debug.h"
46+
4547
using namespace swift;
4648

4749
template <typename... T, typename... U>
@@ -180,6 +182,7 @@ enum class RecordedAccessKind {
180182
class RecordedAccess {
181183
private:
182184
RecordedAccessKind RecordKind;
185+
183186
union {
184187
BeginAccessInst *Inst;
185188
struct {
@@ -815,6 +818,12 @@ static bool isCallToStandardLibrarySwap(ApplyInst *AI, ASTContext &Ctx) {
815818
return FD == Ctx.getSwap(nullptr);
816819
}
817820

821+
static llvm::cl::opt<bool> ShouldAssertOnFailure(
822+
"sil-assert-on-exclusivity-failure",
823+
llvm::cl::desc("Should the compiler assert when it diagnoses conflicting "
824+
"accesses rather than emitting a diagnostic? Intended for "
825+
"use only with debugging."));
826+
818827
/// If making an access of the given kind at the given subpath would
819828
/// would conflict, returns the first recorded access it would conflict
820829
/// with. Otherwise, returns None.
@@ -824,7 +833,10 @@ shouldReportAccess(const AccessInfo &Info,swift::SILAccessKind Kind,
824833
if (Info.alreadyHadConflict())
825834
return None;
826835

827-
return Info.conflictsWithAccess(Kind, SubPath);
836+
auto result = Info.conflictsWithAccess(Kind, SubPath);
837+
if (ShouldAssertOnFailure && result.hasValue())
838+
llvm_unreachable("Standard assertion routine.");
839+
return result;
828840
}
829841

830842
/// For each projection that the summarized function accesses on its

test/SILOptimizer/exclusivity_static_diagnostics.sil

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ sil @takesOneInout : $@convention(thin) (@inout Int) -> ()
1111
sil @makesInt : $@convention(thin) () -> Int
1212
sil @takesInoutAndNoEscapeClosure : $@convention(thin) (@inout Int, @noescape @owned @callee_owned () -> ()) -> ()
1313
sil @takesInoutAndNoEscapeClosureTakingArgument : $@convention(thin) (@inout Int, @noescape @owned @callee_owned (Int) -> ()) -> ()
14+
sil @takesInoutAndNoEscapeGuaranteedClosureTakingArgument : $@convention(thin) (@inout Int, @noescape @guaranteed @callee_guaranteed (Int) -> ()) -> ()
1415
sil @takesInoutAndNoEscapeClosureWithGenericReturn : $@convention(thin) <T> (@inout Int, @owned @noescape @callee_owned (Int) -> @out T) -> ()
1516
sil @takesInoutAndNoEscapeBlockClosure : $@convention(thin) (@inout Int, @owned @convention(block) @noescape () -> ()) -> ()
1617
sil @takesInoutAndNoEscapeOptionalBlockClosure : $@convention(thin) (@inout Int, @owned Optional<@convention(block) @noescape () -> ()>) -> ()
@@ -579,7 +580,7 @@ bb0(%0 : $Int):
579580

580581
sil hidden @closureWithArgument_1 : $@convention(thin) (Int, @inout_aliasable Int) -> () {
581582
bb0(%0 : $Int, %1 : $*Int):
582-
%2 = begin_access [modify] [unknown] %1 : $*Int // expected-note {{conflicting access is here}}
583+
%2 = begin_access [modify] [unknown] %1 : $*Int // expected-note 2 {{conflicting access is here}}
583584
end_access %2 : $*Int
584585
%3 = tuple ()
585586
return %3 : $()
@@ -603,6 +604,26 @@ bb0(%0 : $Int):
603604
return %9 : $()
604605
}
605606

607+
// CHECK-LABEL: sil hidden @inProgressConflictWithNoEscapeGuaranteedClosureArgument : $@convention(thin) (Int) -> () {
608+
sil hidden @inProgressConflictWithNoEscapeGuaranteedClosureArgument : $@convention(thin) (Int) -> () {
609+
bb0(%0 : $Int):
610+
%2 = alloc_box ${ var Int }
611+
%3 = project_box %2 : ${ var Int }, 0
612+
store %0 to [trivial] %3 : $*Int
613+
%4 = function_ref @takesInoutAndNoEscapeGuaranteedClosureTakingArgument : $@convention(thin) (@inout Int, @noescape @guaranteed @callee_guaranteed (Int) -> ()) -> ()
614+
%5 = function_ref @closureWithArgument_1 : $@convention(thin) (Int, @inout_aliasable Int) -> ()
615+
%6 = partial_apply [callee_guaranteed] %5(%3) : $@convention(thin) (Int, @inout_aliasable Int) -> ()
616+
%conv = convert_function %6 : $@callee_guaranteed (Int) -> () to $@callee_guaranteed @noescape (Int) -> ()
617+
%bconv = begin_borrow %conv : $@callee_guaranteed @noescape (Int) -> ()
618+
%7 = begin_access [modify] [unknown] %3 : $*Int // expected-error {{overlapping accesses, but modification requires exclusive access; consider copying to a local variable}}
619+
%8 = apply %4(%3, %bconv) : $@convention(thin) (@inout Int, @noescape @guaranteed @callee_guaranteed (Int) -> ()) -> ()
620+
end_access %7: $*Int
621+
end_borrow %bconv from %conv : $@callee_guaranteed @noescape (Int) -> (), $@callee_guaranteed @noescape (Int) -> ()
622+
destroy_value %2 : ${ var Int }
623+
%9 = tuple ()
624+
return %9 : $()
625+
}
626+
606627
sil hidden @closureThatModifiesCapture_2 : $@convention(thin) (@inout_aliasable Int) -> () {
607628
bb0(%0 : $*Int):
608629
%1 = begin_access [modify] [unknown] %0 : $*Int // expected-error {{overlapping accesses, but modification requires exclusive access; consider copying to a local variable}}

0 commit comments

Comments
 (0)