fix(swift-sections): guard SymbolIndexStore race + align section readers with ABI by Mx-Iris · Pull Request #78 · MxIris-Reverse-Engineering/MachOSwiftSection

Mx-Iris · 2026-04-22T06:20:31Z

Summary

1. Data race fix — `SymbolIndexStore` (commit `e61952b`)

Guard Storage.demangledNodeBySymbol with @Mutex — closes the race documented in KNOWN_ISSUES.md under "SymbolIndexStore.demangledNode(for:in:) data race under parallel tests". Sibling test suites had been corrupting the unsynchronized Dictionary, surfacing non-deterministically as SIGSEGV in the hash lookup, or later as .invalidContextDescriptor when the trashed bytes were reinterpreted as context-descriptor flags.

With the mutex in place, --no-parallel and @Suite(.serialized) are no longer needed — reverted in 67a2824.

2. Swift section reader ABI alignment (commit `67a2824`)

Replace the generic _readRelativeDescriptors approach with ABI-accurate wrappers for each section entry type, mirroring swift/include/swift/ABI/Metadata.h:

TypeMetadataRecord (Metadata.h:2720) — struct over RelativeDirectPointerIntPair<ContextDescriptorWrapper, TypeReferenceKind>. Resolution branches on the 2-bit TypeReferenceKind exactly like TargetTypeMetadataRecord::getContextDescriptor does:
- DirectTypeDescriptor → RelativeDirectPointer
- IndirectTypeDescriptor → RelativeIndirectPointer
- DirectObjCClassName / IndirectObjCClass → nil (mirrors the runtime nullptr fallback at Metadata.h:2753, which notes these kinds "are just never used in these lists")
ProtocolRecord (Metadata.h:2766) — struct over RelativeIndirectablePointerIntPair<ProtocolDescriptor?, Bit, Pointer<…>>. Optional pointee matches nullable=true at MetadataRef.h:109; the reserved low-bit tag is exposed via Bit (runtime ignores it — see MetadataLookup.cpp:821 only calling getPointer()).
__swift5_proto — left on RelativeDirectPointer<ProtocolConformanceDescriptor> on both ends to match the naked using typealias at Metadata.h:3054 (no IntPair, no indirect).

Infrastructure

New RelativeDirectPointerIntPair / TargetRelativeDirectPointerIntPair mirroring swift/include/swift/Basic/RelativePointer.h:575-619 (low 2 bits = int value, no indirect bit — distinct from the existing RelativeIndirectablePointerIntPair).
Both MachOFile.Swift and MachOImage.Swift readers now throw strictly (no try?), symmetric across ends.
Drop retroactive LayoutProtocol conformances that are no longer reached.

Test plan

macOS CI green in both debug and release modes
Re-run the macOS workflow 2–3× to confirm stability (the original race was non-deterministic)
Local swift build across the full package
Follow-up: move the SymbolIndexStore.demangledNode(for:in:) race out of "deferred" in KNOWN_ISSUES.md

…est race CI was hitting a documented race in `demangledNode(for:in:)` where the cache-miss branch wrote to `demangledNodeBySymbol` without synchronization. When sibling test suites ran in parallel, concurrent writers corrupted the Dictionary's internal layout, surfacing as SIGSEGV in the hash lookup or as downstream `.invalidContextDescriptor` when subsequently reading trashed bytes as context-descriptor flags. - SymbolIndexStore: decorate `demangledNodeBySymbol` with `@Mutex` so the generated `_modify` accessor keeps `dict[key] = value` atomic. - GenericSpecializationTests: fold the API tests into the same suite and mark it `.serialized`. - macOS.yml: pass `--no-parallel` to both `swift test` invocations and drop the merged-away `GenericSpecializerAPITests` filter alternative.

gemini-code-assist

Code Review

This pull request introduces thread safety to the demangledNodeBySymbol dictionary using a @Mutex property wrapper and refactors the test suite by merging GenericSpecializerAPITests into GenericSpecializationTests with serialized execution. The review feedback highlights potential performance bottlenecks when using @Mutex with value types like Dictionary due to copy-on-write behavior and suggests ensuring atomic updates to prevent redundant operations.

