Handle unions in struct encoding parsing and harden offset scan - #140
Open
rootkiller6788 wants to merge 1 commit into
Open
Handle unions in struct encoding parsing and harden offset scan#140rootkiller6788 wants to merge 1 commit into
rootkiller6788 wants to merge 1 commit into
Conversation
The struct encoding parser treated a union (encoded by the runtime as "(name=...)" or "(?=...)") as a plain literal, stopping at the first quoted member name inside the union. For unions with named members this produced a malformed encoding like "(?=" plus the union members as sequential struct fields, which shifted every subsequent offset inside the struct and mislocated (or missed) the strong references after it. Consume a union as one opaque type instead (scanToClosingParen), so the fields that follow a union are parsed and offset correctly. While here, FBGetReferencesForObjectsInStructEncoding ignored the return value of NSGetSizeAndAlignment, which does not throw on unsupported encodings (e.g. bitfields or C++ types) - it logs a warning and returns NO. The scan then computed offsets from zero/garbage size and alignment. Bail out of the scan when the size and alignment cannot be obtained. Adds parser tests covering named and anonymous unions followed by an object, plus a literal-encoding regression test.
rootkiller6788
marked this pull request as ready for review
August 26, 2026 15:10
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
1 participant
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.
What
This fixes struct parsing when a struct contains a union, and hardens the struct-offset scan against type encodings
NSGetSizeAndAlignmentcannot handle.1. Union parsing (
FBStructEncodingParser.mm) - fixes #21ObjC runtime type encodings represent a union as
(name=...), or(?=...)for anonymous unions. The parser did not recognize(as a grouping character: it fell through to the literal branch and stopped at the first quoted member name inside the union.For a union with named members such as
"someUnion"(?="x"i"y"f), the parser previously produced:Type(name="someUnion", encoding="(?=")Type(name="x", encoding="i")Type(name="y", encoding="f)")…treating overlapping union members as sequential struct fields and leaving a malformed
(?=encoding. Every subsequent field's offset inside the struct was then wrong, so an object reference stored after the union was located at a garbage index (read of the wrong memory during cycle detection).The fix adds
_StringScanner::scanToClosingParen()and consumes the whole union as one opaqueType, keeping the offsets of the types that follow it correct.2. Layout scan hardening (
FBClassStrongLayout.mm) - related to #126NSGetSizeAndAlignmentdoes not throw on encodings it cannot size (bitfieldsb, C++ types, …): it logs the "unsupported type encoding spec" warning and returnsNO. The code ignored the return value and then computedoffset % align/offset += sizefrom zero or garbage values. The scan now checks the return value and bails out (same intent as the existing@catchbranch) instead of producing misaligned references.Tests
Added to
FBStructEncodingParserTests.mm:testThatParserWillParseUnionAsSingleOpaqueType- literal encoding regression test asserting the exact parsed types.testThatParserWillParseStructWithNamedUnionAndObject- real ivar encoding with a named union.testThatParserWillParseStructWithAnonymousUnionAndObject- real ivar encoding with an anonymous union.Verified with a standalone C++ build of the parser (ObjC bits mechanically stubbed) that runs the new union cases plus all pre-existing parser cases (primitives, objects, nested structs, bitfields, type paths) with no regressions.