diff --git a/tslang/docs/cross-module-dynamic-import-instanceof-design.md b/tslang/docs/cross-module-dynamic-import-instanceof-design.md index f6bfd4131..a406d0b45 100644 --- a/tslang/docs/cross-module-dynamic-import-instanceof-design.md +++ b/tslang/docs/cross-module-dynamic-import-instanceof-design.md @@ -1,11 +1,16 @@ # Cross-module `.instanceOf` resolution under `-shared`: design -Status: **investigated, NOT implemented** — three fix attempts this session, each -found to cause real regressions, all reverted. Written up for a future attempt. +Status: **FIXED** — implemented in a follow-up session (2026-07-22) exactly along +§9's recommendation, after the first session's three attempts (§4-§7) were all +reverted for regressions. All six formerly disabled `-shared` class-extends tests +(basic/multilevel/diamond × compile/JIT) now pass and are enabled; see §10 for +what the working fix actually was — three co-operating changes, two of them in +`DeclarationPrinter` and only found by fixing the first. §1-§9 below are the +original investigation write-up, kept intact as the map that made §10 possible. Follow-up to PR #274 (`Fix cross-module class extends crash and add regression -coverage`), whose commit message names this exact gap as a known, disabled issue. -All anchors below verified by code inspection and live `ctest` runs on -`main@89eb9869` during this investigation. +coverage`), whose commit message named this exact gap as a known, disabled issue. +All anchors in §1-§9 verified by code inspection and live `ctest` runs on +`main@89eb9869` during the original investigation. ## 1. The problem @@ -245,3 +250,71 @@ Whatever the mechanism, verify with the *full* `ctest` suite (not just the class-extends tests) before considering it done — both attempt-2 and attempt-3's regressions were invisible from the class-extends tests alone and only surfaced project-wide. + +## 10. The fix that worked (2026-07-22 session) + +§9's "dedicated, self-contained resolution path" taken to its logical +conclusion: don't register anything anywhere — resolve **in place**. Three +changes, each exposing the next once the previous error stopped masking it: + +1. **`ClassMethodAccess` inline dlsym fallback** (`MLIRGenImpl.h`, the + `isDynamicImport` instance branch). Resolution order is now: a FuncOp + **with a body** (locally defined, e.g. the importer's own synthesized + `.instanceOf` override — the bodiless-declaration case is explicitly + excluded, since it would lower to an unlinkable external symbol reference); + then the registered dlsym-global (statics/ctors/`.new`, the existing + mechanism); then, new: an inline + `SearchForAddressOfSymbolOp(funcName)` + cast, exactly the recipe + `mlirGenFunctionLikeDeclarationDynamicImport`'s initializer uses — but + emitted at the call site, with **no global registration at all**. This + sidesteps every §5 scope-fragility mode by construction: no + `fullNameGlobalsMap` interaction, valid in both discovery (ops land in the + throwaway module) and real passes; cost is one symbol lookup per call site. + The DLL is guaranteed loaded first because the import's + `LoadLibraryPermanentlyOp` ctor precedes all per-symbol resolution (same + ordering assumption `mlirGenImportSharedLib` already documents). This alone + made the basic test pass compile+link+run. + +2. **Class vtable slots owned by a dynamic-import base resolve at runtime** + (`MLIRGenClasses.cpp`, `mlirGenClassVirtualTableDefinition`). The vtable is + extends-recursive, so a derived class's vtable contains slots for inherited + members — with `ADD_STATIC_MEMBERS_TO_VTABLE`, that includes the base's + RTTI statics `.rtti`/`.size` — whose symbols live in the imported DLL. A + constant `SymbolRefOp` to those is a link error (`lld: undefined symbol: + M.Animal..size referenced by .data`): with no import library the address is + not a link-time constant. Such slots now emit + `SearchForAddressOfSymbolOp` + cast instead; `GlobalOpLowering` already + routes any initializer containing that op through the `__cctor` + global-constructor path, so no lowering changes were needed. Ownership is + determined structurally (walk self + transitive `baseClasses`, match the + exact `funcName`/`globalVariableName`), not by name-prefix guessing. + +3. **Two `DeclarationPrinter` bugs**, exposed only once the above let the + multilevel test (the first with `class B extends A` *inside* the exported + decl text) get far enough to parse/run: + - `print(ClassInfo::TypePtr)`'s extends clause printed + **`classType->fullName` instead of `baseClass->fullName`** — the loop + variable was never used, so the DLL's decl text said `class B extends + M.B`. The importer then built `B.baseClasses = [B]`, a self-cycle, and + `ClassInfo::getVirtualTable`'s unguarded recursion stack-overflowed the + compiler (0xC00000FD; root-caused via ProcDump + WinDbg on the dump — + every frame `getVirtualTable`). + - The class fields loop printed the **synthetic base-class storage field** + (a derived class's storage embeds each base's storage as a first field + whose id is the base's full name, `mlirGenClassHeritageClause`) as if it + were a source member: `M.A: [.vtbl:Opaque, a:number];`. The importer + parsed it as a real extra field, shifting every subsequent field's offset + — classic silent data corruption: the DLL's own methods read `b=22` + (correct) while the importer read `c.b == 0` (one slot past). Now + filtered by exact id match against `baseClasses[i]->fullName` (the + `extends` clause the importer re-processes reconstructs the same layout + itself). + +What was **not** touched, per §9: `fullNameGlobalsMap` scoping, the generic +`dynamicImport` branch's name computation, and `.instanceOf`'s missing +decorator (moot — the inline fallback makes the decorator unnecessary). All +six formerly-disabled tests (`test-{compile,jit}-shared-export-import-class-extends{,-multilevel,-implements-diamond}`) +enabled and green; full suite green (767 tests). The pre-existing +`getVirtualTable` unguarded recursion on a (now impossible via decl-text, but +still user-writable) cyclic extends chain remains a latent robustness gap — +out of scope here. diff --git a/tslang/lib/TypeScript/DeclarationPrinter.cpp b/tslang/lib/TypeScript/DeclarationPrinter.cpp index 0bf9df4c7..90fe9a7e9 100644 --- a/tslang/lib/TypeScript/DeclarationPrinter.cpp +++ b/tslang/lib/TypeScript/DeclarationPrinter.cpp @@ -364,12 +364,12 @@ namespace typescript auto any = false; for (auto baseClass : classType->baseClasses) { - if (any) + if (any) { os << ", "; } - os << classType->fullName; + os << baseClass->fullName; any = true; } } @@ -426,6 +426,21 @@ namespace typescript if (filterField(field.id)) continue; + // a derived class's storage embeds each base class's storage as a synthetic + // first field whose id is the base's full name (mlirGenClassHeritageClause); + // that is memory layout, not a source member - printing it would make the + // importer parse it as a real extra field, shifting every subsequent field's + // offset and silently corrupting cross-module field access (the `extends` + // clause printed above already carries the inheritance). + if (auto strId = dyn_cast(field.id)) + { + if (llvm::any_of(classType->baseClasses, + [&](auto &baseClass) { return strId.getValue() == baseClass->fullName; })) + { + continue; + } + } + os.indent(4); if (field.accessLevel == mlir_ts::AccessLevel::Protected) diff --git a/tslang/lib/TypeScript/MLIRGenClasses.cpp b/tslang/lib/TypeScript/MLIRGenClasses.cpp index 547310ec7..c25a44353 100644 --- a/tslang/lib/TypeScript/MLIRGenClasses.cpp +++ b/tslang/lib/TypeScript/MLIRGenClasses.cpp @@ -1734,26 +1734,80 @@ genContext); } else { + // The vtable is extends-recursive, so a derived class's vtable can + // contain slots whose symbols (inherited virtual methods, and - with + // ADD_STATIC_MEMBERS_TO_VTABLE - inherited static fields like the + // RTTI `.rtti`/`.size`) are owned by a base class that lives in a + // dynamically imported module. Those cannot be constant SymbolRefOp + // references: with no import library, the address is not a link-time + // constant. Resolve them at runtime instead (SearchForAddressOfSymbolOp + // + cast) - GlobalOpLowering already routes any initializer containing + // a SearchForAddressOfSymbolOp through the __cctor global-constructor + // path, and the module-load ctor is emitted before all per-symbol + // ctors, so the DLL is loaded by the time this resolves. + auto isOwnedByDynamicImport = [&](mlir::StringRef symbolName, bool isStaticField) { + std::function findOwner = + [&](ClassInfo::TypePtr cls) -> ClassInfo::TypePtr { + if (isStaticField + ? llvm::any_of(cls->staticFields, [&](auto &f) { return f.globalVariableName == symbolName; }) + : llvm::any_of(cls->methods, [&](auto &m) { return m.funcName == symbolName; })) + { + return cls; + } + + for (auto &base : cls->baseClasses) + { + if (auto owner = findOwner(base)) + { + return owner; + } + } + + return ClassInfo::TypePtr(); + }; + + auto owner = findOwner(newClassPtr); + return owner && owner->isDynamicImport; + }; + mlir::Value methodOrFieldNameRef; + mlir::StringRef symbolName; + mlir::Type slotType; if (!vtRecord.isStaticField) { if (vtRecord.methodInfo.isAbstract) { emitError(location) << "Abstract method '" << vtRecord.methodInfo.name << "' is not implemented in '" << newClassPtr->name << "'"; - return TypeValueInitType{mlir::Type(), mlir::Value(), TypeProvided::No}; + return TypeValueInitType{mlir::Type(), mlir::Value(), TypeProvided::No}; } - methodOrFieldNameRef = builder.create( - location, vtRecord.methodInfo.funcType, - mlir::FlatSymbolRefAttr::get(builder.getContext(), - vtRecord.methodInfo.funcName)); + symbolName = vtRecord.methodInfo.funcName; + slotType = vtRecord.methodInfo.funcType; + } + else + { + symbolName = vtRecord.staticFieldInfo.globalVariableName; + slotType = mlir_ts::RefType::get(vtRecord.staticFieldInfo.type); + } + + if (isOwnedByDynamicImport(symbolName, vtRecord.isStaticField)) + { + auto symbolNameValue = V(mlirGenStringValue(location, symbolName.str(), true)); + auto referenceToSymbolOpaque = builder.create( + location, getOpaqueType(), symbolNameValue); + auto castResult = cast(location, slotType, referenceToSymbolOpaque, genContext); + if (castResult.failed_or_no_value()) + { + return TypeValueInitType{mlir::Type(), mlir::Value(), TypeProvided::No}; + } + + methodOrFieldNameRef = V(castResult); } else { methodOrFieldNameRef = builder.create( - location, mlir_ts::RefType::get(vtRecord.staticFieldInfo.type), - mlir::FlatSymbolRefAttr::get(builder.getContext(), - vtRecord.staticFieldInfo.globalVariableName)); + location, slotType, + mlir::FlatSymbolRefAttr::get(builder.getContext(), symbolName)); } vtableValue = builder.create( diff --git a/tslang/lib/TypeScript/MLIRGenImpl.h b/tslang/lib/TypeScript/MLIRGenImpl.h index f133cf5b7..02e58637f 100644 --- a/tslang/lib/TypeScript/MLIRGenImpl.h +++ b/tslang/lib/TypeScript/MLIRGenImpl.h @@ -5446,32 +5446,60 @@ class MLIRGenImpl if (classInfo->isDynamicImport) { - // need to resolve global variable + // Direct (non-virtual) access to a dynamic-import class member - e.g. a + // cross-module `super.method(...)` call, or a non-virtual inherited method. + // Resolution order: // - // 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. + // 1. A FuncOp WITH a body: the method is actually defined in this module + // (compiler-synthesized methods like .instanceOf get real FuncOps even + // for isDynamicImport classes - see mlirGenClassInstanceOfMethod). Only + // a defined body qualifies: a bodyless declaration FuncOp would lower + // to a plain external symbol reference, which the dynamic import mode + // (-shared without an import .lib) cannot link. if (auto funcOp = theModule.lookupSymbol(funcName)) { - auto thisSymbOp = builder.create( - location, getBoundFunctionType(effectiveFuncType), effectiveThisValue, - mlir::FlatSymbolRefAttr::get(builder.getContext(), funcName)); - return thisSymbOp; + if (!funcOp.getBody().empty()) + { + auto thisSymbOp = builder.create( + location, getBoundFunctionType(effectiveFuncType), effectiveThisValue, + mlir::FlatSymbolRefAttr::get(builder.getContext(), funcName)); + return thisSymbOp; + } } + // 2. The dlsym-style global variable mlirGenClassMethodMemberDynamicImport / + // mlirGenFunctionLikeDeclarationDynamicImport registered for @dllimport + // members (statics/constructors/.new today). auto globalFuncVar = resolveFullNameIdentifier(location, funcName, false, genContext); if (!globalFuncVar) { - emitError(location, "Class member '") << funcName << "' can't be resolved (dynamic import)"; - return mlir::Value(); + // 3. Inline dlsym. Compiler-synthesized methods (.instanceOf) and plain + // instance methods of an imported class have neither of the above: no + // per-member @dllimport decorator ever routes them through the + // registration path, and their FuncOp (when one exists at all) is a + // bodyless declaration. Registering a global lazily from HERE is not + // an option either - this can run inside a transient discovery scope + // ("simulate scope"), where a fullNameGlobalsMap registration is torn + // down with the scope, or worse trips its LIFO assert; see + // docs/cross-module-dynamic-import-instanceof-design.md §5-§7 for the + // two reverted attempts. So resolve the symbol in place, exactly like + // the registered variant's initializer does + // (mlirGenFunctionLikeDeclarationDynamicImport): the DLL is already + // loaded by the import's LoadLibraryPermanentlyOp global ctor by the + // time any method body runs. Self-contained: no global state, valid + // in both discovery (ops land in the throwaway module) and real + // passes, at the cost of one symbol lookup per call site. + auto symbolNameValue = V(mlirGenStringValue(location, funcName.str(), true)); + auto referenceToFuncOpaque = builder.create( + location, getOpaqueType(), symbolNameValue); + auto castResult = cast(location, effectiveFuncType, referenceToFuncOpaque, genContext); + if (castResult.failed_or_no_value()) + { + emitError(location, "Class member '") << funcName << "' can't be resolved (dynamic import)"; + return mlir::Value(); + } + + globalFuncVar = V(castResult); } CAST_A(opaqueThisValue, location, getOpaqueType(), effectiveThisValue, genContext); diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index c1b6d91e5..0debe9e6b 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -865,30 +865,19 @@ 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") +# Cross-module `class extends` across a real DLL boundary. Formerly disabled as a +# known issue; fixed by three co-operating changes (see +# docs/cross-module-dynamic-import-instanceof-design.md for the full history): +# ClassMethodAccess resolves unregistered dynamic-import members via inline +# SearchForAddressOfSymbolOp (no fragile global registration), the class vtable +# builder emits runtime symbol resolution for slots owned by a dynamic-import base +# (a link-time address of a DLL-resident symbol does not exist), and +# DeclarationPrinter no longer prints a wrong extends target (own name instead of +# the base's) nor the synthetic base-class storage field (which shifted every +# subsequent field's offset in the importer). +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") @@ -915,10 +904,9 @@ 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-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")