Conditionally enable bounds checking for unsafe buffer pointers

docs/StandardLibraryProgrammersManual.md

@@ -415,11 +415,15 @@ return
 This should be rarely used. It informs the SIL optimizer that any code dominated by it should be treated as the innermost loop of a performance critical section of code. It cranks optimizer heuristics to 11. Injudicious use of this will degrade performance and bloat binary size.
 
 
-#### <a name="precondition"></a>`_precondition`, `_debugPrecondition`, and `_internalInvariant`
+#### <a name="precondition"></a>`_precondition`, `_debugPrecondition`, `_boundsCheckPrecondition`, and `_internalInvariant`
 
-These three functions are assertions that will trigger a run time trap if violated.
+These four functions are assertions that will trigger a run time trap if violated.
 
 * `_precondition` executes in all build configurations. Use this for invariant enforcement in all user code build configurations
+* `_boundsCheckPrecondition` executes in **used code** is built in either a debug build, or when the upcoming feature `UnsafePointerBoundsSafety`
+    has been enabled. Use this only for `_debugPreconditions` that have
+    historically only been `_debugPrecondition`, but are being updated
+    for Swift 6.
 * `_debugPrecondition` will execute when **user code** is built with assertions enabled. Use this for invariant enforcement that's useful while debugging, but might be prohibitively expensive when user code is configured without assertions.
 * `_internalInvariant` will execute when **standard library code** is built with assertions enabled. Use this for internal only invariant checks that useful for debugging the standard library itself.
 

include/swift/AST/SILOptions.h

