Skip to content

Commit 174a5a7

Browse files
glessardDougGregor
authored andcommitted
Conditionally enable bounds checking for unsafe buffer pointers
Currently, bounds checking is only enabled for unsafe buffer pointers in debug builds. Introduce a new precondition check kind, `_boundsCheckPrecondition`, and use it for the bounds checking of unsafe buffer pointers. This check is enabled both in debug builds and in builds where the experimental feature `UnsafePointerBoundsSafety` is enabled.
1 parent ae9238e commit 174a5a7

File tree

3 files changed

+84
-21
lines changed

3 files changed

+84
-21
lines changed

stdlib/public/core/Assert.swift

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,31 @@ internal func _debugPreconditionFailure(
377377
#endif
378378
}
379379

380+
/// Precondition checks for bounds-checking of unsafe pointers.
381+
///
382+
/// Debug library precondition checks are on in debug mode and in release
383+
/// mode with additional bounds safety enabled. If release and unchecked
384+
/// modes, they are disabled.
385+
/// They are meant to be used for bounds-safety checks.
386+
@usableFromInline @_transparent @_alwaysEmitIntoClient
387+
internal func _boundsCheckPrecondition(
388+
_ condition: @autoclosure () -> Bool, _ message: StaticString = StaticString(),
389+
file: StaticString = #file, line: UInt = #line
390+
) {
391+
#if SWIFT_STDLIB_ENABLE_DEBUG_PRECONDITIONS_IN_RELEASE
392+
_precondition(condition(), message, file: file, line: line)
393+
#else
394+
// Only check in debug and release w/ bounds checks.
395+
if _isDebugAssertConfiguration() ||
396+
_isReleaseAssertWithBoundsSafetyConfiguration() {
397+
if !_fastPath(condition()) {
398+
_fatalErrorMessage("Fatal error", message, file: file, line: line,
399+
flags: _fatalErrorFlags())
400+
}
401+
}
402+
#endif
403+
}
404+
380405
/// Internal checks.
381406
///
382407
/// Internal checks are to be used for checking correctness conditions in the

stdlib/public/core/UnsafeBufferPointer.swift.gyb

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -304,16 +304,15 @@ extension Unsafe${Mutable}BufferPointer: ${Mutable}Collection, RandomAccessColle
304304

305305
@inlinable // unsafe-performance
306306
public func _failEarlyRangeCheck(_ index: Int, bounds: Range<Int>) {
307-
// NOTE: In release mode, this method is a no-op for performance reasons.
308-
_debugPrecondition(index >= bounds.lowerBound)
309-
_debugPrecondition(index < bounds.upperBound)
307+
_boundsCheckPrecondition(index >= bounds.lowerBound && index < bounds.upperBound)
310308
}
311309

312310
@inlinable // unsafe-performance
313311
public func _failEarlyRangeCheck(_ range: Range<Int>, bounds: Range<Int>) {
314-
// NOTE: In release mode, this method is a no-op for performance reasons.
315-
_debugPrecondition(range.lowerBound >= bounds.lowerBound)
316-
_debugPrecondition(range.upperBound <= bounds.upperBound)
312+
_boundsCheckPrecondition(
313+
range.lowerBound >= bounds.lowerBound &&
314+
range.upperBound <= bounds.upperBound
315+
)
317316
}
318317

