From ef8a7171a9297ab0aaffd64efdd0fcecb098e76f Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Sat, 10 Oct 2026 14:58:47 -0700 Subject: [PATCH] Borrow the bound receiver of optional chains An optional chain such as `resolved.members?.get(name)` took three references to its receiver: the owned receiver value, a +1 extraction into the bind slot, and a +1 copy for every chainRecv use, each released out of line (scr_map_release for maps). Now the bind slot borrows the payload whenever another owner outlives the body (the statement frame owns the receiver value), chainRecv reads are borrowable receivers and call arguments, and a receiver projected from a borrowable owner is not retained at all when the body preserves reference edges. Optional chains and chain receivers count as edge-preserving themselves; their children decide. Stack receivers, multi-arm unions and bodies that may remove an edge keep the owned forms. Corpus 4620 covers chains whose body clears or replaces the receiver field, drops the last other reference, nested and missing links, call and element receivers, self-clearing optional calls, and values that outlive later stores; a mutation that ignores the body check is caught by ASan (heap-use-after-free). --- packages/compiler/src/backend/llvm/emitter.ts | 15 +++ .../compiler/src/backend/llvm/expr-control.ts | 42 +++++- .../src/backend/llvm/reference-effects.ts | 4 + .../test/ts7/baselines/order-parity.json | 6 + .../4620-optional-chain-borrowed-receivers.ts | 125 ++++++++++++++++++ 5 files changed, 186 insertions(+), 6 deletions(-) create mode 100644 tests/corpus/4620-optional-chain-borrowed-receivers.ts diff --git a/packages/compiler/src/backend/llvm/emitter.ts b/packages/compiler/src/backend/llvm/emitter.ts index 8b9bb381d..c91461efd 100644 --- a/packages/compiler/src/backend/llvm/emitter.ts +++ b/packages/compiler/src/backend/llvm/emitter.ts @@ -5326,6 +5326,9 @@ export class LlEmitter { this.canBorrowCallArgument(value.then) && this.canBorrowCallArgument(value.else_) ); + // An optional chain's bound receiver stays unchanged until its body + // finishes (see emitReadReceiver). + if (value.kind === "chainRecv") return this.chainSlots.has(value.id); if (value.kind !== "varRef") return false; const binding = this.binding(value.localId); return ( @@ -7247,6 +7250,16 @@ export class LlEmitter { this.B.line(`${value} = load ptr, ptr ${binding.slot}`); return { name: value, type: e.type }; } + if (e.kind === "chainRecv") { + // The chain's bind slot holds its receiver unchanged until the body + // finishes, owned by the slot itself or by the chain's receiver. + const bound = this.chainSlots.get(e.id); + if (!bound) + throw new InternalCompilerError(`llvm emitter bug: chainRecv "${e.id}" outside its chain`); + const value = this.B.tmp(); + this.B.line(`${value} = load ${this.llType(bound.type)}, ptr ${bound.name}`); + return { name: value, type: e.type }; + } if (e.kind === "arrayGet" && this.canBorrowReceiver(e)) { // The array keeps the element alive until the consumer; a missing // element still traps exactly like the owned read. @@ -7499,6 +7512,8 @@ export class LlEmitter { canBorrowReceiver(e: IrExpr): boolean { switch (e.kind) { + case "chainRecv": + return this.chainSlots.has(e.id); case "varRef": { const binding = this.binding(e.localId); return ( diff --git a/packages/compiler/src/backend/llvm/expr-control.ts b/packages/compiler/src/backend/llvm/expr-control.ts index 4f0d29977..634671057 100644 --- a/packages/compiler/src/backend/llvm/expr-control.ts +++ b/packages/compiler/src/backend/llvm/expr-control.ts @@ -282,21 +282,51 @@ export function emitControlExpr( throw new InternalCompilerError("llvm emitter bug: optChain union arms"); const multiple = def.arms.length - unitTags.length > 1; const narrowed = multiple ? e.receiver.type : def.arms[narrowIdx]!; + // `owner.field?.member`: a receiver projected from a borrowable owner + // needs no reference of its own while the body cannot remove an edge + // (so nothing can overwrite the projection before the body finishes). + const borrowReceiver = + !multiple && + isRefCounted(narrowed) && + host.canBorrowReceiver(e.receiver) && + host.referenceEffects.preserves(e.body); // A single present arm is only tested and extracted (+1) before the // body runs, so a fresh receiver box can stay on the stack. - const r = - !multiple && host.canStackReceiver(e.receiver) - ? host.emitReadReceiver(e.receiver) - : host.emitExpr(e.receiver); + const stacked = !borrowReceiver && !multiple && host.canStackReceiver(e.receiver); + let r: LlValue; + const nullableField = borrowReceiver ? host.nullableFieldGet(e.receiver) : null; + if (nullableField !== null && e.receiver.kind === "fieldGet") { + // A nullable-pointer field stores no union: test and peek the + // pointer itself, through a private stack box when the union's own + // representation is boxed (emitUnionProjection's form). + const owner = host.emitReadReceiver(e.receiver.obj); + const { ptr } = host.classFieldPtr(owner.name, e.receiver.className, e.receiver.field); + const p = B.tmp(); + B.line(`${p} = load ptr, ptr ${ptr}${host.fieldAliasAttachment(ptr)}`); + r = { + name: host.nullableUnions.get(def.id) ? p : host.stackNullableBox(p, nullableField), + type: e.receiver.type, + }; + } else if (borrowReceiver || stacked) r = host.emitReadReceiver(e.receiver); + else r = host.emitExpr(e.receiver); + // The bind slot borrows a ref payload whenever something else keeps it + // alive until the body finishes: the borrowed projection above, or the + // receiver value the statement frame owns (emitExpr). Stack receivers + // may borrow their payload from a container, so they keep their +1. + const borrowBind = !multiple && !stacked && isRefCounted(narrowed); const bind = B.slot(); B.entryAllocas.push(`${bind} = alloca ${host.llType(narrowed)}`); B.line( `store ${host.llType(narrowed)} ${host.llType(narrowed) === "ptr" ? "null" : host.llType(narrowed) === "double" ? f64Lit(0) : "false"}, ptr ${bind}`, ); - host.ownSlot(bind, narrowed); + if (!borrowBind) host.ownSlot(bind, narrowed); const isUnit = host.tagInSet(r.name, def.id, unitTags); const extract = (): string => - multiple ? host.retainValue(r.name, narrowed) : host.unionExtract(r.name, def.id, narrowed); + multiple + ? host.retainValue(r.name, narrowed) + : borrowBind + ? host.unionPeek(r.name, def.id) + : host.unionExtract(r.name, def.id, narrowed); if (e.type.kind === "void") { // Statement form (cb?.()): no result value at all. const lb = B.newLabel("oc.b"); diff --git a/packages/compiler/src/backend/llvm/reference-effects.ts b/packages/compiler/src/backend/llvm/reference-effects.ts index a1e7dcb11..ba6521298 100644 --- a/packages/compiler/src/backend/llvm/reference-effects.ts +++ b/packages/compiler/src/backend/llvm/reference-effects.ts @@ -80,6 +80,10 @@ function expressionPreservesEdges( case "caughtCheck": // An immortal class object, or a fresh one retaining captured boxes. case "classRef": + // A chain evaluates its receiver and body (checked as children); its + // bound receiver is a read. + case "optChain": + case "chainRecv": return true; // Formatting a primitive allocates a string and runs no user code. case "toString": diff --git a/packages/compiler/test/ts7/baselines/order-parity.json b/packages/compiler/test/ts7/baselines/order-parity.json index 1c7788be6..32307ce8e 100644 --- a/packages/compiler/test/ts7/baselines/order-parity.json +++ b/packages/compiler/test/ts7/baselines/order-parity.json @@ -15748,6 +15748,12 @@ "/tests/corpus/4596-tdz-nullable-forward-capture.ts" ], "diags": [] + }, + "/tests/corpus/4620-optional-chain-borrowed-receivers.ts": { + "order": [ + "/tests/corpus/4620-optional-chain-borrowed-receivers.ts" + ], + "diags": [] } } } diff --git a/tests/corpus/4620-optional-chain-borrowed-receivers.ts b/tests/corpus/4620-optional-chain-borrowed-receivers.ts new file mode 100644 index 000000000..5dfeddb5a --- /dev/null +++ b/tests/corpus/4620-optional-chain-borrowed-receivers.ts @@ -0,0 +1,125 @@ +// An optional chain binds its receiver once. A receiver projected from a +// stable owner (`owner.field?.member`) is borrowed while the chain's body +// cannot remove a reference edge, and the bound value is borrowed by the body +// whenever the chain's receiver keeps it alive. Bodies that replace or clear +// the receiver's field, or drop the last other reference, must still see the +// value the chain bound first, exactly as Node does. + +class Sym { + flags: number; + name: string; + constructor(name: string, flags: number) { + this.name = name; + this.flags = flags; + } +} + +class Table { + members: Map | undefined = undefined; + first: Sym | undefined = undefined; + items: (Sym | undefined)[] = []; + next: Table | undefined = undefined; + onRead: ((s: string) => number) | undefined = undefined; + label(): string { + return `table(${this.members?.size ?? 0})`; + } +} + +class Owner { + table: Table | undefined; + constructor(table: Table | undefined) { + this.table = table; + } +} + +function lookup(t: Table, name: string): Sym | undefined { + const s = t.members?.get(name); + if (s !== undefined && s.flags !== 0) return s; + return undefined; +} + +function deep(o: Owner, name: string): number { + return o.table?.next?.members?.get(name)?.flags ?? -1; +} + +let cleared = 0; +function clearAndCount(o: Owner): number { + o.table = undefined; + cleared++; + return cleared; +} + +function replaceMembers(t: Table): Map { + const fresh = new Map(); + fresh.set("fresh", new Sym("fresh", 9)); + t.members = fresh; + return fresh; +} + +function makeTable(n: number): Table { + const t = new Table(); + const m = new Map(); + for (let i = 0; i < n; i++) m.set(`k${i}`, new Sym(`k${i}`, i)); + t.members = m; + t.first = new Sym("first", 1); + t.items = [new Sym("a", 1), undefined, new Sym("c", 3)]; + return t; +} + +const t = makeTable(4); +console.log(lookup(t, "k2")?.name, lookup(t, "k0")?.name, lookup(t, "zz")?.name); + +// The field is cleared by the body's argument: the chain still reads the map +// it bound first, and the map stays alive for the whole call. +const o = new Owner(t); +console.log(o.table?.members?.get(`k${clearAndCount(o)}`)?.name, o.table === undefined); + +// The body replaces the field the receiver was read from. +const t2 = makeTable(3); +console.log(t2.members?.get(replaceMembers(t2).has("fresh") ? "k1" : "k2")?.name, t2.members?.size); + +// The only other reference disappears inside the body. +const holder = new Owner(makeTable(2)); +const flags = holder.table?.first?.flags; +const read = holder.table?.items.map((s) => { + holder.table = undefined; + return s?.name ?? "-"; +}); +console.log(flags, read?.join(","), holder.table === undefined); + +// Nested chains over stable owners, including missing links. +const a = makeTable(2); +const b = makeTable(5); +a.next = b; +console.log(deep(new Owner(a), "k4"), deep(new Owner(b), "k4"), deep(new Owner(undefined), "k1")); + +// Chains over call results and element reads. +function current(): Table | undefined { + return a; +} +console.log(current()?.members?.get("k1")?.name, a.items[2]?.name, a.items[1]?.name ?? "hole"); + +// Optional calls through a field that the callback itself clears. +function callRead(t: Table, s: string): number | undefined { + return t.onRead?.(s); +} +const c = makeTable(1); +c.onRead = (s: string): number => { + c.onRead = undefined; + return s.length; +}; +console.log(callRead(c, "abc"), callRead(c, "abcd"), c.label()); + +// Values read through a chain outlive later stores to its receiver. +const d = makeTable(3); +const kept = d.members?.get("k2"); +d.members = undefined; +console.log(kept?.name, kept?.flags, lookup(d, "k2")); + +// A loop that exercises the borrowed lookup heavily. +let total = 0; +for (let i = 0; i < 2000; i++) { + const s = b.members?.get(`k${i % 7}`); + total += s?.flags ?? 100; +} +console.log(total);