From a2e96fbe81f9b4fb29160b7f169df74b01e15ca4 Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Tue, 21 Jul 2026 20:59:31 +0100 Subject: [PATCH] Fix cross-module class extends crash and add regression coverage ClassMethodAccess's isDynamicImport branches (called for a class whose base lives in another module) fed an unchecked, possibly-null Value into CreateBoundFunctionOp when the target method's global variable couldn't be resolved by name - an intermittent, heap-layout-sensitive access violation (masked whenever a debugger happened to be attached, which made it look like a flaky Heisenbug). Adds a null-check that turns the failure into a clean compile error, plus a theModule.lookupSymbol fallback since compiler- synthesized methods (e.g. .instanceOf) are registered as real FuncOps rather than the dlsym-style global variables regular imported methods use. Adds the project's first regression coverage for cross-module class inheritance: basic 2-level extends, a 3-level chain, and extends+implements combined. The non-shared-lib variants pass and are enabled; the -shared (AOT/JIT) variants hit a separate, deeper pre-existing bug in when .instanceOf gets synthesized for a class compiled across a real DLL boundary - left disabled with a detailed comment rather than patched further, since two targeted fix attempts each introduced a worse regression (an infinite loop in unrelated same-module class tests). Co-Authored-By: Claude Sonnet 5 --- tslang/lib/TypeScript/MLIRGenImpl.h | 37 +++++++++++++++-- tslang/test/tester/CMakeLists.txt | 31 ++++++++++++++ .../test/tester/tests/export_class_extends.ts | 19 +++++++++ ...export_class_extends_implements_diamond.ts | 29 +++++++++++++ .../tests/export_class_extends_multilevel.ts | 34 +++++++++++++++ .../test/tester/tests/import_class_extends.ts | 27 ++++++++++++ ...import_class_extends_implements_diamond.ts | 41 +++++++++++++++++++ .../tests/import_class_extends_multilevel.ts | 37 +++++++++++++++++ 8 files changed, 252 insertions(+), 3 deletions(-) create mode 100644 tslang/test/tester/tests/export_class_extends.ts create mode 100644 tslang/test/tester/tests/export_class_extends_implements_diamond.ts create mode 100644 tslang/test/tester/tests/export_class_extends_multilevel.ts create mode 100644 tslang/test/tester/tests/import_class_extends.ts create mode 100644 tslang/test/tester/tests/import_class_extends_implements_diamond.ts create mode 100644 tslang/test/tester/tests/import_class_extends_multilevel.ts diff --git a/tslang/lib/TypeScript/MLIRGenImpl.h b/tslang/lib/TypeScript/MLIRGenImpl.h index 04bd81096..f133cf5b7 100644 --- a/tslang/lib/TypeScript/MLIRGenImpl.h +++ b/tslang/lib/TypeScript/MLIRGenImpl.h @@ -5353,13 +5353,18 @@ class MLIRGenImpl { // need to resolve global variable auto globalFuncVar = resolveFullNameIdentifier(location, funcName, false, genContext); + if (!globalFuncVar) + { + emitError(location, "Class member '") << funcName << "' can't be resolved (dynamic import)"; + return mlir::Value(); + } if (!isThisValueClassRef) { CAST_A(opaqueThisValue, location, getOpaqueType(), thisValue, genContext); auto boundMethodValue = builder.create( - location, getBoundFunctionType(effectiveFuncType), opaqueThisValue, globalFuncVar); - return boundMethodValue; + location, getBoundFunctionType(effectiveFuncType), opaqueThisValue, globalFuncVar); + return boundMethodValue; } return globalFuncVar; @@ -5442,7 +5447,33 @@ class MLIRGenImpl if (classInfo->isDynamicImport) { // need to resolve global variable + // + // Not every method of an isDynamicImport class is actually + // registered as a dlsym-style global variable - a + // compiler-synthesized method (e.g. .instanceOf, ForceVirtual, + // see mlirGenClassInstanceOfMethod) never carries its own + // @dllimport decorator (that's only ever attached to + // source-declared methods reprinted under `@dllimport class + // ... { ... }`), so mlirGenFunctionLikeDeclaration's decorator + // check never routes it through + // mlirGenFunctionLikeDeclarationDynamicImport - it gets a real + // (bodyless-for-a-declaration) FuncOp registered directly + // instead, just like a same-module method. Try that first. + if (auto funcOp = theModule.lookupSymbol(funcName)) + { + auto thisSymbOp = builder.create( + location, getBoundFunctionType(effectiveFuncType), effectiveThisValue, + mlir::FlatSymbolRefAttr::get(builder.getContext(), funcName)); + return thisSymbOp; + } + auto globalFuncVar = resolveFullNameIdentifier(location, funcName, false, genContext); + if (!globalFuncVar) + { + emitError(location, "Class member '") << funcName << "' can't be resolved (dynamic import)"; + return mlir::Value(); + } + CAST_A(opaqueThisValue, location, getOpaqueType(), effectiveThisValue, genContext); auto boundMethodValue = builder.create( location, getBoundFunctionType(effectiveFuncType), opaqueThisValue, globalFuncVar); @@ -5457,7 +5488,7 @@ class MLIRGenImpl return thisSymbOp; } } - } + } mlir::Value ClassGenericMethodAccess(ClassInfo::TypePtr classInfo, mlir::Location location, mlir::Value thisValue, int genericMethodIndex, diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index cf5ca67f8..c1b6d91e5 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -847,6 +847,9 @@ add_test(NAME test-compile-include-global-var COMMAND test-runner "${PROJECT_SOU # imports support only compile mode add_test(NAME test-compile-import-component COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/component.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/service.ts") add_test(NAME test-compile-export-import-class-interface COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_interface.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_interface.ts") +add_test(NAME test-compile-export-import-class-extends COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends.ts") +add_test(NAME test-compile-export-import-class-extends-multilevel COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends_multilevel.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends_multilevel.ts") +add_test(NAME test-compile-export-import-class-extends-implements-diamond COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends_implements_diamond.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends_implements_diamond.ts") # shared libs tests (dlls/compile-time) add_test(NAME test-compile-time-shared-decl-emit-type COMMAND test-runner -shared -compile-time "${PROJECT_SOURCE_DIR}/test/tester/tests/emit_compiletime_func.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/decl_type.ts") @@ -862,6 +865,30 @@ add_test(NAME test-compile-shared-decl-emit-class COMMAND test-runner -shared "$ # shared libs tests (exports/imports) add_test(NAME test-compile-shared-export-import-class-interface COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_interface.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_interface.ts") add_test(NAME test-compile-shared-export-import-object-literal-with-class-types COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_with_class_types.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_with_class_types.ts") +# KNOWN ISSUE (pre-existing, not a regression from the crash fix below): a +# derived class whose base class lives in another module ("shared lib" / +# cross-module `class extends`) does not reliably work end-to-end through +# test-runner's actual -shared invocation (no explicit --shared-libs for the +# imported DLL; the import statement's own auto-load is all that's used) - +# AOT fails to link (missing dllimport linkage on the compiler-synthesized +# `.instanceOf` method every class gets) and JIT fails to even compile +# (`.instanceOf` sometimes never gets synthesized/registered at all - a +# discovery/partial-resolve pass ordering bug: it depends sensitively on +# exact compile-time sequencing, e.g. a manual repro that adds an extra +# --shared-libs flag "accidentally" avoids it). Two attempts at a targeted +# fix (deferring synthesis until a non-speculative pass) were reverted after +# each caused an INFINITE LOOP in unrelated same-module class tests (a +# same-module class's own discovery retry loop can also always run under +# allowPartialResolve, so deferring "until a real pass" can mean forever) - +# worse than the original clean compile error, so left as a known issue +# rather than risk that regression again. What IS fixed and verified: the +# crash (access violation) that used to occur instead of this clean error - +# see MLIRGenFunctions.cpp's mlirGenFunctionLikeDeclarationDynamicImport +# (registration name/map fix) and MLIRGenImpl.h's ClassMethodAccess +# (FuncOp-or-variable dual lookup with a null-check backstop). +# add_test(NAME test-compile-shared-export-import-class-extends COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends.ts") +# add_test(NAME test-compile-shared-export-import-class-extends-implements-diamond COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends_implements_diamond.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends_implements_diamond.ts") +# add_test(NAME test-compile-shared-export-import-class-extends-multilevel COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends_multilevel.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends_multilevel.ts") add_test(NAME test-compile-shared-export-import-object-literal-with-interface COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_with_interface.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_with_interface.ts") add_test(NAME test-compile-shared-export-import-object-literal-untyped COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_untyped.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_untyped.ts") add_test(NAME test-compile-shared-export-import-object-literal-untyped-multi-method COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_untyped_multi_method.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_untyped_multi_method.ts") @@ -888,6 +915,10 @@ add_test(NAME test-jit-shared-decl-emit-class COMMAND test-runner -jit -shared " # shared libs tests (exports/imports) add_test(NAME test-jit-shared-export-import-class-interface COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_interface.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_interface.ts") add_test(NAME test-jit-shared-export-import-object-literal-with-class-types COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_with_class_types.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_with_class_types.ts") +# KNOWN ISSUE - see the matching commented-out compile-shared entries above. +# add_test(NAME test-jit-shared-export-import-class-extends COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends.ts") +# add_test(NAME test-jit-shared-export-import-class-extends-multilevel COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends_multilevel.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends_multilevel.ts") +# add_test(NAME test-jit-shared-export-import-class-extends-implements-diamond COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends_implements_diamond.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends_implements_diamond.ts") add_test(NAME test-jit-shared-export-import-object-literal-with-interface COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_with_interface.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_with_interface.ts") add_test(NAME test-jit-shared-export-import-object-literal-untyped COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_untyped.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_untyped.ts") add_test(NAME test-jit-shared-export-import-object-literal-untyped-multi-method COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_untyped_multi_method.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_untyped_multi_method.ts") diff --git a/tslang/test/tester/tests/export_class_extends.ts b/tslang/test/tester/tests/export_class_extends.ts new file mode 100644 index 000000000..b1ea05ea3 --- /dev/null +++ b/tslang/test/tester/tests/export_class_extends.ts @@ -0,0 +1,19 @@ +namespace M { + + // Base class exported so another module can `extends` it - covers the + // cross-module analogue of 00class_super.ts's same-module class extends, + // which was never verified across the module boundary (unlike interface + // extends, which got dedicated cross-module coverage in #268-#270). + + export class Animal { + name: string; + + constructor(name: string) { + this.name = name; + } + + speak(): string { + return `${this.name} makes a noise.`; + } + } +} diff --git a/tslang/test/tester/tests/export_class_extends_implements_diamond.ts b/tslang/test/tester/tests/export_class_extends_implements_diamond.ts new file mode 100644 index 000000000..430ae52f5 --- /dev/null +++ b/tslang/test/tester/tests/export_class_extends_implements_diamond.ts @@ -0,0 +1,29 @@ +namespace M { + + // A class can only `extends` one class, but can `implements` several + // interfaces - this is the class-shaped equivalent of the interface + // diamond coverage (Combined extends Left, Right, #268): a cross-module + // base class plus two cross-module interfaces, both satisfied by one + // most-derived class in the importer. Exercises the same per-object + // vtable-patching machinery from a different angle (class vtable slots + // for inherited methods interleaved with interface vtable slots for + // implemented methods). + + export class Base { + base: number = 1; + + addBase(n: number): void { + this.base = this.base + n; + } + } + + export interface Left { + left: number; + addLeft(n: number): void; + } + + export interface Right { + right: number; + addRight(n: number): void; + } +} diff --git a/tslang/test/tester/tests/export_class_extends_multilevel.ts b/tslang/test/tester/tests/export_class_extends_multilevel.ts new file mode 100644 index 000000000..97db7d7c3 --- /dev/null +++ b/tslang/test/tester/tests/export_class_extends_multilevel.ts @@ -0,0 +1,34 @@ +namespace M { + + // 3-level chain with the first two levels defined (and already linked to + // each other) in the exporting module, so the importer's further-derived + // class must extend a base whose OWN vtable was built across a module + // boundary already - this is the class analogue of the cross-module + // interface multilevel coverage (extends_interface_multilevel.ts), which + // found real vtable-offset bugs when a 2nd/3rd extends target's methods + // were mis-patched. + + export class A { + a: number = 1; + + addA(n: number): void { + this.a = this.a + n; + } + + describe(): string { + return `A:${this.a}`; + } + } + + export class B extends A { + b: number = 2; + + addB(n: number): void { + this.b = this.b + n; + } + + describe(): string { + return `B:${this.b}/${super.describe()}`; + } + } +} diff --git a/tslang/test/tester/tests/import_class_extends.ts b/tslang/test/tester/tests/import_class_extends.ts new file mode 100644 index 000000000..c46b1f515 --- /dev/null +++ b/tslang/test/tester/tests/import_class_extends.ts @@ -0,0 +1,27 @@ +import './export_class_extends' + +class Dog extends M.Animal { + constructor(name: string) { + super(name); + } + + speak(): string { + return `${this.name} barks.`; + } +} + +function main() { + const a = new M.Animal("Generic"); + assert(a.speak() == "Generic makes a noise."); + + const d = new Dog("Mitzie"); + assert(d.name == "Mitzie"); + assert(d.speak() == "Mitzie barks."); + + // virtual dispatch through the base-class-typed reference must still + // resolve to the cross-module-derived override + const asBase: M.Animal = d; + assert(asBase.speak() == "Mitzie barks."); + + print("done."); +} diff --git a/tslang/test/tester/tests/import_class_extends_implements_diamond.ts b/tslang/test/tester/tests/import_class_extends_implements_diamond.ts new file mode 100644 index 000000000..4102618d6 --- /dev/null +++ b/tslang/test/tester/tests/import_class_extends_implements_diamond.ts @@ -0,0 +1,41 @@ +import './export_class_extends_implements_diamond' + +class Combined extends M.Base implements M.Left, M.Right { + left: number = 2; + right: number = 3; + + addLeft(n: number): void { + this.left = this.left + n; + } + + addRight(n: number): void { + this.right = this.right + n; + } +} + +function main() { + const c = new Combined(); + + c.addBase(10); + c.addLeft(20); + c.addRight(30); + + assert(c.base == 11); + assert(c.left == 22); + assert(c.right == 33); + + // cast to each cross-module ancestor/interface type and confirm the + // field/method resolves through the correct (non-corrupted) slot + const asBase: M.Base = c; + assert(asBase.base == 11); + + const asLeft: M.Left = c; + asLeft.addLeft(100); + assert(c.left == 122); + + const asRight: M.Right = c; + asRight.addRight(100); + assert(c.right == 133); + + print("done."); +} diff --git a/tslang/test/tester/tests/import_class_extends_multilevel.ts b/tslang/test/tester/tests/import_class_extends_multilevel.ts new file mode 100644 index 000000000..e5e4ee589 --- /dev/null +++ b/tslang/test/tester/tests/import_class_extends_multilevel.ts @@ -0,0 +1,37 @@ +import './export_class_extends_multilevel' + +class C extends M.B { + c: number = 3; + + addC(n: number): void { + this.c = this.c + n; + } + + describe(): string { + return `C:${this.c}/${super.describe()}`; + } +} + +function main() { + const c = new C(); + + c.addA(10); + c.addB(20); + c.addC(30); + + assert(c.a == 11); + assert(c.b == 22); + assert(c.c == 33); + + assert(c.describe() == "C:33/B:22/A:11"); + + // virtual dispatch through each ancestor-typed reference must still + // reach the most-derived override + const asB: M.B = c; + assert(asB.describe() == "C:33/B:22/A:11"); + + const asA: M.A = c; + assert(asA.describe() == "C:33/B:22/A:11"); + + print("done."); +}