Skip to content

Commit c25745e

Browse files
Fix hybrid-func-to-func cast crash when casting cross-module tuple to interface
castTypeScriptTypes handled narrowing a BoundFunctionType value to a plain FunctionType (the "losing this reference" vtable-slot path) but not the equivalent HybridFunctionType case - the actual type of a method field read off a cross-module reconstructed tuple. That value fell through to castLLVMTypes' dead "value to ref of value" branch and hit llvm_unreachable("review usage") at CastLogicHelper.h:765, crashing the compiler on `<Counter>importedObj` in the importing module. Add the missing branch, mirroring the BoundFunctionType case: extract the raw function pointer via GetMethodOp and warn about the dropped receiver (the interface vtable call re-supplies its own thisVal). Note: this fixes the compile-time crash only. Verifying end-to-end surfaced a separate pre-existing bug - the object clone built for the interface cast writes its fields in reversed order, so the vtable's field offsets read the wrong slots at runtime. Recorded in docs/interface-vtable-simplification-design.md for a follow-up investigation; no regression test added since the full scenario still fails on that second bug. Full ctest suite: 720/720 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent b11b684 commit c25745e

2 files changed

Lines changed: 47 additions & 0 deletions

File tree

tslang/docs/interface-vtable-simplification-design.md

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,3 +399,39 @@ its definition site, `export var counter: Counter = {...}`, so the cast
399399
happens in the exporting module where a local `funcOp` genuinely exists)
400400
remains the only currently-working way to share a method-bearing object
401401
across modules, and is unaffected by any of the above.
402+
403+
### Bug 2 crash fixed, but a third bug blocks full correctness (2026-07-19)
404+
405+
The `CastLogicHelper.h:765` `llvm_unreachable("review usage")` from bug 2
406+
above is fixed: `castTypeScriptTypes`'s `resFuncType`/`BoundFunctionType`
407+
branch (the "losing this reference" case, used when a bound method value is
408+
narrowed to a plain function pointer for a vtable slot) was missing the
409+
equivalent `HybridFunctionType` case - the actual type of a method field
410+
read off a cross-module reconstructed tuple. Added the missing branch,
411+
mirroring the existing `BoundFunctionType` one exactly (same `GetMethodOp`
412+
extraction, same "losing this reference" warning, same rationale: an
413+
interface vtable call re-supplies its own `thisVal`, so the value's own
414+
captured/bound `this` is safe to drop).
415+
416+
This alone does **not** make the scenario work end-to-end. Verifying it
417+
with a real repro (`counterObj: {count,inc}` exported untyped-as-interface,
418+
cast to `Counter` in the importer, called via `-shared` compile+link+run,
419+
confirmed with WinDbg breakpoints on the actual call site and live
420+
register/memory inspection) surfaced a **third, independent, pre-existing
421+
bug**: the object clone built when casting the cross-module tuple VALUE to
422+
an interface (the one behind the "Cloned object is used" warning) writes
423+
its two fields in the wrong order - `inc`'s function pointer lands at
424+
offset 0 and `count` at offset 8, reversed from the tuple's actual declared
425+
layout (`struct<(f64, ptr)>`, count@0/inc@8) that the vtable's field-offset
426+
slot for `count` is computed against. Net effect at runtime: `c.inc()`
427+
doesn't crash, but silently corrupts the `inc` funcptr slot instead of
428+
incrementing `count` (confirmed: `rax`/`rcx` and `dq rcx L2` at the call
429+
site show `[thisVal+0] = <inc's real address>`, `[thisVal+8] = 0`, exactly
430+
backwards from what the vtable's `count` offset assumes). This is a
431+
different code path than `CastLogicHelper.h` - almost certainly in
432+
`MLIRGenCast.cpp`'s `castObjectToInterface` object-cloning logic (wherever
433+
the "Cloned object is used" warning is emitted) - and is a separate,
434+
not-yet-investigated bug. No regression test was added for the
435+
now-fixed crash (a full end-to-end test would still fail, on this new bug,
436+
not the old one); this section records the fix and the newly-found blocker
437+
for whoever picks up the clone-bug investigation next.

tslang/include/TypeScript/LowerToLLVM/CastLogicHelper.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,17 @@ class CastLogicHelper
241241
*/
242242
return rewriter.create<mlir_ts::GetMethodOp>(loc, resFuncType, in);
243243
}
244+
245+
if (auto inHybridFunc = dyn_cast<mlir_ts::HybridFunctionType>(inType))
246+
{
247+
// same rationale as the BoundFunctionType case above: a plain FunctionType
248+
// value carries no slot for a bound `this`, so the receiver is dropped here
249+
// and re-supplied by the caller (e.g. an interface vtable call passes its own
250+
// thisVal) - this is the vtable-method-pointer path for a cross-module tuple
251+
// value cast to a method-bearing interface.
252+
op->emitWarning("losing this reference");
253+
return rewriter.create<mlir_ts::GetMethodOp>(loc, resFuncType, in);
254+
}
244255
}
245256

246257
if (auto resHybridFunc = dyn_cast<mlir_ts::HybridFunctionType>(resType))

0 commit comments

Comments
 (0)