From f09f454f45d667a95b38be61ba6fde193bd08774 Mon Sep 17 00:00:00 2001 From: Kaoru0822 Date: Wed, 14 Oct 2026 21:22:40 -0700 Subject: [PATCH] fix(native): retain valid Objective-C properties beside a malformed record (#3833) * fix(native): retain valid Objective-C properties beside a malformed record readObjcPropertyList decoded a whole property_list_t in one operation. When a later entry's name pointer was unmapped, the list-level catch discarded the properties already decoded. An empty property name passed decoding and then failed schema validation of the whole result, so inspect-native-dispatch-metadata exited 1 with no metadata. Each property_t entry is now decoded on its own. A failing entry, or an empty name, becomes a property_decode_failed issue at the entry address, and sibling properties are retained. Incomplete-attribute issues also report their entry address. List-header failures still fail only that list. Fixes #3336 Co-Authored-By: Claude Opus 5.5 * test(native): cover property record isolation on every platform Move the synthetic Objective-C property Mach-O into a shared fixture and decode it through decodeAppleDispatchMetadata in a portable test, so Linux and Windows runs also catch a regression that drops valid sibling properties. The macOS CLI cases keep exercising the built command. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- docs/native-investigation.md | 5 +- src/native/AppleDispatchMetadata.test.ts | 33 +++++++ src/native/AppleObjcProperties.ts | 62 ++++++++---- .../nativeDispatchPropertyAttributes.test.ts | 94 ++++++++++--------- tests/fixtures/objcPropertyMacho.ts | 51 ++++++++++ 5 files changed, 181 insertions(+), 64 deletions(-) create mode 100644 tests/fixtures/objcPropertyMacho.ts diff --git a/docs/native-investigation.md b/docs/native-investigation.md index 3df66a93f..ed9ffe46f 100644 --- a/docs/native-investigation.md +++ b/docs/native-investigation.md @@ -380,7 +380,10 @@ complete observation. missing or incomplete attribute metadata leaves `atomicity` and any unestablished read-only value null. A type attribute whose quoted class name never closes is incomplete and marks `objc_properties_categories` partial - with `property_attributes_incomplete`. `is_meta_class`, `is_root_class`, and + with `property_attributes_incomplete`. A property record that cannot be + decoded, such as one with an empty or unmapped name, is reported as + `property_decode_failed` at its entry address while sibling properties are + retained. `is_meta_class`, `is_root_class`, and `ivar_count` are also nullable when the selected provider exposes only symbol names. Class implementation methods have null `is_required` and `is_optional` fields because those facts apply to protocol declarations; decoded protocol diff --git a/src/native/AppleDispatchMetadata.test.ts b/src/native/AppleDispatchMetadata.test.ts index 4509e2373..f1af48e43 100644 --- a/src/native/AppleDispatchMetadata.test.ts +++ b/src/native/AppleDispatchMetadata.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from "vitest"; +import { machoWithPropertyAttributes } from "../../tests/fixtures/objcPropertyMacho.js"; import { decodeAppleDispatchMetadata } from "./AppleDispatchMetadata.js"; const base = 0x100000000n; @@ -423,6 +424,38 @@ describe("Apple dispatch coverage ownership", () => { }); }); +describe("malformed Objective-C property records", () => { + it.each([ + ["an unmapped name", { secondNameOffset: 0x2000 }, "first", "0x100000518"], + ["an empty name", { firstName: "" }, "second", "0x100000508"], + ] as const)( + "retains the valid sibling of a property with %s", + (_, layout, kept, location) => { + const result = decodeAppleDispatchMetadata( + machoWithPropertyAttributes("Tq,N", layout), + 100, + provenance, + ); + expect( + result.objc_classes.map(({ name, properties }) => ({ + name, + properties: properties.map((property) => property.name), + })), + ).toEqual([{ name: "Fixture", properties: [kept] }]); + expect( + result.coverage.find( + ({ facet }) => facet === "objc_properties_categories", + ), + ).toMatchObject({ + status: "partial", + reason: expect.stringContaining( + `property_decode_failed at ${location}: Fixture: property`, + ), + }); + }, + ); +}); + describe("malformed Apple dispatch metadata", () => { it("retains valid class siblings when one class has an empty name", () => { const result = decodeAppleDispatchMetadata( diff --git a/src/native/AppleObjcProperties.ts b/src/native/AppleObjcProperties.ts index 5b78b5ab9..266d5ead6 100644 --- a/src/native/AppleObjcProperties.ts +++ b/src/native/AppleObjcProperties.ts @@ -91,36 +91,62 @@ const splitAttributes = ( }; }; -/** Decode `property_list_t { entsize_and_flags, count, property_t[] }`. */ +/** One property-level decode problem, relative to the list's owner. */ +interface PropertyListIssue { + readonly code: "property_decode_failed" | "property_attributes_incomplete"; + readonly message: string; + readonly location: string; +} + +/** + * Decode `property_list_t { entsize_and_flags, count, property_t[] }`. Each + * entry is decoded on its own, so one malformed entry is reported without + * discarding the properties decoded before or after it. + */ export const readObjcPropertyList = ( read: ObjcPropertyReaders, list: bigint, admit: () => boolean, ): { readonly properties: ObjcProperty[]; - /** Names of properties whose attribute string is incomplete. */ - readonly incomplete: string[]; + readonly issues: PropertyListIssue[]; } => { - const incomplete: string[] = []; - if (list === 0n) return { properties: [], incomplete }; + const properties: ObjcProperty[] = []; + const issues: PropertyListIssue[] = []; + if (list === 0n) return { properties, issues }; const entrySize = read.u32(list) & ~3; const count = read.u32(list + 4n); if (entrySize !== 16) throw new TypeError( `Unsupported Objective-C property entry size ${entrySize}`, ); - const properties: ObjcProperty[] = []; for (let index = 0; index < count; index++) { if (!admit()) break; const entry = list + 8n + BigInt(index * entrySize); - const decoded = decodePropertyAttributes( - read.string(read.pointer(entry)), - read.string(read.pointer(entry + 8n)), - ); - properties.push(decoded.property); - if (!decoded.complete) incomplete.push(decoded.property.name); + const location = `0x${entry.toString(16)}`; + try { + const name = read.string(read.pointer(entry)); + if (name.length === 0) throw new TypeError("the property name is empty"); + const decoded = decodePropertyAttributes( + name, + read.string(read.pointer(entry + 8n)), + ); + properties.push(decoded.property); + if (!decoded.complete) + issues.push({ + code: "property_attributes_incomplete", + message: `property ${name} has an unterminated quoted class name in its type attribute, so atomicity and unestablished read-only state are unknown`, + location, + }); + } catch (cause: unknown) { + issues.push({ + code: "property_decode_failed", + message: `property ${String(index)} could not be decoded: ${cause instanceof Error ? cause.message : String(cause)}`, + location, + }); + } } - return { properties, incomplete }; + return { properties, issues }; }; /** Decode a property list, recording a failure for `owner` instead of throwing. */ @@ -135,18 +161,18 @@ export const readObjcPropertiesOf = ( }, ): ObjcProperty[] => { try { - const { properties, incomplete } = readObjcPropertyList( + const { properties, issues } = readObjcPropertyList( read, list, context.admit, ); - for (const name of incomplete) + for (const item of issues) context.issues.push( issue( "objc_properties_categories", - "property_attributes_incomplete", - `${context.owner}.${name}: the type attribute has an unterminated quoted class name, so atomicity and unestablished read-only state are unknown`, - context.location, + item.code, + `${context.owner}: ${item.message}`, + item.location, ), ); return properties; diff --git a/tests/boundary/cli/nativeDispatchPropertyAttributes.test.ts b/tests/boundary/cli/nativeDispatchPropertyAttributes.test.ts index 51ab9dd4b..7aa614afc 100644 --- a/tests/boundary/cli/nativeDispatchPropertyAttributes.test.ts +++ b/tests/boundary/cli/nativeDispatchPropertyAttributes.test.ts @@ -3,54 +3,10 @@ import { join } from "node:path"; import { describe, expect } from "vitest"; +import { machoWithPropertyAttributes } from "../../fixtures/objcPropertyMacho.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { cliTest } from "../../support/cli/cliFixture.js"; -/** arm64 Mach-O whose two class properties share one attribute string. */ -const machoWithPropertyAttributes = (attributes: string): Buffer => { - const bytes = Buffer.alloc(4096); - const base = 0x100000000n; - const u32 = (at: number, value: number) => bytes.writeUInt32LE(value, at); - const pointer = (at: number, offset: number) => - bytes.writeBigUInt64LE(base + BigInt(offset), at); - u32(0, 0xfeedfacf); - u32(4, 0x0100000c); - u32(12, 2); - u32(16, 1); - u32(20, 152); - u32(32, 0x19); - u32(36, 152); - bytes.write("__DATA", 40); - pointer(56, 0); - bytes.writeBigUInt64LE(4096n, 64); - bytes.writeBigUInt64LE(4096n, 80); - u32(88, 5); - u32(92, 5); - u32(96, 1); - bytes.write("__objc_classlist", 104); - bytes.write("__DATA", 120); - pointer(136, 0x180); - bytes.writeBigUInt64LE(8n, 144); - u32(152, 0x180); - pointer(0x180, 0x200); - pointer(0x200 + 32, 0x300); - u32(0x300, 2); - u32(0x308, 16); - pointer(0x300 + 24, 0x400); - bytes.write("Fixture\0", 0x400); - pointer(0x340, 0x500); - u32(0x500, 16); - u32(0x504, 2); - pointer(0x508, 0x600); - pointer(0x510, 0x620); - pointer(0x518, 0x640); - pointer(0x520, 0x620); - bytes.write("first\0", 0x600); - bytes.write(`${attributes}\0`, 0x620); - bytes.write("second\0", 0x640); - return bytes; -}; - describe.skipIf(process.platform !== "darwin")( "native dispatch property attributes", () => { @@ -108,3 +64,51 @@ describe.skipIf(process.platform !== "darwin")( ); }, ); + +describe.skipIf(process.platform !== "darwin")( + "native dispatch property record isolation", + () => { + cliTest.for([ + [ + "an unmapped name pointer", + { secondNameOffset: 0x2000 }, + "first", + "0x100000518", + ], + ["an empty name", { firstName: "" }, "second", "0x100000508"], + ] as const)( + "retains the valid sibling of a property with $0", + async ([, layout, kept, location], { cli }) => { + const directory = await createTestTempDirectory("rea-objc-property-"); + const path = join(directory, "fixture.bin"); + await writeFile(path, machoWithPropertyAttributes("Tq,N", layout)); + const output = await cli.run({ + arguments: ["inspect-native-dispatch-metadata", path, "--json"], + environment: { REA_LOG_LEVEL: "silent" }, + }); + expect(output.exitCode).toBe(0); + expect(output.json).toMatchObject({ + normalized_result: { + result: { + objc_classes: [ + { + name: "Fixture", + properties: [{ name: kept, atomicity: "nonatomic" }], + }, + ], + coverage: expect.arrayContaining([ + expect.objectContaining({ + facet: "objc_properties_categories", + status: "partial", + reason: expect.stringContaining( + `property_decode_failed at ${location}: Fixture: property`, + ), + }), + ]), + }, + }, + }); + }, + ); + }, +); diff --git a/tests/fixtures/objcPropertyMacho.ts b/tests/fixtures/objcPropertyMacho.ts new file mode 100644 index 000000000..49bfe6674 --- /dev/null +++ b/tests/fixtures/objcPropertyMacho.ts @@ -0,0 +1,51 @@ +/** + * arm64 Mach-O with one Objective-C class, `Fixture`, whose two properties + * `first` and `second` share one attribute string. The first name may be + * replaced and the second name pointer may target an unmapped offset. + */ +export const machoWithPropertyAttributes = ( + attributes: string, + { firstName = "first", secondNameOffset = 0x640 } = {}, +): Buffer => { + const bytes = Buffer.alloc(4096); + const base = 0x100000000n; + const u32 = (at: number, value: number) => bytes.writeUInt32LE(value, at); + const pointer = (at: number, offset: number) => + bytes.writeBigUInt64LE(base + BigInt(offset), at); + u32(0, 0xfeedfacf); + u32(4, 0x0100000c); + u32(12, 2); + u32(16, 1); + u32(20, 152); + u32(32, 0x19); + u32(36, 152); + bytes.write("__DATA", 40); + pointer(56, 0); + bytes.writeBigUInt64LE(4096n, 64); + bytes.writeBigUInt64LE(4096n, 80); + u32(88, 5); + u32(92, 5); + u32(96, 1); + bytes.write("__objc_classlist", 104); + bytes.write("__DATA", 120); + pointer(136, 0x180); + bytes.writeBigUInt64LE(8n, 144); + u32(152, 0x180); + pointer(0x180, 0x200); + pointer(0x200 + 32, 0x300); + u32(0x300, 2); + u32(0x308, 16); + pointer(0x300 + 24, 0x400); + bytes.write("Fixture\0", 0x400); + pointer(0x340, 0x500); + u32(0x500, 16); + u32(0x504, 2); + pointer(0x508, 0x600); + pointer(0x510, 0x620); + pointer(0x518, secondNameOffset); + pointer(0x520, 0x620); + bytes.write(`${firstName}\0`, 0x600); + bytes.write(`${attributes}\0`, 0x620); + bytes.write("second\0", 0x640); + return bytes; +};