gemini-code-assist · 2026-04-22T06:22:57Z

+        @Mutex
        private(set) var demangledNodeBySymbol: [Symbol: Node] = [:]


While @Mutex correctly synchronizes access to demangledNodeBySymbol to prevent data races, using a property wrapper on a Dictionary (a value type) can have performance implications. In Swift, any mutation to a property-wrapped value type (like the subscript assignment in setDemangledNode) typically involves a read-modify-write cycle that copies the entire collection if it's not uniquely referenced. For a large symbol index, this could become a bottleneck. Additionally, the check-then-set pattern in demangledNode(for:in:) is not atomic across the mutex boundary, which might lead to redundant demangling operations. If the @Mutex implementation provides a withLock method on its projected value, using it would allow for more efficient, atomic updates.

Copilot

Pull request overview

This PR addresses a documented CI-only data race in SymbolIndexStore.demangledNode(for:in:) by adding synchronization around the shared demangle cache and reducing test parallelism to prevent cross-suite corruption during swift test.

Changes:

Add @Mutex protection to SymbolIndexStore.Storage.demangledNodeBySymbol.
Fold the former GenericSpecializerAPITests into GenericSpecializationTests and serialize the suite.
Disable test parallelism in macOS CI (swift test --no-parallel) and update test filters accordingly.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
`Tests/SwiftInterfaceTests/GenericSpecializationTests.swift`	Merges API tests into the existing suite and marks it serialized to avoid concurrency-related flakiness.
`Sources/MachOSymbols/SymbolIndexStore.swift`	Synchronizes the shared demangled-node cache to prevent concurrent dictionary mutation/read races.
`.github/workflows/macOS.yml`	Disables parallel test execution and removes the merged-away suite from the filter list.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@suite

…lization _readRelativeDescriptors now uses RelativeIndirectablePointer<Descriptor, Pointer<Descriptor>> instead of RelativeDirectPointer<Descriptor>. Some Swift section entries are indirect on CI binaries; treating them as direct pointers reads the wrong offset and is the likely root cause of the release-mode invalidContextDescriptor failure (the prior SymbolIndexStore race fix did not clear release CI). Adds retroactive LayoutProtocol conformances for RelativeIndirectPointer, RelativeIndirectablePointer, and RelativeIndirectablePointerIntPair so that AnyLocatableLayoutWrapper accepts them. Also reverts --no-parallel on both test steps and @suite(.serialized) on GenericSpecializationTests to check whether the pointer-type change is sufficient on its own. @Mutex on demangledNodeBySymbol is kept as independent concurrency hardening.

Replace the Indirectable-as-everything workaround from ab0d033 with ABI-accurate wrappers for each Swift section entry type, matching swift/include/swift/ABI/Metadata.h: - TypeMetadataRecord (Metadata.h:2720) — struct around RelativeDirectPointerIntPair<ContextDescriptorWrapper, TypeReferenceKind>. Resolution branches on the 2-bit TypeReferenceKind like TargetTypeMetadataRecord::getContextDescriptor: direct → RelativeDirectPointer, indirect → RelativeIndirectPointer, ObjC kinds → nil (mirroring the runtime nullptr fallback at Metadata.h:2753). - ProtocolRecord (Metadata.h:2766) — struct around RelativeIndirectablePointerIntPair<ProtocolDescriptor?, Bit, Pointer<…>>. Optional Pointee matches nullable=true at MetadataRef.h:109; the reserved low-bit tag is kept as Bit (runtime reads only the pointer at MetadataLookup.cpp:821). - __swift5_proto — left on RelativeDirectPointer<Descriptor> on both ends to match the naked typealias at Metadata.h:3054. Infra: - Add RelativeDirectPointerIntPair / TargetRelativeDirectPointerIntPair mirroring RelativePointer.h:575-619 (low 2 bits = int, no indirect bit — distinct from the existing indirectable variant). - MachOFile and MachOImage readers now throw strictly (no try?), symmetric across ends. - Drop retroactive LayoutProtocol conformances that are no longer reached.

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-22T09:07:24Z

+        @Mutex
        private(set) var demangledNodeBySymbol: [Symbol: Node] = [:]



