stdlib: add a build-script option --enable-array-cow-checks to enable compilation of COW checks.

eeckstein · eeckstein · commit eb9621fe8643 · 2021-09-30T18:06:52.000+02:00
And set this option in various presets for buildbots.

Don't enable the checks by default because when linking against the OS library (which does not support COW checking) it will result in unresolved symbol errors.
So far it was handled by an availability checks against 9999 (which was a hack), but this does not work anymore.

Note, all this is only relevant for assert builds of the stdlib.

rdar://83673798
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}")
