Downgrade dynamic exclusivity failures to a warning in Swift 3 compatibility mode.

Author: rjmccall

include/swift/ABI/MetadataValues.h

@@ -570,12 +570,28 @@ class FieldType {
 enum class ExclusivityFlags : uintptr_t {
   Read             = 0x0,
   Modify           = 0x1,
-  ActionMask       = 0x1
+  // Leave space for other actions.
+  // Don't rely on ActionMask in stable ABI.
+  ActionMask       = 0x1,
+
+  // Downgrade exclusivity failures to a warning.
+  WarningOnly      = 0x10
 };
+static inline ExclusivityFlags operator|(ExclusivityFlags lhs,
+                                         ExclusivityFlags rhs) {
+  return ExclusivityFlags(uintptr_t(lhs) | uintptr_t(rhs));
+}
+static inline ExclusivityFlags &operator|=(ExclusivityFlags &lhs,
+                                           ExclusivityFlags rhs) {
+  return (lhs = (lhs | rhs));
+}
 static inline ExclusivityFlags getAccessAction(ExclusivityFlags flags) {
   return ExclusivityFlags(uintptr_t(flags)
                         & uintptr_t(ExclusivityFlags::ActionMask));
 }
+static inline bool isWarningOnly(ExclusivityFlags flags) {
+  return uintptr_t(flags) & uintptr_t(ExclusivityFlags::WarningOnly);
+}
 
 } // end namespace swift
 

lib/IRGen/IRGenSIL.cpp

@@ -3952,7 +3952,7 @@ void IRGenSILFunction::visitProjectBoxInst(swift::ProjectBoxInst *i) {
   }
 }
 
