fix: prevent SubClassFinder availability crash - #8457
Draft
philipphofmann wants to merge 40 commits into
Draft
Conversation
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.
philipphofmann
commented
Jul 17, 2026
itaybre
reviewed
Jul 19, 2026
itaybre
reviewed
Jul 19, 2026
|
|
||
| let count = Int(size) / MemoryLayout<UnsafeRawPointer>.size | ||
| return section.withMemoryRebound(to: UnsafeRawPointer.self, capacity: count) { classes in | ||
| (0..<count).map { unsafeBitCast(classes[$0], to: AnyClass.self) } |
Contributor
There was a problem hiding this comment.
m: I don't expect any issue here, but just double check this works on arm64e due to pointer authentication
Member
Author
There was a problem hiding this comment.
Double-checked. This is safe on arm64e:
- The
unsafeBitCastonly reinterprets the pointer's type (UnsafeRawPointer→AnyClass); it doesn't strip or re-sign anything. These are the exact classref pointers the ObjC runtime itself stores in__objc_classlistand hands to the loader, so they're already correctly formed for the runtime. - We never do hand-rolled pointer authentication/stripping. Every subsequent use goes through the runtime (
class_getSuperclass,class_getName), which authenticates internally. - App Store binaries ship plain arm64, so arm64e mostly matters for the dyld shared cache / system frameworks, which we walk the same way.
Definitive confirmation needs a real arm64e device run (CI sims are arm64/x86_64); tracked as device-only follow-up in the PR.
Member
Author
There was a problem hiding this comment.
Validated safe on arm64e. Reading __objc_classlist raw and unsafeBitCast-ing to AnyClass works on arm64e because dyld applies chained fixups in place at load time — by the time we read the in-memory section the entries are already runtime-correct pointers, so class_getSuperclass/class_getName accept them directly with no ptrauth strip. Confirmed on a physical arm64e device: 44 classrefs read, 31 view controllers swizzled, zero EXC_BAD_ACCESS.
How this was verified with Claude Code (reproducible)
This investigation was done with Claude Code. To re-run it, plug in an A12+ physical device (iPhone XS or newer — arm64e requires A12), open this branch, and paste the following into Claude Code:
Build the
iOS-Swiftsample as arm64e and run it on my connected physical device to check whetherSentryDefaultObjCRuntimeWrapper.classes(forImage:)is safe when it reads PAC-signed__objc_classlistentries. Steps:
- Get the device UDID:
xcrun devicectl list devices(find theconnectedphysicalone).- Build for that device forcing the arm64e slice, using the project's own signing (do NOT override CODE_SIGN_STYLE/DEVELOPMENT_TEAM — the sample already has manual signing configured):
xcodebuild -project Samples/iOS-Swift/iOS-Swift.xcodeproj -scheme iOS-Swift \ -configuration Debug -destination 'id=<UDID>' -derivedDataPath /tmp/arm64e-build \ ARCHS=arm64e ONLY_ACTIVE_ARCH=NO build- Confirm the built slices are actually arm64e:
(Debug builds put the Swift classes — and thuslipo -archs /tmp/arm64e-build/Build/Products/Debug-iphoneos/iOS-Swift.app/iOS-Swift lipo -archs /tmp/arm64e-build/Build/Products/Debug-iphoneos/iOS-Swift.app/iOS-Swift.debug.dylib__objc_classlist— iniOS-Swift.debug.dylib, so that one matters most.)- Install and launch with the console attached:
xcrun devicectl device install app --device <UDID> /tmp/arm64e-build/Build/Products/Debug-iphoneos/iOS-Swift.app xcrun devicectl device process launch --console --device <UDID> io.sentry.sample.iOS-Swift- In the console output, verify: (a)
SentrySubClassFinderlogsFound N number of classes in image: .../iOS-Swift.debug.dylib, (b) it logsThe following UIViewControllers ... will generate automatic transactions: <list of iOS_Swift.* VCs>, and (c) there is noEXC_BAD_ACCESS/SIGSEGV/crash and the process stays alive.If it reads the classes, names them, and swizzles the app's view controllers without crashing, the raw arm64e classref read is safe. If it crashes in
class_getSuperclass/class_getName, a ptrauth-aware read would be required.
Result on an iPhone 12 (A14, iOS 26.5): app + iOS-Swift.debug.dylib + Sentry.framework all arm64e; SentrySubClassFinder read 44 classrefs from the arm64e __objc_classlist, and SentryUIViewControllerSwizzling swizzled 31 UIViewController subclasses (BenchmarkingViewController, CoreDataViewController, nested/mangled ones like _TtCC9iOS_Swift18PageViewController17RedViewController, …). No crash across two launches; process stayed alive at a live UI.
Why this is the right test: the PAC signing of __objc_classlist entries depends on the slice the image was built as, not just the CPU — an arm64 app on an A12+ device has no signed classrefs. Forcing ARCHS=arm64e on the app (and its .debug.dylib) is what actually exercises signed classrefs. CI and the simulator run arm64/x86_64 where ptrauth is a no-op, which is why this needs a device.
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.
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.
Member
Author
|
Pushed
Changelog now references #8457. Leaving as a draft for a final look before marking ready. |
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.
`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.
Explain we don't re-read _dyld_image_count per iteration like the CxaThrowSwapper, since we search for one already-loaded image.
SwiftUI was only referenced in comments, never in code.
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.
Follow the test<Function>_when<Condition>_should<Expected> naming convention and the arrange-act-assert layout.
classes(forImage:) confirmed safe on a physical arm64e iPhone 12: 44 classrefs read, 31 view controllers swizzled, no EXC_BAD_ACCESS.
philipphofmann
force-pushed
the
fix/subclassfinder-availability-crash
branch
from
July 21, 2026 08:08
ae2d6da to
b83614b
Compare
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.
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.
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:).
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.
…ailability-crash # Conflicts: # Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift # Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift
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.
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.
noahsmartin
reviewed
Jul 21, 2026
noahsmartin
left a comment
noahsmartin
left a comment
Contributor
There was a problem hiding this comment.
Overall approach approach looks good to me, I would just advise testing on all platforms (watchOS, tvOS, etc...) before shipping
Match the caller and swizzler #if to classes(forImage:), which is declared only on 64-bit iOS/tvOS/visionOS. Keeps the required SPI method and its gate consistent instead of making it @objc optional.
Correct the misleading Finding 1 comment in classes(forImage:) and the tracking docs. A read-side fix (cache/lock/pin) cannot prevent the crash: the returned class pointers are dereferenced later at superclass-walk and swizzle time. Reachable only for a concurrently-unloaded MH_BUNDLE; main executable and frameworks (MH_DYLIB) do not unload. Accepted as a documented limitation; no functional change.
classes(forImage:) reads a Mach-O __objc_classlist section via dyld, not the Objective-C runtime, so it doesn't belong on SentryObjCRuntimeWrapper. Move it (and its section-parsing helper) to a dedicated SentryImageClassProvider protocol + default impl, and inject that into SentrySubClassFinder.
Commit af5d804 claimed swizzling an @available-gated view controller on an older OS is always safe. It is not: the swizzler messages the class (class_getInstanceMethod / class_getMethodImplementation / superclass), which realizes it via realizeClassMaybeSwiftMaybeRelock. A Swift VC subclass with a stored property of a gated newer-framework type (e.g. RoomPlan.CapturedStructure?) crashes at swizzle time on older OS versions - a residual GH-8152 case, confirmed on the iOS 16.4 simulator. Correct the call-site comment in SentryUIViewControllerSwizzling and the doc note in SentrySubClassFinder to describe this as a known limitation. Discovery remains safe (never realizes); only the later swizzle realizes, so the common non-VC crashers are still fixed while a gated-VC-with- gated-field is the remaining exposure. Track it as Finding 4 in REVIEW-PR-8457.md and HANDOFF-subclassfinder-fix.md with the repro and the two rejected guard approaches. No behavior change.
Strip REVIEW-PR-8457.md and HANDOFF-subclassfinder-fix.md down to the crash edge cases still to fix (Finding 4 gated-VC swizzle crash, Finding 2 unremapped classlist pointers) plus the pre-merge checklist. The fix mechanism and the rationale for each limitation already live in the code comments, so the docs no longer restate completed work. Also de-reference the now-removed Finding 1 from the image-unload comment in SentryDefaultImageClassProvider (it stands on its own).
Answer to the open question: swizzling cannot avoid realizing a class (dedup message send, class_getInstanceMethod, and class_replaceMethod all realize; the mutated method list only exists post-realization), and there is no safe unavailability probe. Record the designed fix instead: swizzle base UIViewController inits and defer per-subclass swizzling to first instantiation, dispatched async to main after the original init returns to avoid the GH-1355 crash shape that reverted this approach in 2021 (#1361). Scope (full SubClassFinder removal vs minimal reroute) is left as an open decision with both variants documented.
The iOS-Swift sample now demonstrates the documented workaround for the residual GH-8152 swizzle-time crash (tracked in GH-8548): the two gated crasher fixtures are added to swizzleClassNameExcludes via a new per-app additionalOptionsConfiguration hook, so they are discovered but never realized. Verified on the iOS 16.4 simulator: launch is clean and the SDK logs the exclusion. Removing the excludes re-arms the crash repro, the acceptance gate for the GH-8548 follow-up.
Repoint the known-limitation code comments from the repo-root tracking docs to the new follow-up issue GH-8548, which now holds the full Finding 4 analysis, the deferred first-instantiation swizzle design, and Finding 2. Delete REVIEW-PR-8457.md and HANDOFF-subclassfinder-fix.md; all their content lives in GH-8548 and the PR description.
philipphofmann
marked this pull request as ready for review
July 24, 2026 07:04
philipphofmann
added
ready-to-merge (deprecated)
📲 Install BuildsiOS
|
philipphofmann
marked this pull request as draft
July 24, 2026 08:29
Contributor
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d6d8b6a | 1226.16 ms | 1261.88 ms | 35.72 ms |
| 71e6611 | 1213.27 ms | 1242.25 ms | 28.98 ms |
| ddad953 | 1228.76 ms | 1262.47 ms | 33.71 ms |
| cee6024 | 1220.48 ms | 1257.02 ms | 36.54 ms |
| 1c5ecda | 1219.35 ms | 1253.76 ms | 34.41 ms |
| b994300 | 1227.37 ms | 1269.49 ms | 42.12 ms |
| 09627e8 | 1223.68 ms | 1261.95 ms | 38.27 ms |
| cbc75eb | 1239.77 ms | 1270.89 ms | 31.12 ms |
| a48979e | 1244.58 ms | 1269.00 ms | 24.42 ms |
| a8fc1c8 | 1237.62 ms | 1261.57 ms | 23.95 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d6d8b6a | 24.14 KiB | 1.22 MiB | 1.20 MiB |
| 71e6611 | 24.14 KiB | 1.15 MiB | 1.12 MiB |
| ddad953 | 24.14 KiB | 1.17 MiB | 1.15 MiB |
| cee6024 | 24.14 KiB | 1.24 MiB | 1.22 MiB |
| 1c5ecda | 24.14 KiB | 1.15 MiB | 1.12 MiB |
| b994300 | 24.14 KiB | 1.24 MiB | 1.22 MiB |
| 09627e8 | 24.14 KiB | 1.17 MiB | 1.14 MiB |
| cbc75eb | 24.14 KiB | 1.24 MiB | 1.22 MiB |
| a48979e | 24.14 KiB | 1.17 MiB | 1.15 MiB |
| a8fc1c8 | 24.14 KiB | 1.23 MiB | 1.20 MiB |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📜 Description
Discover
UIViewControllersubclasses without realizing classes. Instead ofobjc_copyClassNamesForImage+NSClassFromString,SentryDefaultImageClassProvider.classes(forImage:)reads the image's__objc_classlistMach-O section to get class pointers (dyld-bound, not realized), then reuses the existingclass_getSuperclasssubclass walk. The confirmed view-controller class pointers are handed straight to the swizzle block on the main thread —NSClassFromStringis no longer used (it realizes the class, and could resolve a same-named class from a different image). The swizzling logic itself is unchanged; only class discovery changed.Neither the superclass walk (
class_getSuperclass) norclass_getNamesends a message to the class, so+initializeis never triggered on the background thread — important becauseUIViewControllers assume they run on the main thread.Guarded with
MH_MAGIC_64before rebinding tomach_header_64(also skips FAT headers), gated to 64-bit archs, and documented as off-main-thread only (the_dyld_*calls take the dyld loader lock).This change removes the common crash but is not a complete fix for #8152, and the SDK can still crash during swizzling on older OS versions.
Coordinators,RoomPlan/ActivityKitwrappers) — are no longer touched.UIViewControllersubclasses are still swizzled, and swizzling messages/introspects the class (class_getInstanceMethod/class_getMethodImplementation/superclass), which realizes it. A Swift view-controller subclass gated to a newer OS that has a stored property of a gated newer-framework type (e.g. an@available(iOS 17, *)VC holding aRoomPlan.CapturedStructure?) is discovered safely, then crashes at swizzle time during Swift type-metadata completion. Confirmed on the iOS 16.4 simulator (crash:swizzleViewLayoutSubViews:→realizeClassMaybeSwiftMaybeRelock→swift_getSingletonMetadata→ type metadata completion →0x0,EXC_BAD_ACCESS).Workaround: add the gated class name to
options.swizzleClassNameExcludes— the finder then skips the class before it ever reaches the swizzler, so it is never realized. The iOS-Swift sample applies exactly this for its regression fixtures (seeAppDelegate.swift); removing those excludes re-arms the crash repro on the iOS 16.4 simulator.There is no cheap, crash-free way to detect that shape before swizzling: reading the "realized" bit skips essentially all VCs at SDK start (none are realized yet → ~0% coverage), and probing
swift_checkMetadataStateitself drives initialization and crashes the same way. A real fix — deferring per-subclass swizzling to first instantiation — is designed and tracked in #8548, together with the full analysis and one further narrow edge case (unremapped__objc_classlistpointers). The regression fixture and UI test added here (SubClassFinderRegressionViewController/SubClassFinderRegressionUITests) are the acceptance gate for that follow-up.💡 Motivation and Context
Fixes #8152. On SDK start, the old code called
NSClassFromStringon every class in the app image, which realizes it. Realizing a Swift class whose metadata references an@available-gated newer-framework type crashes withEXC_BAD_ACCESSon OS versions below that framework's availability (reproducible on the iOS 16.4 simulator too, not only real devices). Real-world crashers aren't view controllers — SwiftUI gestureCoordinators,RoomPlan/ActivityKitwrappers — so the SDK must avoid realizing unrelated classes. Underlying Swift/ObjC runtime bug: swiftlang/swift#72657.💚 How did you test it?
__objc_classlistenumeration matchesobjc_copyClassNamesForImageacross every loaded image; a section-parsing test covers null entries; a regression test drives the real provider through the finder against the test bundle; andtestGettingSubclasses_DoesNotCallInitializerguards that discovery never triggers+initialize.ARCHS=arm64eand ran it on a physical iPhone 12 (A14, iOS 26.5) — 44 classrefs read, 31 view controllers swizzled, zeroEXC_BAD_ACCESS. Confirms the raw classref read is safe on PAC-signed__objc_classlistentries (dyld applies chained fixups in place at load time). Repro steps in this comment.swizzleClassNameExcludesin place, the iOS-Swift sample launches clean on the iOS 16.4 simulator — the gated crasher fixtures are enumerated but skipped before realization (the SDK logs the exclusion). Removing the excludes reproduces the residual crash described in Swizzling @available-gated UIViewController subclasses still crashes below their gate (residual #8152) #8548;SubClassFinderRegressionUITestsguards the launch path.📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.