319318
@inlinable // unsafe-performance
@@ -365,14 +364,14 @@ extension Unsafe${Mutable}BufferPointer: ${Mutable}Collection, RandomAccessColle
365364
@inlinable // unsafe-performance
366365
public subscript(i: Int) -> Element {
367366
get {
368-
_debugPrecondition(i >= 0)
369-
_debugPrecondition(i < endIndex)
367+
_boundsCheckPrecondition(i >= 0)
368+
_boundsCheckPrecondition(i < endIndex)
370369
return _position._unsafelyUnwrappedUnchecked[i]
371370
}
372371
%if Mutable:
373372
nonmutating _modify {
374-
_debugPrecondition(i >= 0)
375-
_debugPrecondition(i < endIndex)
373+
_boundsCheckPrecondition(i >= 0)
374+
_boundsCheckPrecondition(i < endIndex)
376375
yield &_position._unsafelyUnwrappedUnchecked[i]
377376
}
378377
%end
@@ -438,16 +437,16 @@ extension Unsafe${Mutable}BufferPointer: ${Mutable}Collection, RandomAccessColle
438437
-> Slice<Unsafe${Mutable}BufferPointer<Element>>
439438
{
440439
get {
441-
_debugPrecondition(bounds.lowerBound >= startIndex)
442-
_debugPrecondition(bounds.upperBound <= endIndex)
440+
_boundsCheckPrecondition(bounds.lowerBound >= startIndex)
441+
_boundsCheckPrecondition(bounds.upperBound <= endIndex)
443442
return Slice(
444443
base: self, bounds: bounds)
445444
}
446445
% if Mutable:
447446
nonmutating set {
448-
_debugPrecondition(bounds.lowerBound >= startIndex)
449-
_debugPrecondition(bounds.upperBound <= endIndex)
450-
_debugPrecondition(bounds.count == newValue.count)
447+
_boundsCheckPrecondition(bounds.lowerBound >= startIndex)
448+
_boundsCheckPrecondition(bounds.upperBound <= endIndex)
449+
_boundsCheckPrecondition(bounds.count == newValue.count)
451450

452451
// FIXME: swift-3-indexing-model: tests.
453452
if !newValue.isEmpty {
@@ -472,8 +471,8 @@ extension Unsafe${Mutable}BufferPointer: ${Mutable}Collection, RandomAccessColle
472471
@inlinable // unsafe-performance
473472
public func swapAt(_ i: Int, _ j: Int) {
474473
guard i != j else { return }
475-
_debugPrecondition(i >= 0 && j >= 0)
476-
_debugPrecondition(i < endIndex && j < endIndex)
474+
_boundsCheckPrecondition(i >= 0 && j >= 0)
475+
_boundsCheckPrecondition(i < endIndex && j < endIndex)
477476
let pi = (_position! + i)
478477
let pj = (_position! + j)
479478
let tmp = pi.move()
@@ -554,7 +553,7 @@ extension Unsafe${Mutable}BufferPointer {
554553
// We only do this in debug builds to prevent a measurable performance
555554
// degradation wrt passing around pointers not wrapped in a BufferPointer
556555
// construct.
557-
_debugPrecondition(
556+
_boundsCheckPrecondition(
558557
slice.startIndex >= 0 && slice.endIndex <= slice.base.count,
559558
"Invalid slice")
560559
let base = slice.base.baseAddress?.advanced(by: slice.startIndex)
@@ -1029,7 +1028,7 @@ extension Unsafe${Mutable}BufferPointer {
10291028
@inlinable
10301029
@_alwaysEmitIntoClient
10311030
public func initializeElement(at index: Index, to value: Element) {
1032-
_debugPrecondition(startIndex <= index && index < endIndex)
1031+
_boundsCheckPrecondition(startIndex <= index && index < endIndex)
10331032
let p = baseAddress._unsafelyUnwrappedUnchecked.advanced(by: index)
10341033
p.initialize(to: value)
10351034
}
@@ -1047,7 +1046,7 @@ extension Unsafe${Mutable}BufferPointer {
10471046
@inlinable
10481047
@_alwaysEmitIntoClient
10491048
public func moveElement(from index: Index) -> Element {
1050-
_debugPrecondition(startIndex <= index && index < endIndex)
1049+
_boundsCheckPrecondition(startIndex <= index && index < endIndex)
10511050
return baseAddress._unsafelyUnwrappedUnchecked.advanced(by: index).move()
10521051
}
10531052

@@ -1062,7 +1061,7 @@ extension Unsafe${Mutable}BufferPointer {
10621061
@inlinable
10631062
@_alwaysEmitIntoClient
10641063
public func deinitializeElement(at index: Index) {
1065-
_debugPrecondition(startIndex <= index && index < endIndex)
1064+
_boundsCheckPrecondition(startIndex <= index && index < endIndex)
10661065
let p = baseAddress._unsafelyUnwrappedUnchecked.advanced(by: index)
10671066
p.deinitialize(count: 1)
10681067
}

test/stdlib/BoundsCheckTraps.swift

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-build-swift %s -o %t/a.out_Debug -Onone
3+
// RUN: %target-build-swift %s -o %t/a.out_Release -O
4+
// RUN: %target-build-swift %s -o %t/a.out_ReleaseWithBoundsSafety -O -enable-experimental-feature UnsafePointerBoundsSafety
5+
// RUN: %target-codesign %t/a.out_Debug
6+
// RUN: %target-codesign %t/a.out_Release
7+
// RUN: %target-codesign %t/a.out_ReleaseWithBoundsSafety
8+
//
9+
// RUN: %target-run %t/a.out_Debug
10+
// RUN: %target-run %t/a.out_Release
11+
// RUN: %target-run %t/a.out_ReleaseWithBoundsSafety
12+
// REQUIRES: executable_test
13+
// REQUIRES: asserts
14+
// UNSUPPORTED: OS=wasi
15+
16+
import StdlibUnittest
17+
18+
let testSuiteSuffix = _isDebugAssertConfiguration() ? "_debug"
19+
: _isReleaseAssertConfiguration() ? "_release" : "_releaseWithBoundsSafety"
20+
21+
var BoundsCheckTraps = TestSuite("BoundsCheckTraps" + testSuiteSuffix)
22+
23+
BoundsCheckTraps.test("UnsafeMutableBufferPointer")
24+
.skip(.custom(
25+
{ _isFastAssertConfiguration() || _isReleaseAssertConfiguration() },
26+
reason: "this trap is not guaranteed to happen"))
27+
.code {
28+
expectCrashLater()
29+
var array = [1, 2, 3]
30+
array.withUnsafeBufferPointer { buffer in
31+
print(buffer[3])
32+
}
33+
array.withUnsafeMutableBufferPointer { buffer in
34+
buffer[3] = 17
35+
}
36+
_blackHole(array)
37+
}
38+
39+
runAllTests()

0 commit comments

Comments
 (0)