@@ -198,6 +198,7 @@ class SILOptions {
     Debug = 0,     // Enables all asserts.
     Release = 1,   // Disables asserts.
     Unchecked = 2, // Disables asserts, preconditions, and runtime checks.
+    ReleaseWithBoundsSafety = 3, // Enables all asserts + bounds checks for unsafe pointers
 
     // Leave the assert_configuration instruction around.
     DisableReplacement = UINT_MAX

include/swift/Basic/Features.def

@@ -129,6 +129,8 @@ UPCOMING_FEATURE(FullTypedThrows, 413, 6)
 
 UPCOMING_FEATURE(ExistentialAny, 335, 7)
 
+EXPERIMENTAL_FEATURE(UnsafePointerBoundsSafety, true)
+
 EXPERIMENTAL_FEATURE(StaticAssert, false)
 EXPERIMENTAL_FEATURE(NamedOpaqueTypes, false)
 EXPERIMENTAL_FEATURE(FlowSensitiveConcurrencyCaptures, false)

include/swift/Option/Options.td

@@ -1055,7 +1055,7 @@ def debug_info_store_invocation : Flag<["-"], "debug-info-store-invocation">,
 def AssertConfig : Separate<["-"], "assert-config">,
   Flags<[FrontendOption, ModuleInterfaceOption]>,
   HelpText<"Specify the assert_configuration replacement. "
-           "Possible values are Debug, Release, Unchecked, DisableReplacement.">;
+           "Possible values are Debug, Release, Unchecked, ReleaseWithBoundsSafety, DisableReplacement.">;
 
 // Code formatting options
 

include/swift/SILOptimizer/OptimizerBridging.h

@@ -298,7 +298,8 @@ struct BridgedPassContext {
   enum class AssertConfiguration {
     Debug = 0,
     Release = 1,
-    Unchecked = 2
+    Unchecked = 2,
+    ReleaseWithBoundsSafety = 3,
   };
 
   BRIDGED_INLINE bool enableStackProtection() const;

include/swift/SILOptimizer/OptimizerBridgingImpl.h

@@ -443,6 +443,7 @@ static_assert((int)BridgedPassContext::SILStage::Lowered == (int)swift::SILStage
 static_assert((int)BridgedPassContext::AssertConfiguration::Debug == (int)swift::SILOptions::Debug);
 static_assert((int)BridgedPassContext::AssertConfiguration::Release == (int)swift::SILOptions::Release);
 static_assert((int)BridgedPassContext::AssertConfiguration::Unchecked == (int)swift::SILOptions::Unchecked);
+static_assert((int)BridgedPassContext::AssertConfiguration::ReleaseWithBoundsSafety == (int)swift::SILOptions::ReleaseWithBoundsSafety);
 
 SWIFT_END_NULLABILITY_ANNOTATIONS
 

lib/AST/ASTPrinter.cpp

@@ -3944,6 +3944,10 @@ static bool usesFeaturePreconcurrencyConformances(Decl *decl) {
 
 static bool usesFeatureBorrowingSwitch(Decl *decl) { return false; }
 
+static bool usesFeatureUnsafePointerBoundsSafety(Decl *decl) {
+  return false;
+}
+
 /// Suppress the printing of a particular feature.
 static void suppressingFeature(PrintOptions &options, Feature feature,
                                llvm::function_ref<void()> action) {

lib/ASTGen/Sources/ASTGen/Macros.swift

@@ -608,7 +608,7 @@ func findSyntaxNodeInSourceFile<Node: SyntaxProtocol>(
 
   let sourceFilePtr = sourceFilePtr.assumingMemoryBound(to: ExportedSourceFile.self)
 
-  // Find the offset.
+  // Find the offset in this buffer.
   let buffer = sourceFilePtr.pointee.buffer
   let offset = sourceLocationPtr - buffer.baseAddress!
   if offset < 0 || offset >= buffer.count {

lib/Frontend/CompilerInvocation.cpp

@@ -2030,6 +2030,7 @@ void parseExclusivityEnforcementOptions(const llvm::opt::Arg *A,
 
 static bool ParseSILArgs(SILOptions &Opts, ArgList &Args,
                          IRGenOptions &IRGenOpts,
+                         const LangOptions &LangOpts,
                          const FrontendOptions &FEOpts,
                          const TypeCheckerOptions &TCOpts,
                          DiagnosticEngine &Diags,
@@ -2118,6 +2119,8 @@ static bool ParseSILArgs(SILOptions &Opts, ArgList &Args,
       Opts.AssertConfig = SILOptions::Release;
     } else if (Configuration == "Unchecked") {
       Opts.AssertConfig = SILOptions::Unchecked;
+    } else if (Configuration == "ReleaseWithBoundsSafety") {
+      Opts.AssertConfig = SILOptions::ReleaseWithBoundsSafety;
     } else {
       Diags.diagnose(SourceLoc(), diag::error_invalid_arg_value,
                      A->getAsString(Args), A->getValue());
@@ -2131,7 +2134,11 @@ static bool ParseSILArgs(SILOptions &Opts, ArgList &Args,
     // Set the assert configuration according to the optimization level if it
     // has not been set by the -Ounchecked flag.
     Opts.AssertConfig =
-        (IRGenOpts.shouldOptimize() ? SILOptions::Release : SILOptions::Debug);
+        IRGenOpts.shouldOptimize()
+         ? (LangOpts.hasFeature(Feature::UnsafePointerBoundsSafety)
+              ? SILOptions::ReleaseWithBoundsSafety
+              : SILOptions::Release)
+         : SILOptions::Debug;
   }
 
   // -Ounchecked might also set removal of runtime asserts (cond_fail).
@@ -3203,7 +3210,7 @@ bool CompilerInvocation::parseArgs(
     return true;
   }
 
-  if (ParseSILArgs(SILOpts, ParsedArgs, IRGenOpts, FrontendOpts,
+  if (ParseSILArgs(SILOpts, ParsedArgs, IRGenOpts, LangOpts, FrontendOpts,
                    TypeCheckerOpts, Diags,
                    LangOpts.Target, ClangImporterOpts)) {
     return true;

stdlib/public/Concurrency/ExecutorAssertions.swift

@@ -53,7 +53,8 @@ extension SerialExecutor {
       _ message: @autoclosure () -> String = String(),
       file: StaticString = #fileID, line: UInt = #line
   ) {
-    guard _isDebugAssertConfiguration() || _isReleaseAssertConfiguration() else {
+    guard _isDebugAssertConfiguration() || _isReleaseAssertConfiguration() ||
+        _isReleaseAssertWithBoundsSafetyConfiguration() else {
       return
     }
 
@@ -102,7 +103,8 @@ extension Actor {
       _ message: @autoclosure () -> String = String(),
       file: StaticString = #fileID, line: UInt = #line
   ) {
-    guard _isDebugAssertConfiguration() || _isReleaseAssertConfiguration() else {
+    guard _isDebugAssertConfiguration() || _isReleaseAssertConfiguration() ||
+        _isReleaseAssertWithBoundsSafetyConfiguration() else {
       return
     }
 

stdlib/public/Differentiation/ArrayDifferentiation.swift

@@ -193,7 +193,7 @@ extension Array: Differentiable where Element: Differentiable {
 
 extension Array where Element: Differentiable {
   @usableFromInline
-  @derivative(of: subscript)
+  @derivative(of: subscript(_:))
   func _vjpSubscript(index: Int) -> (
     value: Element, pullback: (Element.TangentVector) -> TangentVector
   ) {
@@ -208,7 +208,7 @@ extension Array where Element: Differentiable {
   }
 
   @usableFromInline
-  @derivative(of: subscript)
+  @derivative(of: subscript(_:))
   func _jvpSubscript(index: Int) -> (
     value: Element, differential: (TangentVector) -> Element.TangentVector
   ) {

stdlib/public/Distributed/DistributedAssertions.swift

@@ -47,7 +47,8 @@ extension DistributedActor {
       _ message: @autoclosure () -> String = String(),
       file: StaticString = #fileID, line: UInt = #line
   ) {
-    guard _isDebugAssertConfiguration() || _isReleaseAssertConfiguration() else {
+    guard _isDebugAssertConfiguration() || _isReleaseAssertConfiguration() ||
+        _isReleaseAssertWithBoundsSafetyConfiguration() else {
       return
     }
 

stdlib/public/core/Array.swift

@@ -400,6 +400,29 @@ extension Array {
     return _DependenceToken()
   }
 
+  /// Check that the given `index` is valid for subscripting, i.e.
+  /// `0 ≤ index < count`, but only in debug buikds.
+  @inlinable
+  @_semantics("array.check_subscript")
+  @_effects(notEscaping self.**)
+  @_alwaysEmitIntoClient
+  public // @testable
+  func _debugCheckSubscript(
+    _ index: Int, wasNativeTypeChecked: Bool
+  ) -> _DependenceToken {
+#if _runtime(_ObjC)
+    // There is no need to do bounds checking for the non-native case because
+    // ObjectiveC arrays do bounds checking by their own.
+    // And in the native-non-type-checked case, it's also not needed to do bounds
+    // checking here, because it's done in ArrayBuffer._getElementSlowPath.
+    if _fastPath(wasNativeTypeChecked) {
+      _buffer._native._debugCheckValidSubscript(index)
+    }
+#else
+    _buffer._debugCheckValidSubscript(index)
+#endif
+    return _DependenceToken()
+  }
   /// Check that the given `index` is valid for subscripting, i.e.
   /// `0 ≤ index < count`.
   ///
@@ -411,6 +434,17 @@ extension Array {
     _buffer._checkValidSubscriptMutating(index)
   }
 
+  /// Check that the given `index` is valid for subscripting, i.e.
+  /// `0 ≤ index < count`, but only in debug configurations.
+  ///
+  /// - Precondition: The buffer must be uniquely referenced and native.
+  @_alwaysEmitIntoClient
+  @_semantics("array.check_subscript")
+  @_effects(notEscaping self.**)
+  internal func _debugCheckSubscript_mutating(_ index: Int) {
+    _buffer._debugCheckValidSubscriptMutating(index)
+  }
+
   /// Check that the specified `index` is valid, i.e. `0 ≤ index ≤ count`.
   @inlinable
   @_semantics("array.check_index")
@@ -420,6 +454,17 @@ extension Array {
     _precondition(index >= startIndex, "Negative Array index is out of range")
   }
 
+  /// Check that the specified `index` is valid, i.e. `0 ≤ index ≤ count`,
+  /// but only in debug configurations.
+  @inlinable
+  @_semantics("array.check_index")
+  @_effects(notEscaping self.**)
+  @_alwaysEmitIntoClient
+  internal func _debugCheckIndex(_ index: Int) {
+    _debugPrecondition(index <= endIndex, "Array index is out of range")
+    _debugPrecondition(index >= startIndex, "Negative Array index is out of range")
+  }
+
   @_semantics("array.get_element")
   @_effects(notEscaping self.value**)
   @_effects(escaping self.value**.class*.value** -> return.value**)
@@ -761,6 +806,38 @@ extension Array: RandomAccessCollection, MutableCollection {
     }
   }
 
+  /// Accesses the element at the specified position without bounds
+  /// checking.
+  ///
+  /// This unsafe operation should only be used when performance analysis
+  /// has determined that the bounds checks are not being eliminated
+  /// by the optimizer despite being ensured by a higher-level invariant.
+  @inlinable @_alwaysEmitIntoClient
+  public subscript(unchecked index: Int) -> Element {
+    get {
+      // This call may be hoisted or eliminated by the optimizer.  If
+      // there is an inout violation, this value may be stale so needs to be
+      // checked again below.
+      let wasNativeTypeChecked = _hoistableIsNativeTypeChecked()
+
+      // Make sure the index is in range and wasNativeTypeChecked is
+      // still valid.
+      let token = _debugCheckSubscript(
+        index, wasNativeTypeChecked: wasNativeTypeChecked)
+
+      return _getElement(
+        index, wasNativeTypeChecked: wasNativeTypeChecked,
+        matchingSubscriptCheck: token)
+    }
+    _modify {
+      _makeMutableAndUnique() // makes the array native, too
+      _debugCheckSubscript_mutating(index)
+      let address = _buffer.mutableFirstElementAddress + index
+      defer { _endMutation() }
+      yield &address.pointee
+    }
+  }
+
   /// Accesses a contiguous subrange of the array's elements.
   ///
   /// The returned `ArraySlice` instance uses the same indices for the same
@@ -804,6 +881,31 @@ extension Array: RandomAccessCollection, MutableCollection {
     }
   }
 
+  /// Accesses a contiguous subrange of the array's elements without
+  /// bounds checks on the range.
+  ///
+  /// This unsafe operation should only be used when performance analysis
+  /// has determined that the bounds checks are not being eliminated
+  /// by the optimizer despite being ensured by a higher-level invariant.
+  @inlinable @_alwaysEmitIntoClient
+  public subscript(uncheckedBounds bounds: Range<Int>) -> ArraySlice<Element> {
+    get {
+      _debugCheckIndex(bounds.lowerBound)
+      _debugCheckIndex(bounds.upperBound)
+      return ArraySlice(_buffer: _buffer[bounds])
+    }
+    set(rhs) {
+      _debugCheckIndex(bounds.lowerBound)
+      _debugCheckIndex(bounds.upperBound)
+      // If the replacement buffer has same identity, and the ranges match,
+      // then this was a pinned in-place modification, nothing further needed.
+      if self[bounds]._buffer.identity != rhs._buffer.identity
+      || bounds != rhs.startIndex..<rhs.endIndex {
+        self.replaceSubrange(bounds, with: rhs)
+      }
+    }
+  }
+
   /// The number of elements in the array.
   @inlinable
   @_semantics("array.get_count")

stdlib/public/core/ArrayBuffer.swift

@@ -467,6 +467,15 @@ extension _ArrayBuffer {
     _native._checkValidSubscriptMutating(index)
   }
 
+  /// Traps unless the given `index` is valid for subscripting, i.e.
+  /// `0 ≤ index < count`.
+  ///
+  /// - Precondition: The buffer must be mutable.
+  @inlinable
+  @_alwaysEmitIntoClient
+  internal func _debugCheckValidSubscriptMutating(_ index: Int) {
+    _native._debugCheckValidSubscriptMutating(index)
+  }
   /// The number of elements the buffer can store without reallocation.
   ///
   /// This property is obsolete. It's only used for the ArrayBufferProtocol and