@Mutex on demangledNodeBySymbol alone may not eliminate the race if call sites mutate/read the Dictionary via subscripting (which can go through an in-place _modify access). To guarantee the lock is always held (as SharedCache does for storageByIdentifier), access this property via the wrapper’s withLockUnchecked (e.g. _demangledNodeBySymbol.withLockUnchecked { ... }) for both reads in demangledNode(for:in:) and writes in setDemangledNode(_:,for:) / setDemangledNodeBySymbol(_:).

Copilot · 2026-04-22T09:07:24Z

          swift test \
            -c debug \
            --build-path .build-test-debug \
-            --filter '\.(SymbolTestsCoreDumpSnapshotTests|SymbolTestsCoreInterfaceSnapshotTests|SymbolTestsCoreCoverageInvariantTests|STCoreE2ETests|STCoreTests|GenericSpecializationTests|GenericSpecializerAPITests|MultiPayloadEnumTests|MetadataReaderDemanglingTests)(/|$)'
+            --filter '\.(SymbolTestsCoreDumpSnapshotTests|SymbolTestsCoreInterfaceSnapshotTests|SymbolTestsCoreCoverageInvariantTests|STCoreE2ETests|STCoreTests|GenericSpecializationTests|MultiPayloadEnumTests|MetadataReaderDemanglingTests)(/|$)'



PR description says CI should pass --no-parallel to swift test, but these invocations still run with default parallelism. If disabling parallelism is required to avoid cross-suite races, add --no-parallel here (or update the PR description if that’s no longer the approach).

Copilot · 2026-04-22T09:07:25Z

    }
-}
-
-// MARK: - GenericSpecializer API Tests
-
-@Suite
-struct GenericSpecializerAPITests {
+
    @Test func makeRequest() async throws {
        let machO = MachOImage.current()


PR description mentions marking the merged GenericSpecialization test suite as .serialized, but this file still has no @Suite(.serialized) annotation on the suite type. If serialization is still intended (to avoid intra-suite parallelism), add the attribute; otherwise update the PR description to match the implementation.

Copilot AI review requested due to automatic review settings April 22, 2026 06:20

Copilot started reviewing on behalf of Mx-Iris April 22, 2026 06:20 View session

gemini-code-assist Bot reviewed Apr 22, 2026

View reviewed changes

Copilot AI reviewed Apr 22, 2026

View reviewed changes

Mx-Iris added 2 commits April 22, 2026 16:06

Copilot AI review requested due to automatic review settings April 22, 2026 09:00

Mx-Iris changed the title ~~fix(SymbolIndexStore): guard demangledNodeBySymbol (parallel-test race)~~ fix(swift-sections): guard SymbolIndexStore race + align section readers with ABI Apr 22, 2026

Copilot started reviewing on behalf of Mx-Iris April 22, 2026 09:00 View session

Copilot AI reviewed Apr 22, 2026

View reviewed changes

This was referenced Apr 22, 2026

release: 0.11.0 #79

Merged

release: 0.10.1 #77

Closed

Mx-Iris merged commit 46378d0 into main Apr 22, 2026
6 checks passed

Mx-Iris deleted the fix/context-descriptor-resolve-kind-coverage branch April 22, 2026 10:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(swift-sections): guard SymbolIndexStore race + align section readers with ABI#78

fix(swift-sections): guard SymbolIndexStore race + align section readers with ABI#78
Mx-Iris merged 3 commits intomainfrom
fix/context-descriptor-resolve-kind-coverage

Mx-Iris commented Apr 22, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Apr 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 22, 2026

Uh oh!

Copilot AI Apr 22, 2026

Uh oh!

Copilot AI Apr 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@Mutex
		private(set) var demangledNodeBySymbol: [Symbol: Node] = [:]

Conversation

Mx-Iris commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

1. Data race fix — SymbolIndexStore (commit e61952b)

2. Swift section reader ABI alignment (commit 67a2824)

Infrastructure

Test plan

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Mx-Iris commented Apr 22, 2026 •

edited

Loading

1. Data race fix — `SymbolIndexStore` (commit `e61952b`)

2. Swift section reader ABI alignment (commit `67a2824`)