From b60f3c505e079833d1f2a5be3cd41ae3a2186576 Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Thu, 16 Jul 2026 16:16:13 +0200 Subject: [PATCH 01/20] fix: prevent SubClassFinder availability crash (#8152) --- CHANGELOG.md | 4 + HANDOFF-subclassfinder-fix.md | 98 ++++++++ .../iOS-Swift/App/Sources/AppDelegate.swift | 14 ++ Sources/SentryObjC/Public/SentryObjCOptions.h | 4 + .../Performance/SentrySubClassFinder.swift | 57 ++--- .../SentryDefaultObjCRuntimeWrapper.swift | 36 ++- .../Helper/SentryObjCRuntimeWrapper.swift | 5 + Sources/Swift/Options.swift | 6 + .../Helper/SentryTestObjCRuntimeWrapper.h | 6 + .../Helper/SentryTestObjCRuntimeWrapper.m | 11 + .../SentrySubClassFinderTests.swift | 232 ++++++++++++++++-- 11 files changed, 418 insertions(+), 55 deletions(-) create mode 100644 HANDOFF-subclassfinder-fix.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 2170ba838fb..f738217acf1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Only expose `experimental.dataCollection` APIs in SDK V10 (#8435) +### Fixes + +- Prevent crash on SDK start when the app binary contains classes that reference `@available`-gated APIs. `SentrySubClassFinder` now detects `UIViewController` subclasses without realizing unrelated classes, so it no longer triggers the Swift runtime crash for classes like SwiftUI gesture coordinators or `RoomPlan`/`ActivityKit` wrappers (#8152) + ## 9.22.0 ### Features diff --git a/HANDOFF-subclassfinder-fix.md b/HANDOFF-subclassfinder-fix.md new file mode 100644 index 00000000000..4356486a99d --- /dev/null +++ b/HANDOFF-subclassfinder-fix.md @@ -0,0 +1,98 @@ +# Handoff: Fix `SentrySubClassFinder` availability crash (GH #8152) + +> Temporary working note so this task can be picked up in a later session. Not meant to ship. +> Delete before opening the PR (or move relevant parts into the PR description). + +## TL;DR + +- **Issue:** [#8152](https://github.com/getsentry/sentry-cocoa/issues/8152) (related: [#3798](https://github.com/getsentry/sentry-cocoa/issues/3798), Swift bug [swiftlang/swift#72657](https://github.com/swiftlang/swift/issues/72657)). +- **Crash:** SDK crashes on start (UIViewController performance tracing) because `SentrySubClassFinder` calls `NSClassFromString` on every class in the app image. That **realizes** the class; realizing a Swift class whose metadata references an `@available`-gated newer-framework type forces Swift metadata completion and crashes with `EXC_BAD_ACCESS` in `swift_getSingletonMetadata` on OS versions below the framework's availability. Reproduces only on **real iOS 17.x (and older) devices**, not simulators. +- **Real-world crashers are NOT view controllers:** SwiftUI gesture `Coordinator: NSObject` (conforms under `UIGestureRecognizerRepresentable`, iOS 18+), `RoomPlan`/`ActivityKit` wrappers. +- **Old mitigation:** users manually set `options.swizzleClassNameExcludes` after discovering the crash. Goal of this work: **stop the crash by default**. +- **Fix (simple, pure Swift):** enumerate classes by reading the image's `__objc_classlist` Mach-O section (gives class **pointers**, NOT realized) instead of `objc_copyClassNamesForImage` + `NSClassFromString`. Then use the _existing_ `class_getSuperclass`-based `isClass(_:subClassOf:)` walk (already safe, never realizes). `NSClassFromString` is only called at swizzle time, on the main thread, for confirmed view controllers. +- **Status:** implemented, unit-tested (7/7 pass on iOS 18.6 sim), differential test added, user confirmed sample no longer crashes. **Committed on branch, not pushed to a PR yet.** No draft PR opened. + +## Root-cause chain + +`SentrySubClassFinder.actOnSubclassesOfViewController(inImage:)` (background queue): + +1. old: `objcRuntimeWrapper.copyClassNamesForImage` → class name C-strings (safe, no realization). +2. old: for each name → `NSClassFromString(name)` ← **THE CRASH** (realizes) → `isClass(subclassOf: UIViewController)` walk → keep name if VC. +3. main thread: `NSClassFromString(name)` again → hand `Class` to swizzle block. + +Only step 2's `NSClassFromString` was the problem. `class_getSuperclass` (used by `isClass`) was always safe. + +## The fix (what changed) + +New approach = same structure, but get class **pointers** without realizing: + +- Added `classes(forImage:) -> [AnyClass]` to `SentryObjCRuntimeWrapper` protocol. +- Default impl (`SentryDefaultObjCRuntimeWrapper`): `import MachO`; find the image via `_dyld_image_count`/`_dyld_get_image_name`; read `getsectiondata(header, "__DATA_CONST", "__objc_classlist", &size)` (fallback `"__DATA"`); `unsafeBitCast` each entry to `AnyClass`. These are dyld-bound but **not realized**. +- Finder: iterate `classes(forImage:)`, run the unchanged `isClass` superclass walk, read the name of matches via `class_getName` (does NOT realize, does NOT call `+initialize`), apply `swizzleClassNameExcludes`, collect names. Main-thread pass unchanged (`NSClassFromString` only on confirmed VCs). +- `swizzleClassNameExcludes` kept + doc-reframed as a _fallback_ for the rare residual case (a real VC subclass that itself crashes when realized at swizzle time — unavoidable since swizzling requires realization). + +### Files changed (10) + +- `Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift` — swap enumeration; net simpler. +- `Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift` — add `classes(forImage:)` to protocol. +- `Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift` — implement it (`import MachO`). +- `Sources/Swift/Options.swift` + `Sources/SentryObjC/Public/SentryObjCOptions.h` — reframe `swizzleClassNameExcludes` doc as fallback. +- `CHANGELOG.md` — `## Unreleased` → `### Fixes` entry (#8152). +- `Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift` — reworked to new seam + new differential test. +- `Tests/SentryTests/Helper/SentryTestObjCRuntimeWrapper.{h,m}` — added `classes` override block (parallels existing `classesNames`). +- `Samples/iOS-Swift/App/Sources/AppDelegate.swift` — **user's own** RoomPlan repro scaffolding; leave as-is (don't touch). + +## Why it's safe (verified, not assumed) + +Verified against **objc4 source** (`apple-oss-distributions/objc4` main) + empirical tests on Apple M5 Max (arm64): + +1. `class_getSuperclass` = `cls->getSuperclass()` — pure field read, **never realizes**, handles arm64e ptrauth internally (`objc-class.mm:802`). The only realizing/crashing call was `NSClassFromString`. +2. `objc_copyClassNamesForImage` returns `demangledName(false)`; `class_getName` returns the same demangled form. Neither realizes. So new names == old names by construction. +3. `class_getName` and the superclass walk do **not** trigger `+initialize`. A Swift `[AnyClass]` append does not either (holds metatype, no retain-message) — unlike ObjC `NSArray addObject:` which does. Tested empirically. +4. **Classes with a Swift-generic superclass** (e.g. `UIHostingController` subclass like the repro's `TestHorizontalGestureVC`) are **not in `__objc_classlist` at all** — AND `objc_copyClassNamesForImage` doesn't return them either (same section), even after instantiation. So switching enumeration loses **zero** coverage; those VCs were never image-enumerated (they're handled by the root-VC swizzle path). This is the key finding that made the complex approach unnecessary. +5. arm64e: no hand-rolled pointer stripping in the fix (we call `class_getSuperclass`, which authenticates correctly). App images are almost always plain arm64 anyway. + +### Differential/oracle evidence (ran locally, throwaway scripts in /tmp) + +- Enumeration equivalence: across **163 loaded images / 9,610 classes**, new (`__objc_classlist` + `class_getName`) == old (`objc_copyClassNamesForImage`) name set. **0 mismatches.** +- Superclass-walk safety: walked all 9,610 classes with `class_getSuperclass` — no crash, no hang, max chain depth 7, 0 runaway/cyclic chains. + +## Rejected over-engineered approach (do NOT resurrect unless needed) + +First attempt added a new C file `SentryViewControllerClassScanner.c` doing raw `vm_read`/`sentrycrashmem_copySafely` reads of `class_t.superclass`, `ptrauth_strip`, name-demangle matching, plus a `_SentryPrivate.h` import. Problems: hit the `scripts/check-sentrycrash-imports.sh` ratchet (86/86, "never increase"), and it was all unnecessary once we confirmed `class_getSuperclass` is realization-free. The SentryCrash module (`Sources/SentryCrash/Recording/Tools/SentryCrashObjC.c` etc.) does have battle-tested non-realizing class introspection if ever needed, but note its name-read path assumes a _realized_ class and its `isMemoryReadable` uses a non-thread-safe shared static buffer. + +## Tests + +- `SentrySubClassFinderTests.swift` reworked: fixture injects `[AnyClass]` via the new `classes` seam (replaced the `classesNames` string seam). Existing behavioral tests preserved (find VCs, honor excludes, ignore non-VC, wrong image, no `+initialize`). +- **New:** `testClassListEnumerationMatchesCopyClassNamesForImage` — iterates every loaded image, asserts new enumeration == `objc_copyClassNamesForImage` name set. Never calls `NSClassFromString` (safe on every OS). Permanent CI guard for the one thing that changed. +- Repro test `testActOnSubclassesOfViewController_WithAvailabilityGatedGestureClass` — real runtime, gated gesture `Coordinator` in binary, asserts no crash + a plain VC is found. +- Requires `@_spi(Private) @testable import Sentry` (the default wrapper is `@_spi(Private)`). + +### Verified passing + +`make test-ios FOR_AGENTS=true IOS_DEVICE_NAME="iPhone 16" IOS_SIMULATOR_OS=18.6 ONLY_TESTING=SentryTests/SentrySubClassFinderTests` +→ "Executed 7 tests, with 0 failures". The 2 key tests also confirmed by name (2/2 pass). + +## Environment / invocation pitfalls (bit me; save time later) + +- **Local branch was 7 commits behind `origin/main`**, which independently broke `SentryTestUtils/ClearTestState.swift` (`SentryAppStartMeasurementProvider.reset()` gated behind `#if SENTRY_TEST` didn't exist yet). **Merged `origin/main` into the branch** (local merge commit `25319bf65`, then this work) to fix. This is why the test target wouldn't compile at first. +- Test target only compiles under the **`Test` build configuration** (defines `SENTRY_TEST`). Running via XcodeBuildMCP `test_sim` with default `Debug` FAILS with "no member 'reset'". Use `make test-ios` (sets `-configuration Test`). +- Makefile hardcodes `--os 18.4 --device "iPhone 16 Pro"`; this machine only has iOS **18.6 / iPhone 16**. Override: `IOS_DEVICE_NAME="iPhone 16" IOS_SIMULATOR_OS=18.6`. +- `SWIFT_TREAT_WARNINGS_AS_ERRORS=YES` and `GCC_TREAT_WARNINGS_AS_ERRORS=YES`. E.g. `let x = NSClassFromString(...)` needs explicit `: AnyClass` or the build fails on the inference warning. +- SwiftPM `swift build --product Sentry` compiles the SDK but fails at `verify-emitted-module-interface` (unrelated `SentryHeaders` module-interface quirk on macOS standalone). Add `-Xswiftc -no-verify-emitted-module-interface` for a clean SDK compile check. +- `make build-ios`/`make test-ios` also honor `FOR_AGENTS=true` for reduced output; full logs land in `raw-*-output.log` (gitignored). + +## Remaining gaps / next steps + +1. **Real-device confirmation** on iOS 16/17 with the repro (gated non-VC like RoomPlan wrapper + gesture Coordinator) — user confirmed sample doesn't crash on their run; more devices/OSes + an arm64e-built app would fully close it. (Repro is device-only; CI sims can't reproduce.) +2. **Run the broader swizzling/perf tests** (`SentryUIViewControllerSwizzlingTests`) to be thorough — not yet run this session. +3. `make format` + `make analyze` full pass before PR (swiftlint + clang-format already pass on touched files). +4. **When opening the PR:** delete this handoff file; move summary into the PR body; check `git log main..HEAD` for a claudescope `Agent transcript:` line to include (per AGENTS.md). `feat`/`fix` needs a changelog entry (have it). One maintainer approval; `ready-to-merge` label for full CI. +5. Consider the other verification options not yet done: a written device-test checklist, and/or a safety fallback (if classlist read returns empty, fall back to old behavior for that image) as defense-in-depth. + +## Memory + +Persistent notes saved in this session's memory: + +- `subclassfinder-availability-crash-fix` (the fix + all findings) +- `sentrycrash-raw-objc-introspection` (the rejected-approach primitives, for reference) diff --git a/Samples/iOS-Swift/App/Sources/AppDelegate.swift b/Samples/iOS-Swift/App/Sources/AppDelegate.swift index 4c1acaa5963..c300b10fb75 100644 --- a/Samples/iOS-Swift/App/Sources/AppDelegate.swift +++ b/Samples/iOS-Swift/App/Sources/AppDelegate.swift @@ -1,3 +1,4 @@ +import RoomPlan import SentrySampleShared import UIKit @@ -47,6 +48,14 @@ class AppDelegate: UIResponder, UIApplicationDelegate { randomDistributionTimer?.invalidate() randomDistributionTimer = nil + + if #available(iOS 17.0, *) { + let room = RoomPlanWrapper() + print("[hello room: \(room)") + } else { + // Fallback on earlier versions + } + } // Workaround for 'Stored properties cannot be marked potentially unavailable with '@available'' @@ -77,3 +86,8 @@ class AppDelegate: UIResponder, UIApplicationDelegate { extension Notification.Name { static let apnsTokenReceived = Notification.Name("io.sentry.apns-token-received") } + +@available(iOS 17.0, *) +class RoomPlanWrapper { + private var finalResults: CapturedStructure? +} diff --git a/Sources/SentryObjC/Public/SentryObjCOptions.h b/Sources/SentryObjC/Public/SentryObjCOptions.h index 056a82ef8f5..159ae786aa0 100644 --- a/Sources/SentryObjC/Public/SentryObjCOptions.h +++ b/Sources/SentryObjC/Public/SentryObjCOptions.h @@ -362,6 +362,10 @@ NS_ASSUME_NONNULL_BEGIN /** * A set of class names to ignore for swizzling. * The SDK checks if a class name of a class to swizzle contains a class name of this set. + * @note The SDK discovers @c UIViewController subclasses to swizzle without realizing unrelated + * classes, so it no longer crashes by default for classes that reference @c \@available -gated + * APIs. Use this option only as a fallback for the rare case of a @c UIViewController subclass that + * itself crashes when realized on an unsupported OS version. * @note Default is an empty set. */ @property (nonatomic, copy) NSSet *swizzleClassNameExcludes; diff --git a/Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift b/Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift index 2ed9acddbe5..fa8d5987187 100644 --- a/Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift +++ b/Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift @@ -40,50 +40,43 @@ class SentrySubClassFinder: NSObject { return } - var count: UInt32 = 0 - guard let classes = self.objcRuntimeWrapper.copyClassNamesForImage(cImageName, &count) else { - return - } - - SentrySDKLog.debug("Found \(count) number of classes in image: \(imageName).") - - // Storing the actual classes in an NSArray would call initializer of the class, which we - // must avoid as we are on a background thread here and dealing with UIViewControllers, - // which assume they are running on the main thread. Therefore, we store the class name - // instead so we can search for the subclasses on a background thread. We can't use - // NSObject:isSubclassOfClass as not all classes in the runtime in classes inherit from - // NSObject and a call to isSubclassOfClass would call the initializer of the class, which - // we can't allow because of the problem with UIViewControllers mentioned above. - // - // Turns out the approach to search all the view controllers inside the app binary image is - // fast and we don't need to include this restriction that will cause confusion. - // In a project with 1000 classes (a big project), it took only ~3ms to check all classes. + // Get the image's classes from its objc class list. This gives us the classes without + // realizing them. We must not use NSClassFromString to find the subclasses, because it + // realizes the class, and realizing a class whose Swift metadata references an + // `@available`-gated newer-framework type crashes on older OS versions + // (https://github.com/getsentry/sentry-cocoa/issues/8152, + // https://github.com/swiftlang/swift/issues/72657). Walking the superclass chain with + // `class_getSuperclass` below doesn't realize any class, so it can't trigger that crash. + let classes = self.objcRuntimeWrapper.classes(forImage: cImageName) + + SentrySDKLog.debug("Found \(classes.count) number of classes in image: \(imageName).") + + // We keep the class names instead of the classes, so we can hand them to the swizzling on + // the main thread. Sending a message to a class (for example adding it to an NSArray + // retains it) calls its initializer, which we must avoid on this background thread as + // UIViewControllers assume they run on the main thread. Walking the superclass chain and + // reading the name with `class_getName` don't message the class. var classesToSwizzle: [String] = [] + for cls in classes { + guard self.isClass(cls, subClassOf: viewControllerClass) else { + continue + } - for i in 0.., _ outCount: UnsafeMutablePointer?) -> UnsafeMutablePointer>? { return objc_copyClassNamesForImage(image, outCount) } - + @_spi(Private) public func classGetImageName(_ cls: AnyClass) -> UnsafePointer? { return class_getImageName(cls) } + + @_spi(Private) + public func classes(forImage image: UnsafePointer) -> [AnyClass] { + let imageName = String(cString: image) + for index in 0..<_dyld_image_count() { + guard let cName = _dyld_get_image_name(index), String(cString: cName) == imageName else { + continue + } + guard let header = _dyld_get_image_header(index) else { + return [] + } + + // Read the class pointers from the image's `__objc_classlist` section (`__DATA_CONST` on + // modern binaries, `__DATA` on older ones). dyld binds these pointers at load time, but + // the classes aren't realized, so callers can inspect them without running class + // initialization. This is why `SentrySubClassFinder` uses this instead of NSClassFromString. + var size: UInt = 0 + let section = header.withMemoryRebound(to: mach_header_64.self, capacity: 1) { header in + getsectiondata(header, "__DATA_CONST", "__objc_classlist", &size) + ?? getsectiondata(header, "__DATA", "__objc_classlist", &size) + } + guard let section else { + return [] + } + + let count = Int(size) / MemoryLayout.size + return section.withMemoryRebound(to: UnsafeRawPointer.self, capacity: count) { classes in + (0.., _ outCount: UnsafeMutablePointer?) -> UnsafeMutablePointer>? @objc(class_getImageName:) func classGetImageName(_ cls: AnyClass) -> UnsafePointer? + /// Returns the classes defined in the given image by reading its `__objc_classlist` section. + /// The classes are not realized, so callers can inspect them (e.g. walk their superclass chain) + /// without triggering class initialization. + @objc(classesForImage:) + func classes(forImage image: UnsafePointer) -> [AnyClass] } // swiftlint:enable missing_docs diff --git a/Sources/Swift/Options.swift b/Sources/Swift/Options.swift index a8658bcbf63..98c7bf38dff 100644 --- a/Sources/Swift/Options.swift +++ b/Sources/Swift/Options.swift @@ -449,6 +449,12 @@ /// The SDK checks if a class name of a class to swizzle contains a class name of this array. /// For example, if you add MyUIViewController to this list, the SDK excludes the following classes /// from swizzling: YourApp.MyUIViewController, YourApp.MyUIViewControllerA, MyApp.MyUIViewController. + /// + /// The SDK discovers `UIViewController` subclasses to swizzle without realizing unrelated classes, + /// so it no longer crashes by default for classes that reference `@available`-gated APIs. Use this + /// option only as a fallback for the rare case of a `UIViewController` subclass that itself crashes + /// when realized on an unsupported OS version. + /// /// Default is an empty set. @objc public var swizzleClassNameExcludes: Set = [] diff --git a/Tests/SentryTests/Helper/SentryTestObjCRuntimeWrapper.h b/Tests/SentryTests/Helper/SentryTestObjCRuntimeWrapper.h index 37777713349..80f45eb78c1 100644 --- a/Tests/SentryTests/Helper/SentryTestObjCRuntimeWrapper.h +++ b/Tests/SentryTests/Helper/SentryTestObjCRuntimeWrapper.h @@ -15,6 +15,12 @@ @property (nullable, nonatomic, copy) NSArray *_Nullable (^classesNames) (NSArray *_Nullable); +/** + * Overrides the classes returned for an image. Receives the classes the real wrapper found and + * returns the classes to use. When @c nil, the real implementation is used unchanged. + */ +@property (nullable, nonatomic, copy) NSArray *_Nonnull (^classes)(NSArray *_Nonnull); + @property (nullable, nonatomic) const char *imageName; @end diff --git a/Tests/SentryTests/Helper/SentryTestObjCRuntimeWrapper.m b/Tests/SentryTests/Helper/SentryTestObjCRuntimeWrapper.m index 72e24d18576..e67c6d54dce 100644 --- a/Tests/SentryTests/Helper/SentryTestObjCRuntimeWrapper.m +++ b/Tests/SentryTests/Helper/SentryTestObjCRuntimeWrapper.m @@ -64,4 +64,15 @@ - (const char *)class_getImageName:(Class)cls return self.imageName; } +- (NSArray *)classesForImage:(const char *)image +{ + NSArray *result = [self.objcRuntimeWrapper classesForImage:image]; + + if (self.classes != nil) { + result = self.classes(result); + } + + return result; +} + @end diff --git a/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift b/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift index 1692952508b..336a6827f0f 100644 --- a/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift +++ b/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift @@ -1,25 +1,30 @@ @_spi(Private) import SentryTestUtils -@testable import Sentry +@_spi(Private) @testable import Sentry +import MachO import ObjectiveC import XCTest +#if canImport(SwiftUI) +import SwiftUI +#endif + #if os(iOS) || os(tvOS) class SentrySubClassFinderTests: XCTestCase { private class Fixture { lazy var runtimeWrapper: SentryTestObjCRuntimeWrapper = { let result = SentryTestObjCRuntimeWrapper() - result.classesNames = { _ in - return self.testClassesNames + result.classes = { _ in + return self.testClasses } return result }() let imageName: String - let testClassesNames = [NSStringFromClass(FirstViewController.self), - NSStringFromClass(SecondViewController.self), - NSStringFromClass(ViewControllerNumberThree.self), - NSStringFromClass(VCAnyNaming.self), - NSStringFromClass(FakeViewController.self)] + let testClasses: [AnyClass] = [FirstViewController.self, + SecondViewController.self, + ViewControllerNumberThree.self, + VCAnyNaming.self, + FakeViewController.self] init() { if let name = class_getImageName(FirstViewController.self) { imageName = String(cString: name, encoding: .utf8) ?? "" @@ -27,47 +32,48 @@ class SentrySubClassFinderTests: XCTestCase { imageName = "" } } - + func getSut(swizzleClassNameExcludes: Set = []) -> SentrySubClassFinder { return SentrySubClassFinder(dispatchQueue: TestSentryDispatchQueueWrapper(), objcRuntimeWrapper: runtimeWrapper, swizzleClassNameExcludes: swizzleClassNameExcludes) } } - + private var fixture: Fixture! - + override func setUp() { super.setUp() fixture = Fixture() } - + func testActOnSubclassesOfViewController() { assertActOnSubclassesOfViewController(expected: [FirstViewController.self, SecondViewController.self, ViewControllerNumberThree.self, VCAnyNaming.self]) } - + func testActOnSubclassesOfViewController_WithSwizzleClassNameExcludes() { assertActOnSubclassesOfViewController(expected: [SecondViewController.self, ViewControllerNumberThree.self], swizzleClassNameExcludes: ["FirstViewController", "VCAnyNaming"]) } - + func testActOnSubclassesOfViewController_NoViewController() { - fixture.runtimeWrapper.classesNames = { _ in [] } + fixture.runtimeWrapper.classes = { _ in [] } assertActOnSubclassesOfViewController(expected: []) } - + func testActOnSubclassesOfViewController_IgnoreFakeViewController() { - fixture.runtimeWrapper.classesNames = { _ in [NSStringFromClass(FakeViewController.self)] } + fixture.runtimeWrapper.classes = { _ in [FakeViewController.self] } assertActOnSubclassesOfViewController(expected: []) } - + func testActOnSubclassesOfViewController_WrongImage_NoViewController() { - fixture.runtimeWrapper.classesNames = nil + fixture.runtimeWrapper.classes = nil assertActOnSubclassesOfViewController(expected: [], imageName: "OtherImage") } - + func testGettingSubclasses_DoesNotCallInitializer() throws { let trackedClassName = try XCTUnwrap( SentryInitializeForGettingSubclassesCalled.registerDynamicClass()) - fixture.runtimeWrapper.classesNames = { _ in - return self.fixture.testClassesNames + [trackedClassName] + let trackedClass: AnyClass = try XCTUnwrap(NSClassFromString(trackedClassName)) + fixture.runtimeWrapper.classes = { _ in + return self.fixture.testClasses + [trackedClass] } assertActOnSubclassesOfViewController( @@ -80,6 +86,49 @@ class SentrySubClassFinderTests: XCTestCase { XCTAssertFalse(SentryInitializeForGettingSubclassesCalled.wasCalled()) } + + /// Differential test proving the one thing the fix changed — how classes are enumerated — is + /// equivalent to the old way, across every image loaded in the test process (thousands of real + /// classes from every linked framework), on whatever platform and architecture CI runs. + /// + /// The fix replaced `objc_copyClassNamesForImage` (which returns names) with reading the + /// `__objc_classlist` section (which returns class pointers). The subsequent subclass check + /// (`class_getSuperclass`) is unchanged. So the only new behavior to guard is that the class set + /// from `__objc_classlist` matches the class set from `objc_copyClassNamesForImage`. + /// + /// This test deliberately does NOT call `NSClassFromString`: that realizes classes, which is the + /// exact operation that crashes for availability-gated classes on older OS versions (GH-8152). + /// Comparing the two enumerations by name never realizes anything, so the test is safe on every + /// OS — including ones where the old path would have crashed. + func testClassListEnumerationMatchesCopyClassNamesForImage() throws { + let sut = SentryDefaultObjCRuntimeWrapper() + + var comparedImages = 0 + for index in 0..<_dyld_image_count() { + let imageName = try XCTUnwrap(_dyld_get_image_name(index).map { String(cString: $0) }) + + // New enumeration: class pointers from __objc_classlist, named via class_getName. + let fromClassList = Set(sut.classes(forImage: imageName).map { NSStringFromClass($0) }) + + // Old enumeration: names from objc_copyClassNamesForImage. + var fromCopyNames: Set = [] + imageName.withCString { cImageName in + var count: UInt32 = 0 + guard let names = objc_copyClassNamesForImage(cImageName, &count) else { return } + defer { free(UnsafeMutableRawPointer(mutating: names)) } + for i in 0.. = []) { assertActOnSubclassesOfViewController(expected: expected, imageName: fixture.imageName, swizzleClassNameExcludes: swizzleClassNameExcludes) @@ -120,3 +169,142 @@ class ViewControllerNumberThree: UIViewController {} class VCAnyNaming: UIViewController {} class FakeViewController {} #endif + +// MARK: - Availability-gated gesture crash reproduction +// +// Mirrors a real crash: an app defines a gesture conforming to +// UIGestureRecognizerRepresentable (iOS 18+) behind @available. +// The nested Coordinator (NSObject subclass) is registered in the ObjC runtime. +// Calling NSClassFromString on it realizes the class, and Swift metadata +// resolution triggers protocol conformance lookup for +// UIGestureRecognizerRepresentable. On iOS versions where the protocol doesn't +// exist, this crashes (GH-8152, swiftlang/swift#72657). +// +// The finder now avoids this by reading the image's class list and walking each +// class's superclass chain with class_getSuperclass, which doesn't realize any +// class. The Coordinator walks to NSObject (it's not a UIViewController), so it +// is never realized and never crashes. + +#if os(iOS) + +@available(iOS 26.0, *) +private struct TestHorizontalPanGesture: UIGestureRecognizerRepresentable { + var onChanged: ((CGSize) -> Void)? + var onEnded: ((CGSize) -> Void)? + + func makeUIGestureRecognizer(context: Context) -> UIPanGestureRecognizer { + let gesture = UIPanGestureRecognizer( + target: context.coordinator, + action: #selector(Coordinator.handlePan(_:)) + ) + gesture.maximumNumberOfTouches = 1 + gesture.cancelsTouchesInView = false + gesture.delegate = context.coordinator + return gesture + } + + func updateUIGestureRecognizer( + _ recognizer: UIPanGestureRecognizer, + context: Context + ) { + context.coordinator.onChanged = onChanged + context.coordinator.onEnded = onEnded + } + + func makeCoordinator(converter: CoordinateSpaceConverter) -> Coordinator { + Coordinator() + } + + func onChanged(_ action: @escaping (CGSize) -> Void) -> Self { + var copy = self + copy.onChanged = action + return copy + } + + func onEnded(_ action: @escaping (CGSize) -> Void) -> Self { + var copy = self + copy.onEnded = action + return copy + } + + final class Coordinator: NSObject, UIGestureRecognizerDelegate { + var onChanged: ((CGSize) -> Void)? + var onEnded: ((CGSize) -> Void)? + + func gestureRecognizerShouldBegin(_ gestureRecognizer: UIGestureRecognizer) -> Bool { + guard let panGesture = gestureRecognizer as? UIPanGestureRecognizer else { + return false + } + let velocity = panGesture.velocity(in: gestureRecognizer.view) + return abs(velocity.x) > abs(velocity.y) + } + + @objc func handlePan(_ gestureRecognizer: UIPanGestureRecognizer) { + let translation = gestureRecognizer.translation(in: gestureRecognizer.view) + let size = CGSize(width: translation.x, height: translation.y) + switch gestureRecognizer.state { + case .changed: + onChanged?(size) + case .ended, .cancelled, .failed: + onEnded?(size) + default: + break + } + } + } +} + +@available(iOS 26.0, *) +private struct TestGestureView: View { + var body: some View { + Text("Hello") + .gesture( + TestHorizontalPanGesture() + .onChanged { _ in } + .onEnded { _ in } + ) + } +} + +@available(iOS 26.0, *) +@objc(TestHorizontalGestureVC) +private class TestHorizontalGestureVC: UIHostingController {} + +extension SentrySubClassFinderTests { + + func testActOnSubclassesOfViewController_WithAvailabilityGatedGestureClass() { + // Use the real runtime wrapper (no injected classes) so the finder reads the actual + // __objc_classlist of the test binary, which contains TestHorizontalPanGesture.Coordinator, + // an @available(iOS 26.0, *) NSObject subclass whose parent type conforms to + // UIGestureRecognizerRepresentable. Realizing it (via NSClassFromString) would trigger Swift + // metadata resolution that can crash on iOS versions where the protocol doesn't exist. The + // finder must walk its superclass chain (reaching NSObject) without realizing it, so it's + // skipped as a non-view-controller and never crashes. + fixture.runtimeWrapper.classes = nil + + let expect = expectation(description: "SubClassFinder callback") + expect.assertForOverFulfill = false + + var foundClasses: [AnyClass] = [] + let sut = fixture.getSut() + sut.actOnSubclassesOfViewController(inImage: fixture.imageName) { subClass in + XCTAssertTrue(Thread.isMainThread) + foundClasses.append(subClass) + expect.fulfill() + } + + wait(for: [expect], timeout: 5) + + // The plain UIViewController subclasses in the test binary are still found, proving the scan + // completed without crashing on the availability-gated gesture types. + XCTAssertTrue(foundClasses.contains(where: { $0 == FirstViewController.self })) + + // Reference the gated view controller so its gesture types (including the Coordinator that + // triggers the crash) are compiled into the test binary and exercised by the scan above. + if #available(iOS 26.0, *) { + XCTAssertNotNil(TestHorizontalGestureVC.self) + } + } +} + +#endif From 81b9ddbe886a7cf39310e0707dc0e89d364bafdf Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Fri, 17 Jul 2026 06:29:21 +0200 Subject: [PATCH 02/20] fix: guard classes(forImage:) to iOS/tvOS/visionOS getsectiondata with mach_header_64 doesn't compile on 32-bit watchOS device slices (arm64_32, armv7k). Match the only caller, SentrySubClassFinder, which is iOS/tvOS/visionOS only. --- HANDOFF-subclassfinder-fix.md | 6 ++++++ .../Performance/SentryUIViewControllerSwizzling.swift | 3 ++- Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift | 5 +++++ Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift | 6 ++++++ Tests/SentryTests/Helper/SentryTestObjCRuntimeWrapper.m | 2 ++ 5 files changed, 21 insertions(+), 1 deletion(-) diff --git a/HANDOFF-subclassfinder-fix.md b/HANDOFF-subclassfinder-fix.md index 4356486a99d..1c2e4ffe3c6 100644 --- a/HANDOFF-subclassfinder-fix.md +++ b/HANDOFF-subclassfinder-fix.md @@ -82,6 +82,12 @@ First attempt added a new C file `SentryViewControllerClassScanner.c` doing raw - SwiftPM `swift build --product Sentry` compiles the SDK but fails at `verify-emitted-module-interface` (unrelated `SentryHeaders` module-interface quirk on macOS standalone). Add `-Xswiftc -no-verify-emitted-module-interface` for a clean SDK compile check. - `make build-ios`/`make test-ios` also honor `FOR_AGENTS=true` for reduced output; full logs land in `raw-*-output.log` (gitignored). +## Open investigations (before merge) + +- **Reuse `SentryBinaryImageCache` instead of scanning all dyld images?** `classes(forImage:)` currently loops `_dyld_image_count()` to find the header. `SentryBinaryImageInfo.address` IS the in-memory `mach_header` pointer (`SentryCrashDynamicLinker.c:414` `buffer->address = (uintptr_t)header`; confirmed by the remove-callback match `src->image.address == (uintptr_t)mh`). So we could pass that straight to `getsectiondata` and skip the loop. Caveats to resolve first: (1) cache is populated **async on a background thread** in prod (`dispatch_async` in `sentrycrashbic_startCache`), so it may be empty/incomplete when the finder runs at startup — need a fallback to the dyld scan; (2) cache is indexed **by address, not name** — no by-name lookup exists, would need `getAllBinaryImages()` linear scan or a new accessor. Decide if the win is worth the coupling. +- **Performance:** measure the actual cost of `classes(forImage:)` + the superclass walk vs the old path. Old finder comment claimed ~3ms for 1000 classes. Confirm the section read + walk isn't slower; profile on a real device. +- **Correctness / integration validation:** confirm the new enumeration swizzles the SAME set of view controllers as the old path, not just "doesn't crash." The differential unit test (`testClassListEnumerationMatchesCopyClassNamesForImage`) proves enumeration equivalence, but we want end-to-end integration coverage that the swizzled VC set matches. Do more integration testing before merge. + ## Remaining gaps / next steps 1. **Real-device confirmation** on iOS 16/17 with the repro (gated non-VC like RoomPlan wrapper + gesture Coordinator) — user confirmed sample doesn't crash on their run; more devices/OSes + an arm64e-built app would fully close it. (Repro is device-only; CI sims can't reproduce.) diff --git a/Sources/Swift/Core/Integrations/Performance/SentryUIViewControllerSwizzling.swift b/Sources/Swift/Core/Integrations/Performance/SentryUIViewControllerSwizzling.swift index 3b74dd9791e..2503ae80447 100644 --- a/Sources/Swift/Core/Integrations/Performance/SentryUIViewControllerSwizzling.swift +++ b/Sources/Swift/Core/Integrations/Performance/SentryUIViewControllerSwizzling.swift @@ -78,7 +78,8 @@ class SentryUIViewControllerSwizzling { // not to lose auto-generated transactions for the initial view controller. As we use // SentrySwizzleModeOncePerClassAndSuperclasses, we don't have to worry about swizzling // twice. We could also use objc_getClassList to lookup sub classes of UIViewController, but - // the lookup can take around 60ms, which is not acceptable. + // the lookup can take around 60ms, which is not acceptable. Furthermore, objc_getClassList + // realizes every class in the process. if !swizzleRootViewControllerFromUIApplication(app) { SentrySDKLog.debug("Failed to find root UIViewController from UIApplicationDelegate. Trying to use UISceneWillConnectNotification notification.") diff --git a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift index e12029700a0..2c3f59159f0 100644 --- a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift +++ b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift @@ -15,6 +15,10 @@ public final class SentryDefaultObjCRuntimeWrapper: NSObject, SentryObjCRuntimeW return class_getImageName(cls) } + // Only supported on iOS, tvOS, and visionOS, matching `SentrySubClassFinder`, its only caller. + // It reads `mach_header_64` via `getsectiondata`, which isn't available on 32-bit watchOS device + // slices (`arm64_32`, `armv7k`), so we don't build it there. +#if os(iOS) || os(tvOS) || os(visionOS) @_spi(Private) public func classes(forImage image: UnsafePointer) -> [AnyClass] { let imageName = String(cString: image) @@ -47,5 +51,6 @@ public final class SentryDefaultObjCRuntimeWrapper: NSObject, SentryObjCRuntimeW return [] } +#endif } // swiftlint:enable missing_docs diff --git a/Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift b/Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift index 411dcfaaece..34d3f838a9e 100644 --- a/Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift +++ b/Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift @@ -10,7 +10,13 @@ public protocol SentryObjCRuntimeWrapper { /// Returns the classes defined in the given image by reading its `__objc_classlist` section. /// The classes are not realized, so callers can inspect them (e.g. walk their superclass chain) /// without triggering class initialization. + /// + /// Only supported on iOS, tvOS, and visionOS, matching `SentrySubClassFinder`, its only caller. + /// It relies on `getsectiondata` with `mach_header_64`, which isn't available on 32-bit watchOS + /// device slices (`arm64_32`, `armv7k`). +#if os(iOS) || os(tvOS) || os(visionOS) @objc(classesForImage:) func classes(forImage image: UnsafePointer) -> [AnyClass] +#endif } // swiftlint:enable missing_docs diff --git a/Tests/SentryTests/Helper/SentryTestObjCRuntimeWrapper.m b/Tests/SentryTests/Helper/SentryTestObjCRuntimeWrapper.m index e67c6d54dce..a6916b17060 100644 --- a/Tests/SentryTests/Helper/SentryTestObjCRuntimeWrapper.m +++ b/Tests/SentryTests/Helper/SentryTestObjCRuntimeWrapper.m @@ -64,6 +64,7 @@ - (const char *)class_getImageName:(Class)cls return self.imageName; } +#if TARGET_OS_IOS || TARGET_OS_TV || TARGET_OS_VISION - (NSArray *)classesForImage:(const char *)image { NSArray *result = [self.objcRuntimeWrapper classesForImage:image]; @@ -74,5 +75,6 @@ - (const char *)class_getImageName:(Class)cls return result; } +#endif @end From 3d5efa0b229664a6733e2170ab0e23e6ccb4c2cc Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Fri, 17 Jul 2026 06:32:30 +0200 Subject: [PATCH 03/20] ditch changes --- .../SentrySubClassFinderTests.swift | 139 ------------------ 1 file changed, 139 deletions(-) diff --git a/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift b/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift index 336a6827f0f..5a4ca04279c 100644 --- a/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift +++ b/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift @@ -169,142 +169,3 @@ class ViewControllerNumberThree: UIViewController {} class VCAnyNaming: UIViewController {} class FakeViewController {} #endif - -// MARK: - Availability-gated gesture crash reproduction -// -// Mirrors a real crash: an app defines a gesture conforming to -// UIGestureRecognizerRepresentable (iOS 18+) behind @available. -// The nested Coordinator (NSObject subclass) is registered in the ObjC runtime. -// Calling NSClassFromString on it realizes the class, and Swift metadata -// resolution triggers protocol conformance lookup for -// UIGestureRecognizerRepresentable. On iOS versions where the protocol doesn't -// exist, this crashes (GH-8152, swiftlang/swift#72657). -// -// The finder now avoids this by reading the image's class list and walking each -// class's superclass chain with class_getSuperclass, which doesn't realize any -// class. The Coordinator walks to NSObject (it's not a UIViewController), so it -// is never realized and never crashes. - -#if os(iOS) - -@available(iOS 26.0, *) -private struct TestHorizontalPanGesture: UIGestureRecognizerRepresentable { - var onChanged: ((CGSize) -> Void)? - var onEnded: ((CGSize) -> Void)? - - func makeUIGestureRecognizer(context: Context) -> UIPanGestureRecognizer { - let gesture = UIPanGestureRecognizer( - target: context.coordinator, - action: #selector(Coordinator.handlePan(_:)) - ) - gesture.maximumNumberOfTouches = 1 - gesture.cancelsTouchesInView = false - gesture.delegate = context.coordinator - return gesture - } - - func updateUIGestureRecognizer( - _ recognizer: UIPanGestureRecognizer, - context: Context - ) { - context.coordinator.onChanged = onChanged - context.coordinator.onEnded = onEnded - } - - func makeCoordinator(converter: CoordinateSpaceConverter) -> Coordinator { - Coordinator() - } - - func onChanged(_ action: @escaping (CGSize) -> Void) -> Self { - var copy = self - copy.onChanged = action - return copy - } - - func onEnded(_ action: @escaping (CGSize) -> Void) -> Self { - var copy = self - copy.onEnded = action - return copy - } - - final class Coordinator: NSObject, UIGestureRecognizerDelegate { - var onChanged: ((CGSize) -> Void)? - var onEnded: ((CGSize) -> Void)? - - func gestureRecognizerShouldBegin(_ gestureRecognizer: UIGestureRecognizer) -> Bool { - guard let panGesture = gestureRecognizer as? UIPanGestureRecognizer else { - return false - } - let velocity = panGesture.velocity(in: gestureRecognizer.view) - return abs(velocity.x) > abs(velocity.y) - } - - @objc func handlePan(_ gestureRecognizer: UIPanGestureRecognizer) { - let translation = gestureRecognizer.translation(in: gestureRecognizer.view) - let size = CGSize(width: translation.x, height: translation.y) - switch gestureRecognizer.state { - case .changed: - onChanged?(size) - case .ended, .cancelled, .failed: - onEnded?(size) - default: - break - } - } - } -} - -@available(iOS 26.0, *) -private struct TestGestureView: View { - var body: some View { - Text("Hello") - .gesture( - TestHorizontalPanGesture() - .onChanged { _ in } - .onEnded { _ in } - ) - } -} - -@available(iOS 26.0, *) -@objc(TestHorizontalGestureVC) -private class TestHorizontalGestureVC: UIHostingController {} - -extension SentrySubClassFinderTests { - - func testActOnSubclassesOfViewController_WithAvailabilityGatedGestureClass() { - // Use the real runtime wrapper (no injected classes) so the finder reads the actual - // __objc_classlist of the test binary, which contains TestHorizontalPanGesture.Coordinator, - // an @available(iOS 26.0, *) NSObject subclass whose parent type conforms to - // UIGestureRecognizerRepresentable. Realizing it (via NSClassFromString) would trigger Swift - // metadata resolution that can crash on iOS versions where the protocol doesn't exist. The - // finder must walk its superclass chain (reaching NSObject) without realizing it, so it's - // skipped as a non-view-controller and never crashes. - fixture.runtimeWrapper.classes = nil - - let expect = expectation(description: "SubClassFinder callback") - expect.assertForOverFulfill = false - - var foundClasses: [AnyClass] = [] - let sut = fixture.getSut() - sut.actOnSubclassesOfViewController(inImage: fixture.imageName) { subClass in - XCTAssertTrue(Thread.isMainThread) - foundClasses.append(subClass) - expect.fulfill() - } - - wait(for: [expect], timeout: 5) - - // The plain UIViewController subclasses in the test binary are still found, proving the scan - // completed without crashing on the availability-gated gesture types. - XCTAssertTrue(foundClasses.contains(where: { $0 == FirstViewController.self })) - - // Reference the gated view controller so its gesture types (including the Coordinator that - // triggers the crash) are compiled into the test binary and exercised by the scan above. - if #available(iOS 26.0, *) { - XCTAssertNotNil(TestHorizontalGestureVC.self) - } - } -} - -#endif From 62b002c8d61eae48623633acf9dab3c26bb1acea Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Tue, 21 Jul 2026 05:28:53 +0200 Subject: [PATCH 04/20] fix: gate classes(forImage:) to 64-bit archs The section read binds to mach_header_64, so guard the conditional with arch(arm64) || arch(x86_64) to exclude 32-bit watchOS device slices (arm64_32, armv7k). Covers all slices iOS/tvOS/visionOS ship. --- .../Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift | 8 +++++--- Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift | 6 +++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift index 2c3f59159f0..6b871223005 100644 --- a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift +++ b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift @@ -16,9 +16,11 @@ public final class SentryDefaultObjCRuntimeWrapper: NSObject, SentryObjCRuntimeW } // Only supported on iOS, tvOS, and visionOS, matching `SentrySubClassFinder`, its only caller. - // It reads `mach_header_64` via `getsectiondata`, which isn't available on 32-bit watchOS device - // slices (`arm64_32`, `armv7k`), so we don't build it there. -#if os(iOS) || os(tvOS) || os(visionOS) + // It reads `mach_header_64` via `getsectiondata`, so we gate it to 64-bit architectures. This + // covers every slice these platforms ship (`arm64`/`arm64e` devices, `arm64`/`x86_64` + // simulators) and excludes the 32-bit watchOS device slices (`arm64_32`, `armv7k`), where the + // 64-bit header layout doesn't apply. +#if (os(iOS) || os(tvOS) || os(visionOS)) && (arch(arm64) || arch(x86_64)) @_spi(Private) public func classes(forImage image: UnsafePointer) -> [AnyClass] { let imageName = String(cString: image) diff --git a/Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift b/Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift index 34d3f838a9e..f2db5e1362a 100644 --- a/Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift +++ b/Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift @@ -12,9 +12,9 @@ public protocol SentryObjCRuntimeWrapper { /// without triggering class initialization. /// /// Only supported on iOS, tvOS, and visionOS, matching `SentrySubClassFinder`, its only caller. - /// It relies on `getsectiondata` with `mach_header_64`, which isn't available on 32-bit watchOS - /// device slices (`arm64_32`, `armv7k`). -#if os(iOS) || os(tvOS) || os(visionOS) + /// It relies on `getsectiondata` with `mach_header_64`, so it's gated to 64-bit architectures and + /// excludes the 32-bit watchOS device slices (`arm64_32`, `armv7k`). +#if (os(iOS) || os(tvOS) || os(visionOS)) && (arch(arm64) || arch(x86_64)) @objc(classesForImage:) func classes(forImage image: UnsafePointer) -> [AnyClass] #endif From b4755ae7cfd1f7e684ffc982f5f6396ed8706b39 Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Tue, 21 Jul 2026 06:20:11 +0200 Subject: [PATCH 05/20] fix: address SubClassFinder review feedback Guard classes(forImage:) with MH_MAGIC_64 before rebinding the header to mach_header_64, which also safely skips a FAT header instead of misreading it. Document that _dyld_get_image_header/_dyld_get_image_name take the dyld loader read lock, so the method must run off the main thread (its only caller already does). Drop the swizzleClassNameExcludes doc paragraph, remove the sample RoomPlan repro scaffolding, and add a regression test driving the real runtime wrapper against the test bundle. --- CHANGELOG.md | 2 +- HANDOFF-subclassfinder-fix.md | 256 ++++++++++++------ .../iOS-Swift/App/Sources/AppDelegate.swift | 14 - Sources/SentryObjC/Public/SentryObjCOptions.h | 4 - .../SentryDefaultObjCRuntimeWrapper.swift | 16 +- .../Helper/SentryObjCRuntimeWrapper.swift | 4 + Sources/Swift/Options.swift | 6 - .../SentrySubClassFinderTests.swift | 46 ++++ 8 files changed, 242 insertions(+), 106 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f738217acf1..0552c807ee5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ ### Fixes -- Prevent crash on SDK start when the app binary contains classes that reference `@available`-gated APIs. `SentrySubClassFinder` now detects `UIViewController` subclasses without realizing unrelated classes, so it no longer triggers the Swift runtime crash for classes like SwiftUI gesture coordinators or `RoomPlan`/`ActivityKit` wrappers (#8152) +- Prevent crash on SDK start when the app binary contains classes that reference `@available`-gated APIs. `SentrySubClassFinder` now detects `UIViewController` subclasses without realizing unrelated classes, so it no longer triggers the Swift runtime crash for classes like SwiftUI gesture coordinators or `RoomPlan`/`ActivityKit` wrappers (#8457) ## 9.22.0 diff --git a/HANDOFF-subclassfinder-fix.md b/HANDOFF-subclassfinder-fix.md index 1c2e4ffe3c6..1ee4b8e71e2 100644 --- a/HANDOFF-subclassfinder-fix.md +++ b/HANDOFF-subclassfinder-fix.md @@ -1,104 +1,202 @@ # Handoff: Fix `SentrySubClassFinder` availability crash (GH #8152) -> Temporary working note so this task can be picked up in a later session. Not meant to ship. -> Delete before opening the PR (or move relevant parts into the PR description). +> Working note for picking this up across sessions. Tracks the live PR, decisions, +> and the one deferred optimization. Not part of the shipped SDK. + +## Status + +- **PR:** [#8457](https://github.com/getsentry/sentry-cocoa/pull/8457), branch + `fix/subclassfinder-availability-crash`. Open as a **draft** (intentionally — not + yet marked ready-to-merge; double-checking first). +- **Reviews:** three rounds (philipphofmann, NinjaLikesCheez, itaybre). All raised + comments addressed in code + replies; see "Review feedback" below. +- **Tests:** `SentrySubClassFinderTests` pass on the local sim. Includes the + differential enumeration test and a new real-wrapper regression test. ## TL;DR -- **Issue:** [#8152](https://github.com/getsentry/sentry-cocoa/issues/8152) (related: [#3798](https://github.com/getsentry/sentry-cocoa/issues/3798), Swift bug [swiftlang/swift#72657](https://github.com/swiftlang/swift/issues/72657)). -- **Crash:** SDK crashes on start (UIViewController performance tracing) because `SentrySubClassFinder` calls `NSClassFromString` on every class in the app image. That **realizes** the class; realizing a Swift class whose metadata references an `@available`-gated newer-framework type forces Swift metadata completion and crashes with `EXC_BAD_ACCESS` in `swift_getSingletonMetadata` on OS versions below the framework's availability. Reproduces only on **real iOS 17.x (and older) devices**, not simulators. -- **Real-world crashers are NOT view controllers:** SwiftUI gesture `Coordinator: NSObject` (conforms under `UIGestureRecognizerRepresentable`, iOS 18+), `RoomPlan`/`ActivityKit` wrappers. -- **Old mitigation:** users manually set `options.swizzleClassNameExcludes` after discovering the crash. Goal of this work: **stop the crash by default**. -- **Fix (simple, pure Swift):** enumerate classes by reading the image's `__objc_classlist` Mach-O section (gives class **pointers**, NOT realized) instead of `objc_copyClassNamesForImage` + `NSClassFromString`. Then use the _existing_ `class_getSuperclass`-based `isClass(_:subClassOf:)` walk (already safe, never realizes). `NSClassFromString` is only called at swizzle time, on the main thread, for confirmed view controllers. -- **Status:** implemented, unit-tested (7/7 pass on iOS 18.6 sim), differential test added, user confirmed sample no longer crashes. **Committed on branch, not pushed to a PR yet.** No draft PR opened. +- **Issue:** [#8152](https://github.com/getsentry/sentry-cocoa/issues/8152) (related: + [#3798](https://github.com/getsentry/sentry-cocoa/issues/3798), Swift bug + [swiftlang/swift#72657](https://github.com/swiftlang/swift/issues/72657)). +- **Crash:** SDK crashes on start (UIViewController performance tracing) because + `SentrySubClassFinder` called `NSClassFromString` on every class in the app image. + That **realizes** the class; realizing a Swift class whose metadata references an + `@available`-gated newer-framework type forces Swift metadata completion and + crashes with `EXC_BAD_ACCESS` in `swift_getSingletonMetadata` on OS versions below + the framework's availability. Reproduces only on **real iOS 17.x (and older) + devices**, not simulators. +- **Real-world crashers are NOT view controllers:** SwiftUI gesture + `Coordinator: NSObject` (conforms under `UIGestureRecognizerRepresentable`, iOS + 18+), `RoomPlan`/`ActivityKit` wrappers. +- **Fix (pure Swift):** enumerate classes by reading the image's `__objc_classlist` + Mach-O section (gives class **pointers**, NOT realized) instead of + `objc_copyClassNamesForImage` + `NSClassFromString`. Then use the existing + `class_getSuperclass`-based `isClass(_:subClassOf:)` walk (already safe, never + realizes). `NSClassFromString` is only called at swizzle time, on the main thread, + for confirmed view controllers. ## Root-cause chain `SentrySubClassFinder.actOnSubclassesOfViewController(inImage:)` (background queue): -1. old: `objcRuntimeWrapper.copyClassNamesForImage` → class name C-strings (safe, no realization). -2. old: for each name → `NSClassFromString(name)` ← **THE CRASH** (realizes) → `isClass(subclassOf: UIViewController)` walk → keep name if VC. +1. old: `objcRuntimeWrapper.copyClassNamesForImage` → class name C-strings (safe). +2. old: for each name → `NSClassFromString(name)` ← **THE CRASH** (realizes) → + `isClass(subclassOf: UIViewController)` walk → keep name if VC. 3. main thread: `NSClassFromString(name)` again → hand `Class` to swizzle block. -Only step 2's `NSClassFromString` was the problem. `class_getSuperclass` (used by `isClass`) was always safe. +Only step 2's `NSClassFromString` was the problem. `class_getSuperclass` (used by +`isClass`) was always safe. ## The fix (what changed) -New approach = same structure, but get class **pointers** without realizing: +Same structure, but get class **pointers** without realizing: - Added `classes(forImage:) -> [AnyClass]` to `SentryObjCRuntimeWrapper` protocol. -- Default impl (`SentryDefaultObjCRuntimeWrapper`): `import MachO`; find the image via `_dyld_image_count`/`_dyld_get_image_name`; read `getsectiondata(header, "__DATA_CONST", "__objc_classlist", &size)` (fallback `"__DATA"`); `unsafeBitCast` each entry to `AnyClass`. These are dyld-bound but **not realized**. -- Finder: iterate `classes(forImage:)`, run the unchanged `isClass` superclass walk, read the name of matches via `class_getName` (does NOT realize, does NOT call `+initialize`), apply `swizzleClassNameExcludes`, collect names. Main-thread pass unchanged (`NSClassFromString` only on confirmed VCs). -- `swizzleClassNameExcludes` kept + doc-reframed as a _fallback_ for the rare residual case (a real VC subclass that itself crashes when realized at swizzle time — unavoidable since swizzling requires realization). - -### Files changed (10) - -- `Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift` — swap enumeration; net simpler. -- `Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift` — add `classes(forImage:)` to protocol. -- `Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift` — implement it (`import MachO`). -- `Sources/Swift/Options.swift` + `Sources/SentryObjC/Public/SentryObjCOptions.h` — reframe `swizzleClassNameExcludes` doc as fallback. -- `CHANGELOG.md` — `## Unreleased` → `### Fixes` entry (#8152). -- `Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift` — reworked to new seam + new differential test. -- `Tests/SentryTests/Helper/SentryTestObjCRuntimeWrapper.{h,m}` — added `classes` override block (parallels existing `classesNames`). -- `Samples/iOS-Swift/App/Sources/AppDelegate.swift` — **user's own** RoomPlan repro scaffolding; leave as-is (don't touch). +- Default impl (`SentryDefaultObjCRuntimeWrapper`): `import MachO`; find the image + via `_dyld_image_count`/`_dyld_get_image_name`; read + `getsectiondata(header, "__DATA_CONST", "__objc_classlist", &size)` (fallback + `"__DATA"`); `unsafeBitCast` each entry to `AnyClass`. These are dyld-bound but + **not realized**. +- Finder: iterate `classes(forImage:)`, run the unchanged `isClass` superclass walk, + read matches' names via `class_getName` (does NOT realize, does NOT call + `+initialize`), apply `swizzleClassNameExcludes`, collect names. Main-thread pass + unchanged (`NSClassFromString` only on confirmed VCs). + +### Files changed (PR #8457, vs origin/main) + +- `Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift` — swap + enumeration; net simpler. +- `Sources/Swift/Core/Integrations/Performance/SentryUIViewControllerSwizzling.swift` + — comment note that `objc_getClassList` realizes every class. +- `Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift` — add `classes(forImage:)` + to protocol (+ threading/arch doc). +- `Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift` — implement it + (`import MachO`), arch guard, `MH_MAGIC_64` guard, off-main-thread doc. +- `CHANGELOG.md` — `## Unreleased` → `### Fixes` entry (#8457). +- `Tests/.../SentrySubClassFinderTests.swift` — new seam + differential test + real- + wrapper regression test + gated `AvailabilityGatedNonViewController` fixture. +- `Tests/SentryTests/Helper/SentryTestObjCRuntimeWrapper.{h,m}` — added `classes` + override block (parallels existing `classesNames`). + +Note: `Sources/Swift/Options.swift` and `Sources/SentryObjC/Public/SentryObjCOptions.h` +are NOT changed for docs — an earlier draft added a `swizzleClassNameExcludes` +"fallback" paragraph that was removed per review; the option's doc is unchanged from +`main`. + +## Review feedback (all addressed) + +- **arch guard** (NinjaLikesCheez, `m`): gate the `#if` to 64-bit archs since we bind + to `mach_header_64`. Done: `(arch(arm64) || arch(x86_64))`. Used `x86_64` (not + `arm64e`) because Swift's `arch(arm64)` already matches `arm64e`, and `x86_64` is + needed for the Intel simulator. Commit `62b002c8d`. +- **magic check** (itaybre `l` → NinjaLikesCheez upgraded to `h`, tied to the FAT + thread): verify `header.pointee.magic == MH_MAGIC_64` before rebinding. Done. Also + answers FAT: a FAT container has `FAT_MAGIC`, fails the check, so we skip rather + than misread. No FAT parsing added — `_dyld_get_image_header` only returns thin, + in-memory, native-arch slices, so a FAT header can't reach this code. Mirrors the + repo's only precedent, `firstCmdAfterHeader` in `SentryCrashDynamicLinker.c`. +- **arm64e ptrauth** (itaybre, `m`): the `unsafeBitCast` just reinterprets a + dyld-bound classref (the same pointers the ObjC runtime stores in + `__objc_classlist`); we never hand-strip pointers, and the subsequent + `class_getSuperclass`/`class_getName` authenticate internally. App Store apps ship + plain arm64. A real arm64e device run is the definitive confirmation (device-only). +- **`unsafeBitcast` to `mach_header_64` to save the rebind** (NinjaLikesCheez, `l`): + kept `withMemoryRebound` for clarity. +- **dyld lock (new, self-found):** verified against Apple dyld source + (`DyldAPIs.cpp`): `_dyld_image_count()` does NOT lock, but + `_dyld_get_image_header`/`_dyld_get_image_name` acquire the dyld loader **read + lock** (`withLoadersReadLock`). So `classes(forImage:)` must run off the main + thread. Added doc comments on both the impl and the protocol. No behavior change: + the sole caller already runs it on a background queue via `dispatchAsync`. ## Why it's safe (verified, not assumed) -Verified against **objc4 source** (`apple-oss-distributions/objc4` main) + empirical tests on Apple M5 Max (arm64): - -1. `class_getSuperclass` = `cls->getSuperclass()` — pure field read, **never realizes**, handles arm64e ptrauth internally (`objc-class.mm:802`). The only realizing/crashing call was `NSClassFromString`. -2. `objc_copyClassNamesForImage` returns `demangledName(false)`; `class_getName` returns the same demangled form. Neither realizes. So new names == old names by construction. -3. `class_getName` and the superclass walk do **not** trigger `+initialize`. A Swift `[AnyClass]` append does not either (holds metatype, no retain-message) — unlike ObjC `NSArray addObject:` which does. Tested empirically. -4. **Classes with a Swift-generic superclass** (e.g. `UIHostingController` subclass like the repro's `TestHorizontalGestureVC`) are **not in `__objc_classlist` at all** — AND `objc_copyClassNamesForImage` doesn't return them either (same section), even after instantiation. So switching enumeration loses **zero** coverage; those VCs were never image-enumerated (they're handled by the root-VC swizzle path). This is the key finding that made the complex approach unnecessary. -5. arm64e: no hand-rolled pointer stripping in the fix (we call `class_getSuperclass`, which authenticates correctly). App images are almost always plain arm64 anyway. - -### Differential/oracle evidence (ran locally, throwaway scripts in /tmp) - -- Enumeration equivalence: across **163 loaded images / 9,610 classes**, new (`__objc_classlist` + `class_getName`) == old (`objc_copyClassNamesForImage`) name set. **0 mismatches.** -- Superclass-walk safety: walked all 9,610 classes with `class_getSuperclass` — no crash, no hang, max chain depth 7, 0 runaway/cyclic chains. - -## Rejected over-engineered approach (do NOT resurrect unless needed) - -First attempt added a new C file `SentryViewControllerClassScanner.c` doing raw `vm_read`/`sentrycrashmem_copySafely` reads of `class_t.superclass`, `ptrauth_strip`, name-demangle matching, plus a `_SentryPrivate.h` import. Problems: hit the `scripts/check-sentrycrash-imports.sh` ratchet (86/86, "never increase"), and it was all unnecessary once we confirmed `class_getSuperclass` is realization-free. The SentryCrash module (`Sources/SentryCrash/Recording/Tools/SentryCrashObjC.c` etc.) does have battle-tested non-realizing class introspection if ever needed, but note its name-read path assumes a _realized_ class and its `isMemoryReadable` uses a non-thread-safe shared static buffer. +Verified against **objc4 source** + empirical tests on Apple silicon (arm64): + +1. `class_getSuperclass` = `cls->getSuperclass()` — pure field read, **never + realizes**, handles arm64e ptrauth internally. The only realizing/crashing call + was `NSClassFromString`. +2. `objc_copyClassNamesForImage` returns `demangledName(false)`; `class_getName` + returns the same demangled form. Neither realizes. So new names == old names by + construction. +3. `class_getName` and the superclass walk do **not** trigger `+initialize`. A Swift + `[AnyClass]` append doesn't either (holds a metatype, no retain-message) — unlike + ObjC `NSArray addObject:`. Tested empirically. +4. **Classes with a Swift-generic superclass** (e.g. a `UIHostingController` + subclass) are **not in `__objc_classlist`** — and `objc_copyClassNamesForImage` + doesn't return them either (same section). So switching enumeration loses **zero** + coverage; those VCs were never image-enumerated (handled by the root-VC swizzle + path). Key finding that made a more complex approach unnecessary. +5. arm64e: no hand-rolled pointer stripping (we call `class_getSuperclass`, which + authenticates correctly). App images are almost always plain arm64 anyway. + +### Differential/oracle evidence + +- `testClassListEnumerationMatchesCopyClassNamesForImage` iterates every loaded image + and asserts the new (`__objc_classlist` + `class_getName`) name set equals the old + (`objc_copyClassNamesForImage`) set. Never calls `NSClassFromString`, so it's safe + on every OS. Permanent CI guard for the one thing that changed. +- Local throwaway oracle (earlier session): across ~163 images / ~9,610 classes, new + == old, 0 mismatches; superclass walk over all classes: no crash, max depth 7, 0 + cycles. ## Tests -- `SentrySubClassFinderTests.swift` reworked: fixture injects `[AnyClass]` via the new `classes` seam (replaced the `classesNames` string seam). Existing behavioral tests preserved (find VCs, honor excludes, ignore non-VC, wrong image, no `+initialize`). -- **New:** `testClassListEnumerationMatchesCopyClassNamesForImage` — iterates every loaded image, asserts new enumeration == `objc_copyClassNamesForImage` name set. Never calls `NSClassFromString` (safe on every OS). Permanent CI guard for the one thing that changed. -- Repro test `testActOnSubclassesOfViewController_WithAvailabilityGatedGestureClass` — real runtime, gated gesture `Coordinator` in binary, asserts no crash + a plain VC is found. -- Requires `@_spi(Private) @testable import Sentry` (the default wrapper is `@_spi(Private)`). +- Behavioral tests inject `[AnyClass]` via the new `classes` seam (find VCs, honor + excludes, ignore non-VC, wrong image, no `+initialize`). +- `testClassListEnumerationMatchesCopyClassNamesForImage` — differential enumeration + equivalence (above). +- `testActOnSubclassesOfViewController_withRealRuntimeWrapper_findsBundleViewControllers` + — drives the **real** `SentryDefaultObjCRuntimeWrapper` through the finder against + this bundle; asserts the bundle's VCs are found and non-VCs (incl. the gated + `AvailabilityGatedNonViewController`) are not. The actual `EXC_BAD_ACCESS` is + device/OS-version-specific and can't be reproduced on CI sims, so this guards the + enumeration + selection behavior, not the crash itself. +- Requires `@_spi(Private) @testable import Sentry` (the default wrapper is + `@_spi(Private)`). + +## Deferred / future work (NOT blocking this PR) + +- **Reuse `SentryBinaryImageCache` instead of scanning all dyld images.** + `classes(forImage:)` currently loops `_dyld_image_count()` to find the header. + `SentryBinaryImageInfo.address` IS the in-memory `mach_header` pointer + (`SentryCrashDynamicLinker.c:414` `buffer->address = (uintptr_t)header`), so we + could pass that straight to `getsectiondata` and skip the loop. **Caveats to + resolve first:** (1) the cache is populated **async on a background thread** in prod + (`dispatch_async` in `sentrycrashbic_startCache`), so it may be empty/incomplete + when the finder runs at startup — needs a fallback to the dyld scan; (2) it's + indexed **by address, not name** — no by-name lookup exists, would need a linear + scan of `getAllBinaryImages()` or a new accessor. Decide if the win is worth the + coupling. (User may tackle this later.) +- **On-device confirmation** on iOS 16/17 with the repro (gated non-VC like a RoomPlan + wrapper + gesture Coordinator), ideally an arm64e-built app. Device-only; CI sims + can't reproduce. +- **Perf** of the section read + superclass walk vs the old path on a real device. -### Verified passing - -`make test-ios FOR_AGENTS=true IOS_DEVICE_NAME="iPhone 16" IOS_SIMULATOR_OS=18.6 ONLY_TESTING=SentryTests/SentrySubClassFinderTests` -→ "Executed 7 tests, with 0 failures". The 2 key tests also confirmed by name (2/2 pass). - -## Environment / invocation pitfalls (bit me; save time later) - -- **Local branch was 7 commits behind `origin/main`**, which independently broke `SentryTestUtils/ClearTestState.swift` (`SentryAppStartMeasurementProvider.reset()` gated behind `#if SENTRY_TEST` didn't exist yet). **Merged `origin/main` into the branch** (local merge commit `25319bf65`, then this work) to fix. This is why the test target wouldn't compile at first. -- Test target only compiles under the **`Test` build configuration** (defines `SENTRY_TEST`). Running via XcodeBuildMCP `test_sim` with default `Debug` FAILS with "no member 'reset'". Use `make test-ios` (sets `-configuration Test`). -- Makefile hardcodes `--os 18.4 --device "iPhone 16 Pro"`; this machine only has iOS **18.6 / iPhone 16**. Override: `IOS_DEVICE_NAME="iPhone 16" IOS_SIMULATOR_OS=18.6`. -- `SWIFT_TREAT_WARNINGS_AS_ERRORS=YES` and `GCC_TREAT_WARNINGS_AS_ERRORS=YES`. E.g. `let x = NSClassFromString(...)` needs explicit `: AnyClass` or the build fails on the inference warning. -- SwiftPM `swift build --product Sentry` compiles the SDK but fails at `verify-emitted-module-interface` (unrelated `SentryHeaders` module-interface quirk on macOS standalone). Add `-Xswiftc -no-verify-emitted-module-interface` for a clean SDK compile check. -- `make build-ios`/`make test-ios` also honor `FOR_AGENTS=true` for reduced output; full logs land in `raw-*-output.log` (gitignored). - -## Open investigations (before merge) - -- **Reuse `SentryBinaryImageCache` instead of scanning all dyld images?** `classes(forImage:)` currently loops `_dyld_image_count()` to find the header. `SentryBinaryImageInfo.address` IS the in-memory `mach_header` pointer (`SentryCrashDynamicLinker.c:414` `buffer->address = (uintptr_t)header`; confirmed by the remove-callback match `src->image.address == (uintptr_t)mh`). So we could pass that straight to `getsectiondata` and skip the loop. Caveats to resolve first: (1) cache is populated **async on a background thread** in prod (`dispatch_async` in `sentrycrashbic_startCache`), so it may be empty/incomplete when the finder runs at startup — need a fallback to the dyld scan; (2) cache is indexed **by address, not name** — no by-name lookup exists, would need `getAllBinaryImages()` linear scan or a new accessor. Decide if the win is worth the coupling. -- **Performance:** measure the actual cost of `classes(forImage:)` + the superclass walk vs the old path. Old finder comment claimed ~3ms for 1000 classes. Confirm the section read + walk isn't slower; profile on a real device. -- **Correctness / integration validation:** confirm the new enumeration swizzles the SAME set of view controllers as the old path, not just "doesn't crash." The differential unit test (`testClassListEnumerationMatchesCopyClassNamesForImage`) proves enumeration equivalence, but we want end-to-end integration coverage that the swizzled VC set matches. Do more integration testing before merge. - -## Remaining gaps / next steps - -1. **Real-device confirmation** on iOS 16/17 with the repro (gated non-VC like RoomPlan wrapper + gesture Coordinator) — user confirmed sample doesn't crash on their run; more devices/OSes + an arm64e-built app would fully close it. (Repro is device-only; CI sims can't reproduce.) -2. **Run the broader swizzling/perf tests** (`SentryUIViewControllerSwizzlingTests`) to be thorough — not yet run this session. -3. `make format` + `make analyze` full pass before PR (swiftlint + clang-format already pass on touched files). -4. **When opening the PR:** delete this handoff file; move summary into the PR body; check `git log main..HEAD` for a claudescope `Agent transcript:` line to include (per AGENTS.md). `feat`/`fix` needs a changelog entry (have it). One maintainer approval; `ready-to-merge` label for full CI. -5. Consider the other verification options not yet done: a written device-test checklist, and/or a safety fallback (if classlist read returns empty, fall back to old behavior for that image) as defense-in-depth. - -## Memory - -Persistent notes saved in this session's memory: +## Rejected over-engineered approach (do NOT resurrect unless needed) -- `subclassfinder-availability-crash-fix` (the fix + all findings) -- `sentrycrash-raw-objc-introspection` (the rejected-approach primitives, for reference) +First attempt added a C file doing raw `vm_read`/`sentrycrashmem_copySafely` reads of +`class_t.superclass`, `ptrauth_strip`, name-demangle matching, plus a `_SentryPrivate.h` +import. It hit the `scripts/check-sentrycrash-imports.sh` ratchet and was unnecessary +once we confirmed `class_getSuperclass` is realization-free. The SentryCrash module has +battle-tested non-realizing class introspection if ever needed, but note its name-read +path assumes a _realized_ class and its `isMemoryReadable` uses a non-thread-safe shared +static buffer. + +## Environment / invocation pitfalls + +- Test target only compiles under the **`Test` build configuration** (defines + `SENTRY_TEST`). Use `make test-ios` (sets `-configuration Test`), not a bare + XcodeBuildMCP `test_sim` with `Debug`. +- This machine's sim: override `IOS_DEVICE_NAME="iPhone 17 Pro" IOS_SIMULATOR_OS=26.4`. +- `SWIFT_TREAT_WARNINGS_AS_ERRORS=YES` / `GCC_TREAT_WARNINGS_AS_ERRORS=YES`. +- `make build-ios`/`make test-ios` honor `FOR_AGENTS=true`; full logs in + `raw-*-output.log` (gitignored). + +## Before marking ready + +- `make format` + `make analyze` clean; `SentrySubClassFinderTests` green. +- `classes(forImage:)` is `@_spi(Private)` → not in `sdk_api.json` (confirmed absent); + no `make generate-public-api` needed. +- Confirm Danger passes (changelog references **#8457**, the PR number). +- Then flip draft → ready and add `ready-to-merge` for full CI. diff --git a/Samples/iOS-Swift/App/Sources/AppDelegate.swift b/Samples/iOS-Swift/App/Sources/AppDelegate.swift index c300b10fb75..4c1acaa5963 100644 --- a/Samples/iOS-Swift/App/Sources/AppDelegate.swift +++ b/Samples/iOS-Swift/App/Sources/AppDelegate.swift @@ -1,4 +1,3 @@ -import RoomPlan import SentrySampleShared import UIKit @@ -48,14 +47,6 @@ class AppDelegate: UIResponder, UIApplicationDelegate { randomDistributionTimer?.invalidate() randomDistributionTimer = nil - - if #available(iOS 17.0, *) { - let room = RoomPlanWrapper() - print("[hello room: \(room)") - } else { - // Fallback on earlier versions - } - } // Workaround for 'Stored properties cannot be marked potentially unavailable with '@available'' @@ -86,8 +77,3 @@ class AppDelegate: UIResponder, UIApplicationDelegate { extension Notification.Name { static let apnsTokenReceived = Notification.Name("io.sentry.apns-token-received") } - -@available(iOS 17.0, *) -class RoomPlanWrapper { - private var finalResults: CapturedStructure? -} diff --git a/Sources/SentryObjC/Public/SentryObjCOptions.h b/Sources/SentryObjC/Public/SentryObjCOptions.h index 159ae786aa0..056a82ef8f5 100644 --- a/Sources/SentryObjC/Public/SentryObjCOptions.h +++ b/Sources/SentryObjC/Public/SentryObjCOptions.h @@ -362,10 +362,6 @@ NS_ASSUME_NONNULL_BEGIN /** * A set of class names to ignore for swizzling. * The SDK checks if a class name of a class to swizzle contains a class name of this set. - * @note The SDK discovers @c UIViewController subclasses to swizzle without realizing unrelated - * classes, so it no longer crashes by default for classes that reference @c \@available -gated - * APIs. Use this option only as a fallback for the rare case of a @c UIViewController subclass that - * itself crashes when realized on an unsupported OS version. * @note Default is an empty set. */ @property (nonatomic, copy) NSSet *swizzleClassNameExcludes; diff --git a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift index 6b871223005..9d065c82129 100644 --- a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift +++ b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift @@ -20,6 +20,11 @@ public final class SentryDefaultObjCRuntimeWrapper: NSObject, SentryObjCRuntimeW // covers every slice these platforms ship (`arm64`/`arm64e` devices, `arm64`/`x86_64` // simulators) and excludes the 32-bit watchOS device slices (`arm64_32`, `armv7k`), where the // 64-bit header layout doesn't apply. + // + // Call this off the main thread. It walks all loaded images with `_dyld_get_image_header` and + // `_dyld_get_image_name`, both of which acquire the dyld loader read lock. That lock is + // contended by every image load/unload (for example `dlopen`), so calling this on the main + // thread risks blocking it. #if (os(iOS) || os(tvOS) || os(visionOS)) && (arch(arm64) || arch(x86_64)) @_spi(Private) public func classes(forImage image: UnsafePointer) -> [AnyClass] { @@ -37,8 +42,15 @@ public final class SentryDefaultObjCRuntimeWrapper: NSObject, SentryObjCRuntimeW // the classes aren't realized, so callers can inspect them without running class // initialization. This is why `SentrySubClassFinder` uses this instead of NSClassFromString. var size: UInt = 0 - let section = header.withMemoryRebound(to: mach_header_64.self, capacity: 1) { header in - getsectiondata(header, "__DATA_CONST", "__objc_classlist", &size) + let section = header.withMemoryRebound(to: mach_header_64.self, capacity: 1) { header -> UnsafeMutablePointer? in + // Only read the header as a `mach_header_64` if it actually is one. dyld hands us + // thin, native-arch, in-memory slices, so this holds in practice; the check is + // defensive. It also rejects a FAT header (`FAT_MAGIC`), which would otherwise be + // misread, so we skip such an image instead of going off into the weeds. + guard header.pointee.magic == MH_MAGIC_64 else { + return nil + } + return getsectiondata(header, "__DATA_CONST", "__objc_classlist", &size) ?? getsectiondata(header, "__DATA", "__objc_classlist", &size) } guard let section else { diff --git a/Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift b/Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift index f2db5e1362a..42ad211e502 100644 --- a/Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift +++ b/Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift @@ -11,6 +11,10 @@ public protocol SentryObjCRuntimeWrapper { /// The classes are not realized, so callers can inspect them (e.g. walk their superclass chain) /// without triggering class initialization. /// + /// Call this off the main thread. It walks all loaded images with `_dyld_get_image_header` and + /// `_dyld_get_image_name`, both of which acquire the dyld loader read lock that every image + /// load/unload contends, so calling it on the main thread risks blocking it. + /// /// Only supported on iOS, tvOS, and visionOS, matching `SentrySubClassFinder`, its only caller. /// It relies on `getsectiondata` with `mach_header_64`, so it's gated to 64-bit architectures and /// excludes the 32-bit watchOS device slices (`arm64_32`, `armv7k`). diff --git a/Sources/Swift/Options.swift b/Sources/Swift/Options.swift index 98c7bf38dff..a8658bcbf63 100644 --- a/Sources/Swift/Options.swift +++ b/Sources/Swift/Options.swift @@ -449,12 +449,6 @@ /// The SDK checks if a class name of a class to swizzle contains a class name of this array. /// For example, if you add MyUIViewController to this list, the SDK excludes the following classes /// from swizzling: YourApp.MyUIViewController, YourApp.MyUIViewControllerA, MyApp.MyUIViewController. - /// - /// The SDK discovers `UIViewController` subclasses to swizzle without realizing unrelated classes, - /// so it no longer crashes by default for classes that reference `@available`-gated APIs. Use this - /// option only as a fallback for the rare case of a `UIViewController` subclass that itself crashes - /// when realized on an unsupported OS version. - /// /// Default is an empty set. @objc public var swizzleClassNameExcludes: Set = [] diff --git a/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift b/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift index 5a4ca04279c..ecbcd3f2641 100644 --- a/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift +++ b/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift @@ -130,6 +130,46 @@ class SentrySubClassFinderTests: XCTestCase { XCTAssertGreaterThan(comparedImages, 0, "Expected at least one image with classes") } + /// End-to-end regression test for GH-8152. The other tests inject a fake class list through the + /// stub wrapper; this one drives the REAL `SentryDefaultObjCRuntimeWrapper` through the finder + /// against this test bundle's image. It proves the production enumeration path — reading the + /// `__objc_classlist` section and walking `class_getSuperclass` — discovers the bundle's + /// `UIViewController` subclasses and ignores everything else, end to end. + /// + /// `AvailabilityGatedNonViewController` is an `@available`-gated `NSObject` subclass compiled + /// into the same image, standing in for the real-world crashers (SwiftUI gesture coordinators, + /// `RoomPlan`/`ActivityKit` wrappers). The old path realized every class here via + /// `NSClassFromString`; realizing such a gated class on an OS older than its availability is + /// what crashed in Swift metadata completion. The new path never realizes classes during + /// discovery, so it can't trigger that. The actual `EXC_BAD_ACCESS` is device- and + /// OS-version-specific and cannot be reproduced on CI simulators, so this test guards the + /// enumeration and selection behavior rather than the crash itself. + func testRealRuntimeWrapper_whenReadingBundleImage_findsBundleViewControllers() throws { + let realWrapper = SentryDefaultObjCRuntimeWrapper() + let imageName = try XCTUnwrap( + class_getImageName(FirstViewController.self).map { String(cString: $0) }) + + let sut = SentrySubClassFinder( + dispatchQueue: TestSentryDispatchQueueWrapper(), + objcRuntimeWrapper: realWrapper, + swizzleClassNameExcludes: []) + + var found: [AnyClass] = [] + sut.actOnSubclassesOfViewController(inImage: imageName) { found.append($0) } + + for expected in [FirstViewController.self, SecondViewController.self, + ViewControllerNumberThree.self, VCAnyNaming.self] { + XCTAssertTrue(found.contains { $0 == expected }, + "Expected \(expected) to be discovered via the real enumeration path") + } + XCTAssertFalse(found.contains { $0 == FakeViewController.self }, + "Non-UIViewController must not be discovered") + if #available(iOS 17.0, tvOS 17.0, *) { + XCTAssertFalse(found.contains { $0 == AvailabilityGatedNonViewController.self }, + "Availability-gated non-UIViewController must not be discovered") + } + } + private func assertActOnSubclassesOfViewController(expected: [AnyClass], swizzleClassNameExcludes: Set = []) { assertActOnSubclassesOfViewController(expected: expected, imageName: fixture.imageName, swizzleClassNameExcludes: swizzleClassNameExcludes) } @@ -168,4 +208,10 @@ class SecondViewController: UIViewController {} class ViewControllerNumberThree: UIViewController {} class VCAnyNaming: UIViewController {} class FakeViewController {} + +/// An `@available`-gated `NSObject` subclass, standing in for the real-world classes that crashed +/// GH-8152 (SwiftUI gesture coordinators, `RoomPlan`/`ActivityKit` wrappers). It is compiled into +/// the test image's `__objc_classlist` so the enumeration must encounter it without realizing it. +@available(iOS 17.0, tvOS 17.0, *) +class AvailabilityGatedNonViewController: NSObject {} #endif From 915421707ce4506bcb4f23bc8996f607737b01f3 Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Tue, 21 Jul 2026 06:33:46 +0200 Subject: [PATCH 06/20] ref: tidy classes(forImage:) comments and image match Lead with the off-main-thread requirement, drop caller references from the doc comments, and compare the image name with strcmp instead of round-tripping the C string through String. --- .../SentryDefaultObjCRuntimeWrapper.swift | 21 ++++++++----------- .../Helper/SentryObjCRuntimeWrapper.swift | 14 ++++++------- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift index 9d065c82129..8ba4bcf73ba 100644 --- a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift +++ b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift @@ -15,22 +15,19 @@ public final class SentryDefaultObjCRuntimeWrapper: NSObject, SentryObjCRuntimeW return class_getImageName(cls) } - // Only supported on iOS, tvOS, and visionOS, matching `SentrySubClassFinder`, its only caller. - // It reads `mach_header_64` via `getsectiondata`, so we gate it to 64-bit architectures. This - // covers every slice these platforms ship (`arm64`/`arm64e` devices, `arm64`/`x86_64` - // simulators) and excludes the 32-bit watchOS device slices (`arm64_32`, `armv7k`), where the - // 64-bit header layout doesn't apply. + // IMPORTANT: Call this off the main thread. `_dyld_get_image_header` and `_dyld_get_image_name` + // acquire the dyld loader read lock, which every image load/unload (for example `dlopen`) + // contends, so calling this on the main thread risks blocking it. // - // Call this off the main thread. It walks all loaded images with `_dyld_get_image_header` and - // `_dyld_get_image_name`, both of which acquire the dyld loader read lock. That lock is - // contended by every image load/unload (for example `dlopen`), so calling this on the main - // thread risks blocking it. + // Only supported on iOS, tvOS, and visionOS. It reads `mach_header_64` via `getsectiondata`, so + // we gate it to 64-bit architectures: every slice these platforms ship (`arm64`/`arm64e` + // devices, `arm64`/`x86_64` simulators), excluding the 32-bit watchOS device slices + // (`arm64_32`, `armv7k`) where the 64-bit header layout doesn't apply. #if (os(iOS) || os(tvOS) || os(visionOS)) && (arch(arm64) || arch(x86_64)) @_spi(Private) public func classes(forImage image: UnsafePointer) -> [AnyClass] { - let imageName = String(cString: image) for index in 0..<_dyld_image_count() { - guard let cName = _dyld_get_image_name(index), String(cString: cName) == imageName else { + guard let cName = _dyld_get_image_name(index), strcmp(cName, image) == 0 else { continue } guard let header = _dyld_get_image_header(index) else { @@ -40,7 +37,7 @@ public final class SentryDefaultObjCRuntimeWrapper: NSObject, SentryObjCRuntimeW // Read the class pointers from the image's `__objc_classlist` section (`__DATA_CONST` on // modern binaries, `__DATA` on older ones). dyld binds these pointers at load time, but // the classes aren't realized, so callers can inspect them without running class - // initialization. This is why `SentrySubClassFinder` uses this instead of NSClassFromString. + // initialization, unlike `NSClassFromString`. var size: UInt = 0 let section = header.withMemoryRebound(to: mach_header_64.self, capacity: 1) { header -> UnsafeMutablePointer? in // Only read the header as a `mach_header_64` if it actually is one. dyld hands us diff --git a/Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift b/Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift index 42ad211e502..f5279ffe21e 100644 --- a/Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift +++ b/Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift @@ -7,17 +7,17 @@ public protocol SentryObjCRuntimeWrapper { func copyClassNamesForImage(_ image: UnsafePointer, _ outCount: UnsafeMutablePointer?) -> UnsafeMutablePointer>? @objc(class_getImageName:) func classGetImageName(_ cls: AnyClass) -> UnsafePointer? + /// IMPORTANT: Call this off the main thread. `_dyld_get_image_header` and `_dyld_get_image_name` + /// acquire the dyld loader read lock that every image load/unload contends, so calling it on the + /// main thread risks blocking it. + /// /// Returns the classes defined in the given image by reading its `__objc_classlist` section. /// The classes are not realized, so callers can inspect them (e.g. walk their superclass chain) /// without triggering class initialization. /// - /// Call this off the main thread. It walks all loaded images with `_dyld_get_image_header` and - /// `_dyld_get_image_name`, both of which acquire the dyld loader read lock that every image - /// load/unload contends, so calling it on the main thread risks blocking it. - /// - /// Only supported on iOS, tvOS, and visionOS, matching `SentrySubClassFinder`, its only caller. - /// It relies on `getsectiondata` with `mach_header_64`, so it's gated to 64-bit architectures and - /// excludes the 32-bit watchOS device slices (`arm64_32`, `armv7k`). + /// Only supported on iOS, tvOS, and visionOS. It relies on `getsectiondata` with `mach_header_64`, + /// so it's gated to 64-bit architectures and excludes the 32-bit watchOS device slices + /// (`arm64_32`, `armv7k`). #if (os(iOS) || os(tvOS) || os(visionOS)) && (arch(arm64) || arch(x86_64)) @objc(classesForImage:) func classes(forImage image: UnsafePointer) -> [AnyClass] From 2c936e5e9db218d41e70186461390b499f6946c0 Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Tue, 21 Jul 2026 06:36:11 +0200 Subject: [PATCH 07/20] ref: hoist magic check out of rebind closure `magic` is the first field of both `mach_header` and `mach_header_64`, so read it on the original pointer and return [] directly instead of funneling nil through the withMemoryRebound closure. --- .../SentryDefaultObjCRuntimeWrapper.swift | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift index 8ba4bcf73ba..03923e82bd4 100644 --- a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift +++ b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift @@ -34,20 +34,22 @@ public final class SentryDefaultObjCRuntimeWrapper: NSObject, SentryObjCRuntimeW return [] } + // Only treat the header as a `mach_header_64` if it actually is one. dyld hands us thin, + // native-arch, in-memory slices, so this holds in practice; the check is defensive. It + // also rejects a FAT header (`FAT_MAGIC`), which would otherwise be misread, so we skip + // such an image instead of going off into the weeds. `magic` is the first field of both + // `mach_header` and `mach_header_64`, so we can read it before rebinding. + guard header.pointee.magic == MH_MAGIC_64 else { + return [] + } + // Read the class pointers from the image's `__objc_classlist` section (`__DATA_CONST` on // modern binaries, `__DATA` on older ones). dyld binds these pointers at load time, but // the classes aren't realized, so callers can inspect them without running class // initialization, unlike `NSClassFromString`. var size: UInt = 0 - let section = header.withMemoryRebound(to: mach_header_64.self, capacity: 1) { header -> UnsafeMutablePointer? in - // Only read the header as a `mach_header_64` if it actually is one. dyld hands us - // thin, native-arch, in-memory slices, so this holds in practice; the check is - // defensive. It also rejects a FAT header (`FAT_MAGIC`), which would otherwise be - // misread, so we skip such an image instead of going off into the weeds. - guard header.pointee.magic == MH_MAGIC_64 else { - return nil - } - return getsectiondata(header, "__DATA_CONST", "__objc_classlist", &size) + let section = header.withMemoryRebound(to: mach_header_64.self, capacity: 1) { header in + getsectiondata(header, "__DATA_CONST", "__objc_classlist", &size) ?? getsectiondata(header, "__DATA", "__objc_classlist", &size) } guard let section else { From 261743f3fb97688be17c67b33765828888d12d22 Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Tue, 21 Jul 2026 08:54:49 +0200 Subject: [PATCH 08/20] docs: note why image count is read once Explain we don't re-read _dyld_image_count per iteration like the CxaThrowSwapper, since we search for one already-loaded image. --- Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift index 03923e82bd4..0930810e321 100644 --- a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift +++ b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift @@ -26,6 +26,9 @@ public final class SentryDefaultObjCRuntimeWrapper: NSObject, SentryObjCRuntimeW #if (os(iOS) || os(tvOS) || os(visionOS)) && (arch(arm64) || arch(x86_64)) @_spi(Private) public func classes(forImage image: UnsafePointer) -> [AnyClass] { + // We read `_dyld_image_count` once instead of per iteration (unlike the CxaThrowSwapper): we + // search for one already-loaded image, so images added while iterating aren't our target, and + // stale indices from an unload just return nil below. for index in 0..<_dyld_image_count() { guard let cName = _dyld_get_image_name(index), strcmp(cName, image) == 0 else { continue From a0fc0db00ee7f4634f3f403d5846a738e8fcdd86 Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Tue, 21 Jul 2026 09:33:19 +0200 Subject: [PATCH 09/20] test: drop unused SwiftUI import in SubClassFinder tests SwiftUI was only referenced in comments, never in code. --- .../Integrations/Performance/SentrySubClassFinderTests.swift | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift b/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift index ecbcd3f2641..22ba7af9a93 100644 --- a/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift +++ b/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift @@ -4,10 +4,6 @@ import MachO import ObjectiveC import XCTest -#if canImport(SwiftUI) -import SwiftUI -#endif - #if os(iOS) || os(tvOS) class SentrySubClassFinderTests: XCTestCase { From 521b5915a1655887f90b1f618e2b1c36d7a03cad Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Tue, 21 Jul 2026 09:36:29 +0200 Subject: [PATCH 10/20] ref: log skipped images in classes(forImage:) The defensive early returns (missing header, non-mach_header_64, absent __objc_classlist, image not loaded) were silent, so a class list coming back empty was hard to triage. Add short log lines. --- Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift index 0930810e321..fd253300e41 100644 --- a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift +++ b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift @@ -34,6 +34,7 @@ public final class SentryDefaultObjCRuntimeWrapper: NSObject, SentryObjCRuntimeW continue } guard let header = _dyld_get_image_header(index) else { + SentrySDKLog.warning("No header for image: \(String(cString: image)). Skipping class list.") return [] } @@ -43,6 +44,7 @@ public final class SentryDefaultObjCRuntimeWrapper: NSObject, SentryObjCRuntimeW // such an image instead of going off into the weeds. `magic` is the first field of both // `mach_header` and `mach_header_64`, so we can read it before rebinding. guard header.pointee.magic == MH_MAGIC_64 else { + SentrySDKLog.warning("Header for image: \(String(cString: image)) isn't a mach_header_64. Skipping class list.") return [] } @@ -56,6 +58,7 @@ public final class SentryDefaultObjCRuntimeWrapper: NSObject, SentryObjCRuntimeW ?? getsectiondata(header, "__DATA", "__objc_classlist", &size) } guard let section else { + SentrySDKLog.debug("No __objc_classlist section for image: \(String(cString: image)).") return [] } @@ -65,6 +68,7 @@ public final class SentryDefaultObjCRuntimeWrapper: NSObject, SentryObjCRuntimeW } } + SentrySDKLog.debug("Image not found in loaded images: \(String(cString: image)).") return [] } #endif From 3a199bb30b41592076d1e2b716927739620b3ffd Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Tue, 21 Jul 2026 09:36:36 +0200 Subject: [PATCH 11/20] test: rename diff test and apply arrange-act-assert Follow the test_when_should naming convention and the arrange-act-assert layout. --- .../Performance/SentrySubClassFinderTests.swift | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift b/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift index 22ba7af9a93..d8635aae3fe 100644 --- a/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift +++ b/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift @@ -96,13 +96,15 @@ class SentrySubClassFinderTests: XCTestCase { /// exact operation that crashes for availability-gated classes on older OS versions (GH-8152). /// Comparing the two enumerations by name never realizes anything, so the test is safe on every /// OS — including ones where the old path would have crashed. - func testClassListEnumerationMatchesCopyClassNamesForImage() throws { + func testClassesForImage_whenReadingEveryLoadedImage_shouldMatchCopyClassNamesForImage() throws { + // -- Arrange -- let sut = SentryDefaultObjCRuntimeWrapper() - var comparedImages = 0 + for index in 0..<_dyld_image_count() { let imageName = try XCTUnwrap(_dyld_get_image_name(index).map { String(cString: $0) }) + // -- Act -- // New enumeration: class pointers from __objc_classlist, named via class_getName. let fromClassList = Set(sut.classes(forImage: imageName).map { NSStringFromClass($0) }) @@ -117,6 +119,7 @@ class SentrySubClassFinderTests: XCTestCase { } } + // -- Assert -- XCTAssertEqual(fromClassList, fromCopyNames, "Enumeration mismatch for image \(imageName)") if !fromCopyNames.isEmpty { comparedImages += 1 } } From b83614bb87446bd7e6f36b02f121401ffcf56349 Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Tue, 21 Jul 2026 10:04:07 +0200 Subject: [PATCH 12/20] docs: record arm64e device validation in handoff classes(forImage:) confirmed safe on a physical arm64e iPhone 12: 44 classrefs read, 31 view controllers swizzled, no EXC_BAD_ACCESS. --- HANDOFF-subclassfinder-fix.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/HANDOFF-subclassfinder-fix.md b/HANDOFF-subclassfinder-fix.md index 1ee4b8e71e2..4cc638596f9 100644 --- a/HANDOFF-subclassfinder-fix.md +++ b/HANDOFF-subclassfinder-fix.md @@ -95,11 +95,14 @@ are NOT changed for docs — an earlier draft added a `swizzleClassNameExcludes` than misread. No FAT parsing added — `_dyld_get_image_header` only returns thin, in-memory, native-arch slices, so a FAT header can't reach this code. Mirrors the repo's only precedent, `firstCmdAfterHeader` in `SentryCrashDynamicLinker.c`. -- **arm64e ptrauth** (itaybre, `m`): the `unsafeBitCast` just reinterprets a - dyld-bound classref (the same pointers the ObjC runtime stores in - `__objc_classlist`); we never hand-strip pointers, and the subsequent - `class_getSuperclass`/`class_getName` authenticate internally. App Store apps ship - plain arm64. A real arm64e device run is the definitive confirmation (device-only). +- **arm64e ptrauth** (itaybre, `m`): **RESOLVED — device-validated safe.** Built the + iOS-Swift sample `ARCHS=arm64e ONLY_ACTIVE_ARCH=NO` and ran on a physical iPhone 12 + (A14, iOS 26.5); app + `iOS-Swift.debug.dylib` + `Sentry.framework` all arm64e. The + SDK read 44 classrefs from the arm64e `__objc_classlist`, walked superclasses, and + swizzled 31 UIViewController subclasses with zero `EXC_BAD_ACCESS`. dyld applies + chained fixups **in place** at load time, so the in-memory section already holds + runtime-correct pointers that `class_getSuperclass`/`class_getName` accept directly — + no ptrauth strip needed. (Static/on-disk parsers would still need to decode fixups.) - **`unsafeBitcast` to `mach_header_64` to save the rebind** (NinjaLikesCheez, `l`): kept `withMemoryRebound` for clarity. - **dyld lock (new, self-found):** verified against Apple dyld source @@ -168,9 +171,7 @@ Verified against **objc4 source** + empirical tests on Apple silicon (arm64): indexed **by address, not name** — no by-name lookup exists, would need a linear scan of `getAllBinaryImages()` or a new accessor. Decide if the win is worth the coupling. (User may tackle this later.) -- **On-device confirmation** on iOS 16/17 with the repro (gated non-VC like a RoomPlan - wrapper + gesture Coordinator), ideally an arm64e-built app. Device-only; CI sims - can't reproduce. +- **On-device arm64e run: DONE** (see arm64e item above — iPhone 12, no crash). - **Perf** of the section read + superclass walk vs the old path on a real device. ## Rejected over-engineered approach (do NOT resurrect unless needed) From 1fcd2e5a7c08b500f1b5f32cdb2de1c4aa6d85b0 Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Tue, 21 Jul 2026 11:18:11 +0200 Subject: [PATCH 13/20] docs: clarify why background class inspection is safe The old comment was imprecise for the current flow. Explain that the background walk uses only class_getSuperclass and class_getName, which never message the class and so never trigger +initialize, and that we hand names to the main thread where messaging the class is safe. --- .../Performance/SentrySubClassFinder.swift | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift b/Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift index fa8d5987187..a5650a574e3 100644 --- a/Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift +++ b/Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift @@ -51,11 +51,12 @@ class SentrySubClassFinder: NSObject { SentrySDKLog.debug("Found \(classes.count) number of classes in image: \(imageName).") - // We keep the class names instead of the classes, so we can hand them to the swizzling on - // the main thread. Sending a message to a class (for example adding it to an NSArray - // retains it) calls its initializer, which we must avoid on this background thread as - // UIViewControllers assume they run on the main thread. Walking the superclass chain and - // reading the name with `class_getName` don't message the class. + // We inspect the classes on this background thread but only with `class_getSuperclass` + // (in `isClass`) and `class_getName`. Neither sends an Objective-C message to the class, + // so neither triggers its `+initialize`, which only runs on the first message send. This + // matters because UIViewControllers assume they run on the main thread, so we must not + // trigger their `+initialize` here. We collect the class names (not the classes) to hand + // to the swizzling on the main thread, where messaging the class is safe. var classesToSwizzle: [String] = [] for cls in classes { guard self.isClass(cls, subClassOf: viewControllerClass) else { From 953fc8a6ac51a6745fddf14dd370855198a14f74 Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Tue, 21 Jul 2026 11:27:24 +0200 Subject: [PATCH 14/20] ref: read classlist as AnyClass?, drop unsafeBitCast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebind the __objc_classlist section to AnyClass? — the honest Swift type of its Class _Nullable entries — and compactMap out nulls, instead of reading non-optional UnsafeRawPointer and unsafeBitCast-ing each entry to AnyClass. A null entry in a malformed section is now skipped instead of producing an invalid non-optional AnyClass (undefined behavior). Extract the section parsing into an internal static helper so tests can exercise the null-entry case, which a real dyld-loaded section can't be made to contain. --- HANDOFF-subclassfinder-fix.md | 5 +++-- .../SentryDefaultObjCRuntimeWrapper.swift | 17 +++++++++++++---- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/HANDOFF-subclassfinder-fix.md b/HANDOFF-subclassfinder-fix.md index 4cc638596f9..aca61245df7 100644 --- a/HANDOFF-subclassfinder-fix.md +++ b/HANDOFF-subclassfinder-fix.md @@ -55,8 +55,9 @@ Same structure, but get class **pointers** without realizing: - Default impl (`SentryDefaultObjCRuntimeWrapper`): `import MachO`; find the image via `_dyld_image_count`/`_dyld_get_image_name`; read `getsectiondata(header, "__DATA_CONST", "__objc_classlist", &size)` (fallback - `"__DATA"`); `unsafeBitCast` each entry to `AnyClass`. These are dyld-bound but - **not realized**. + `"__DATA"`); rebind the section to `AnyClass?` (the honest type of its + `Class _Nullable` entries — no `unsafeBitCast`) and `compactMap` out nulls. These + are dyld-bound but **not realized**. - Finder: iterate `classes(forImage:)`, run the unchanged `isClass` superclass walk, read matches' names via `class_getName` (does NOT realize, does NOT call `+initialize`), apply `swizzleClassNameExcludes`, collect names. Main-thread pass diff --git a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift index fd253300e41..7ede6fee601 100644 --- a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift +++ b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift @@ -62,15 +62,24 @@ public final class SentryDefaultObjCRuntimeWrapper: NSObject, SentryObjCRuntimeW return [] } - let count = Int(size) / MemoryLayout.size - return section.withMemoryRebound(to: UnsafeRawPointer.self, capacity: count) { classes in - (0.., size: UInt) -> [AnyClass] { + // The section holds ObjC `Class _Nullable` pointers, so `AnyClass?` is their honest + // Swift type: reading through it needs no unsafeBitCast, and a null entry (malformed + // section) becomes `nil` and is skipped instead of producing an invalid `AnyClass`. + let count = Int(size) / MemoryLayout.stride + return section.withMemoryRebound(to: AnyClass?.self, capacity: count) { classes in + UnsafeBufferPointer(start: classes, count: count).compactMap { $0 } + } + } #endif } // swiftlint:enable missing_docs From c4c9502fdc25ca7052827a6dd067698c55250a22 Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Tue, 21 Jul 2026 11:27:34 +0200 Subject: [PATCH 15/20] test: cover null entry in classlist section parsing Feed the new classes(inSection:size:) helper a crafted buffer shaped like getsectiondata's return value with a null slot in the middle, and assert the null is skipped. A real dyld-loaded __objc_classlist section never contains nulls, so this edge case is unreachable through classes(forImage:). --- .../SentrySubClassFinderTests.swift | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift b/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift index d8635aae3fe..d2e3a9456ae 100644 --- a/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift +++ b/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift @@ -129,6 +129,27 @@ class SentrySubClassFinderTests: XCTestCase { XCTAssertGreaterThan(comparedImages, 0, "Expected at least one image with classes") } + /// A real dyld-loaded `__objc_classlist` section never contains null entries (dyld binds every + /// slot at load time), so this edge case can't be produced through `classes(forImage:)`. Instead + /// we hand the section-parsing helper a crafted buffer shaped exactly like `getsectiondata`'s + /// return value — pointer-sized `Class _Nullable` entries — with a null slot in the middle, and + /// assert the null is skipped rather than surfacing as an invalid non-optional `AnyClass`. + func testClassesInSection_whenSectionContainsNullEntry_shouldSkipIt() { + // -- Arrange -- + var entries: [AnyClass?] = [FirstViewController.self, nil, SecondViewController.self] + + // -- Act -- + let classes: [AnyClass] = entries.withUnsafeMutableBytes { raw in + let section = raw.baseAddress!.assumingMemoryBound(to: UInt8.self) + return SentryDefaultObjCRuntimeWrapper.classes(inSection: section, size: UInt(raw.count)) + } + + // -- Assert -- + XCTAssertEqual(classes.count, 2) + XCTAssertTrue(classes[0] == FirstViewController.self) + XCTAssertTrue(classes[1] == SecondViewController.self) + } + /// End-to-end regression test for GH-8152. The other tests inject a fake class list through the /// stub wrapper; this one drives the REAL `SentryDefaultObjCRuntimeWrapper` through the finder /// against this test bundle's image. It proves the production enumeration path — reading the From 38a6aad4fdf479a1cf606c858f5345cca179cd17 Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Tue, 21 Jul 2026 11:56:01 +0200 Subject: [PATCH 16/20] ref: pass class pointers directly to swizzle block Store AnyClass instead of class names in SentrySubClassFinder and call the swizzle block with the stored pointer. This drops the NSClassFromString round-trip on the main thread, which realizes the class and could re-trigger the availability-gated realization crash this branch fixes (GH-8152, swiftlang/swift#72657). It also swizzles exactly the class found in the target image instead of whatever a name lookup resolves for duplicate class names. --- .../Performance/SentrySubClassFinder.swift | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift b/Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift index a5650a574e3..64d7011021e 100644 --- a/Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift +++ b/Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift @@ -55,9 +55,16 @@ class SentrySubClassFinder: NSObject { // (in `isClass`) and `class_getName`. Neither sends an Objective-C message to the class, // so neither triggers its `+initialize`, which only runs on the first message send. This // matters because UIViewControllers assume they run on the main thread, so we must not - // trigger their `+initialize` here. We collect the class names (not the classes) to hand - // to the swizzling on the main thread, where messaging the class is safe. - var classesToSwizzle: [String] = [] + // trigger their `+initialize` here. Storing the unrealized class pointers doesn't message + // them either; the swizzling block on the main thread sends the first message, where + // that is safe. We must not round-trip through NSClassFromString on the main thread + // either: it realizes the class, and realizing a class whose Swift metadata references + // an `@available`-gated newer-framework type crashes on older OS versions + // (https://github.com/getsentry/sentry-cocoa/issues/8152, + // https://github.com/swiftlang/swift/issues/72657) — the same reason we read the class + // list from the image instead (see above). It could also resolve a same-named class + // from a different image. + var classesToSwizzle: [AnyClass] = [] for cls in classes { guard self.isClass(cls, subClassOf: viewControllerClass) else { continue @@ -75,18 +82,16 @@ class SentrySubClassFinder: NSObject { continue } - classesToSwizzle.append(className) + classesToSwizzle.append(cls) } self.dispatchQueue.dispatchAsyncOnMainQueueIfNotMainThread { - for className in classesToSwizzle { - if let cls = NSClassFromString(className) { - block(cls) - } + for cls in classesToSwizzle { + block(cls) } SentrySDKLog.debug( - "The following UIViewControllers for image: \(imageName) will generate automatic transactions: \(classesToSwizzle.joined(separator: ", "))" + "The following UIViewControllers for image: \(imageName) will generate automatic transactions: \(classesToSwizzle.map { String(cString: class_getName($0)) }.joined(separator: ", "))" ) } } From 6e8284f8718f1fc665bafbb10ea545c8f08c7ec3 Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Tue, 21 Jul 2026 13:13:18 +0200 Subject: [PATCH 17/20] docs: sync handoff with current state Reflect the direct class-pointer handoff (no NSClassFromString), the merged origin/main (#8494 early return), the extracted classes(inSection:) helper, and the current test names. --- HANDOFF-subclassfinder-fix.md | 84 +++++++++++++++++++++++------------ 1 file changed, 56 insertions(+), 28 deletions(-) diff --git a/HANDOFF-subclassfinder-fix.md b/HANDOFF-subclassfinder-fix.md index aca61245df7..793e1e332a9 100644 --- a/HANDOFF-subclassfinder-fix.md +++ b/HANDOFF-subclassfinder-fix.md @@ -10,8 +10,14 @@ yet marked ready-to-merge; double-checking first). - **Reviews:** three rounds (philipphofmann, NinjaLikesCheez, itaybre). All raised comments addressed in code + replies; see "Review feedback" below. -- **Tests:** `SentrySubClassFinderTests` pass on the local sim. Includes the - differential enumeration test and a new real-wrapper regression test. +- **`origin/main` merged in** (merge commit, includes `#8494` "reduce SDK-start + overhead"). Conflicts in `SentrySubClassFinder.swift` (kept our `[AnyClass]` flow + + main's empty-check early return; dropped main's `free(classes)`, which was for the + old C-array API) and its test (kept our `classes` seam + main's + `_NoViewController_DoesNotDispatchToMainQueue` test adapted to that seam). +- **Tests:** `SentrySubClassFinderTests` green on the local sim (9 tests) after the + merge. Includes the differential enumeration test, the section null-entry test, and + the real-wrapper regression test. ## TL;DR @@ -32,8 +38,9 @@ Mach-O section (gives class **pointers**, NOT realized) instead of `objc_copyClassNamesForImage` + `NSClassFromString`. Then use the existing `class_getSuperclass`-based `isClass(_:subClassOf:)` walk (already safe, never - realizes). `NSClassFromString` is only called at swizzle time, on the main thread, - for confirmed view controllers. + realizes). The confirmed VC class pointers are carried to the main thread and handed + straight to the swizzle block — `NSClassFromString` is not used at all anymore (it + realizes, and could resolve a same-named class from a different image). ## Root-cause chain @@ -42,10 +49,12 @@ 1. old: `objcRuntimeWrapper.copyClassNamesForImage` → class name C-strings (safe). 2. old: for each name → `NSClassFromString(name)` ← **THE CRASH** (realizes) → `isClass(subclassOf: UIViewController)` walk → keep name if VC. -3. main thread: `NSClassFromString(name)` again → hand `Class` to swizzle block. +3. old: main thread → `NSClassFromString(name)` again → hand `Class` to swizzle block. Only step 2's `NSClassFromString` was the problem. `class_getSuperclass` (used by -`isClass`) was always safe. +`isClass`) was always safe. The new code drops `NSClassFromString` entirely (both +steps 2 and 3) — it walks unrealized class pointers and passes them straight to the +swizzle. ## The fix (what changed) @@ -59,9 +68,11 @@ Same structure, but get class **pointers** without realizing: `Class _Nullable` entries — no `unsafeBitCast`) and `compactMap` out nulls. These are dyld-bound but **not realized**. - Finder: iterate `classes(forImage:)`, run the unchanged `isClass` superclass walk, - read matches' names via `class_getName` (does NOT realize, does NOT call - `+initialize`), apply `swizzleClassNameExcludes`, collect names. Main-thread pass - unchanged (`NSClassFromString` only on confirmed VCs). + read names via `class_getName` only to apply `swizzleClassNameExcludes` (neither + `class_getSuperclass` nor `class_getName` sends a message, so `+initialize` never + runs on the background thread), then collect the class **pointers** and hand them + directly to the main-thread swizzle block. Holding/iterating `AnyClass` does not + message the class either; the swizzle is the first message, and it runs on main. ### Files changed (PR #8457, vs origin/main) @@ -72,10 +83,17 @@ Same structure, but get class **pointers** without realizing: - `Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift` — add `classes(forImage:)` to protocol (+ threading/arch doc). - `Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift` — implement it - (`import MachO`), arch guard, `MH_MAGIC_64` guard, off-main-thread doc. + (`import MachO`), arch guard, `MH_MAGIC_64` guard, off-main-thread doc. Section + parsing extracted to a testable `static func classes(inSection:size:)` that reads + entries as `AnyClass?` and `compactMap`s out nulls (no `unsafeBitCast`). - `CHANGELOG.md` — `## Unreleased` → `### Fixes` entry (#8457). -- `Tests/.../SentrySubClassFinderTests.swift` — new seam + differential test + real- - wrapper regression test + gated `AvailabilityGatedNonViewController` fixture. +- `Tests/.../SentrySubClassFinderTests.swift` — new `classes` seam; differential test + (`testClassesForImage_whenReadingEveryLoadedImage_shouldMatchCopyClassNamesForImage`), + section null-entry test (`testClassesInSection_whenSectionContainsNullEntry_shouldSkipIt`), + real-wrapper regression test + (`testRealRuntimeWrapper_whenReadingBundleImage_findsBundleViewControllers`) + gated + `AvailabilityGatedNonViewController` fixture, and the merged-in + `_NoViewController_DoesNotDispatchToMainQueue` test. - `Tests/SentryTests/Helper/SentryTestObjCRuntimeWrapper.{h,m}` — added `classes` override block (parallels existing `classesNames`). @@ -123,9 +141,14 @@ Verified against **objc4 source** + empirical tests on Apple silicon (arm64): 2. `objc_copyClassNamesForImage` returns `demangledName(false)`; `class_getName` returns the same demangled form. Neither realizes. So new names == old names by construction. -3. `class_getName` and the superclass walk do **not** trigger `+initialize`. A Swift - `[AnyClass]` append doesn't either (holds a metatype, no retain-message) — unlike - ObjC `NSArray addObject:`. Tested empirically. +3. `class_getName` and the superclass walk do **not** trigger `+initialize` — + `+initialize` fires only on the first _message send_, and neither function messages + the class. Holding/iterating an `AnyClass` in a Swift array doesn't message it + either (metatype pointer, no retain-message) — unlike ObjC `NSArray addObject:`. + Guarded by `testGettingSubclasses_DoesNotCallInitializer` (registers a class with a + custom `+initialize`, asserts the finder never calls it). This is why passing the + class pointer to the main thread (instead of a name) is safe: the swizzle block is + the first message and it runs on main. 4. **Classes with a Swift-generic superclass** (e.g. a `UIHostingController` subclass) are **not in `__objc_classlist`** — and `objc_copyClassNamesForImage` doesn't return them either (same section). So switching enumeration loses **zero** @@ -136,23 +159,28 @@ Verified against **objc4 source** + empirical tests on Apple silicon (arm64): ### Differential/oracle evidence -- `testClassListEnumerationMatchesCopyClassNamesForImage` iterates every loaded image - and asserts the new (`__objc_classlist` + `class_getName`) name set equals the old - (`objc_copyClassNamesForImage`) set. Never calls `NSClassFromString`, so it's safe - on every OS. Permanent CI guard for the one thing that changed. +- `testClassesForImage_whenReadingEveryLoadedImage_shouldMatchCopyClassNamesForImage` + iterates every loaded image and asserts the new (`__objc_classlist` + `class_getName`) + name set equals the old (`objc_copyClassNamesForImage`) set. Never calls + `NSClassFromString`, so it's safe on every OS. Permanent CI guard for the one thing + that changed. - Local throwaway oracle (earlier session): across ~163 images / ~9,610 classes, new == old, 0 mismatches; superclass walk over all classes: no crash, max depth 7, 0 cycles. -## Tests - -- Behavioral tests inject `[AnyClass]` via the new `classes` seam (find VCs, honor - excludes, ignore non-VC, wrong image, no `+initialize`). -- `testClassListEnumerationMatchesCopyClassNamesForImage` — differential enumeration - equivalence (above). -- `testActOnSubclassesOfViewController_withRealRuntimeWrapper_findsBundleViewControllers` - — drives the **real** `SentryDefaultObjCRuntimeWrapper` through the finder against - this bundle; asserts the bundle's VCs are found and non-VCs (incl. the gated +## Tests (9 total, all green) + +- Behavioral tests inject `[AnyClass]` via the `classes` seam (find VCs, honor + excludes, ignore non-VC, wrong image, no `+initialize`, and — from the merged + `#8494` — no main-queue dispatch when there's nothing to swizzle). +- `testClassesForImage_whenReadingEveryLoadedImage_shouldMatchCopyClassNamesForImage` + — differential enumeration equivalence (above). +- `testClassesInSection_whenSectionContainsNullEntry_shouldSkipIt` — hands the + `classes(inSection:size:)` helper a crafted buffer with a null slot and asserts the + null is skipped. +- `testRealRuntimeWrapper_whenReadingBundleImage_findsBundleViewControllers` — drives + the **real** `SentryDefaultObjCRuntimeWrapper` through the finder against this + bundle; asserts the bundle's VCs are found and non-VCs (incl. the gated `AvailabilityGatedNonViewController`) are not. The actual `EXC_BAD_ACCESS` is device/OS-version-specific and can't be reproduced on CI sims, so this guards the enumeration + selection behavior, not the crash itself. From 198a7274aba61527ce4bf15646cfa298d1e85b7a Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Tue, 21 Jul 2026 16:01:19 +0200 Subject: [PATCH 18/20] docs: record Finding 3 verdict for PR 8457 --- REVIEW-PR-8457.md | 353 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 353 insertions(+) create mode 100644 REVIEW-PR-8457.md diff --git a/REVIEW-PR-8457.md b/REVIEW-PR-8457.md new file mode 100644 index 00000000000..6e4dae8b473 --- /dev/null +++ b/REVIEW-PR-8457.md @@ -0,0 +1,353 @@ +# PR 8457 Local Review Handoff + +## Review Status + +- PR: `https://github.com/getsentry/sentry-cocoa/pull/8457` +- Title: `fix: prevent SubClassFinder availability crash` +- Reviewed head: `6e8284f8718f1fc665bafbb10ea545c8f08c7ec3` +- Base during review: `main` at `6d9248043b5919d9b675d9e743188924b44b6bf6` +- Review date: July 21, 2026 +- Verdict: **Do not merge yet** +- GitHub activity: no comments, reviews, or mutations were posted +- Local changes from review: this handoff file only + +## PR Intent + +- Fix GH-8152, where `SentrySubClassFinder` realizes unrelated Swift/Objective-C classes through `NSClassFromString`. +- Class realization can crash older OS versions when Swift metadata references an availability-gated framework type. +- Replace `objc_copyClassNamesForImage` plus `NSClassFromString` discovery with direct reads of the image's `__objc_classlist` Mach-O section. +- Walk superclasses with `class_getSuperclass` without realizing classes. +- Pass discovered `AnyClass` pointers to the main-thread swizzling block. + +## Finding 1: Concurrent Image Unload Can Crash + +### Severity + +- **P0 / merge blocker** +- Violates the SDK requirement to never crash the host application. + +### Affected Code + +- `Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift:28` +- `Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift:32` +- `Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift:36` +- `Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift:56` +- `Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift:65` + +### Problem + +- `classes(forImage:)` iterates `_dyld_image_count`, matches an image name, obtains a Mach-O header, obtains a section pointer, and copies class pointers. +- The image is not pinned and no loader/runtime synchronization protects the header or section memory. +- An unload can occur after any of these operations: + - `_dyld_get_image_name(index)` + - `_dyld_get_image_header(index)` + - `getsectiondata(...)` + - before or during `UnsafeBufferPointer(...).compactMap` +- The installed Apple SDK header explicitly documents this iteration as not thread-safe: + +```c +/* + * The following functions allow you to iterate through all loaded images. + * This is not a thread safe operation. Another thread can add or remove + * an image during the iteration. + */ +``` + +- Local header inspected: + - `/Applications/Xcode-16.4.0.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS18.5.sdk/usr/include/mach-o/dyld.h:51` +- The PR comments currently claim `_dyld_get_image_header` and `_dyld_get_image_name` acquire a loader read lock and that stale indices safely return `nil`. Those claims are not supported by the public API contract. +- The old `objc_copyClassNamesForImage` implementation is materially safer. Modern objc4 uses the runtime lock plus Mach-O UUID validation to avoid concurrent-unload ABA problems before reading an image's class list. + +### Deterministic Reproduction + +- A temporary Objective-C `MH_BUNDLE` containing a class was built locally. +- The probe performed these steps: + 1. `dlopen` the bundle. + 2. Find and retain its raw Mach-O header pointer using `_dyld_get_image_header`. + 3. `dlclose` the bundle so it is unmapped. + 4. Call `getsectiondata` using the stale header. +- Result: + +```text +before=45 loaded=348 index=45 header=0x1002dc000 +after=347 afterIndex=-1 afterHeader=0x0 +probe_status=139 +``` + +- Exit status `139` is `SIGSEGV`. +- This demonstrates that an Objective-C image can unload and that dereferencing the saved header or section is unsafe. +- The production race window also extends through the entire class-pointer copy, not only the `getsectiondata` call. + +### Required Resolution + +- Do not dereference an image header or section unless image lifetime is guaranteed for the complete operation. +- Use a mechanism that safely coordinates with image loading/unloading and validates that the header still represents the requested image. +- Possible directions requiring careful validation: + - Use the SDK's binary-image cache/callback infrastructure with appropriate synchronization and image removal handling. + - Pin the target image for the operation and independently verify the header-to-path association before dereferencing it. + - Use an Objective-C runtime API that already owns the necessary runtime lock and remapping behavior, if a supported API is available. +- Merely reading `_dyld_image_count` once does not solve this issue. +- Add an unload-race test using an Objective-C bundle or an equivalent deterministic harness. + +## Finding 2: Raw Class List Entries Are Unremapped + +### Severity + +- **P1 / merge blocker** +- The implementation passes class references to swizzling that are not guaranteed to be the live runtime class objects. + +### Affected Code + +- `Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift:74` +- `Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift:50` +- `Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift:67` +- `Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift:85` +- `Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift:93` + +### Problem + +- objc4 defines the entries in the image class list as `classref_t`: + +```cpp +// classref_t is unremapped class_t* +typedef struct classref * classref_t; +``` + +- Apple runtime code calls `remapClass(classlist[i])` before returning or operating on an image class. +- Remapping can: + - Replace a compiler-emitted class object with a previously allocated future class object. + - Return `nil` for a class ignored because of a missing weak-linked superclass. +- `classes(inSection:size:)` instead rebinds entries directly to `AnyClass?` and returns them unchanged. +- The direct pointers are then retained across a queue hop and passed to the Sentry swizzler. + +### Local Reproduction + +- A future class was allocated with `objc_getFutureClass("SentryFutureVictim")`. +- A bundle defining that class was then loaded. +- The raw class-list pointer was compared with the live runtime lookup. +- Result: + +```text +future=0x1013451a0 live=0x1013451a0 raw=0x100b480c0 +liveName=SentryFutureVictim rawName=SentryFutureVictim +liveSuper=0x1ef149ad8 rawSuper=0x1ef149ad8 +runtimeCount=1 first=SentryFutureVictim rawEqualsLive=no +probe_status=10 +``` + +- The raw pointer and live class pointer differ even though their names and superclasses match. +- This is exactly the case handled by objc4's `remapClass` map. + +### Why Existing Tests Miss It + +- `testClassesForImage_whenReadingEveryLoadedImage_shouldMatchCopyClassNamesForImage` compares sets of class **names**. +- A remapped class and its raw compiler class have the same name. +- The test therefore passes even when pointer identity and runtime validity differ. +- `compactMap` only removes literal null entries; it cannot apply objc4's private remapping table. + +### Consequences + +- Sentry may swizzle a replaced or ignored class object instead of the live class. +- `SentrySwizzle` deduplication records class object identity in `NSMutableSet`; raw and live remapped classes have different identities. +- A later swizzle through the live pointer may bypass once-per-class deduplication and apply a second swizzle. +- Runtime operations on ignored or otherwise remapped raw metadata are outside the supported runtime contract. + +### Recommended Resolution + +- Do not pass raw class-list pointers to the swizzler or retain them across the background-to-main queue hop. +- A safer supported-API design: + 1. Pin/synchronize the image while parsing its class list. + 2. Use raw pointers only transiently for the non-realizing superclass walk. + 3. Store the selected class names, not the raw pointers. + 4. Resolve only the selected `UIViewController` subclasses on the main thread. + 5. Verify `class_getImageName` still matches the requested image before swizzling to avoid same-name cross-image resolution. +- Resolving a selected view-controller class on main does not add a new realization requirement: the swizzling implementation immediately messages and realizes the selected class anyway. +- Add a future-class/remapping regression test; name-set equivalence is insufficient. + +### Follow-Up Review of Local Attempt + +- Follow-up reviewed on July 21, 2026. +- Local changes inspected: + - Added explanatory comments at `Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift:56` justifying the intentional use of unremapped pointers. + - Added `testActOnSubclassesOfViewController_WhenClassDoesNotReachViewController_IsNotSwizzled` at `Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift:157`. +- Conclusion: **the local attempt does not address Finding 2**. +- No production behavior changed. Raw `__objc_classlist` entries are still returned unchanged, retained across the queue hop, and passed to the swizzler. + +#### Disproven Assumption + +- The new comment claims a resolved future class is, in practice, never a `UIViewController` subclass. +- objc4 only prevents a future class from being completed by a Swift class. It does not restrict the Objective-C superclass hierarchy of a future class. +- A deterministic local probe reserved `SentryFutureViewController` through `objc_getFutureClass`, then loaded an Objective-C bundle defining it as an `NSViewController` subclass. +- `NSViewController` exercises the same Objective-C runtime remapping behavior as an Objective-C `UIViewController` subclass. +- Result: + +```text +future=0x10228ffc0 live=0x10228ffc0 raw=0x1026900b8 rawEqualsLive=no +rawName=SentryFutureViewController liveName=SentryFutureViewController +rawSuper=NSViewController liveSuper=NSViewController +rawIsViewController=yes liveIsViewController=yes +``` + +- The raw compiler-emitted pointer: + - Differs from the live runtime pointer. + - Has a superclass chain reaching the view-controller base class. + - Passes the same superclass filter used by `SentrySubClassFinder`. +- The current implementation would therefore forward the unremapped raw pointer to the swizzler. + +#### Why the Added Test Is Insufficient + +- `FakeViewController.self` is a normal, live runtime class pointer. It is not a raw pointer that objc4 remaps to another class or to `nil`. +- The test only reconfirms that an ordinary non-view-controller class is rejected by the superclass walk. +- That behavior was already covered by `testActOnSubclassesOfViewController_IgnoreFakeViewController`. +- The test cannot detect: + - A raw pointer differing from the live runtime class pointer. + - A future class whose raw and live pointers share the same name and superclass. + - Incorrect class-identity deduplication in `SentrySwizzle`. + - A raw pointer that objc4 would remap to `nil`. +- The updated focused suite passed `10` tests with `0` failures on iOS 18.6, but none of those tests exercises Objective-C class remapping. + +#### Finding Status + +- **Finding 2 remains open and remains a merge blocker.** +- Documentation asserting that raw pointers are safe is not a replacement for obtaining the live runtime class identity or redesigning the handoff so raw pointers never reach the swizzler. + +## Finding 3: Required SPI Protocol Method Breaks Old Conformers + +### Severity + +- **P2 / compatibility risk** +- Potential unrecognized-selector host crash for an injected conformer built against an older SDK. + +### Affected Code + +- `Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift:21` +- `Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift:50` +- `Sources/Swift/SentryDependencyContainer.swift:161` + +### Problem + +- `classesForImage:` is added as a required method to the public Objective-C SPI protocol `SentryObjCRuntimeWrapper`. +- `SentryDependencyContainer.objcRuntimeWrapper` is publicly settable through the private SPI surface. +- The new method is called unconditionally. +- An Objective-C conformer compiled against the previous SDK can still be supplied at runtime but does not implement `classesForImage:`. +- Calling the new selector on that object causes an unrecognized-selector exception. + +### Scope Check + +- A read-only GitHub code search across the `getsentry` organization found no external source conformers beyond `sentry-cocoa` tests. +- This lowers the known exposure but does not eliminate private, customer, or downstream SPI users. + +### Recommended Resolution + +- Avoid adding a required method to the existing SPI protocol. +- Options: + - Introduce a separate internal capability protocol used only by the default implementation. + - Make the Objective-C requirement optional and implement an explicit safe fallback. + - Remove this operation from the injectable wrapper if injection is not needed. +- The fallback must not restore the original unsafe realization of every class. + +### Verification Verdict (2026-07-21) + +- **Verdict: TRUE — the mechanism is confirmed, and the P2 severity is correct.** +- Every link in the chain was verified first-hand against the working tree, the diff vs base (`6d9248043`), the generated public header, and the test conformer. + +#### Evidence + +- **Required method, called unconditionally.** The method is added with no `optional` keyword, and the file has no `optional` anywhere: + - `Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift:22` (`@objc(classesForImage:)` + `func classes(forImage:)`) + - `Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift:50` calls it directly; there is no `respondsToSelector` / optional guard anywhere in the Swift layer. +- **Reachable by external Objective-C conformers.** The protocol is `@objc @_spi(Private) public` and is emitted into the shipped `Sentry.framework/Headers/Sentry-Swift.h` as a plain `@protocol SentryObjCRuntimeWrapper` with **no SPI guard**. + - `develop-docs/SWIFT.md:47` confirms `@_spi(Private)` restricts _Swift_ importers only; it has no effect for Objective-C. + - `Sources/Swift/SentryDependencyContainer.swift:161` exposes the setter as `@objc public var objcRuntimeWrapper` (readwrite in the header). +- **Old conformers really are missing the method (smoking gun).** The repo's own Objective-C conformer had to _add_ `classesForImage:` under the same `#if`: + - `Tests/SentryTests/Helper/SentryTestObjCRuntimeWrapper.m:68` + - Objective-C only _warns_ (never errors) on incomplete protocol conformance, so a conformer built before this PR compiles and links, then hits `doesNotRecognizeSelector:` when the new SDK invokes the selector. +- **Default-on trigger path.** The swizzling path that reaches this call is enabled by default: + - `Sources/Swift/Options.swift:257` (`enableAutoPerformanceTracing = true`), `Sources/Swift/Options.swift:280` (`enableUIViewControllerTracing = true`), `Sources/Swift/Options.swift:446` (`enableSwizzling = true`). + +#### Severity Note + +- P2 is correct — not higher. The finding's own scope check holds: the only realistic external consumers are hybrid SDKs, and none conform to this protocol. The method is also platform/arch-gated (`#if (os(iOS) || os(tvOS) || os(visionOS)) && (arch(arm64) || arch(x86_64))`), so watchOS/macOS/32-bit slices are unaffected regardless. Real latent ABI hazard, small blast radius. + +#### Wording Refinement + +- The risk is **Objective-C** conformers only. A _Swift_ conformer would fail to **compile** against the new required method, so it can never reach runtime. Read the finding's "compiled against the previous SDK" as Objective-C-only. + +#### Deferred Remediation (NOT YET IMPLEMENTED) + +- Smallest safe fix, ready to pick up later: + 1. Mark the protocol method `@objc optional func classes(forImage:)` in `Sources/Swift/Helper/SentryObjCRuntimeWrapper.swift:22` (protocol is already `@objc`). + 2. In `Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift:50`, call via optional chaining (`self.objcRuntimeWrapper.classes?(forImage: cImageName)`) and, when `nil`, log a warning and **skip** swizzling for that image. The fallback must **not** realize all classes — that is the exact GH-8152 crash this PR fixes. + 3. Add a regression test in `Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift` that injects a conformer omitting `classesForImage:` and asserts `actOnSubclassesOfViewController` completes without raising and performs no swizzling. + 4. Run `make generate-public-api` and commit any diff to satisfy the `api-stability` CI check. No `SentryObjC`/`SentryObjCCompat` wrapper change is needed (SPI, not the public wrapper SDK surface). +- `SentryDefaultObjCRuntimeWrapper` and the test conformer already implement the method, so the normal SDK path is unchanged; only a foreign incomplete conformer takes the skip branch. + +## Validation Completed + +### Tests + +- Command: + +```bash +IOS_SIMULATOR_OS=18.6 IOS_DEVICE_NAME='iPhone 16 Pro' \ + make test-ios FOR_AGENTS=true \ + ONLY_TESTING=SentryTests/SentrySubClassFinderTests +``` + +- Result: `9` tests executed, `0` failures. + +- Command: + +```bash +IOS_SIMULATOR_OS=15.5 IOS_DEVICE_NAME='iPhone 13 Pro' \ + make test-ios FOR_AGENTS=true \ + ONLY_TESTING=SentryTests/SentrySubClassFinderTests +``` + +- Result: `9` tests executed, `0` failures. +- The iOS 15.5 run is important because the fix targets older runtime/availability behavior. + +### CI State Observed + +- Fast unit tests and fast framework slice checks were passing. +- Full CI jobs were gated by the missing `ready-to-merge` label because the PR was still a draft. +- Danger, lint, analysis, and security checks observed during review were passing. + +### Cross-Platform Build Attempt + +- A local tvOS build was attempted with Xcode 26.6 and the installed tvOS 26.5 simulator. +- It failed during Swift package binary-artifact resolution before compilation: + +```text +downloaded archive of binary target ... does not contain a binary artifact +xcodebuild: error: Could not resolve package dependencies +``` + +- This failure appears unrelated to the PR and does not provide tvOS compilation evidence. +- Raw diagnostics are in `raw-build-output.log` from the review session. + +### arm64e Evidence + +- The PR author tested an arm64e build on a physical iPhone 12 running iOS 26.5. +- The sample read and swizzled classes without pointer-authentication failures. +- This is useful evidence for PAC handling. +- It does not cover concurrent unload or Objective-C class remapping. + +## Suggested Implementation Sequence + +1. Design a loader-safe image lifetime strategy. +2. Stop carrying raw class references to main. +3. Resolve selected names on main and validate image identity. +4. Preserve compatibility for existing runtime-wrapper conformers. +5. Add unload and future-class regression tests. +6. Re-run iOS 15.5 and current iOS tests. +7. Build tvOS, visionOS, Catalyst, and watchOS slices. +8. Re-run format and analysis before merge. + +## Review Conclusion + +- The PR fixes the normal availability-crash path in the tested configurations. +- The current implementation replaces a runtime-managed, unload-aware enumeration path with direct parsing that lacks image lifetime protection and class remapping. +- Both gaps are runtime correctness issues, not theoretical style concerns. +- The unload crash was reproduced locally and the remapped-pointer mismatch was demonstrated locally. +- Address Findings 1 and 2 before marking the PR ready to merge. From 8b6cf2b716adeea3ed58d44cf83ab8e49a1fde2f Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Tue, 21 Jul 2026 16:06:59 +0200 Subject: [PATCH 19/20] docs: mark SubClassFinder Finding 2 as open Unremapped __objc_classlist entries (Finding 2) still reach the swizzler; flag it in the wrapper comment and handoff. Add a filter test that documents it does not close the finding. No behavior change. --- HANDOFF-subclassfinder-fix.md | 37 +++++++++++++++++-- .../SentryDefaultObjCRuntimeWrapper.swift | 15 ++++++++ .../SentrySubClassFinderTests.swift | 17 +++++++++ 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/HANDOFF-subclassfinder-fix.md b/HANDOFF-subclassfinder-fix.md index 793e1e332a9..183ae211dc1 100644 --- a/HANDOFF-subclassfinder-fix.md +++ b/HANDOFF-subclassfinder-fix.md @@ -15,9 +15,40 @@ main's empty-check early return; dropped main's `free(classes)`, which was for the old C-array API) and its test (kept our `classes` seam + main's `_NoViewController_DoesNotDispatchToMainQueue` test adapted to that seam). -- **Tests:** `SentrySubClassFinderTests` green on the local sim (9 tests) after the - merge. Includes the differential enumeration test, the section null-entry test, and - the real-wrapper regression test. +- **Tests:** `SentrySubClassFinderTests` green on the local sim (10 tests) after the + merge. Includes the differential enumeration test, the section null-entry test, the + real-wrapper regression test, and the filter-drop test noted under Open blockers. + +## Open blockers (must resolve before ready-to-merge) + +- **Finding 2 — raw `__objc_classlist` entries are unremapped (OPEN, P1).** Full + writeup + follow-up in `REVIEW-PR-8457.md` §"Finding 2". `classes(inSection:size:)` + returns the raw compiler-emitted pointers without objc4's `remapClass`, and + `SentrySubClassFinder` carries them across the queue hop straight to the swizzler. + For a class objc4 remaps, the raw pointer differs from the live class object, which + can bypass `SentrySwizzle`'s class-identity dedup (`NSMutableSet` in + `SentrySwizzle.m`). + - **An earlier local attempt did NOT fix it** (comment + a filter-drop test only). The + review disproved the comment's core claim: objc4 only forbids a future class from + being _completed by a Swift class_, NOT from having an Objective-C view-controller + superclass. A reserved ObjC `objc_getFutureClass` VC subclass passes + `SentrySubClassFinder`'s `class_getSuperclass` filter yet has raw ≠ live identity — + demonstrated by a local probe (`SentryFutureViewController : NSViewController`, + `rawIsViewController=yes rawEqualsLive=no`). + - **Local changes currently in the tree (uncommitted):** softened comment in + `SentryDefaultObjCRuntimeWrapper.swift` (now flags this as a known open issue, no + longer claims safety) + `testActOnSubclassesOfViewController_WhenClassDoesNotReach` + `ViewController_IsNotSwizzled` (documents the filter drop for the + weak-missing-superclass shape only; explicitly NOT a Finding-2 fix). + - **Constraint on any real fix:** must NOT reintroduce the GH-8152 realization crash. + The review's "store names, resolve on main via `NSClassFromString`" suggestion + realizes the class and can pick a same-named class from another image — the exact + behavior this PR removed. A viable fix likely applies a `remapClass`-equivalent to + each entry (skipping nil results) transiently while keeping the non-realizing path. + - **Test gap:** name-set equivalence and the current filter test are insufficient; a + real regression test needs an ObjC bundle + `objc_getFutureClass` to exercise + remapping and pointer identity. + - Deferred by user decision (2026-07-21): store findings now, pick up the fix later. ## TL;DR diff --git a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift index 7ede6fee601..1c8b52be0e3 100644 --- a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift +++ b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift @@ -52,6 +52,21 @@ public final class SentryDefaultObjCRuntimeWrapper: NSObject, SentryObjCRuntimeW // modern binaries, `__DATA` on older ones). dyld binds these pointers at load time, but // the classes aren't realized, so callers can inspect them without running class // initialization, unlike `NSClassFromString`. + // + // These are the raw, compiler-emitted pointers; we do NOT run objc4's `remapClass` over + // them, so for a class objc4 remaps (a resolved future class from `objc_getFutureClass`, + // or a weak-linked class with a missing superclass that it maps to nil) the entry here can + // differ from the live runtime class or be a disavowed struct. + // + // KNOWN OPEN ISSUE (Finding 2 in REVIEW-PR-8457.md; see HANDOFF-subclassfinder-fix.md): + // an Objective-C future class can have a view-controller superclass, pass + // `SentrySubClassFinder`'s `class_getSuperclass` filter, and reach the swizzler as a raw + // pointer that differs from the live class — bypassing `SentrySwizzle`'s class-identity + // dedup. objc4 only forbids a future class from being *completed by a Swift class*, not + // from having an ObjC view-controller superclass, so this is not merely theoretical. + // This must be resolved before merge, and the fix must not reintroduce the GH-8152 + // realization crash: re-resolving by name via `NSClassFromString` realizes the class and + // can pick a same-named class from another image, which is exactly what this change removed. var size: UInt = 0 let section = header.withMemoryRebound(to: mach_header_64.self, capacity: 1) { header in getsectiondata(header, "__DATA_CONST", "__objc_classlist", &size) diff --git a/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift b/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift index 165e3dd1d75..596689e5ba4 100644 --- a/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift +++ b/Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift @@ -144,6 +144,23 @@ class SentrySubClassFinderTests: XCTestCase { XCTAssertGreaterThan(comparedImages, 0, "Expected at least one image with classes") } + /// Documents that a class whose superclass chain does not reach `UIViewController` is dropped by + /// the `class_getSuperclass` filter even when mixed in with real view controllers. + /// + /// NOTE: this does NOT close review Finding 2 (raw `__objc_classlist` entries aren't run through + /// objc4's `remapClass`). `FakeViewController` is an ordinary live class pointer, not a remapped + /// one, so this only covers the weak-linked-missing-superclass shape (objc4 zeroes the superclass → + /// the walk can't reach `UIViewController`). It does NOT cover a resolved Objective-C future class + /// with a view-controller superclass, which passes this filter yet has raw ≠ live pointer identity. + /// Reproducing that needs an ObjC bundle + `objc_getFutureClass`. Finding 2 remains open — see + /// HANDOFF-subclassfinder-fix.md and REVIEW-PR-8457.md. + func testActOnSubclassesOfViewController_WhenClassDoesNotReachViewController_IsNotSwizzled() { + fixture.runtimeWrapper.classes = { _ in + [FakeViewController.self, FirstViewController.self, SecondViewController.self] + } + assertActOnSubclassesOfViewController(expected: [FirstViewController.self, SecondViewController.self]) + } + /// A real dyld-loaded `__objc_classlist` section never contains null entries (dyld binds every /// slot at load time), so this edge case can't be produced through `classes(forImage:)`. Instead /// we hand the section-parsing helper a crafted buffer shaped exactly like `getsectiondata`'s From 45a729ba7af80e98c56fcd51a5f0cecbf97ab916 Mon Sep 17 00:00:00 2001 From: Philipp Suess Date: Tue, 21 Jul 2026 16:26:59 +0200 Subject: [PATCH 20/20] docs: correct classes(forImage:) dyld unload-race comment --- .../SentryDefaultObjCRuntimeWrapper.swift | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift index 1c8b52be0e3..4075af9f4e2 100644 --- a/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift +++ b/Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift @@ -15,9 +15,18 @@ public final class SentryDefaultObjCRuntimeWrapper: NSObject, SentryObjCRuntimeW return class_getImageName(cls) } - // IMPORTANT: Call this off the main thread. `_dyld_get_image_header` and `_dyld_get_image_name` - // acquire the dyld loader read lock, which every image load/unload (for example `dlopen`) - // contends, so calling this on the main thread risks blocking it. + // Call this off the main thread; the sole caller runs it on a background queue. (These `_dyld_*` + // calls take the dyld loader read lock that image load/unload contends, and the superclass walk + // over every class takes a few ms — neither belongs on the main thread.) + // + // KNOWN OPEN ISSUE (Finding 1 in REVIEW-PR-8457.md; see HANDOFF-subclassfinder-fix.md): iterating + // the dyld image list is NOT thread-safe. `` states: "Another thread can add or + // remove an image during the iteration." If the target image is unloaded between the name match + // and the `getsectiondata` dereference below, the header/section pointer dangles and + // dereferencing it can crash. This is a P0 that must be resolved before merge (fix direction: + // coordinate the read with the loader, e.g. via SentryBinaryImageCache). Not reachable in the + // default config (only the never-unloadable main executable is enumerated); reachable when the + // enumerated image is dynamically unloadable. // // Only supported on iOS, tvOS, and visionOS. It reads `mach_header_64` via `getsectiondata`, so // we gate it to 64-bit architectures: every slice these platforms ship (`arm64`/`arm64e` @@ -27,8 +36,9 @@ public final class SentryDefaultObjCRuntimeWrapper: NSObject, SentryObjCRuntimeW @_spi(Private) public func classes(forImage image: UnsafePointer) -> [AnyClass] { // We read `_dyld_image_count` once instead of per iteration (unlike the CxaThrowSwapper): we - // search for one already-loaded image, so images added while iterating aren't our target, and - // stale indices from an unload just return nil below. + // search for one already-loaded image, so images added while iterating aren't our target. An + // index that becomes out of range after an unload returns nil below, but an in-range index + // whose image was unmapped yields a dangling header — see the KNOWN OPEN ISSUE above. for index in 0..<_dyld_image_count() { guard let cName = _dyld_get_image_name(index), strcmp(cName, image) == 0 else { continue