diff --git a/stdlib/public/core/ClosedRange.swift b/stdlib/public/core/ClosedRange.swift index b368c64ac64fc..7841db6c41f61 100644 --- a/stdlib/public/core/ClosedRange.swift +++ b/stdlib/public/core/ClosedRange.swift @@ -68,6 +68,14 @@ public struct ClosedRange { /// The range's upper bound. public let upperBound: Bound + // This works around _debugPrecondition() impacting the performance of + // optimized code. (rdar://72246338) + @_alwaysEmitIntoClient @inline(__always) + internal init(_uncheckedBounds bounds: (lower: Bound, upper: Bound)) { + self.lowerBound = bounds.lower + self.upperBound = bounds.upper + } + /// Creates an instance with the given bounds. /// /// Because this initializer does not perform any checks, it should be used @@ -78,8 +86,9 @@ public struct ClosedRange { /// - Parameter bounds: A tuple of the lower and upper bounds of the range. @inlinable public init(uncheckedBounds bounds: (lower: Bound, upper: Bound)) { - self.lowerBound = bounds.lower - self.upperBound = bounds.upper + _debugPrecondition(bounds.lower <= bounds.upper, + "ClosedRange requires lowerBound <= upperBound") + self.init(_uncheckedBounds: (lower: bounds.lower, upper: bounds.upper)) } } @@ -100,8 +109,9 @@ extension ClosedRange: RangeExpression { public func relative(to collection: C) -> Range where C.Index == Bound { return Range( - uncheckedBounds: ( - lower: lowerBound, upper: collection.index(after: self.upperBound))) + _uncheckedBounds: ( + lower: lowerBound, + upper: collection.index(after: self.upperBound))) } /// Returns a Boolean value indicating whether the given element is contained @@ -336,7 +346,7 @@ extension Comparable { public static func ... (minimum: Self, maximum: Self) -> ClosedRange { _precondition( minimum <= maximum, "Range requires lowerBound <= upperBound") - return ClosedRange(uncheckedBounds: (lower: minimum, upper: maximum)) + return ClosedRange(_uncheckedBounds: (lower: minimum, upper: maximum)) } } @@ -423,7 +433,7 @@ extension ClosedRange { limits.upperBound < self.upperBound ? limits.upperBound : limits.lowerBound > self.upperBound ? limits.lowerBound : self.upperBound - return ClosedRange(uncheckedBounds: (lower: lower, upper: upper)) + return ClosedRange(_uncheckedBounds: (lower: lower, upper: upper)) } } @@ -439,7 +449,7 @@ extension ClosedRange where Bound: Strideable, Bound.Stride: SignedInteger { public init(_ other: Range) { _precondition(!other.isEmpty, "Can't form an empty closed range") let upperBound = other.upperBound.advanced(by: -1) - self.init(uncheckedBounds: (lower: other.lowerBound, upper: upperBound)) + self.init(_uncheckedBounds: (lower: other.lowerBound, upper: upperBound)) } } @@ -476,7 +486,7 @@ extension ClosedRange: Decodable where Bound: Decodable { codingPath: decoder.codingPath, debugDescription: "Cannot initialize \(ClosedRange.self) with a lowerBound (\(lowerBound)) greater than upperBound (\(upperBound))")) } - self.init(uncheckedBounds: (lower: lowerBound, upper: upperBound)) + self.init(_uncheckedBounds: (lower: lowerBound, upper: upperBound)) } } diff --git a/stdlib/public/core/Range.swift b/stdlib/public/core/Range.swift index 4bcde92bf8153..24fba64e9c245 100644 --- a/stdlib/public/core/Range.swift +++ b/stdlib/public/core/Range.swift @@ -157,6 +157,14 @@ public struct Range { /// instance does not contain its upper bound. public let upperBound: Bound + // This works around _debugPrecondition() impacting the performance of + // optimized code. (rdar://72246338) + @_alwaysEmitIntoClient @inline(__always) + internal init(_uncheckedBounds bounds: (lower: Bound, upper: Bound)) { + self.lowerBound = bounds.lower + self.upperBound = bounds.upper + } + /// Creates an instance with the given bounds. /// /// Because this initializer does not perform any checks, it should be used @@ -167,8 +175,9 @@ public struct Range { /// - Parameter bounds: A tuple of the lower and upper bounds of the range. @inlinable public init(uncheckedBounds bounds: (lower: Bound, upper: Bound)) { - self.lowerBound = bounds.lower - self.upperBound = bounds.upper + _debugPrecondition(bounds.lower <= bounds.upper, + "Range requires lowerBound <= upperBound") + self.init(_uncheckedBounds: (lower: bounds.lower, upper: bounds.upper)) } /// Returns a Boolean value indicating whether the given element is contained @@ -308,7 +317,7 @@ extension Range where Bound: Strideable, Bound.Stride: SignedInteger { /// require an upper bound of `Int.max + 1`, which is not representable as public init(_ other: ClosedRange) { let upperBound = other.upperBound.advanced(by: 1) - self.init(uncheckedBounds: (lower: other.lowerBound, upper: upperBound)) + self.init(_uncheckedBounds: (lower: other.lowerBound, upper: upperBound)) } } @@ -325,7 +334,7 @@ extension Range: RangeExpression { @inlinable // trivial-implementation public func relative(to collection: C) -> Range where C.Index == Bound { - return Range(uncheckedBounds: (lower: lowerBound, upper: upperBound)) + self } } @@ -359,7 +368,7 @@ extension Range { limits.upperBound < self.upperBound ? limits.upperBound : limits.lowerBound > self.upperBound ? limits.lowerBound : self.upperBound - return Range(uncheckedBounds: (lower: lower, upper: upper)) + return Range(_uncheckedBounds: (lower: lower, upper: upper)) } } @@ -435,7 +444,7 @@ extension Range: Decodable where Bound: Decodable { codingPath: decoder.codingPath, debugDescription: "Cannot initialize \(Range.self) with a lowerBound (\(lowerBound)) greater than upperBound (\(upperBound))")) } - self.init(uncheckedBounds: (lower: lowerBound, upper: upperBound)) + self.init(_uncheckedBounds: (lower: lowerBound, upper: upperBound)) } } @@ -729,7 +738,7 @@ extension Comparable { public static func ..< (minimum: Self, maximum: Self) -> Range { _precondition(minimum <= maximum, "Range requires lowerBound <= upperBound") - return Range(uncheckedBounds: (lower: minimum, upper: maximum)) + return Range(_uncheckedBounds: (lower: minimum, upper: maximum)) } /// Returns a partial range up to, but not including, its upper bound. diff --git a/stdlib/public/core/SmallString.swift b/stdlib/public/core/SmallString.swift index 8ef19f5765228..f23619ea980ea 100644 --- a/stdlib/public/core/SmallString.swift +++ b/stdlib/public/core/SmallString.swift @@ -208,7 +208,7 @@ extension _SmallString { // Restore the memory type of self._storage _ = rawPtr.bindMemory(to: RawBitPattern.self, capacity: 1) } - return try f(UnsafeBufferPointer(start: ptr, count: count)) + return try f(UnsafeBufferPointer(_uncheckedStart: ptr, count: count)) } } diff --git a/stdlib/public/core/String.swift b/stdlib/public/core/String.swift index 9176dcfbef9e0..04651d0d78cff 100644 --- a/stdlib/public/core/String.swift +++ b/stdlib/public/core/String.swift @@ -458,6 +458,7 @@ extension String { contigBytes._providesContiguousBytesNoCopy { self = contigBytes.withUnsafeBytes { rawBufPtr in + Builtin.onFastPath() // encourage SIL Optimizer to inline this closure return String._fromUTF8Repairing( UnsafeBufferPointer( start: rawBufPtr.baseAddress?.assumingMemoryBound(to: UInt8.self), diff --git a/stdlib/public/core/StringObject.swift b/stdlib/public/core/StringObject.swift index ebc2ce510be99..89fe1baf12b59 100644 --- a/stdlib/public/core/StringObject.swift +++ b/stdlib/public/core/StringObject.swift @@ -902,7 +902,7 @@ extension _StringObject { return sharedUTF8 } return UnsafeBufferPointer( - start: self.nativeUTF8Start, count: self.largeCount) + _uncheckedStart: self.nativeUTF8Start, count: self.largeCount) } // Whether the object stored can be bridged directly as a NSString diff --git a/stdlib/public/core/StringProtocol.swift b/stdlib/public/core/StringProtocol.swift index 3acd16db29bf7..9523b3db7f360 100644 --- a/stdlib/public/core/StringProtocol.swift +++ b/stdlib/public/core/StringProtocol.swift @@ -153,7 +153,7 @@ extension StringProtocol { let end = endIndex _internalInvariant( start.transcodedOffset == 0 && end.transcodedOffset == 0) - return Range(uncheckedBounds: (start._encodedOffset, end._encodedOffset)) + return Range(_uncheckedBounds: (start._encodedOffset, end._encodedOffset)) } } diff --git a/stdlib/public/core/StringStorageBridge.swift b/stdlib/public/core/StringStorageBridge.swift index 467b5c9d07f9d..083b6810c2f30 100644 --- a/stdlib/public/core/StringStorageBridge.swift +++ b/stdlib/public/core/StringStorageBridge.swift @@ -30,7 +30,7 @@ extension _AbstractStringStorage { "Range out of bounds") let range = Range( - uncheckedBounds: (aRange.location, aRange.location+aRange.length)) + _uncheckedBounds: (aRange.location, aRange.location+aRange.length)) let str = asString str._copyUTF16CodeUnits( into: UnsafeMutableBufferPointer(start: buffer, count: range.count), diff --git a/stdlib/public/core/Substring.swift b/stdlib/public/core/Substring.swift index a4a7029898955..47469337f0be6 100644 --- a/stdlib/public/core/Substring.swift +++ b/stdlib/public/core/Substring.swift @@ -105,7 +105,7 @@ public struct Substring { self._slice = Slice( base: slice.base, - bounds: Range(uncheckedBounds: (start, end))) + bounds: Range(_uncheckedBounds: (start, end))) _invariantCheck() } @@ -132,7 +132,7 @@ extension Substring { @inlinable @inline(__always) internal var _offsetRange: Range { return Range( - uncheckedBounds: (startIndex._encodedOffset, endIndex._encodedOffset)) + _uncheckedBounds: (startIndex._encodedOffset, endIndex._encodedOffset)) } #if !INTERNAL_CHECKS_ENABLED @@ -322,7 +322,8 @@ extension Substring: CustomDebugStringConvertible { extension Substring: LosslessStringConvertible { public init(_ content: String) { - self = content[...] + let range = Range(_uncheckedBounds: (content.startIndex, content.endIndex)) + self.init(Slice(base: content, bounds: range)) } } @@ -727,14 +728,14 @@ extension Substring: RangeReplaceableCollection { public init(_ elements: S) where S.Element == Character { if let str = elements as? String { - self = str[...] + self.init(str) return } if let subStr = elements as? Substring { self = subStr return } - self = String(elements)[...] + self.init(String(elements)) } @inlinable // specialize diff --git a/stdlib/public/core/UnsafeBufferPointer.swift.gyb b/stdlib/public/core/UnsafeBufferPointer.swift.gyb index ee85c9de54d33..33ceb5b769a8a 100644 --- a/stdlib/public/core/UnsafeBufferPointer.swift.gyb +++ b/stdlib/public/core/UnsafeBufferPointer.swift.gyb @@ -207,7 +207,14 @@ extension Unsafe${Mutable}BufferPointer: ${Mutable}Collection, RandomAccessColle // optimizer is not capable of creating partial specializations yet. // NOTE: Range checks are not performed here, because it is done later by // the subscript function. - return end - start + // NOTE: We allow the subtraction to silently overflow in release builds + // to eliminate a superflous check when `start` and `end` are both valid + // indices. (The operation can only overflow if `start` is negative, which + // implies it's an invalid index.) `Collection` does not specify what + // `distance` should return when given an invalid index pair. + let result = end.subtractingReportingOverflow(start) + _debugPrecondition(!result.overflow) + return result.partialValue } @inlinable // unsafe-performance @@ -387,6 +394,17 @@ extension Unsafe${Mutable}BufferPointer: ${Mutable}Collection, RandomAccessColle } extension Unsafe${Mutable}BufferPointer { + // This works around _debugPrecondition() impacting the performance of + // optimized code. (rdar://72246338) + @_alwaysEmitIntoClient + internal init( + @_nonEphemeral _uncheckedStart start: Unsafe${Mutable}Pointer?, + count: Int + ) { + _position = start + self.count = count + } + /// Creates a new buffer pointer over the specified number of contiguous /// instances beginning at the given pointer. /// @@ -401,13 +419,12 @@ extension Unsafe${Mutable}BufferPointer { public init( @_nonEphemeral start: Unsafe${Mutable}Pointer?, count: Int ) { - _precondition( + _debugPrecondition( count >= 0, "Unsafe${Mutable}BufferPointer with negative count") - _precondition( + _debugPrecondition( count == 0 || start != nil, "Unsafe${Mutable}BufferPointer has a nil start and nonzero count") - _position = start - self.count = count + self.init(_uncheckedStart: start, count: _assumeNonNegative(count)) } @inlinable // unsafe-performance @@ -498,6 +515,17 @@ extension Unsafe${Mutable}BufferPointer { /// - Parameter slice: The buffer slice to rebase. @inlinable // unsafe-performance public init(rebasing slice: Slice>) { + // NOTE: `Slice` does not guarantee that its start/end indices are valid + // in `base` -- it merely ensures that `startIndex <= endIndex`. + // We need manually check that we aren't given an invalid slice, + // or the resulting collection would allow access that was + // out-of-bounds with respect to the original base buffer. + // We only do this in debug builds to prevent a measurable performance + // degradation wrt passing around pointers not wrapped in a BufferPointer + // construct. + _debugPrecondition( + slice.startIndex >= 0 && slice.endIndex <= slice.base.count, + "Invalid slice") let base = slice.base.baseAddress?.advanced(by: slice.startIndex) let count = slice.endIndex &- slice.startIndex self.init(start: base, count: count) diff --git a/stdlib/public/core/UnsafeRawBufferPointer.swift.gyb b/stdlib/public/core/UnsafeRawBufferPointer.swift.gyb index 1a7508ccf50a3..1a9d71289b41c 100644 --- a/stdlib/public/core/UnsafeRawBufferPointer.swift.gyb +++ b/stdlib/public/core/UnsafeRawBufferPointer.swift.gyb @@ -253,7 +253,7 @@ extension Unsafe${Mutable}RawBufferPointer: ${Mutable}Collection { @inlinable public var count: Int { if let pos = _position { - return _end! - pos + return _assumeNonNegative(_end! - pos) } return 0 } @@ -432,11 +432,11 @@ extension Unsafe${Mutable}RawBufferPointer { public init( @_nonEphemeral start: Unsafe${Mutable}RawPointer?, count: Int ) { - _precondition(count >= 0, "${Self} with negative count") - _precondition(count == 0 || start != nil, + _debugPrecondition(count >= 0, "${Self} with negative count") + _debugPrecondition(count == 0 || start != nil, "${Self} has a nil start and nonzero count") _position = start - _end = start.map { $0 + count } + _end = start.map { $0 + _assumeNonNegative(count) } } /// Creates a new buffer over the same memory as the given buffer. @@ -511,6 +511,17 @@ extension Unsafe${Mutable}RawBufferPointer { /// - Parameter slice: The raw buffer slice to rebase. @inlinable public init(rebasing slice: Slice) { + // NOTE: `Slice` does not guarantee that its start/end indices are valid + // in `base` -- it merely ensures that `startIndex <= endIndex`. + // We need manually check that we aren't given an invalid slice, + // or the resulting collection would allow access that was + // out-of-bounds with respect to the original base buffer. + // We only do this in debug builds to prevent a measurable performance + // degradation wrt passing around pointers not wrapped in a BufferPointer + // construct. + _debugPrecondition( + slice.startIndex >= 0 && slice.endIndex <= slice.base.count, + "Invalid slice") let base = slice.base.baseAddress?.advanced(by: slice.startIndex) let count = slice.endIndex &- slice.startIndex self.init(start: base, count: count) diff --git a/test/stdlib/RangeTraps.swift b/test/stdlib/RangeTraps.swift index c8e265416ef66..594a329b82ecc 100644 --- a/test/stdlib/RangeTraps.swift +++ b/test/stdlib/RangeTraps.swift @@ -116,5 +116,23 @@ RangeTraps.test("throughNaN") _ = ...Double.nan } +RangeTraps.test("UncheckedHalfOpen") + .xfail(.custom( + { !_isDebugAssertConfiguration() }, + reason: "assertions are disabled in Release and Unchecked mode")) + .code { + expectCrashLater() + var range = Range(uncheckedBounds: (lower: 1, upper: 0)) + } + +RangeTraps.test("UncheckedClosed") + .xfail(.custom( + { !_isDebugAssertConfiguration() }, + reason: "assertions are disabled in Release and Unchecked mode")) + .code { + expectCrashLater() + var range = ClosedRange(uncheckedBounds: (lower: 1, upper: 0)) + } + runAllTests() diff --git a/validation-test/stdlib/ArrayTraps.swift.gyb b/validation-test/stdlib/ArrayTraps.swift.gyb index b632fa0609edf..ca46edbe48881 100644 --- a/validation-test/stdlib/ArrayTraps.swift.gyb +++ b/validation-test/stdlib/ArrayTraps.swift.gyb @@ -172,8 +172,8 @@ ${ArrayTy}Traps.test("${'remove(at:)/%s' % index}") ArrayTraps.test("unsafeLength") .skip(.custom( - { _isFastAssertConfiguration() }, - reason: "this trap is not guaranteed to happen in -Ounchecked")) + { !_isDebugAssertConfiguration() }, + reason: "this trap is not guaranteed to happen in -O or -Ounchecked")) .crashOutputMatches(_isDebugAssertConfiguration() ? "UnsafeBufferPointer with negative count" : "") .code { diff --git a/validation-test/stdlib/Range.swift.gyb b/validation-test/stdlib/Range.swift.gyb index b8fa3838e0f7f..f5898c73eb9fa 100644 --- a/validation-test/stdlib/Range.swift.gyb +++ b/validation-test/stdlib/Range.swift.gyb @@ -335,17 +335,31 @@ extension ${Self} where Bound : TestProtocol1 { var ${TestSuite} = TestSuite("${Self}") ${TestSuite}.test("init(uncheckedBounds:)") - .forEach(in: [(1, 2), (1, 1), (2, 1)]) { + .forEach(in: [(1, 2), (1, 1)]) { (lowerInt, upperInt) in - // Check that 'init(uncheckedBounds:)' does not perform precondition checks, - // allowing to create ranges that break invariants. let r = ${Self}( uncheckedBounds: (lower: ${Bound}(lowerInt), upper: ${Bound}(upperInt))) expectEqual(lowerInt, r.lowerBound.value) expectEqual(upperInt, r.upperBound.value) } +${TestSuite}.test("init(uncheckedBounds:) with invalid values") + .xfail( + .custom( + { _isDebugAssertConfiguration() }, + reason: "unchecked initializer still checks its input in Debug mode")) + .code { + let low = 2 + let up = 1 + // Check that 'init(uncheckedBounds:)' does not perform precondition checks, + // allowing to create ranges that break invariants. + let r = ${Self}( + uncheckedBounds: (lower: ${Bound}(low), upper: ${Bound}(up))) + expectEqual(low, r.lowerBound.value) + expectEqual(up, r.upperBound.value) +} + % for (DestinationSelf, _, _, _) in all_range_types: ${TestSuite}.test("init(${DestinationSelf})/whereBoundIsStrideable") .forEach(in: [(0, 0), (1, 2), (10, 20), (Int.min, Int.max)]) {