From faa5cb601aa70ff219481f7ee3781c3ea413441e Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Wed, 21 Oct 2026 00:15:04 -0500 Subject: [PATCH] Route defineProperty replacements of child methods through the child's box, and size the getter-only error to the name - The scan for properties JavaScript adds to children now also records `Reflect.defineProperty`, `Object.defineProperty` and `Object.defineProperties` on a child, and element writes (`child["kill"] = ...`). A key that is not a literal sends every member through the child's box. A typed call to a method replaced through `Reflect.defineProperty` then calls the replacement instead of the native method. - The "Cannot set property x of # which has only a getter" message is built at its exact length. Before, a name that overflowed the 160-byte buffer passed the untruncated length to the throw, which read past the buffer. - Corpus 6611 adds a module that replaces kill through Reflect.defineProperty and writes to a getter-only property with a 300-character name; 6610 and 6611 get their cost files. --- .../frontend/lowering/handle-properties.ts | 56 +++++++++++++++++-- .../compiler/src/frontend/lowering/lowerer.ts | 2 +- .../main.cjs.json | 1 + packages/runtime/src/scr_dyn_handle_props.c | 16 ++++-- .../definer.cjs | 25 +++++++++ .../main.cjs | 5 +- .../6610-npm-execa-child-mixin/main.ts.json | 1 + .../main.cjs.json | 2 +- 8 files changed, 94 insertions(+), 14 deletions(-) create mode 100644 tests/corpus/6611-child-properties-across-modules/definer.cjs create mode 100644 tests/harness/corpus-costs/6610-npm-execa-child-mixin/main.ts.json diff --git a/packages/compiler/src/frontend/lowering/handle-properties.ts b/packages/compiler/src/frontend/lowering/handle-properties.ts index 4a64fde79..4af4ca54d 100644 --- a/packages/compiler/src/frontend/lowering/handle-properties.ts +++ b/packages/compiler/src/frontend/lowering/handle-properties.ts @@ -102,8 +102,10 @@ const CHILD_MEMBERS: ReadonlySet = new Set([ ]); interface ChildAdditions { - /** Names JavaScript assigns onto typed children. */ + /** Names JavaScript assigns or defines onto typed children. */ written: ReadonlySet; + /** Whether a write or definition uses a key that is not a literal. */ + everyName: boolean; /** Whether the program can add properties to children at all. */ any: boolean; } @@ -123,21 +125,62 @@ function childAdditions(lowerer: Lowerer): ChildAdditions { // A child can reach any module (`module.exports = (child) => { child.x = // ... }`), so once some file loads child_process every JavaScript file's // writes onto children count. + // A key that is not a literal may name any member, so every member then + // goes through the child's box. + let everyName = false; + const isChild = (e: ts.Expression): boolean => + lowerer.mapTypeOf(lowerer.typeOf(e))?.kind === "child"; + const key = (k: ts.Expression | ts.PropertyName): void => { + if (ts.isIdentifier(k) || ts.isStringLiteralLike(k) || ts.isNumericLiteral(k)) + written.add(k.text); + else everyName = true; + }; for (const sf of spawns ? files : []) { const visit = (node: ts.Node): void => { if ( ts.isBinaryExpression(node) && node.operatorToken.kind === ts.SyntaxKind.EqualsToken && - ts.isPropertyAccessExpression(node.left) && - lowerer.mapTypeOf(lowerer.typeOf(node.left.expression))?.kind === "child" + (ts.isPropertyAccessExpression(node.left) || ts.isElementAccessExpression(node.left)) && + isChild(node.left.expression) ) { - written.add(node.left.name.text); + if (ts.isPropertyAccessExpression(node.left)) written.add(node.left.name.text); + else key(node.left.argumentExpression); + } + // `Object.defineProperty(child, "kill", { value })`, its Reflect + // twin, and `Object.defineProperties(child, { kill: ... })`. + if ( + ts.isCallExpression(node) && + ts.isPropertyAccessExpression(node.expression) && + ts.isIdentifier(node.expression.expression) && + (node.expression.expression.text === "Object" || + node.expression.expression.text === "Reflect") + ) { + const method = node.expression.name.text; + const [target, second] = node.arguments.nodes; + if (target && second && isChild(target)) { + if (method === "defineProperty") key(second); + else if (method === "defineProperties") { + if (!ts.isObjectLiteralExpression(second)) everyName = true; + else + for (const prop of second.properties.nodes) { + const name = prop.name; + if ( + name !== undefined && + (ts.isIdentifier(name) || + ts.isStringLiteralLike(name) || + ts.isNumericLiteral(name)) + ) + written.add(name.text); + else everyName = true; + } + } + } } ts.forEachChild(node, visit); }; visit(sf); } - known = { written, any: written.size > 0 || (spawns && defines) }; + known = { written, everyName, any: written.size > 0 || everyName || (spawns && defines) }; lowerer.childAdditions = known; return known; } @@ -158,7 +201,8 @@ function childReceiverOf( const typed = lowerer.mapTypeOf(lowerer.typeOf(access.expression))?.kind === "child"; if (!untyped && (!typed || !additions.any)) return null; const name = access.name.text; - if (!untyped && CHILD_MEMBERS.has(name) && !additions.written.has(name)) return null; + if (!untyped && CHILD_MEMBERS.has(name) && !additions.everyName && !additions.written.has(name)) + return null; const recv = tryLowerExpression(lowerer, access.expression); if (recv?.type.kind !== "child") return null; if (additions.any) lowerer.writesOntoChildren = true; diff --git a/packages/compiler/src/frontend/lowering/lowerer.ts b/packages/compiler/src/frontend/lowering/lowerer.ts index 1bc5fe277..543149f83 100644 --- a/packages/compiler/src/frontend/lowering/lowerer.ts +++ b/packages/compiler/src/frontend/lowering/lowerer.ts @@ -2167,7 +2167,7 @@ export class Lowerer { /** Whether lowered code writes properties onto children, and what the * program's JavaScript adds to them (handle-properties.ts). */ writesOntoChildren = false; - childAdditions: { written: ReadonlySet; any: boolean } | null = null; + childAdditions: { written: ReadonlySet; everyName: boolean; any: boolean } | null = null; /** Bindings whose unchecked string arithmetic can produce either a number * (NaN for an absent read) or a string. Preserve that result union at bare * reads instead of applying the checker-bare string narrowing adapter. */ diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6611-child-properties-across-modules/main.cjs.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6611-child-properties-across-modules/main.cjs.json index a3ba7f068..d4e0b110c 100644 --- a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6611-child-properties-across-modules/main.cjs.json +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6611-child-properties-across-modules/main.cjs.json @@ -2,6 +2,7 @@ "order": [ "/tests/corpus/6611-child-properties-across-modules/spawner.cjs", "/tests/corpus/6611-child-properties-across-modules/patcher.cjs", + "/tests/corpus/6611-child-properties-across-modules/definer.cjs", "/tests/corpus/6611-child-properties-across-modules/main.cjs" ], "diags": [] diff --git a/packages/runtime/src/scr_dyn_handle_props.c b/packages/runtime/src/scr_dyn_handle_props.c index 004d6c50f..c7de963f0 100644 --- a/packages/runtime/src/scr_dyn_handle_props.c +++ b/packages/runtime/src/scr_dyn_handle_props.c @@ -12,7 +12,6 @@ * child (the compiler's handle-properties.ts). */ #include "scr_runtime.h" -#include #include #include @@ -138,10 +137,17 @@ static bool hp_set(void *h, const char *key, size_t key_len, const ScrDyn *value /* An accessor's setter runs on the child. Without one the write throws * the strict-mode TypeError, as the other handle getters do. */ if (!entry->setter) { - char msg[160]; - int n = snprintf(msg, sizeof msg, "Cannot set property %.*s of # which has only a getter", - (int)key_len, key); - scr_throw_error_msg(SCR_ERR_TYPE, msg, n < 0 ? 0 : (size_t)n); + /* Built at its exact length: a property name has no length limit. */ + static const char head[] = "Cannot set property "; + static const char tail[] = " of # which has only a getter"; + size_t len = sizeof head - 1 + key_len + sizeof tail - 1; + char *msg = malloc(len); + if (!msg) scr_trap("scriptc: out of memory\n"); + memcpy(msg, head, sizeof head - 1); + memcpy(msg + sizeof head - 1, key, key_len); + memcpy(msg + sizeof head - 1 + key_len, tail, sizeof tail - 1); + scr_throw_error_msg(SCR_ERR_TYPE, msg, len); + free(msg); return true; } ScrDyn *setter = scr_dyn_retain(entry->setter); diff --git a/tests/corpus/6611-child-properties-across-modules/definer.cjs b/tests/corpus/6611-child-properties-across-modules/definer.cjs new file mode 100644 index 000000000..8b1c8f1a3 --- /dev/null +++ b/tests/corpus/6611-child-properties-across-modules/definer.cjs @@ -0,0 +1,25 @@ +"use strict"; +// Replaces kill through Reflect.defineProperty on a child it receives, and +// defines a getter-only property with a long name. +const start = require("./spawner.cjs"); +module.exports = function run() { + const child = start(); + const original = child.kill.bind(child); + const kill = { + value(signal) { + child.killedBy = "replaced " + signal; + return original(signal); + }, + configurable: true, + writable: true, + }; + Reflect.defineProperty(child, "kill", kill); + Reflect.defineProperty(child, "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", { get() { return 1; }, configurable: true }); + try { + child.xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx = 2; + } catch (e) { + console.log(e instanceof TypeError, e.message.length, e.message.slice(0, 30), e.message.slice(-40)); + } + child.on("close", (code) => console.log("close", code, child.killedBy)); + child.kill("SIGTERM"); +}; diff --git a/tests/corpus/6611-child-properties-across-modules/main.cjs b/tests/corpus/6611-child-properties-across-modules/main.cjs index 945707897..a3f422c64 100644 --- a/tests/corpus/6611-child-properties-across-modules/main.cjs +++ b/tests/corpus/6611-child-properties-across-modules/main.cjs @@ -1,8 +1,10 @@ "use strict"; // Properties a JavaScript module adds to a child it did not spawn: reads, // accessor calls on the child, a write to a getter-only accessor, and the -// replaced kill. +// replaced kill. definer.cjs replaces kill through Reflect.defineProperty +// and writes a getter-only property with a long name. const patched = require("./patcher.cjs"); +const definer = require("./definer.cjs"); const child = patched(); console.log(child.label); child.label = "x"; @@ -16,3 +18,4 @@ child.on("close", (code) => { console.log("close", code, child.killedBy); }); child.kill("SIGTERM"); +child.on("exit", () => definer()); diff --git a/tests/harness/corpus-costs/6610-npm-execa-child-mixin/main.ts.json b/tests/harness/corpus-costs/6610-npm-execa-child-mixin/main.ts.json new file mode 100644 index 000000000..bed55ed2c --- /dev/null +++ b/tests/harness/corpus-costs/6610-npm-execa-child-mixin/main.ts.json @@ -0,0 +1 @@ +{ "plain": 9021, "san": 6459 } diff --git a/tests/harness/corpus-costs/6611-child-properties-across-modules/main.cjs.json b/tests/harness/corpus-costs/6611-child-properties-across-modules/main.cjs.json index fd82b8987..741999ef6 100644 --- a/tests/harness/corpus-costs/6611-child-properties-across-modules/main.cjs.json +++ b/tests/harness/corpus-costs/6611-child-properties-across-modules/main.cjs.json @@ -1 +1 @@ -{ "plain": 5088, "san": 4845 } +{ "plain": 2689, "san": 4020 }