From 496296e50327c621db6947fbdfa61d3bb60bbf07 Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Mon, 2 Dec 2024 09:24:12 -0800 Subject: [PATCH] [stdlib] Fix RawSpan initializer lifetime dependencies. Two are fixes needed in most of the `RawSpan` and `Span` initializers. For example: ``` let baseAddress = buffer.baseAddress let span = RawSpan(_unchecked: baseAddress, byteCount: buffer.count) // As a trivial value, 'baseAddress' does not formally depend on the // lifetime of 'buffer'. Make the dependence explicit. self = _overrideLifetime(span, borrowing: buffer) ``` Fix #1. baseAddress needs to be a variable `span` has a lifetime dependence on `baseAddress` via its initializer. Therefore, the lifetime of `baseAddress` needs to include the call to `_overrideLifetime`. The override sets the lifetime dependency of its result, not its argument. It's argument still needs to be non-escaping when it is passed in. Alternatives: - Make the RawSpan initializer `@_unsafeNonescapableResult`. Any occurrence of `@_unsafeNonescapableResult` actually signals a bug. We never want to expose this annotation. In addition to being gross, it would totally disable enforcement of the initialized span. But we really don't want to side-step `_overrideLifetime` where it makes sense. We want the library author to explicitly indicate that they understand exactly which dependence is unsafe. And we do want to eventually expose the `_overrideLifetime` API, which needs to be well understood, supported, and tested. - Add lifetime annotations to a bunch of `UnsafePointer`-family APIs so the compiler can see that the resulting pointer is derived from self, where self is an incoming `Unsafe[Buffer]Pointer`. This would create a massive lifetime annotation burden on the `UnsafePointer`-family APIs, which don't really have anything to do with lifetime dependence. It makes more sense for the author of `Span`-like APIs to reason about pointer lifetimes. Fix #2. `_overrideLifetime` changes the lifetime dependency of span to be on an incoming argument rather than a local variable. This makes it legal to escape the function (by assigning it to self). Remember that self is implicitly returned, so the `@lifetime(borrow buffer)` tells the compiler that `self` is valid within `buffer`'s borrow scope. --- stdlib/public/core/Span/RawSpan.swift | 79 ++++++++++++++++++------- stdlib/public/core/Span/Span.swift | 85 +++++++++++++++++++++------ 2 files changed, 126 insertions(+), 38 deletions(-) diff --git a/stdlib/public/core/Span/RawSpan.swift b/stdlib/public/core/Span/RawSpan.swift index 5e45a2cfa842f..fdb410ac0f932 100644 --- a/stdlib/public/core/Span/RawSpan.swift +++ b/stdlib/public/core/Span/RawSpan.swift @@ -97,9 +97,11 @@ extension RawSpan { public init( _unsafeBytes buffer: UnsafeRawBufferPointer ) { - self.init( - _unchecked: buffer.baseAddress, byteCount: buffer.count - ) + let baseAddress = buffer.baseAddress + let span = RawSpan(_unchecked: baseAddress, byteCount: buffer.count) + // As a trivial value, 'baseAddress' does not formally depend on the + // lifetime of 'buffer'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, borrows: buffer) } /// Unsafely create a `RawSpan` over initialized memory. @@ -115,7 +117,11 @@ extension RawSpan { public init( _unsafeBytes buffer: borrowing Slice ) { - self.init(_unsafeBytes: UnsafeRawBufferPointer(rebasing: buffer)) + let rawBuffer = UnsafeRawBufferPointer(rebasing: buffer) + let span = RawSpan(_unsafeBytes: rawBuffer) + // As a trivial value, 'rawBuffer' does not formally depend on the + // lifetime of 'buffer'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, borrows: buffer) } /// Unsafely create a `RawSpan` over initialized memory. @@ -131,7 +137,11 @@ extension RawSpan { public init( _unsafeBytes buffer: UnsafeMutableRawBufferPointer ) { - self.init(_unsafeBytes: UnsafeRawBufferPointer(buffer)) + let rawBuffer = UnsafeRawBufferPointer(buffer) + let span = RawSpan(_unsafeBytes: rawBuffer) + // As a trivial value, 'rawBuffer' does not formally depend on the + // lifetime of 'buffer'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, borrows: buffer) } @_alwaysEmitIntoClient @@ -139,7 +149,12 @@ extension RawSpan { public init( _unsafeBytes buffer: borrowing Slice ) { - self.init(_unsafeBytes: UnsafeRawBufferPointer(rebasing: buffer)) + let rawBuffer = + UnsafeRawBufferPointer(UnsafeMutableRawBufferPointer(rebasing: buffer)) + let span = RawSpan(_unsafeBytes: rawBuffer) + // As a trivial value, 'rawBuffer' does not formally depend on the + // lifetime of 'buffer'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, borrows: buffer) } /// Unsafely create a `RawSpan` over initialized memory. @@ -175,7 +190,11 @@ extension RawSpan { public init( _unsafeElements buffer: UnsafeBufferPointer ) { - self.init(_unsafeBytes: UnsafeRawBufferPointer(buffer)) + let rawBuffer = UnsafeRawBufferPointer(buffer) + let span = RawSpan(_unsafeBytes: rawBuffer) + // As a trivial value, 'rawBuffer' does not formally depend on the + // lifetime of 'buffer'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, borrows: buffer) } /// Unsafely create a `RawSpan` over initialized memory. @@ -191,9 +210,11 @@ extension RawSpan { public init( _unsafeElements buffer: borrowing Slice> ) { - self.init( - _unsafeBytes: .init(UnsafeBufferPointer(rebasing: buffer)) - ) + let rawBuffer = UnsafeRawBufferPointer(UnsafeBufferPointer(rebasing: buffer)) + let span = RawSpan(_unsafeBytes: rawBuffer) + // As a trivial value, 'rawBuffer' does not formally depend on the + // lifetime of 'buffer'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, borrows: buffer) } /// Unsafely create a `RawSpan` over initialized memory. @@ -209,7 +230,11 @@ extension RawSpan { public init( _unsafeElements buffer: UnsafeMutableBufferPointer ) { - self.init(_unsafeElements: UnsafeBufferPointer(buffer)) + let rawBuffer = UnsafeRawBufferPointer(buffer) + let span = RawSpan(_unsafeBytes: rawBuffer) + // As a trivial value, 'rawBuffer' does not formally depend on the + // lifetime of 'buffer'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, borrows: buffer) } /// Unsafely create a `RawSpan` over initialized memory. @@ -225,9 +250,12 @@ extension RawSpan { public init( _unsafeElements buffer: borrowing Slice> ) { - self.init( - _unsafeBytes: .init(UnsafeBufferPointer(rebasing: buffer)) - ) + let rawBuffer = + UnsafeRawBufferPointer(UnsafeMutableBufferPointer(rebasing: buffer)) + let span = RawSpan(_unsafeBytes: rawBuffer) + // As a trivial value, 'rawBuffer' does not formally depend on the + // lifetime of 'buffer'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, borrows: buffer) } /// Unsafely create a `RawSpan` over initialized memory. @@ -344,10 +372,9 @@ extension RawSpan { @_alwaysEmitIntoClient @lifetime(self) public func _extracting(unchecked bounds: Range) -> Self { - RawSpan( - _unchecked: _pointer?.advanced(by: bounds.lowerBound), - byteCount: bounds.count - ) + let newStart = _pointer?.advanced(by: bounds.lowerBound) + let newSpan = RawSpan(_unchecked: newStart, byteCount: bounds.count) + return _unsafeLifetime(dependent: newSpan, dependsOn: self) } /// Constructs a new span over the bytes within the supplied range of @@ -457,7 +484,11 @@ extension RawSpan { consuming public func _unsafeView( as type: T.Type ) -> Span { - Span(_unsafeBytes: .init(start: _pointer, count: _count)) + let rawBuffer = UnsafeRawBufferPointer(start: _pointer, count: _count) + let newSpan = Span(_unsafeBytes: rawBuffer) + // As a trivial value, 'rawBuffer' does not formally depend on the + // lifetime of 'self'. Make the dependence explicit. + return _unsafeLifetime(dependent: newSpan, dependsOn: self) } } @@ -677,7 +708,10 @@ extension RawSpan { _precondition(maxLength >= 0, "Can't have a suffix of negative length") let newCount = min(maxLength, byteCount) let newStart = _pointer?.advanced(by: byteCount &- newCount) - return Self(_unchecked: newStart, byteCount: newCount) + let newSpan = RawSpan(_unchecked: newStart, byteCount: newCount) + // As a trivial value, 'newStart' does not formally depend on the + // lifetime of 'self'. Make the dependence explicit. + return _unsafeLifetime(dependent: newSpan, dependsOn: self) } /// Returns a span over all but the given number of initial bytes. @@ -700,6 +734,9 @@ extension RawSpan { _precondition(k >= 0, "Can't drop a negative number of elements") let droppedCount = min(k, byteCount) let newStart = _pointer?.advanced(by: droppedCount) - return Self(_unchecked: newStart, byteCount: byteCount &- droppedCount) + let newSpan = RawSpan(_unchecked: newStart, byteCount: byteCount &- droppedCount) + // As a trivial value, 'newStart' does not formally depend on the + // lifetime of 'self'. Make the dependence explicit. + return _unsafeLifetime(dependent: newSpan, dependsOn: self) } } diff --git a/stdlib/public/core/Span/Span.swift b/stdlib/public/core/Span/Span.swift index a2fd6386c80cf..4eef5bf72e08a 100644 --- a/stdlib/public/core/Span/Span.swift +++ b/stdlib/public/core/Span/Span.swift @@ -100,13 +100,16 @@ extension Span where Element: ~Copyable { _unsafeElements buffer: UnsafeBufferPointer ) { //FIXME: Workaround for https://github.com/swiftlang/swift/issues/77235 - let baseAddress = buffer.baseAddress + let baseAddress = UnsafeRawPointer(buffer.baseAddress) _precondition( ((Int(bitPattern: baseAddress) & (MemoryLayout.alignment &- 1)) == 0), "baseAddress must be properly aligned to access Element" ) - self.init(_unchecked: baseAddress, count: buffer.count) + let span = Span(_unchecked: baseAddress, count: buffer.count) + // As a trivial value, 'baseAddress' does not formally depend on the + // lifetime of 'buffer'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, borrows: buffer) } /// Unsafely create a `Span` over initialized memory. @@ -122,7 +125,11 @@ extension Span where Element: ~Copyable { public init( _unsafeElements buffer: UnsafeMutableBufferPointer ) { - self.init(_unsafeElements: UnsafeBufferPointer(buffer)) + let buf = UnsafeBufferPointer(buffer) + let span = Span(_unsafeElements: buf) + // As a trivial value, 'buf' does not formally depend on the + // lifetime of 'buffer'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, borrows: buffer) } /// Unsafely create a `Span` over initialized memory. @@ -142,7 +149,11 @@ extension Span where Element: ~Copyable { count: Int ) { _precondition(count >= 0, "Count must not be negative") - self.init(_unsafeElements: .init(start: pointer, count: count)) + let buf = UnsafeBufferPointer(start: pointer, count: count) + let span = Span(_unsafeElements: buf) + // As a trivial value, 'buf' does not formally depend on the + // lifetime of 'pointer'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, borrows: pointer) } } @@ -162,7 +173,11 @@ extension Span { public init( _unsafeElements buffer: borrowing Slice> ) { - self.init(_unsafeElements: UnsafeBufferPointer(rebasing: buffer)) + let buf = UnsafeBufferPointer(rebasing: buffer) + let span = Span(_unsafeElements: buf) + // As a trivial value, 'buf' does not formally depend on the + // lifetime of 'buffer'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, borrows: buffer) } /// Unsafely create a `Span` over initialized memory. @@ -178,7 +193,11 @@ extension Span { public init( _unsafeElements buffer: borrowing Slice> ) { - self.init(_unsafeElements: UnsafeBufferPointer(rebasing: buffer)) + let buf = UnsafeBufferPointer(rebasing: buffer) + let span = Span(_unsafeElements: buf) + // As a trivial value, 'buf' does not formally depend on the + // lifetime of 'buffer'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, borrows: buffer) } } @@ -214,7 +233,10 @@ extension Span where Element: BitwiseCopyable { _precondition( remainder == 0, "Span must contain a whole number of elements" ) - self.init(_unchecked: baseAddress, count: count) + let span = Span(_unchecked: baseAddress, count: count) + // As a trivial value, 'baseAddress' does not formally depend on the + // lifetime of 'buffer'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, borrows: buffer) } /// Unsafely create a `Span` over initialized memory. @@ -234,7 +256,11 @@ extension Span where Element: BitwiseCopyable { public init( _unsafeBytes buffer: UnsafeMutableRawBufferPointer ) { - self.init(_unsafeBytes: UnsafeRawBufferPointer(buffer)) + let rawBuffer = UnsafeRawBufferPointer(buffer) + let span = Span(_unsafeBytes: rawBuffer) + // As a trivial value, 'buf' does not formally depend on the + // lifetime of 'buffer'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, borrows: buffer) } /// Unsafely create a `Span` over initialized memory. @@ -258,7 +284,11 @@ extension Span where Element: BitwiseCopyable { byteCount: Int ) { _precondition(byteCount >= 0, "Count must not be negative") - self.init(_unsafeBytes: .init(start: pointer, count: byteCount)) + let rawBuffer = UnsafeRawBufferPointer(start: pointer, count: byteCount) + let span = Span(_unsafeBytes: rawBuffer) + // As a trivial value, 'rawBuffer' does not formally depend on the + // lifetime of 'pointer'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, borrows: pointer) } /// Unsafely create a `Span` over initialized memory. @@ -278,7 +308,11 @@ extension Span where Element: BitwiseCopyable { public init( _unsafeBytes buffer: borrowing Slice ) { - self.init(_unsafeBytes: UnsafeRawBufferPointer(rebasing: buffer)) + let rawBuffer = UnsafeRawBufferPointer(rebasing: buffer) + let span = Span(_unsafeBytes: rawBuffer) + // As a trivial value, 'rawBuffer' does not formally depend on the + // lifetime of 'buffer'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, borrows: buffer) } /// Unsafely create a `Span` over initialized memory. @@ -298,7 +332,11 @@ extension Span where Element: BitwiseCopyable { public init( _unsafeBytes buffer: borrowing Slice ) { - self.init(_unsafeBytes: UnsafeRawBufferPointer(rebasing: buffer)) + let rawBuffer = UnsafeRawBufferPointer(rebasing: buffer) + let span = Span(_unsafeBytes: rawBuffer) + // As a trivial value, 'rawBuffer' does not formally depend on the + // lifetime of 'buffer'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, borrows: buffer) } /// Create a `Span` over the bytes represented by a `RawSpan` @@ -309,9 +347,12 @@ extension Span where Element: BitwiseCopyable { @_alwaysEmitIntoClient @lifetime(bytes) public init(_bytes bytes: consuming RawSpan) { - self.init( - _unsafeBytes: .init(start: bytes._pointer, count: bytes.byteCount) - ) + let rawBuffer = + UnsafeRawBufferPointer(start: bytes._pointer, count: bytes.byteCount) + let span = Span(_unsafeBytes: rawBuffer) + // As a trivial value, 'rawBuffer' does not formally depend on the + // lifetime of 'bytes'. Make the dependence explicit. + self = _unsafeLifetime(dependent: span, dependsOn: bytes) } } @@ -481,7 +522,11 @@ extension Span where Element: ~Copyable { @lifetime(self) public func _extracting(unchecked bounds: Range) -> Self { let delta = bounds.lowerBound &* MemoryLayout.stride - return Span(_unchecked: _pointer?.advanced(by: delta), count: bounds.count) + let newStart = _pointer?.advanced(by: delta) + let newSpan = Span(_unchecked: newStart, count: bounds.count) + // As a trivial value, 'newStart' does not formally depend on the + // lifetime of 'self'. Make the dependence explicit. + return _unsafeLifetime(dependent: newSpan, dependsOn: self) } /// Constructs a new span over the items within the supplied range of @@ -704,7 +749,10 @@ extension Span where Element: ~Copyable { let newCount = min(maxLength, count) let offset = (count &- newCount) * MemoryLayout.stride let newStart = _pointer?.advanced(by: offset) - return Self(_unchecked: newStart, count: newCount) + let newSpan = Span(_unchecked: newStart, count: newCount) + // As a trivial value, 'newStart' does not formally depend on the + // lifetime of 'buffer'. Make the dependence explicit. + return _unsafeLifetime(dependent: newSpan, dependsOn: self) } /// Returns a span over all but the given number of initial elements. @@ -728,6 +776,9 @@ extension Span where Element: ~Copyable { let droppedCount = min(k, count) let offset = droppedCount * MemoryLayout.stride let newStart = _pointer?.advanced(by: offset) - return Self(_unchecked: newStart, count: count &- droppedCount) + let newSpan = Span(_unchecked: newStart, count: count &- droppedCount) + // As a trivial value, 'newStart' does not formally depend on the + // lifetime of 'buffer'. Make the dependence explicit. + return _unsafeLifetime(dependent: newSpan, dependsOn: self) } }