Merge pull request swiftlang#39520 from eeckstein/fix-cow-checks

eeckstein · web-flow · commit 2135f247d44f · 2021-10-01T07:08:51.000+02:00
stdlib: add a build-script option --enable-array-cow-checks to enable compilation of COW checks.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -397,6 +397,10 @@ option(SWIFT_CHECK_INCREMENTAL_COMPILATION
     "Check if incremental compilation works when compiling the Swift libraries"
     FALSE)
 
+option(SWIFT_ENABLE_ARRAY_COW_CHECKS
+    "Compile the stdlib with Array COW checks enabled (only relevant for assert builds)"
+    FALSE)
+
 option(SWIFT_REPORT_STATISTICS
     "Create json files which contain internal compilation statistics"
     FALSE)
diff --git a/stdlib/public/core/ArrayBuffer.swift b/stdlib/public/core/ArrayBuffer.swift
@@ -128,7 +128,7 @@ extension _ArrayBuffer {
     } else {
       isUnique = _isNative
     }
-#if INTERNAL_CHECKS_ENABLED
+#if INTERNAL_CHECKS_ENABLED && COW_CHECKS_ENABLED
     if isUnique {
       _native.isImmutable = false
     }
@@ -145,7 +145,7 @@ extension _ArrayBuffer {
   @_alwaysEmitIntoClient
   @inline(__always)
   internal mutating func endCOWMutation() {
-#if INTERNAL_CHECKS_ENABLED
+#if INTERNAL_CHECKS_ENABLED && COW_CHECKS_ENABLED
     _native.isImmutable = true
 #endif
     _storage.endCOWMutation()
diff --git a/stdlib/public/core/Builtin.swift b/stdlib/public/core/Builtin.swift
@@ -345,19 +345,11 @@ internal func _class_getInstancePositiveExtentSize(_ theClass: AnyClass) -> Int
 #endif
 }
 
-#if INTERNAL_CHECKS_ENABLED
-// "9999" means: enable if linked with a built library, but not when linked with
-// the OS libraries.
-// Note: this must not be changed to a "real" OS version.
-@available(macOS 9999, iOS 9999, tvOS 9999, watchOS 9999, *)
+#if INTERNAL_CHECKS_ENABLED && COW_CHECKS_ENABLED
 @usableFromInline
 @_silgen_name("_swift_isImmutableCOWBuffer")
 internal func _swift_isImmutableCOWBuffer(_ object: AnyObject) -> Bool
 
-// "9999" means: enable if linked with a built library, but not when linked with
-// the OS libraries.
-// Note: this must not be changed to a "real" OS version.
-@available(macOS 9999, iOS 9999, tvOS 9999, watchOS 9999, *)
 @usableFromInline
 @_silgen_name("_swift_setImmutableCOWBuffer")
 internal func _swift_setImmutableCOWBuffer(_ object: AnyObject, _ immutable: Bool) -> Bool
diff --git a/stdlib/public/core/CMakeLists.txt b/stdlib/public/core/CMakeLists.txt
@@ -283,6 +283,9 @@ endif()
 if(SWIFT_STDLIB_ENABLE_STDLIBCORE_EXCLUSIVITY_CHECKING)
   list(APPEND swift_stdlib_compile_flags "-enforce-exclusivity=checked")
 endif()
+if(SWIFT_ENABLE_ARRAY_COW_CHECKS)
+  list(APPEND swift_stdlib_compile_flags "-DCOW_CHECKS_ENABLED")
+endif()
 
 # STAGING: Temporarily avoids having to write #fileID in Swift.swiftinterface.
 list(APPEND swift_stdlib_compile_flags "-Xfrontend" "-enable-experimental-concise-pound-file")
diff --git a/stdlib/public/core/ContiguousArrayBuffer.swift b/stdlib/public/core/ContiguousArrayBuffer.swift
@@ -12,11 +12,7 @@
 
 import SwiftShims
 
-#if INTERNAL_CHECKS_ENABLED
-// "9999" means: enable if linked with a built library, but not when linked with
-// the OS libraries.
-// Note: this must not be changed to a "real" OS version.
-@available(macOS 9999, iOS 9999, tvOS 9999, watchOS 9999, *)
+#if INTERNAL_CHECKS_ENABLED && COW_CHECKS_ENABLED
 @_silgen_name("swift_COWChecksEnabled")
 public func _COWChecksEnabled() -> Bool
 #endif
@@ -428,7 +424,7 @@ internal struct _ContiguousArrayBuffer<Element>: _ArrayBufferProtocol {
   @_alwaysEmitIntoClient
   @inline(__always)
   internal var immutableStorage : __ContiguousArrayStorageBase {
-#if INTERNAL_CHECKS_ENABLED
+#if INTERNAL_CHECKS_ENABLED && COW_CHECKS_ENABLED
     _internalInvariant(isImmutable, "Array storage is not immutable")
 #endif
     return Builtin.COWBufferForReading(_storage)
@@ -440,7 +436,7 @@ internal struct _ContiguousArrayBuffer<Element>: _ArrayBufferProtocol {
   @_alwaysEmitIntoClient
   @inline(__always)
   internal var mutableStorage : __ContiguousArrayStorageBase {
-#if INTERNAL_CHECKS_ENABLED
+#if INTERNAL_CHECKS_ENABLED && COW_CHECKS_ENABLED
     _internalInvariant(isMutable, "Array storage is immutable")
 #endif
     return _storage
@@ -452,44 +448,34 @@ internal struct _ContiguousArrayBuffer<Element>: _ArrayBufferProtocol {
   @_alwaysEmitIntoClient
   @inline(__always)
   internal var mutableOrEmptyStorage : __ContiguousArrayStorageBase {
-#if INTERNAL_CHECKS_ENABLED
+#if INTERNAL_CHECKS_ENABLED && COW_CHECKS_ENABLED
     _internalInvariant(isMutable || _storage.countAndCapacity.capacity == 0,
                        "Array storage is immutable and not empty")
 #endif
     return _storage
   }
 
-#if INTERNAL_CHECKS_ENABLED
+#if INTERNAL_CHECKS_ENABLED && COW_CHECKS_ENABLED
   @_alwaysEmitIntoClient
   internal var isImmutable: Bool {
     get {
-      // "9999" means: enable if linked with a built library, but not when
-      // linked with the OS libraries.
-      // Note: this must not be changed to a "real" OS version.
-      if #available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *) {
-        if (_COWChecksEnabled()) {
-          return capacity == 0 || _swift_isImmutableCOWBuffer(_storage)
-        }
+      if (_COWChecksEnabled()) {
+        return capacity == 0 || _swift_isImmutableCOWBuffer(_storage)
       }
       return true
     }
     nonmutating set {
-      // "9999" means: enable if linked with a built library, but not when
-      // linked with the OS libraries.
-      // Note: this must not be changed to a "real" OS version.
-      if #available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *) {
-        if (_COWChecksEnabled()) {
-          // Make sure to not modify the empty array singleton (which has a
-          // capacity of 0).
-          if capacity > 0 {
-            let wasImmutable = _swift_setImmutableCOWBuffer(_storage, newValue)
-            if newValue {
-              _internalInvariant(!wasImmutable,
-                "re-setting immutable array buffer to immutable")
-            } else {
-              _internalInvariant(wasImmutable,
-                "re-setting mutable array buffer to mutable")
-            }
+      if (_COWChecksEnabled()) {
+        // Make sure to not modify the empty array singleton (which has a
+        // capacity of 0).
+        if capacity > 0 {
+          let wasImmutable = _swift_setImmutableCOWBuffer(_storage, newValue)
+          if newValue {
+            _internalInvariant(!wasImmutable,
+              "re-setting immutable array buffer to immutable")
+          } else {
+            _internalInvariant(wasImmutable,
+              "re-setting mutable array buffer to mutable")
           }
         }
       }
@@ -498,13 +484,8 @@ internal struct _ContiguousArrayBuffer<Element>: _ArrayBufferProtocol {
   
   @_alwaysEmitIntoClient
   internal var isMutable: Bool {
-      // "9999" means: enable if linked with a built library, but not when
-      // linked with the OS libraries.
-      // Note: this must not be changed to a "real" OS version.
-    if #available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *) {
-      if (_COWChecksEnabled()) {
-        return !_swift_isImmutableCOWBuffer(_storage)
-      }
+    if (_COWChecksEnabled()) {
+      return !_swift_isImmutableCOWBuffer(_storage)
     }
     return true
   }
@@ -704,7 +685,7 @@ internal struct _ContiguousArrayBuffer<Element>: _ArrayBufferProtocol {
   @_alwaysEmitIntoClient
   internal mutating func beginCOWMutation() -> Bool {
     if Bool(Builtin.beginCOWMutation(&_storage)) {
-#if INTERNAL_CHECKS_ENABLED
+#if INTERNAL_CHECKS_ENABLED && COW_CHECKS_ENABLED
       isImmutable = false
 #endif
       return true
@@ -721,7 +702,7 @@ internal struct _ContiguousArrayBuffer<Element>: _ArrayBufferProtocol {
   @_alwaysEmitIntoClient
   @inline(__always)
   internal mutating func endCOWMutation() {
-#if INTERNAL_CHECKS_ENABLED
+#if INTERNAL_CHECKS_ENABLED && COW_CHECKS_ENABLED
     isImmutable = true
 #endif
     Builtin.endCOWMutation(&_storage)
diff --git a/stdlib/public/core/SliceBuffer.swift b/stdlib/public/core/SliceBuffer.swift
@@ -325,7 +325,7 @@ internal struct _SliceBuffer<Element>
       return false
     }
     if Bool(Builtin.beginCOWMutation(&owner)) {
-#if INTERNAL_CHECKS_ENABLED
+#if INTERNAL_CHECKS_ENABLED && COW_CHECKS_ENABLED
       nativeBuffer.isImmutable = false
 #endif
       return true
@@ -342,7 +342,7 @@ internal struct _SliceBuffer<Element>
   @_alwaysEmitIntoClient
   @inline(__always)
   internal mutating func endCOWMutation() {
-#if INTERNAL_CHECKS_ENABLED
+#if INTERNAL_CHECKS_ENABLED && COW_CHECKS_ENABLED
     nativeBuffer.isImmutable = true
 #endif
     Builtin.endCOWMutation(&owner)
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
@@ -159,6 +159,11 @@ if (SWIFT_TEST_USE_LEAKS)
   list(APPEND SWIFT_LIT_ARGS "--param" "leaks-all")
 endif()
 
+if (SWIFT_ENABLE_ARRAY_COW_CHECKS)
+  list(APPEND SWIFT_LIT_ARGS
+       "--param" "array_cow_checks")
+endif()
+
 if(NOT CMAKE_CFG_INTDIR STREQUAL ".")
   list(APPEND SWIFT_LIT_ARGS
        "--param" "build_mode=${CMAKE_CFG_INTDIR}")
diff --git a/test/api-digester/stability-stdlib-abi-with-asserts.test b/test/api-digester/stability-stdlib-abi-with-asserts.test
@@ -38,7 +38,7 @@
 // Thank you for your help ensuring the stdlib remains compatible with its past!
 //                                            -- Your friendly stdlib engineers
 
-// REQUIRES: swift_stdlib_asserts
+// REQUIRES: swift_stdlib_asserts, array_cow_checks
 
 // *** DO NOT DISABLE OR XFAIL THIS TEST. *** (See comment above.)
 
@@ -60,5 +60,8 @@ Protocol _RuntimeFunctionCountersStats is a new API without @available attribute
 Struct _GlobalRuntimeFunctionCountersState is a new API without @available attribute
 Struct _ObjectRuntimeFunctionCountersState is a new API without @available attribute
 Struct _RuntimeFunctionCounters is a new API without @available attribute
+Func _COWChecksEnabled() is a new API without @available attribute
+Func _swift_isImmutableCOWBuffer(_:) is a new API without @available attribute
+Func _swift_setImmutableCOWBuffer(_:_:) is a new API without @available attribute
 
 // *** DO NOT DISABLE OR XFAIL THIS TEST. *** (See comment above.)
diff --git a/test/lit.cfg b/test/lit.cfg
@@ -264,6 +264,10 @@ native_swift_tools_path = lit_config.params.get('native_swift_tools_path')
 if native_swift_tools_path is not None:
     append_to_env_path(native_swift_tools_path)
 
+array_cow_checks = lit_config.params.get('array_cow_checks')
+if array_cow_checks is not None:
+    config.available_features.add('array_cow_checks')
+
 if lit_config.params.get('libswift', None) is not None:
     config.available_features.add('libswift')
 
diff --git a/utils/build-presets.ini b/utils/build-presets.ini
@@ -94,6 +94,8 @@ test-optimize-for-size
 swift-stdlib-build-type=RelWithDebInfo
 swift-stdlib-enable-assertions=true
 
+enable-array-cow-checks
+
 # This is a release non-incremental build. Run sil-verify-all.
 sil-verify-all
 
@@ -111,6 +113,8 @@ test-optimized
 swift-stdlib-build-type=RelWithDebInfo
 swift-stdlib-enable-assertions=true
 
+enable-array-cow-checks
+
 # This is a release non-incremental build. Run sil-verify-all.
 sil-verify-all
 
@@ -153,6 +157,8 @@ assertions
 swift-stdlib-build-type=Debug
 swift-stdlib-enable-assertions=true
 
+enable-array-cow-checks
+
 
 [preset: buildbot,tools=RA,stdlib=DA]
 mixin-preset=
@@ -328,6 +334,8 @@ build-subdir=buildbot_incremental
 release
 assertions
 
+enable-array-cow-checks
+
 build-swift-stdlib-unittest-extra
 
 libcxx
@@ -1056,6 +1064,8 @@ installable-package=%(installable_package)s
 assertions
 release
 
+enable-array-cow-checks
+
 verbose-build
 
 test
diff --git a/utils/build-script-impl b/utils/build-script-impl
@@ -50,6 +50,7 @@ KNOWN_SETTINGS=(
     check-args-only                               ""                "set to check all arguments are known. Exit with status 0 if success, non zero otherwise"
     only-execute                                  "all"             "Only execute the named action (see implementation)"
     check-incremental-compilation                 "0"               "set to 1 to compile swift libraries multiple times to check if incremental compilation works"
+    enable-array-cow-checks                       "0"               "set tp 1 compile the stdlib with Array COW checks enabled (only relevant for assert builds)"
 
     ## Build Mode Options
     enable-asan                                   ""                "enable Address Sanitizer"
@@ -1994,6 +1995,7 @@ for host in "${ALL_HOSTS[@]}"; do
                     -DSWIFT_BUILD_REMOTE_MIRROR:BOOL=$(true_false "${BUILD_SWIFT_REMOTE_MIRROR}")
                     -DSWIFT_STDLIB_SIL_DEBUGGING:BOOL=$(true_false "${BUILD_SIL_DEBUGGING_STDLIB}")
                     -DSWIFT_CHECK_INCREMENTAL_COMPILATION:BOOL=$(true_false "${CHECK_INCREMENTAL_COMPILATION}")
+                    -DSWIFT_ENABLE_ARRAY_COW_CHECKS:BOOL=$(true_false "${ENABLE_ARRAY_COW_CHECKS}")
                     -DSWIFT_REPORT_STATISTICS:BOOL=$(true_false "${REPORT_STATISTICS}")
                     -DSWIFT_BUILD_DYNAMIC_STDLIB:BOOL=$(true_false "${BUILD_SWIFT_DYNAMIC_STDLIB}")
                     -DSWIFT_BUILD_STATIC_STDLIB:BOOL=$(true_false "${BUILD_SWIFT_STATIC_STDLIB}")
