Conversation
Minor release superseding the planned 0.10.1: - Picks up the parallel-test SymbolIndexStore data race fix (@Mutex on demangledNodeBySymbol) — lets --no-parallel / @suite(.serialized) be dropped again. - Picks up ABI-accurate Swift section readers — __swift5_types and __swift5_protos indirect entries are now read correctly, via the new public types TypeMetadataRecord / ProtocolRecord / RelativeDirectPointerIntPair. - Carries forward everything planned for 0.10.1: generic-context dispatch signal-11 fix, swift-demangling 0.3.0 async-API migration, version-check CI, KNOWN_ISSUES race annotations. Changelogs/0.11.0.md consolidates all of the above.
There was a problem hiding this comment.
Pull request overview
Bumps the CLI/library bundled version to 0.11.0 and adds the corresponding 0.11.0 release notes, aligning with the repo’s version-check and release workflows that require a matching Changelogs/<version>.md.
Changes:
- Update
BundledVersion.valuefrom0.10.0to0.11.0. - Add
Changelogs/0.11.0.mddescribing the0.11.0release contents and requirements.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Sources/swift-section/Version.swift | Version bump to 0.11.0 (drives CI/version-check + release tag validation). |
| Changelogs/0.11.0.md | New release notes file required by version-check and used for GitHub release notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the project version to 0.11.0 and introduces a comprehensive changelog. The review feedback identifies several significant discrepancies in the changelog, noting that various claimed improvements—including the promotion of protocol requirements for witness-table dispatch, the implementation of thread-safe storage using @Mutex, and the addition of ABI-accurate section readers—are either missing from the current branch or incorrectly described relative to the actual code implementation.
| ### Generic context dispatch (signal-11 crash) | ||
|
|
||
| - The `ReadingContext` overload of `genericContext(in:)` introduced during the reading-context refactor was declared as a protocol extension only, with no override on `TypeContextDescriptorProtocol`. Calls through `any ContextDescriptorProtocol` therefore statically dispatched to the base implementation, which built a `TargetGenericContext<GenericContextDescriptorHeader>` (8-byte header) even when the descriptor was a `StructDescriptor` / `ClassDescriptor` / `EnumDescriptor`. Those descriptors actually start with `TypeGenericContextDescriptorHeader` (16 bytes — two relative offsets preceding the `base` header), so the read was misaligned by 8 bytes. The misaligned bytes usually surfaced as a garbage `valueHeader.numValues` of ~`UInt32.max`, after which `readWrapperElements` walked ~4 billion fabricated `GenericValueDescriptor` slots and crashed with `EXC_BAD_ACCESS`. | ||
| - Promoted `genericContext(in:)` and `parent(in:)` to protocol requirements on `ContextDescriptorProtocol` so they participate in witness-table dispatch. |
There was a problem hiding this comment.
The changelog claims that genericContext(in:) and parent(in:) were promoted to protocol requirements on ContextDescriptorProtocol. However, in the provided source code (Sources/MachOSwiftSection/Models/Type/TypeContextDescriptorProtocol.swift, lines 17 and 52), these are still implemented as extension methods. To achieve witness-table dispatch as described, these should be declared within the protocol declaration itself.
| ### `SymbolIndexStore` parallel-test data race | ||
|
|
||
| - `Storage.demangledNodeBySymbol` was previously a plain `Dictionary` mutated by `setDemangledNode(_:for:)` without synchronization. When swift-testing ran sibling suites in parallel, two builders could concurrently hit a cache miss on the same `SymbolIndexStore.Storage`; both would insert into the dictionary, and the unsynchronized read+write corrupted its internal layout — surfacing intermittently as `NSInvalidArgumentException: -[NSTaggedPointerString objectForKey:]`, SIGSEGV in the hash lookup, or later as `.invalidContextDescriptor` when the trashed bytes were reinterpreted as context-descriptor flags. | ||
| - Fixed by guarding the dictionary with `@Mutex` (`FrameworkToolbox` / `SwiftStdlibToolbox`). The CI workaround (`--no-parallel` and `@Suite(.serialized)`) is no longer needed and has been removed. This moves the entry out of `KNOWN_ISSUES.md` "deferred". |
There was a problem hiding this comment.
This entry states that the SymbolIndexStore data race was fixed using @Mutex. However, in Sources/MachOSymbols/SymbolIndexStore.swift (line 155), demangledNodeBySymbol is still a standard Dictionary without synchronization, and the Storage class is still marked @unchecked Sendable. This suggests the actual implementation of the fix is missing from this branch.
| ### `__swift5_types` / `__swift5_protos` indirect entries | ||
|
|
||
| - The generic section reader was reading every entry as `RelativeDirectPointer<Descriptor>`. Per the ABI (`swift/include/swift/ABI/Metadata.h:2720, 2766`), entries in these sections are tagged pointers — the low bits encode a `TypeReferenceKind` (for `__swift5_types`) or an indirect flag + reserved bit (for `__swift5_protos`). On a cross-module binary any entry emitted as `IndirectTypeDescriptor` (see the direct-vs-indirect decision at `lib/IRGen/GenDecl.cpp:4195-4224`) was read at the wrong offset, producing a descriptor pointer to unrelated bytes and surfacing as `.invalidContextDescriptor`. | ||
| - Readers now decode the ABI correctly: `TypeMetadataRecord` branches on `TypeReferenceKind` to pick direct / indirect resolution (and drops ObjC kinds to `nil`, matching the runtime `nullptr` fallback at `Metadata.h:2753`); `ProtocolRecord` uses `RelativeIndirectablePointerIntPair` with nullable `Optional` pointee to match `MetadataRef.h:109`. Both `MachOFile.Swift` and `MachOImage.Swift` readers now throw strictly on error and are symmetric across ends. |
There was a problem hiding this comment.
The changelog mentions that readers now decode the ABI correctly using new types like TypeMetadataRecord and ProtocolRecord, and throw strictly on error. However, these new model files are missing from the branch context, and MachOFile.Swift._readRelativeDescriptors (line 119) still uses compactMap with try? (swallowing errors) and RelativeDirectPointer (which does not appear to handle the tagged/indirect resolution described).
| - Added `GenericSpecializationTests`, `MultiPayloadEnumTests`, and `MetadataReaderDemanglingTests` to the macOS matrix filter so future regressions in the generic-context dispatch path or the section reader path surface in CI instead of as local crashes. `GenericSpecializerAPITests` was folded into `GenericSpecializationTests`, and the corresponding filter entry removed. | ||
| - `Sources/swift-section/Version.swift` is the single source of truth for the CLI version: | ||
| - `release.yml` no longer injects the version via heredoc; it fails a tagged release when `BundledVersion.value != github.ref_name`. | ||
| - A new `version-check.yml` workflow runs on every push to `main` and every PR, and fails when `Changelogs/<BundledVersion.value>.md` does not exist. |
There was a problem hiding this comment.
The changelog mentions a new version-check.yml workflow and updates to release.yml. However, these files are not present in the provided branch context. If these were intended to be part of the 0.11.0 release, they appear to be missing from this pull request.
Summary
Minor release superseding the planned
0.10.1. Folds in everything that was in flight for0.10.1plus the two items in PR #78:SymbolIndexStoreparallel-test data race —@MutexondemangledNodeBySymbol. Removes the--no-parallel/@Suite(.serialized)CI workarounds. Moves the matching entry out ofKNOWN_ISSUES.md"deferred".__swift5_types/__swift5_protosindirect entries are now read per ABI (swift/include/swift/ABI/Metadata.h:2720, 2766). New public typesTypeMetadataRecord,ProtocolRecord,RelativeDirectPointerIntPair,TargetRelativeDirectPointerIntPair,RelativeDirectPointerIntPairProtocol.0.10.1draft (already onmain): generic-context dispatch signal-11 fix,swift-demangling0.3.0 async-API migration,version-check.ymlCI,release.ymlhardening,KNOWN_ISSUES.mdannotations.The full write-up is in
Changelogs/0.11.0.md(follows the0.10.0.mdstructure).Files
Sources/swift-section/Version.swift—"0.10.0"→"0.11.0"Changelogs/0.11.0.md(new)Merge order
Merge after #78 (
fix(swift-sections)) so the code referenced in the changelog actually lands onmainbefore the tag is cut. This PR will rebase cleanly.Supersedes #77 (
release: 0.10.1) — that PR is being closed.Test plan
Version Checkworkflow is green (confirmsChangelogs/0.11.0.mdexists and matchesBundledVersion.value)mainfirst, then this rebased on top0.11.0cuts a clean release (release.ymlversion-match step passes)SymbolIndexStore.demangledNoderace entry inKNOWN_ISSUES.mdfrom "deferred" to "fixed" once a few CI runs have confirmed stability