Skip to content

Commit 95b97a6

Browse files
authored
Remove Stubs.cpp and reimplement what it has in Swift. (#929)
This PR removes Stubs.cpp, which currently houses some thunks for functions that are conditionally unavailable in glibc, and replaces it with runtime function lookups in Swift. Is there potentially a one-time non-zero performance cost? Yes. Is that performance cost prohibitive given that the functions are only looked up once and then cached? No. These functions won't get called on Linux if `SWT_NO_DYNAMIC_LINKING` is defined but we don't currently support that combination anyway. Even if you're using a statically-linked Swift standard library, we'd expect Linux to still support calling `dlsym()`. ### 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 faaabba commit 95b97a6

File tree

5 files changed

+31
-70
lines changed

5 files changed

+31
-70
lines changed

Sources/Testing/ExitTests/SpawnProcess.swift

+17-4
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,18 @@ typealias ProcessID = HANDLE
2626
typealias ProcessID = Never
2727
#endif
2828

29+
#if os(Linux) && !SWT_NO_DYNAMIC_LINKING
30+
/// Close file descriptors above a given value when spawing a new process.
31+
///
32+
/// This symbol is provided because the underlying function was added to glibc
33+
/// relatively recently and may not be available on all targets. Checking
34+
/// `__GLIBC_PREREQ()` is insufficient because `_DEFAULT_SOURCE` may not be
35+
/// defined at the point spawn.h is first included.
36+
private let _posix_spawn_file_actions_addclosefrom_np = symbol(named: "posix_spawn_file_actions_addclosefrom_np").map {
37+
unsafeBitCast($0, to: (@convention(c) (UnsafeMutablePointer<posix_spawn_file_actions_t>, CInt) -> CInt).self)
38+
}
39+
#endif
40+
2941
/// Spawn a process and wait for it to terminate.
3042
///
3143
/// - Parameters:
@@ -141,17 +153,18 @@ func spawnExecutable(
141153
// Close all other file descriptors open in the parent.
142154
flags |= CShort(POSIX_SPAWN_CLOEXEC_DEFAULT)
143155
#elseif os(Linux)
156+
#if !SWT_NO_DYNAMIC_LINKING
144157
// This platform doesn't have POSIX_SPAWN_CLOEXEC_DEFAULT, but we can at
145158
// least close all file descriptors higher than the highest inherited one.
146159
// We are assuming here that the caller didn't set FD_CLOEXEC on any of
147160
// these file descriptors.
148-
_ = swt_posix_spawn_file_actions_addclosefrom_np(fileActions, highestFD + 1)
161+
_ = _posix_spawn_file_actions_addclosefrom_np?(fileActions, highestFD + 1)
162+
#endif
149163
#elseif os(FreeBSD)
150164
// Like Linux, this platform doesn't have POSIX_SPAWN_CLOEXEC_DEFAULT.
151165
// Unlike Linux, all non-EOL FreeBSD versions (≥13.1) support
152-
// `posix_spawn_file_actions_addclosefrom_np`. Therefore, we don't need
153-
// `swt_posix_spawn_file_actions_addclosefrom_np` to guard the
154-
// availability of this function.
166+
// `posix_spawn_file_actions_addclosefrom_np`, and FreeBSD does not use
167+
// glibc nor guard symbols behind `_DEFAULT_SOURCE`.
155168
_ = posix_spawn_file_actions_addclosefrom_np(fileActions, highestFD + 1)
156169
#elseif os(OpenBSD)
157170
// OpenBSD does not have posix_spawn_file_actions_addclosefrom_np().

Sources/Testing/ExitTests/WaitFor.swift

+14-1
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,17 @@ private nonisolated(unsafe) let _waitThreadNoChildrenCondition = {
9494
return result
9595
}()
9696

97+
#if os(Linux) && !SWT_NO_DYNAMIC_LINKING
98+
/// Set the name of the current thread.
99+
///
100+
/// This function declaration is provided because `pthread_setname_np()` is
101+
/// only declared if `_GNU_SOURCE` is set, but setting it causes build errors
102+
/// due to conflicts with Swift's Glibc module.
103+
private let _pthread_setname_np = symbol(named: "pthread_setname_np").map {
104+
unsafeBitCast($0, to: (@convention(c) (pthread_t, UnsafePointer<CChar>) -> CInt).self)
105+
}
106+
#endif
107+
97108
/// Create a waiter thread that is responsible for waiting for child processes
98109
/// to exit.
99110
private let _createWaitThread: Void = {
@@ -152,7 +163,9 @@ private let _createWaitThread: Void = {
152163
#if SWT_TARGET_OS_APPLE
153164
_ = pthread_setname_np("Swift Testing exit test monitor")
154165
#elseif os(Linux)
155-
_ = swt_pthread_setname_np(pthread_self(), "SWT ExT monitor")
166+
#if !SWT_NO_DYNAMIC_LINKING
167+
_ = _pthread_setname_np?(pthread_self(), "SWT ExT monitor")
168+
#endif
156169
#elseif os(FreeBSD)
157170
_ = pthread_set_name_np(pthread_self(), "SWT ex test monitor")
158171
#elseif os(OpenBSD)

Sources/_TestingInternals/CMakeLists.txt

-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ include(LibraryVersion)
1212
include(TargetTriple)
1313
add_library(_TestingInternals STATIC
1414
Discovery.cpp
15-
Stubs.cpp
1615
Versions.cpp
1716
WillThrow.cpp)
1817
target_include_directories(_TestingInternals PUBLIC

Sources/_TestingInternals/Stubs.cpp

-45
This file was deleted.

Sources/_TestingInternals/include/Stubs.h

-19
Original file line numberDiff line numberDiff line change
@@ -117,25 +117,6 @@ static char *_Nullable *_Null_unspecified swt_environ(void) {
117117
}
118118
#endif
119119

120-
#if defined(__linux__)
121-
/// Set the name of the current thread.
122-
///
123-
/// This function declaration is provided because `pthread_setname_np()` is
124-
/// only declared if `_GNU_SOURCE` is set, but setting it causes build errors
125-
/// due to conflicts with Swift's Glibc module.
126-
SWT_EXTERN int swt_pthread_setname_np(pthread_t thread, const char *name);
127-
#endif
128-
129-
#if defined(__GLIBC__)
130-
/// Close file descriptors above a given value when spawing a new process.
131-
///
132-
/// This symbol is provided because the underlying function was added to glibc
133-
/// relatively recently and may not be available on all targets. Checking
134-
/// `__GLIBC_PREREQ()` is insufficient because `_DEFAULT_SOURCE` may not be
135-
/// defined at the point spawn.h is first included.
136-
SWT_EXTERN int swt_posix_spawn_file_actions_addclosefrom_np(posix_spawn_file_actions_t *fileActions, int from);
137-
#endif
138-
139120
#if !defined(__ANDROID__)
140121
#if __has_include(<signal.h>) && defined(si_pid)
141122
/// Get the value of the `si_pid` field of a `siginfo_t` structure.

0 commit comments

Comments
 (0)