-static ExclusivityFlags getExclusivityFlags(SILAccessKind kind) {
+static ExclusivityFlags getExclusivityAction(SILAccessKind kind) {
   switch (kind) {
   case SILAccessKind::Read:
     return ExclusivityFlags::Read;
@@ -3965,9 +3965,20 @@ static ExclusivityFlags getExclusivityFlags(SILAccessKind kind) {
   llvm_unreachable("bad access kind");
 }
 
+static ExclusivityFlags getExclusivityFlags(SILModule &M,
+                                            SILAccessKind kind) {
+  auto flags = getExclusivityAction(kind);
+
+  // In old Swift compatibility modes, downgrade this to a warning.
+  if (M.getASTContext().LangOpts.isSwiftVersion3())
+    flags |= ExclusivityFlags::WarningOnly;
+
+  return flags;
+}
+
 template <class Inst>
 static ExclusivityFlags getExclusivityFlags(Inst *i) {
-  return getExclusivityFlags(i->getAccessKind());
+  return getExclusivityFlags(i->getModule(), i->getAccessKind());
 }
 
 void IRGenSILFunction::visitBeginAccessInst(BeginAccessInst *access) {

stdlib/public/runtime/Exclusivity.cpp

@@ -19,6 +19,7 @@
 #include "swift/Runtime/Exclusivity.h"
 #include "swift/Runtime/Metadata.h"
 #include <memory>
+#include <stdio.h>
 
 // Pick an implementation strategy.
 #ifndef SWIFT_EXCLUSIVITY_USE_THREADLOCAL
@@ -61,8 +62,37 @@ static const char *getAccessName(ExclusivityFlags flags) {
   switch (flags) {
   case ExclusivityFlags::Read: return "read";
   case ExclusivityFlags::Modify: return "modify";
+  default: return "unknown";
   }
-  return "unknown";
+}
+
+static void reportExclusivityConflict(ExclusivityFlags oldAction, void *oldPC,
+                                      ExclusivityFlags newFlags, void *newPC,
+                                      void *pointer) {
+  // TODO: print something about where the pointer came from?
+  // TODO: if we do something more computationally intense here,
+  //   suppress warnings if the total warning count exceeds some limit
+
+  if (isWarningOnly(newFlags)) {
+    fprintf(stderr,
+            "WARNING: %s/%s access conflict detected on address %p\n"
+            "  first access started at PC=%p\n"
+            "  second access started at PC=%p\n",
+            getAccessName(oldAction),
+            getAccessName(getAccessAction(newFlags)),
+            pointer, oldPC, newPC);
+    return;
+  }
+
+  // TODO: try to recover source-location information from the return
+  // address.
+  fatalError(0,
+             "%s/%s access conflict detected on address %p, aborting\n"
+             "  first access started at PC=%p\n"
+             "  second access started at PC=%p\n",
+             getAccessName(oldAction),
+             getAccessName(getAccessAction(newFlags)),
+             pointer, oldPC, newPC);
 }
 
 namespace {
@@ -123,13 +153,11 @@ class AccessSet {
         continue;
 
       // Otherwise, it's a conflict.
-      // TODO: try to recover source-location information from the return
-      // address.
-      fatalError(0, "%s(%p) / %s(%p) access conflict detected on address %p,"
-                 " aborting\n",
-                 getAccessName(cur->getAccessAction()), cur->PC,
-                 getAccessName(action), pc,
-                 pointer);
+      reportExclusivityConflict(cur->getAccessAction(), cur->PC,
+                                flags, pc, pointer);
+
+      // If we're only warning, don't report multiple conflicts.
+      break;
     }
 
     // Insert to the front of the array so that remove tends to find it faster.

test/IRGen/access_markers.sil

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend -enforce-exclusivity=checked -assume-parsing-unqualified-ownership-sil %s -emit-ir | %FileCheck %s --check-prefix=CHECK
+// RUN: %target-swift-frontend -swift-version 4 -enforce-exclusivity=checked -assume-parsing-unqualified-ownership-sil %s -emit-ir | %FileCheck %s --check-prefix=CHECK
 
 import Builtin
 import Swift

test/IRGen/exclusivity.sil

@@ -1,4 +1,5 @@
-// RUN: %target-swift-frontend %s -emit-ir -enforce-exclusivity=checked | %FileCheck --check-prefix=CHECK --check-prefix=CHECK-%target-cpu %s
+// RUN: %target-swift-frontend %s -swift-version 4 -emit-ir -enforce-exclusivity=checked | %FileCheck --check-prefix=CHECK --check-prefix=CHECK-SWIFT4 --check-prefix=CHECK-%target-cpu %s
+// RUN: %target-swift-frontend %s -swift-version 3 -emit-ir -enforce-exclusivity=checked | %FileCheck --check-prefix=CHECK  --check-prefix=CHECK-SWIFT3 --check-prefix=CHECK-%target-cpu %s
 
 sil_stage canonical
 
@@ -24,8 +25,9 @@ bb0(%0 : $A):
   // CHECK: [[T0:%.*]] = bitcast [[BUFFER_T]]* [[SCRATCH0]] to i8*
   // CHECK: call void @llvm.lifetime.start(i64 -1, i8* [[T0]])
   // CHECK: [[T0:%.*]] = bitcast [[INT:%TSi]]* [[PROP]] to i8*
-  // CHECK: call void @swift_beginAccess(i8* [[T0]], [[BUFFER_T]]* [[SCRATCH0]], [[SIZE_T:i32|i64]] 0, i8* null)
-  %4 = begin_access [read] [dynamic] %3 : $*Int
+  // CHECK-SWIFT3: call void @swift_beginAccess(i8* [[T0]], [[BUFFER_T]]* [[SCRATCH0]], [[SIZE_T:i32|i64]] 16, i8* null)
+  // CHECK-SWIFT4: call void @swift_beginAccess(i8* [[T0]], [[BUFFER_T]]* [[SCRATCH0]], [[SIZE_T:i32|i64]] 0, i8* null)
+    %4 = begin_access [read] [dynamic] %3 : $*Int
 
   // CHECK: [[T0:%.*]] = getelementptr inbounds [[INT]], [[INT]]* %1, i32 0, i32 0
   // CHECK: load {{.*}}* [[T0]]
@@ -44,7 +46,8 @@ bb0(%0 : $A):
   // CHECK: [[T0:%.*]] = bitcast [[BUFFER_T]]* [[SCRATCH1]] to i8*
   // CHECK: call void @llvm.lifetime.start(i64 -1, i8* [[T0]])
   // CHECK: [[T0:%.*]] = bitcast [[INT:%TSi]]* [[PROP]] to i8*
-  // CHECK: call void @swift_beginAccess(i8* [[T0]], [[BUFFER_T]]* [[SCRATCH1]], [[SIZE_T]] 1, i8* null)
+  // CHECK-SWIFT3: call void @swift_beginAccess(i8* [[T0]], [[BUFFER_T]]* [[SCRATCH1]], [[SIZE_T:i32|i64]] 17, i8* null)
+  // CHECK-SWIFT4: call void @swift_beginAccess(i8* [[T0]], [[BUFFER_T]]* [[SCRATCH1]], [[SIZE_T:i32|i64]] 1, i8* null)
   %12 = begin_access [modify] [dynamic] %11 : $*Int
 
   // CHECK: call {{.*}} @changeInt([[INT]]*{{.*}} [[PROP]])

test/Interpreter/enforce_exclusive_access.swift

@@ -1,6 +1,6 @@
 // RUN: rm -rf %t
 // RUN: mkdir -p %t
-// RUN: %target-build-swift %s -o %t/a.out -enforce-exclusivity=checked -Onone
+// RUN: %target-build-swift -swift-version 4 %s -o %t/a.out -enforce-exclusivity=checked -Onone
 //
 // RUN: %target-run %t/a.out
 // REQUIRES: executable_test