From fb54d9f25acf408dcbb9c107bcd88a595020a8e1 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 9 Sep 2024 11:55:37 -0400 Subject: [PATCH 1/3] Ask Foundation to capture `NSError`/`CFError` backtraces for us. On Apple platforms, when an error occurs in an Objective-C method or C function, convention is to return the error as an instance of `NSError`/`CFError` via an out-parameter. When that Objective-C method or C function is called by a Swift function, the Swift function detects the error, then effectively rethrows it, at which point our `swift_willThrow` hook is triggered. This can obscure the real origin of the error which may be many stack frames down from the point Swift takes over. This PR asks Foundation, via a relatively new internal function, to capture backtraces for instances of `NSError` and `CFError` at the point they are created. Then, when Swift Testing attempts to look up the backtrace for an error it has caught, if Foundation has generated one, then Swift Testing can substitute it in place of the one it generated in the `swift_willThrow` hook. Resolves rdar://114386243. --- .../Testing/SourceAttribution/Backtrace.swift | 39 ++++++++++++++++++- Tests/TestingTests/BacktraceTests.swift | 34 ++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/Sources/Testing/SourceAttribution/Backtrace.swift b/Sources/Testing/SourceAttribution/Backtrace.swift index 96b57c8bf..5db85fe1b 100644 --- a/Sources/Testing/SourceAttribution/Backtrace.swift +++ b/Sources/Testing/SourceAttribution/Backtrace.swift @@ -322,12 +322,39 @@ extension Backtrace { forward(errorType) } + /// Whether or not Foundation provides a function that triggers the capture of + /// backtaces when instances of `NSError` or `CFError` are created. + /// + /// A backtrace created by said function represents the point in execution + /// where the error was created by an Objective-C or C stack frame. For an + /// error thrown from Objective-C or C through Swift before being caught by + /// the testing library, that backtrace is closer to the point of failure than + /// the one that would be captured at the point `swift_willThrow()` is called. + /// + /// On non-Apple platforms, the value of this property is always `false`. + /// + /// - Note: The underlying Foundation function is called (if present) the + /// first time the value of this property is read. + static let isFoundationCaptureEnabled = { +#if SWT_TARGET_OS_APPLE && !SWT_NO_DYNAMIC_LINKING + let _CFErrorSetCallStackCaptureEnabled = symbol(named: "_CFErrorSetCallStackCaptureEnabled").map { + unsafeBitCast($0, to: (@convention(c) (UInt8) -> UInt8).self) + } + _ = _CFErrorSetCallStackCaptureEnabled?(1) + return _CFErrorSetCallStackCaptureEnabled != nil +#else + false +#endif + }() + /// The implementation of ``Backtrace/startCachingForThrownErrors()``, run /// only once. /// /// This value is named oddly so that it shows up clearly in symbolicated /// backtraces. private static let __SWIFT_TESTING_IS_CAPTURING_A_BACKTRACE_FOR_A_THROWN_ERROR__: Void = { + _ = isFoundationCaptureEnabled + _oldWillThrowHandler.withLock { oldWillThrowHandler in oldWillThrowHandler = swt_setWillThrowHandler { errorAddress in let backtrace = Backtrace.current() @@ -369,6 +396,9 @@ extension Backtrace { /// /// - Parameters: /// - error: The error for which a backtrace is needed. + /// - checkFoundation: Whether or not to check for a backtrace created by + /// Foundation with `_CFErrorSetCallStackCaptureEnabled()`. On non-Apple + /// platforms, this argument has no effect. /// /// If no backtrace information is available for the specified error, this /// initializer returns `nil`. To start capturing backtraces, call @@ -379,7 +409,14 @@ extension Backtrace { /// because doing so will cause Swift-native errors to be unboxed into /// existential containers with different addresses. @inline(never) - init?(forFirstThrowOf error: any Error) { + init?(forFirstThrowOf error: any Error, checkFoundation: Bool = true) { + if checkFoundation && Self.isFoundationCaptureEnabled, + let userInfo = error._userInfo as? [String: Any], + let addresses = userInfo["NSCallStackReturnAddresses"] as? [Address], !addresses.isEmpty { + self.init(addresses: addresses) + return + } + let entry = Self._errorMappingCache.withLock { cache in cache[.init(error)] } diff --git a/Tests/TestingTests/BacktraceTests.swift b/Tests/TestingTests/BacktraceTests.swift index 8b836de9d..055e8e501 100644 --- a/Tests/TestingTests/BacktraceTests.swift +++ b/Tests/TestingTests/BacktraceTests.swift @@ -95,8 +95,42 @@ struct BacktraceTests { await runner.run() } } + + @inline(never) + func throwNSError() throws { + let error = NSError(domain: "Oh no!", code: 123, userInfo: [:]) + throw error + } + + @inline(never) + func throwBacktracedRefCountedError() throws { + throw BacktracedRefCountedError() + } + + @Test("Thrown NSError has a different backtrace than we generated", .enabled(if: Backtrace.isFoundationCaptureEnabled)) + func foundationGeneratedNSError() throws { + do { + try throwNSError() + } catch { + let backtrace1 = Backtrace(forFirstThrowOf: error, checkFoundation: true) + let backtrace2 = Backtrace(forFirstThrowOf: error, checkFoundation: false) + #expect(backtrace1 != backtrace2) + } + + // Foundation won't capture backtraces for reference-counted errors that + // don't inherit from NSError (even though the existential error box itself + // is of an NSError subclass.) + do { + try throwBacktracedRefCountedError() + } catch { + let backtrace1 = Backtrace(forFirstThrowOf: error, checkFoundation: true) + let backtrace2 = Backtrace(forFirstThrowOf: error, checkFoundation: false) + #expect(backtrace1 == backtrace2) + } + } #endif + @Test("Backtrace.current() is populated") func currentBacktrace() { let backtrace = Backtrace.current() From 9a9dd8bb7a665be68797e9123bef1106f9d56060 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 9 Sep 2024 12:08:47 -0400 Subject: [PATCH 2/3] Fix typos --- Tests/TestingTests/BacktraceTests.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Tests/TestingTests/BacktraceTests.swift b/Tests/TestingTests/BacktraceTests.swift index 055e8e501..ac0fc98f5 100644 --- a/Tests/TestingTests/BacktraceTests.swift +++ b/Tests/TestingTests/BacktraceTests.swift @@ -108,7 +108,7 @@ struct BacktraceTests { } @Test("Thrown NSError has a different backtrace than we generated", .enabled(if: Backtrace.isFoundationCaptureEnabled)) - func foundationGeneratedNSError() throws { + func foundationGeneratedNSError() { do { try throwNSError() } catch { @@ -130,7 +130,6 @@ struct BacktraceTests { } #endif - @Test("Backtrace.current() is populated") func currentBacktrace() { let backtrace = Backtrace.current() From 773b3255ade32089d6d4746e0132d99c5ee3726f Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 9 Sep 2024 12:10:39 -0400 Subject: [PATCH 3/3] Use DarwinBoolean --- Sources/Testing/SourceAttribution/Backtrace.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Testing/SourceAttribution/Backtrace.swift b/Sources/Testing/SourceAttribution/Backtrace.swift index 5db85fe1b..8e1914790 100644 --- a/Sources/Testing/SourceAttribution/Backtrace.swift +++ b/Sources/Testing/SourceAttribution/Backtrace.swift @@ -338,9 +338,9 @@ extension Backtrace { static let isFoundationCaptureEnabled = { #if SWT_TARGET_OS_APPLE && !SWT_NO_DYNAMIC_LINKING let _CFErrorSetCallStackCaptureEnabled = symbol(named: "_CFErrorSetCallStackCaptureEnabled").map { - unsafeBitCast($0, to: (@convention(c) (UInt8) -> UInt8).self) + unsafeBitCast($0, to: (@convention(c) (DarwinBoolean) -> DarwinBoolean).self) } - _ = _CFErrorSetCallStackCaptureEnabled?(1) + _ = _CFErrorSetCallStackCaptureEnabled?(true) return _CFErrorSetCallStackCaptureEnabled != nil #else false