Skip to content

Commit 900bf8c

Browse files
authored
Add a type argument to the ABI accessor function signature. (#939)
We access the test content record section directly instead of using some (non-existent) type-safe Swift API. On some platforms (Darwin in particular), it is possible for two copies of the testing library to be loaded at runtime, and for them to have incompatible definitions of types like `Test`. That means one library's `@Test` macro could produce a test content record that is then read by the other library's discovery logic, resulting in a stack smash or worse. We can resolve this issue by adding a `type` argument to the accessor function we define for test content records; the body of the accessor can then compare the value of this argument against the expected Swift type of its result and, if they don't match, bail early. The new argument is defined as a pointer _to_ a Swift type rather than as a Swift type directly because `@convention(c)` functions cannot directly reference Swift types. The value of the new argument can be safely ignored if the type of the test content record's value is and always has been `@frozen`. ### Checklist: - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
1 parent f4abd7a commit 900bf8c

File tree

4 files changed

+103
-28
lines changed

4 files changed

+103
-28
lines changed

Documentation/ABI/TestContent.md

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,16 @@ Regardless of platform, all test content records created and discoverable by the
4141
testing library have the following layout:
4242

4343
```swift
44+
typealias Accessor = @convention(c) (
45+
_ outValue: UnsafeMutableRawPointer,
46+
_ type: UnsafeRawPointer,
47+
_ hint: UnsafeRawPointer?
48+
) -> CBool
49+
4450
typealias TestContentRecord = (
4551
kind: UInt32,
4652
reserved1: UInt32,
47-
accessor: (@convention(c) (_ outValue: UnsafeMutableRawPointer, _ hint: UnsafeRawPointer?) -> CBool)?,
53+
accessor: Accessor?,
4854
context: UInt,
4955
reserved2: UInt
5056
)
@@ -54,10 +60,16 @@ This type has natural size, stride, and alignment. Its fields are native-endian.
5460
If needed, this type can be represented in C as a structure:
5561

5662
```c
63+
typedef bool (* SWTAccessor)(
64+
void *outValue,
65+
const void *type,
66+
const void *_Nullable hint
67+
);
68+
5769
struct SWTTestContentRecord {
5870
uint32_t kind;
5971
uint32_t reserved1;
60-
bool (* _Nullable accessor)(void *outValue, const void *_Null_unspecified hint);
72+
SWTAccessor _Nullable accessor;
6173
uintptr_t context;
6274
uintptr_t reserved2;
6375
};
@@ -105,42 +117,59 @@ If `accessor` is `nil`, the test content record is ignored. The testing library
105117
may, in the future, define record kinds that do not provide an accessor function
106118
(that is, they represent pure compile-time information only.)
107119

108-
The second argument to this function, `hint`, is an optional input that can be
120+
The second argument to this function, `type`, is a pointer to the type[^mightNotBeSwift]
121+
(not a bitcast Swift type) of the value expected to be written to `outValue`.
122+
This argument helps to prevent memory corruption if two copies of Swift Testing
123+
or a third-party library are inadvertently loaded into the same process. If the
124+
value at `type` does not match the test content record's expected type, the
125+
accessor function must return `false` and must not modify `outValue`.
126+
127+
<!-- TODO: discuss this argument's value in Embedded Swift (no metatypes) -->
128+
129+
[^mightNotBeSwift]: Although this document primarily deals with Swift, the test
130+
content record section is generally language-agnostic. The use of languages
131+
other than Swift is beyond the scope of this document. With that in mind, it
132+
is _technically_ feasible for a test content accessor to be written in (for
133+
example) C++, expect the `type` argument to point to a C++ value of type
134+
`std::type_info`, and write a C++ class instance to `outValue`.
135+
136+
The third argument to this function, `hint`, is an optional input that can be
109137
passed to help the accessor function determine if its corresponding test content
110138
record matches what the caller is looking for. If the caller passes `nil` as the
111139
`hint` argument, the accessor behaves as if it matched (that is, no additional
112140
filtering is performed.)
113141

114-
The concrete Swift type of the value written to `outValue` and the value pointed
115-
to by `hint` depend on the kind of record:
142+
The concrete Swift type of the value written to `outValue`, the type pointed to
143+
by `type`, and the value pointed to by `hint` depend on the kind of record:
116144

117145
- For test or suite declarations (kind `0x74657374`), the accessor produces an
118-
asynchronous Swift function that returns an instance of `Test`:
146+
asynchronous Swift function[^notAccessorSignature] that returns an instance of
147+
`Testing.Test`:
119148

120149
```swift
121150
@Sendable () async -> Test
122151
```
123152

124-
This signature is not the signature of `accessor`, but of the Swift function
125-
reference it writes to `outValue`. This level of indirection is necessary
126-
because loading a test or suite declaration is an asynchronous operation, but
127-
C functions cannot be `async`.
153+
[^notAccessorSignature]: This signature is not the signature of `accessor`,
154+
but of the Swift function reference it writes to `outValue`. This level of
155+
indirection is necessary because loading a test or suite declaration is an
156+
asynchronous operation, but C functions cannot be `async`.
128157

129158
Test content records of this kind do not specify a type for `hint`. Always
130159
pass `nil`.
131160

132161
- For exit test declarations (kind `0x65786974`), the accessor produces a
133-
structure describing the exit test (of type `__ExitTest`.)
162+
structure describing the exit test (of type `Testing.__ExitTest`.)
134163

135-
Test content records of this kind accept a `hint` of type `__ExitTest.ID`.
164+
Test content records of this kind accept a `hint` of type `Testing.__ExitTest.ID`.
136165
They only produce a result if they represent an exit test declared with the
137166
same ID (or if `hint` is `nil`.)
138167

139168
> [!WARNING]
140169
> Calling code should use [`withUnsafeTemporaryAllocation(of:capacity:_:)`](https://developer.apple.com/documentation/swift/withunsafetemporaryallocation(of:capacity:_:))
141-
> and [`withUnsafePointer(to:_:)`](https://developer.apple.com/documentation/swift/withunsafepointer(to:_:)-35wrn),
142-
> respectively, to ensure the pointers passed to `accessor` are large enough and
143-
> are well-aligned. If they are not large enough to contain values of the
170+
> and/or [`withUnsafePointer(to:_:)`](https://developer.apple.com/documentation/swift/withunsafepointer(to:_:)-35wrn)
171+
> to ensure the pointers passed to `accessor` are large enough and are
172+
> well-aligned. If they are not large enough to contain values of the
144173
> appropriate types (per above), or if `hint` points to uninitialized or
145174
> incorrectly-typed memory, the result is undefined.
146175

Sources/Testing/Discovery.swift

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,26 @@
1010

1111
private import _TestingInternals
1212

13+
/// The type of the accessor function used to access a test content record.
14+
///
15+
/// - Parameters:
16+
/// - outValue: A pointer to uninitialized memory large enough to contain the
17+
/// corresponding test content record's value.
18+
/// - type: A pointer to the expected type of `outValue`. Use `load(as:)` to
19+
/// get the Swift type, not `unsafeBitCast(_:to:)`.
20+
/// - hint: An optional pointer to a hint value.
21+
///
22+
/// - Returns: Whether or not `outValue` was initialized. The caller is
23+
/// responsible for deinitializing `outValue` if it was initialized.
24+
///
25+
/// - Warning: This type is used to implement the `@Test` macro. Do not use it
26+
/// directly.
27+
public typealias __TestContentRecordAccessor = @convention(c) (
28+
_ outValue: UnsafeMutableRawPointer,
29+
_ type: UnsafeRawPointer,
30+
_ hint: UnsafeRawPointer?
31+
) -> CBool
32+
1333
/// The content of a test content record.
1434
///
1535
/// - Parameters:
@@ -24,7 +44,7 @@ private import _TestingInternals
2444
public typealias __TestContentRecord = (
2545
kind: UInt32,
2646
reserved1: UInt32,
27-
accessor: (@convention(c) (_ outValue: UnsafeMutableRawPointer, _ hint: UnsafeRawPointer?) -> CBool)?,
47+
accessor: __TestContentRecordAccessor?,
2848
context: UInt,
2949
reserved2: UInt
3050
)
@@ -54,6 +74,20 @@ protocol TestContent: ~Copyable {
5474
/// By default, this type equals `Never`, indicating that this type of test
5575
/// content does not support hinting during discovery.
5676
associatedtype TestContentAccessorHint: Sendable = Never
77+
78+
/// The type to pass (by address) as the accessor function's `type` argument.
79+
///
80+
/// The default value of this property is `Self.self`. A conforming type can
81+
/// override the default implementation to substitute another type (e.g. if
82+
/// the conforming type is not public but records are created during macro
83+
/// expansion and can only reference public types.)
84+
static var testContentAccessorTypeArgument: any ~Copyable.Type { get }
85+
}
86+
87+
extension TestContent where Self: ~Copyable {
88+
static var testContentAccessorTypeArgument: any ~Copyable.Type {
89+
self
90+
}
5791
}
5892

5993
// MARK: - Individual test content records
@@ -108,18 +142,20 @@ struct TestContentRecord<T>: Sendable where T: TestContent & ~Copyable {
108142
return nil
109143
}
110144

111-
return withUnsafeTemporaryAllocation(of: T.self, capacity: 1) { buffer in
112-
let initialized = if let hint {
113-
withUnsafePointer(to: hint) { hint in
114-
accessor(buffer.baseAddress!, hint)
145+
return withUnsafePointer(to: T.testContentAccessorTypeArgument) { type in
146+
withUnsafeTemporaryAllocation(of: T.self, capacity: 1) { buffer in
147+
let initialized = if let hint {
148+
withUnsafePointer(to: hint) { hint in
149+
accessor(buffer.baseAddress!, type, hint)
150+
}
151+
} else {
152+
accessor(buffer.baseAddress!, type, nil)
115153
}
116-
} else {
117-
accessor(buffer.baseAddress!, nil)
118-
}
119-
guard initialized else {
120-
return nil
154+
guard initialized else {
155+
return nil
156+
}
157+
return buffer.baseAddress!.move()
121158
}
122-
return buffer.baseAddress!.move()
123159
}
124160
}
125161
}

Sources/Testing/Test+Discovery.swift

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,15 @@ extension Test {
2828
0x74657374
2929
}
3030

31+
static var testContentAccessorTypeArgument: any ~Copyable.Type {
32+
Generator.self
33+
}
34+
35+
/// The type of the actual (asynchronous) generator function.
36+
typealias Generator = @Sendable () async -> Test
37+
3138
/// The actual (asynchronous) accessor function.
32-
case generator(@Sendable () async -> Test)
39+
case generator(Generator)
3340
}
3441

3542
/// All available ``Test`` instances in the process, according to the runtime.

Tests/TestingTests/MiscellaneousTests.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,10 @@ struct MiscellaneousTests {
615615
private static let record: __TestContentRecord = (
616616
0xABCD1234,
617617
0,
618-
{ outValue, hint in
618+
{ outValue, type, hint in
619+
guard type.load(as: Any.Type.self) == DiscoverableTestContent.self else {
620+
return false
621+
}
619622
if let hint, hint.load(as: TestContentAccessorHint.self) != expectedHint {
620623
return false
621624
}

0 commit comments

Comments
 (0)