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 #<ChildProcess> 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.
This commit is contained in:
Chris Tate
2026-10-21 00:15:04 -05:00
parent 00840aaca6
commit faa5cb601a
8 changed files with 94 additions and 14 deletions
@@ -102,8 +102,10 @@ const CHILD_MEMBERS: ReadonlySet<string> = new Set([
]);
interface ChildAdditions {
/** Names JavaScript assigns onto typed children. */
/** Names JavaScript assigns or defines onto typed children. */
written: ReadonlySet<string>;
/** 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;
@@ -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<string>; any: boolean } | null = null;
childAdditions: { written: ReadonlySet<string>; 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. */
@@ -2,6 +2,7 @@
"order": [
"<repo>/tests/corpus/6611-child-properties-across-modules/spawner.cjs",
"<repo>/tests/corpus/6611-child-properties-across-modules/patcher.cjs",
"<repo>/tests/corpus/6611-child-properties-across-modules/definer.cjs",
"<repo>/tests/corpus/6611-child-properties-across-modules/main.cjs"
],
"diags": []
+11 -5
View File
@@ -12,7 +12,6 @@
* child (the compiler's handle-properties.ts). */
#include "scr_runtime.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -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 #<ChildProcess> 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 #<ChildProcess> 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);
@@ -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");
};
@@ -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());
@@ -0,0 +1 @@
{ "plain": 9021, "san": 6459 }
@@ -1 +1 @@
{ "plain": 5088, "san": 4845 }
{ "plain": 2689, "san": 4020 }