[Exclusivity] Disable dynamic enforcement in noescape closures.

atrick · atrick · commit 47d5037139dd · 2017-05-11T08:57:02.000-07:00
(cherry picked from commit dd31c40)
diff --git a/lib/SILOptimizer/Mandatory/AccessEnforcementSelection.cpp b/lib/SILOptimizer/Mandatory/AccessEnforcementSelection.cpp
@@ -156,15 +156,6 @@ void SelectEnforcement::analyzeProjection(ProjectBoxInst *projection) {
       if (access->getEnforcement() == SILAccessEnforcement::Unknown)
         Accesses.push_back(access);
     }
-
-    // FIXME: We should distinguish between escapes via arguments to escaping
-    // closures (which must propagate to to all reachable blocks because they
-    // could access the address at any later point in the program) and
-    // non-escaping closures (which only force dynamic enforcement for
-    // accesses that are in progress at the time of the escape).
-    if (auto partialApply = dyn_cast<PartialApplyInst>(user)) {
-      noteEscapingUse(partialApply);
-    }
   }
 }
 
@@ -378,19 +369,15 @@ struct AccessEnforcementSelection : SILFunctionTransform {
     if (auto arg = dyn_cast<SILFunctionArgument>(address)) {
       switch (arg->getArgumentConvention()) {
       case SILArgumentConvention::Indirect_Inout:
+      case SILArgumentConvention::Indirect_InoutAliasable:
         // `inout` arguments are checked on the caller side, either statically
         // or dynamically if necessary. The @inout does not alias and cannot
         // escape within the callee, so static enforcement is always sufficient.
-        setStaticEnforcement(access);
-        break;
-      case SILArgumentConvention::Indirect_InoutAliasable:
-        // `inout_aliasable` are not enforced on the caller side. Dynamic
-        // enforcement is required unless we have special knowledge of how this
-        // closure is used at its call-site.
         //
-        // TODO: optimize closures passed to call sites in which the captured
-        // variable is not modified by any closure passed to the same call.
-        setDynamicEnforcement(access);
+        // FIXME: `inout_aliasable` are not currently enforced on the caller
+        // side. Consequenctly, using static enforcement for noescape closures
+        // may fails to diagnose certain violations.
+        setStaticEnforcement(access);
         break;
       default:
         // @in/@in_guaranteed cannot be mutably accessed, mutably captured, or
diff --git a/test/Interpreter/enforce_exclusive_access.swift b/test/Interpreter/enforce_exclusive_access.swift
@@ -101,47 +101,62 @@ ExclusiveAccessTestSuite.test("ModifyFollowedByModify") {
   globalX = X() // no-trap
 }
 
-ExclusiveAccessTestSuite.test("ClosureCaptureModifyModify")
-.skip(.custom(
-    { _isFastAssertConfiguration() },
-    reason: "this trap is not guaranteed to happen in -Ounchecked"))
-  .crashOutputMatches("modify/modify access conflict detected on address")
-  .code
-{
-  var x = X()
-  modifyAndPerform(&x) {
-    expectCrashLater()
-    x.i = 12
-  }
-}
-
-ExclusiveAccessTestSuite.test("ClosureCaptureReadModify")
-.skip(.custom(
-    { _isFastAssertConfiguration() },
-    reason: "this trap is not guaranteed to happen in -Ounchecked"))
-  .crashOutputMatches("read/modify access conflict detected on address")
-  .code
-{
-  var x = X()
-  modifyAndPerform(&x) {
-    expectCrashLater()
-    _blackHole(x.i)
-  }
-}
-
-ExclusiveAccessTestSuite.test("ClosureCaptureModifyRead")
-.skip(.custom(
-    { _isFastAssertConfiguration() },
-    reason: "this trap is not guaranteed to happen in -Ounchecked"))
-  .crashOutputMatches("modify/read access conflict detected on address")
-  .code
-{
-  var x = X()
-  readAndPerform(&x) {
-    expectCrashLater()
-    x.i = 12
-  }
-}
+// FIXME: This should be a static diagnostics.
+// Once this radar is fixed, conirm that a it is covered by a static diagnostic
+// (-verify) test in exclusivity_static_diagnostics.sil.
+// <rdar://problem/32061282> Enforce exclusive access in noescape closures.
+//
+//ExclusiveAccessTestSuite.test("ClosureCaptureModifyModify")
+//.skip(.custom(
+//    { _isFastAssertConfiguration() },
+//    reason: "this trap is not guaranteed to happen in -Ounchecked"))
+//  .crashOutputMatches("modify/modify access conflict detected on address")
+//  .code
+//{
+//  var x = X()
+//  modifyAndPerform(&x) {
+//    expectCrashLater()
+//    x.i = 12
+//  }
+//}
+
+// FIXME: This should be a static diagnostics.
+// Once this radar is fixed, conirm that a it is covered by a static diagnostic
+// (-verify) test in exclusivity_static_diagnostics.sil.
+// <rdar://problem/32061282> Enforce exclusive access in noescape closures.
+//
+//ExclusiveAccessTestSuite.test("ClosureCaptureReadModify")
+//.skip(.custom(
+//    { _isFastAssertConfiguration() },
+//    reason: "this trap is not guaranteed to happen in -Ounchecked"))
+//  .crashOutputMatches("read/modify access conflict detected on address")
+//  .code
+//{
+//  var x = X()
+//  modifyAndPerform(&x) {
+//    expectCrashLater()
+//    _blackHole(x.i)
+//  }
+//}
+
+// FIXME: This should be a static diagnostics.
+// Once this radar is fixed, conirm that a it is covered by a static diagnostic
+// (-verify) test in exclusivity_static_diagnostics.sil.
+// <rdar://problem/32061282> Enforce exclusive access in noescape closures.
+//
+//ExclusiveAccessTestSuite.test("ClosureCaptureModifyRead")
+//.skip(.custom(
+//    { _isFastAssertConfiguration() },
+//    reason: "this trap is not guaranteed to happen in -Ounchecked"))
+//  .crashOutputMatches("modify/read access conflict detected on address")
+//  .code
+//{
+//  var x = X()
+//  readAndPerform(&x) {
+//    expectCrashLater()
+//    x.i = 12
+//  }
+//}
 
 ExclusiveAccessTestSuite.test("ClosureCaptureReadRead") {
   var x = X()
diff --git a/test/SILOptimizer/access_enforcement_noescape.swift b/test/SILOptimizer/access_enforcement_noescape.swift
@@ -5,7 +5,7 @@
 // (Some static/dynamic enforcement selection is done in SILGen, and some is
 // deferred. That may change over time but we want the outcome to be the same).
 //
-// Each FIXME me line is a case that the current implementation misses.
+// Each FIXME line is a case that the current implementation misses.
 // The model is currently being refined, so this isn't set in stone.
 //
 // TODO: Ensure that each of these cases is covered by
@@ -73,12 +73,14 @@ func readRead() {
 
 // closure #1 in readRead()
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape8readReadyyFyycfU_ : $@convention(thin) (@inout_aliasable Int) -> () {
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [read] [dynamic]
 // FIXME-CHECK: [[ACCESS:%.*]] = begin_access [read] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape8readReadyyFyycfU_'
 
 // closure #2 in readRead()
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape8readReadyyFyycfU0_ : $@convention(thin) (@inout_aliasable Int) -> () {
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [read] [dynamic]
 // FIXME-CHECK: [[ACCESS:%.*]] = begin_access [read] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape8readReadyyFyycfU0_'
@@ -99,12 +101,14 @@ func inoutReadRead(x: inout Int) {
 
 // closure #1 in inoutReadRead(x:)
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape09inoutReadE0ySiz1x_tFyycfU_ : $@convention(thin) (@inout_aliasable Int) -> () {
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [read] [dynamic]
 // FIXME-CHECK: [[ACCESS:%.*]] = begin_access [read] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape09inoutReadE0ySiz1x_tFyycfU_'
 
 // closure #2 in inoutReadRead(x:)
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape09inoutReadE0ySiz1x_tFyycfU0_ : $@convention(thin) (@inout_aliasable Int) -> () {
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [read] [dynamic]
 // FIXME-CHECK: [[ACCESS:%.*]] = begin_access [read] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape09inoutReadE0ySiz1x_tFyycfU0_'
@@ -136,6 +140,7 @@ func readBoxRead() {
 
 // closure #2 in readBoxRead()
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape11readBoxReadyyFyycfU0_ : $@convention(thin) (@inout_aliasable Int) -> () {
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [read] [dynamic]
 // FIXME-CHECK: [[ACCESS:%.*]] = begin_access [read] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape11readBoxReadyyFyycfU0_'
@@ -165,12 +170,14 @@ func readWrite() {
 
 // closure #1 in readWrite()
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape9readWriteyyFyycfU_ : $@convention(thin) (@inout_aliasable Int) -> () {
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [read] [dynamic]
 // FIXME-CHECK: [[ACCESS:%.*]] = begin_access [read] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape9readWriteyyFyycfU_'
 
 // closure #2 in readWrite()
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape9readWriteyyFyycfU0_ : $@convention(thin) (@inout_aliasable Int) -> () {
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [modify] [dynamic]
 // FIXME-CHECK: [[ACCESS:%.*]] = begin_access [modify] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape9readWriteyyFyycfU0_'
@@ -193,12 +200,14 @@ func inoutReadWrite(x: inout Int) {
 
 // closure #1 in inoutReadWrite(x:)
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape14inoutReadWriteySiz1x_tFyycfU_ : $@convention(thin) (@inout_aliasable Int) -> () {
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [read] [dynamic]
 // FIXME-CHECK: [[ACCESS:%.*]] = begin_access [read] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape14inoutReadWriteySiz1x_tFyycfU_'
 
 // closure #2 in inoutReadWrite(x:)
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape14inoutReadWriteySiz1x_tFyycfU0_ : $@convention(thin) (@inout_aliasable Int) -> () {
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [modify] [dynamic]
 // FIXME-CHECK: [[ACCESS:%.*]] = begin_access [modify] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape14inoutReadWriteySiz1x_tFyycfU0_'
@@ -236,6 +245,7 @@ func readBoxWrite() {
 
 // closure #2 in readBoxWrite()
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape12readBoxWriteyyFyycfU0_ : $@convention(thin) (@inout_aliasable Int) -> () {
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [modify] [dynamic]
 // FIXME-CHECK: [[ACCESS:%.*]] = begin_access [modify] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape12readBoxWriteyyFyycfU0_'
@@ -275,6 +285,7 @@ func readWriteBox() {
 
 // closure #2 in readWriteBox()
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape12readWriteBoxyyFyycfU0_ : $@convention(thin) (@inout_aliasable Int) -> () {
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [read] [dynamic]
 // FIXME-CHECK: [[ACCESS:%.*]] = begin_access [read] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape12readWriteBoxyyFyycfU0_'
@@ -305,7 +316,8 @@ func readWriteInout() {
 
 // closure #1 in readWriteInout()
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape14readWriteInoutyyFyycfU_ : $@convention(thin) (@inout_aliasable Int) -> () {
-// FIXME-CHECK: [[ACCESS:%.*]] = begin_access [modify] [static] %0 : $*Int
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [read] [dynamic]
+// FIXME-CHECK: [[ACCESS:%.*]] = begin_access [read] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape14readWriteInoutyyFyycfU_'
 
@@ -328,7 +340,8 @@ func inoutReadWriteInout(x: inout Int) {
 
 // closure #1 in inoutReadWriteInout(x:)
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape19inoutReadWriteInoutySiz1x_tFyycfU_ : $@convention(thin) (@inout_aliasable Int) -> () {
-// FIXME-CHECK: [[ACCESS:%.*]] = begin_access [modify] [static] %0 : $*Int
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [read] [dynamic]
+// FIXME-CHECK: [[ACCESS:%.*]] = begin_access [read] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape19inoutReadWriteInoutySiz1x_tFyycfU_'
 
@@ -381,12 +394,14 @@ func writeWrite() {
 
 // closure #1 in writeWrite()
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape10writeWriteyyFyycfU_ : $@convention(thin) (@inout_aliasable Int) -> () {
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [modify] [dynamic]
 // FIXME-CHECK: [[ACCESS:%.*]] = begin_access [modify] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape10writeWriteyyFyycfU_'
 
 // closure #2 in writeWrite()
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape10writeWriteyyFyycfU0_ : $@convention(thin) (@inout_aliasable Int) -> () {
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [modify] [dynamic]
 // FIXME-CHECK: [[ACCESS:%.*]] = begin_access [modify] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape10writeWriteyyFyycfU0_'
@@ -410,12 +425,14 @@ func inoutWriteWrite(x: inout Int) {
 
 // closure #1 in inoutWriteWrite(x:)
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape010inoutWriteE0ySiz1x_tFyycfU_ : $@convention(thin) (@inout_aliasable Int) -> () {
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [modify] [dynamic]
 // FIXME-CHECK: [[ACCESS:%.*]] = begin_access [modify] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape010inoutWriteE0ySiz1x_tFyycfU_'
 
 // closure #2 in inoutWriteWrite(x:)
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape010inoutWriteE0ySiz1x_tFyycfU0_ : $@convention(thin) (@inout_aliasable Int) -> () {
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [modify] [dynamic]
 // FIXME-CHECK: [[ACCESS:%.*]] = begin_access [modify] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape010inoutWriteE0ySiz1x_tFyycfU0_'
@@ -450,6 +467,7 @@ func writeWriteBox() {
 
 // closure #2 in writeWriteBox()
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape13writeWriteBoxyyFyycfU0_ : $@convention(thin) (@inout_aliasable Int) -> () {
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [modify] [dynamic]
 // FIXME-CHECK: [[ACCESS:%.*]] = begin_access [modify] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape13writeWriteBoxyyFyycfU0_'
@@ -479,6 +497,7 @@ func writeWriteInout() {
 
 // closure #1 in writeWriteInout()
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape15writeWriteInoutyyFyycfU_ : $@convention(thin) (@inout_aliasable Int) -> () {
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [modify] [dynamic]
 // FIXME-CHECK: [[ACCESS:%.*]] = begin_access [modify] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape15writeWriteInoutyyFyycfU_'
@@ -502,6 +521,7 @@ func inoutWriteWriteInout(x: inout Int) {
 
 // closure #1 in inoutWriteWriteInout(x:)
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape010inoutWriteE5InoutySiz1x_tFyycfU_ : $@convention(thin) (@inout_aliasable Int) -> () {
+// CHECK-NOT: [[ACCESS:%.*]] = begin_access [modify] [dynamic]
 // FIXME-CHECK: [[ACCESS:%.*]] = begin_access [modify] [static] %0 : $*Int
 // FIXME-CHECK: end_access [[ACCESS]] 
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape010inoutWriteE5InoutySiz1x_tFyycfU_'
@@ -525,8 +545,8 @@ func writeBoxWriteInout() {
 // closure #1 in writeBoxWriteInout()
 // CHECK-LABEL: sil private @_T027access_enforcement_noescape18writeBoxWriteInoutyyFyycfU_ : $@convention(thin) (@owned { var Int }) -> () {
 // CHECK: [[ADDR:%.*]] = project_box %0 : ${ var Int }, 0
-// FIXME-CHECK: [[ACCESS:%.*]] = begin_access [modify] [dynamic] [[ADDR]] : $*Int
-// FIXME-CHECK: end_access [[ACCESS]]
+// CHECK: [[ACCESS:%.*]] = begin_access [modify] [dynamic] [[ADDR]] : $*Int
+// CHECK: end_access [[ACCESS]]
 // CHECK-LABEL: } // end sil function '_T027access_enforcement_noescape18writeBoxWriteInoutyyFyycfU_'
 
 // Error: Cannot capture inout
diff --git a/test/SILOptimizer/access_enforcement_selection.sil b/test/SILOptimizer/access_enforcement_selection.sil
@@ -68,7 +68,7 @@ sil hidden @markFuncEscape : $@convention(thin) () -> () {
 sil @takesInoutAndClosure : $@convention(thin) (@inout Builtin.Int64, @owned @callee_owned () -> ()) -> ()
 sil @closureCapturingByStorageAddress : $@convention(thin) (@inout_aliasable Builtin.Int64) -> ()
 
-// Test dynamic enforcement of box addresses that escape via closure
+// Test static enforcement of box addresses that escape via closure
 // partial_applys.
 // application.
 // CHECK-LABEL: sil hidden @escapeAsArgumentToPartialApply : $@convention(thin) () -> () {
@@ -78,7 +78,7 @@ sil @closureCapturingByStorageAddress : $@convention(thin) (@inout_aliasable Bui
 // CHECK:  [[FUNC:%.*]] = function_ref @takesInoutAndClosure : $@convention(thin) (@inout Builtin.Int64, @owned @callee_owned () -> ()) -> ()
 // CHECK:  [[CLOSURE:%.*]] = function_ref @closureCapturingByStorageAddress : $@convention(thin) (@inout_aliasable Builtin.Int64) -> ()
 // CHECK:  [[PA:%.*]] = partial_apply [[CLOSURE]]([[ADR]]) : $@convention(thin) (@inout_aliasable Builtin.Int64) -> ()
-// CHECK:  [[MODIFY:%.*]] = begin_access [modify] [dynamic] [[ADR]] : $*Builtin.Int64
+// CHECK:  [[MODIFY:%.*]] = begin_access [modify] [static] [[ADR]] : $*Builtin.Int64
 // CHECK:  %{{.*}} = apply [[FUNC]]([[MODIFY]], [[PA]]) : $@convention(thin) (@inout Builtin.Int64, @owned @callee_owned () -> ()) -> ()
 // CHECK:  end_access [[MODIFY]] : $*Builtin.Int64
 // CHECK:  destroy_value [[BOX]] : ${ var Builtin.Int64 }
diff --git a/test/SILOptimizer/access_enforcement_selection.swift b/test/SILOptimizer/access_enforcement_selection.swift
