Fix cross-module diamond-extends interface silently corrupting data

tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h

@@ -288,18 +288,40 @@ struct InterfaceInfo
 
     // own methods plus every inherited method from `extends`, in the same
     // extends-first-then-own order as getTupleTypeFields/getVirtualTable -
-    // needed wherever code must patch/visit every method slot a cast target
-    // could own, not just this interface's own directly-declared ones.
-    void getAllMethods(llvm::SmallVector<InterfaceMethodInfo *> &allMethods)
+    // paired with the combined vtable slot each one actually occupies IN THIS
+    // (possibly multi-level/diamond `extends`) interface, since
+    // method.virtualIndex is only correct standalone (see findField's doc
+    // comment above); a method inherited through `extends`
+    // needs vtableOffset (the declaring interface's slot-block position) added to
+    // it. Needed by mlirGenCreateInterfaceVTableForObject's per-object patch loop,
+    // which indexes directly into ONE combined vtable for the cast's root
+    // interface - using method.virtualIndex alone there mis-patches every
+    // inherited method beyond the first `extends` target (harmless no-op for a
+    // single `extends` chain, where each level's own slots start where the
+    // previous level's ended and vtableOffset is cumulative from index 0 - but
+    // wrong for a diamond, where a second/third `extends` target's methods all
+    // stack at vtableOffset 0 relative to themselves).
+    void getAllMethodsWithVTableOffset(llvm::SmallVector<std::pair<InterfaceMethodInfo *, int>> &allMethods, int baseOffset = 0)
     {
+        auto offset = baseOffset;
         for (auto &extent : extends)
         {
-            std::get<1>(extent)->getAllMethods(allMethods);
+            std::get<1>(extent)->getAllMethodsWithVTableOffset(allMethods, offset);
+            offset += std::get<1>(extent)->getVTableSize();
         }
 
+        // own methods' virtualIndex (assignCanonicalVirtualIndexes()) is already
+        // absolute WITHIN THIS interface's own combined vtable (it starts counting
+        // after this interface's own extends block) - so at THIS level baseOffset
+        // is 0 and no further adjustment is needed. It's only when THIS interface
+        // is itself reached as an `extends` target (the recursive call above,
+        // baseOffset != 0) that its methods - own AND inherited alike, all already
+        // pushed into `allMethods` by this point - need the caller's baseOffset,
+        // which the recursive call already applied for inherited ones; own ones
+        // get it here via baseOffset.
         for (auto &method : methods)
         {
-            allMethods.push_back(&method);
+            allMethods.push_back({&method, baseOffset});
         }
     }
 

tslang/lib/TypeScript/MLIRGenInterfaces.cpp

@@ -201,8 +201,15 @@ namespace mlirgen
             // same as an own one; only checking newInterfacePtr->methods here left every
             // inherited method's slot holding its initial offset-placeholder value
             // (never a real function pointer), crashing on the first call through it.
-            llvm::SmallVector<InterfaceMethodInfo *> allMethods;
-            newInterfacePtr->getAllMethods(allMethods);
+            // Paired with each method's actual combined-vtable slot
+            // (method.virtualIndex + vtableOffset): virtualIndex alone is only correct
+            // standalone, same caveat as InterfaceInfo::findField's doc comment
+            // (MLIRGenStore.h) - a method inherited from a SECOND OR LATER `extends`
+            // target (diamond `extends A, B`) has virtualIndex relative to its OWN
+            // declaring interface, which collides with the FIRST target's slots unless
+            // offset by where that target's own block starts in the combined vtable.
+            llvm::SmallVector<std::pair<InterfaceMethodInfo *, int>> allMethods;
+            newInterfacePtr->getAllMethodsWithVTableOffset(allMethods);
             if (allMethods.size() > 0) {
 
                 mlir_ts::TupleType storeType;
@@ -226,8 +233,8 @@ namespace mlirgen
                 // imported object type reconstructed from a @dllimport declaration, with no
                 // local funcOp to name) still need their function pointer read out of the
                 // actual object `in` at cast time.
-                llvm::SmallVector<InterfaceMethodInfo *> methodsNeedingPatch;
-                for (auto* methodPtr : allMethods)
+                llvm::SmallVector<std::pair<InterfaceMethodInfo *, int>> methodsNeedingPatch;
+                for (auto& [methodPtr, vtableOffset] : allMethods)
                 {
                     auto& method = *methodPtr;
                     auto fieldId = builder.getStringAttr(method.name);
@@ -240,7 +247,7 @@ namespace mlirgen
                     auto fieldInfo = mth.getFieldInfoByIndex(storeType, index);
                     if (lookupObjectLiteralMethodSymbol(fieldInfo.type, fieldId).empty())
                     {
-                        methodsNeedingPatch.push_back(methodPtr);
+                        methodsNeedingPatch.push_back({methodPtr, vtableOffset});
                     }
                 }
 
@@ -265,7 +272,7 @@ namespace mlirgen
                 builder.create<mlir_ts::StoreOp>(location, valueVTable, heapVTable);
                 auto varVTable = builder.create<mlir_ts::CastOp>(location, globalVTableRefValue.getType(), heapVTable);
 
-                for (auto* methodPtr : methodsNeedingPatch)
+                for (auto& [methodPtr, vtableOffset] : methodsNeedingPatch)
                 {
                     auto& method = *methodPtr;
                     auto index = mth.getFieldIndexByFieldName(storeType, builder.getStringAttr(method.name));
@@ -282,8 +289,9 @@ namespace mlirgen
                                             << "\n\t object method ref: " << V(methodRef) << "\n\n";);
 
                     // where to save
-                    auto fieldInfoVT = mth.getFieldInfoByIndex(vtableType, method.virtualIndex);
-                    auto methodRefVT = builder.create<mlir_ts::PropertyRefOp>(location, fieldInfoVT.type, varVTable, method.virtualIndex);
+                    auto combinedVirtualIndex = method.virtualIndex + vtableOffset;
+                    auto fieldInfoVT = mth.getFieldInfoByIndex(vtableType, combinedVirtualIndex);
+                    auto methodRefVT = builder.create<mlir_ts::PropertyRefOp>(location, fieldInfoVT.type, varVTable, combinedVirtualIndex);
 
                     LLVM_DEBUG(llvm::dbgs() << "\n!!\n\t vtable method: " << method.name
                                             << "\n\t vtable method ref: " << V(methodRefVT) << "\n\n";);
@@ -570,7 +578,23 @@ namespace mlirgen
                     auto interfaceInfo = getInterfaceInfoByFullName(interfaceType.getName().getValue());
                     if (interfaceInfo)
                     {
-                        newInterfacePtr->extends.push_back({-1, interfaceInfo});
+                        // mlirGen(InterfaceDeclaration) can run more than once for the same
+                        // already-declared interface (e.g. re-visited from another module or a
+                        // discovery pass) - mlirGenInterfaceAddFieldMember/addInterfaceMethod are
+                        // idempotent (they check getFieldIndex/getMethodIndex before pushing), but
+                        // this push_back had no equivalent guard, so a diamond `extends A, B` got
+                        // BOTH targets appended again on every re-run, multiplying the combined
+                        // vtable's slot count (and every findField/findMethod/getVirtualTable
+                        // offset derived from it) without bound. Skip targets already present.
+                        auto alreadyExtends = llvm::any_of(newInterfacePtr->extends, [&](auto &extent) {
+                            return std::get<1>(extent)->fullName == interfaceInfo->fullName;
+                        });
+
+                        if (!alreadyExtends)
+                        {
+                            newInterfacePtr->extends.push_back({-1, interfaceInfo});
+                        }
+
                         success = true;
                     }
                 })

tslang/test/tester/CMakeLists.txt

@@ -866,6 +866,8 @@ add_test(NAME test-compile-shared-export-import-object-literal-structural-typed-
 add_test(NAME test-compile-shared-export-import-object-literal-structural-typed-multi-method COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_multi_method.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_multi_method.ts")
 add_test(NAME test-compile-shared-export-import-object-literal-structural-typed-interleaved COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_interleaved.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_interleaved.ts")
 add_test(NAME test-compile-shared-export-import-object-literal-structural-typed-extends-interface COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface.ts")
+add_test(NAME test-compile-shared-export-import-object-literal-structural-typed-extends-interface-multilevel COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface_multilevel.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface_multilevel.ts")
+add_test(NAME test-compile-shared-export-import-object-literal-structural-typed-extends-interface-diamond COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface_diamond.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface_diamond.ts")
 add_test(NAME test-compile-shared-export-import-vars COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_vars.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_vars.ts")
 add_test(NAME test-compile-shared-export-import-vars-2 COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_vars2.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_vars2.ts")
 add_test(NAME test-compile-shared-export-import-enum COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_enum.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_enum.ts")
@@ -888,6 +890,8 @@ add_test(NAME test-jit-shared-export-import-object-literal-structural-typed-para
 add_test(NAME test-jit-shared-export-import-object-literal-structural-typed-multi-method COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_multi_method.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_multi_method.ts")
 add_test(NAME test-jit-shared-export-import-object-literal-structural-typed-interleaved COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_interleaved.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_interleaved.ts")
 add_test(NAME test-jit-shared-export-import-object-literal-structural-typed-extends-interface COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface.ts")
+add_test(NAME test-jit-shared-export-import-object-literal-structural-typed-extends-interface-multilevel COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface_multilevel.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface_multilevel.ts")
+add_test(NAME test-jit-shared-export-import-object-literal-structural-typed-extends-interface-diamond COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface_diamond.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface_diamond.ts")
 add_test(NAME test-jit-shared-export-import-vars COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_vars.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_vars.ts")
 add_test(NAME test-jit-shared-export-import-vars-2 COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_vars2.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_vars2.ts")
 add_test(NAME test-jit-shared-export-import-enum COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_enum.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_enum.ts")

tslang/test/tester/tests/export_object_literal_structural_typed_extends_interface_diamond.ts

@@ -0,0 +1,30 @@
+namespace M2 {
+    export interface Left {
+        left: number;
+        addLeft(n: number): void;
+    }
+    export interface Right {
+        right: number;
+        addRight(n: number): void;
+    }
+    export interface Combined extends Left, Right {
+        combined: number;
+        addCombined(n: number): void;
+    }
+
+    export var rawCombined: {
+        left: number;
+        addLeft(n: number): void;
+        right: number;
+        addRight(n: number): void;
+        combined: number;
+        addCombined(n: number): void;
+    } = {
+        left: 1.0,
+        addLeft(n: number) { this.left = this.left + n; },
+        right: 2.0,
+        addRight(n: number) { this.right = this.right + n; },
+        combined: 3.0,
+        addCombined(n: number) { this.combined = this.combined + n; },
+    };
+}

tslang/test/tester/tests/export_object_literal_structural_typed_extends_interface_multilevel.ts

@@ -0,0 +1,67 @@
+namespace M {
+
+    // Extends export_object_literal_structural_typed_extends_interface.ts's
+    // coverage (single-level: Accumulator extends Base) to a cross-module
+    // 3-level chain (C extends B extends A) and an interface with TWO
+    // extends targets (Combined extends Left, Right) - the same coverage
+    // 00object_annotated_method_extends_interface_multilevel.ts locked in
+    // same-module, verified here to also survive the module boundary.
+
+    export interface A {
+        a: number;
+        addA(n: number): void;
+    }
+    export interface B extends A {
+        b: number;
+        addB(n: number): void;
+    }
+    export interface C extends B {
+        c: number;
+        addC(n: number): void;
+    }
+
+    export var raw: {
+        a: number;
+        addA(n: number): void;
+        b: number;
+        addB(n: number): void;
+        c: number;
+        addC(n: number): void;
+    } = {
+        a: 1.0,
+        addA(n: number) { this.a = this.a + n; },
+        b: 2.0,
+        addB(n: number) { this.b = this.b + n; },
+        c: 3.0,
+        addC(n: number) { this.c = this.c + n; },
+    };
+
+    export interface Left {
+        left: number;
+        addLeft(n: number): void;
+    }
+    export interface Right {
+        right: number;
+        addRight(n: number): void;
+    }
+    export interface Combined extends Left, Right {
+        combined: number;
+        addCombined(n: number): void;
+    }
+
+    export var rawCombined: {
+        left: number;
+        addLeft(n: number): void;
+        right: number;
+        addRight(n: number): void;
+        combined: number;
+        addCombined(n: number): void;
+    } = {
+        left: 1.0,
+        addLeft(n: number) { this.left = this.left + n; },
+        right: 2.0,
+        addRight(n: number) { this.right = this.right + n; },
+        combined: 3.0,
+        addCombined(n: number) { this.combined = this.combined + n; },
+    };
+}

tslang/test/tester/tests/import_object_literal_structural_typed_extends_interface_diamond.ts

@@ -0,0 +1,17 @@
+import './export_object_literal_structural_typed_extends_interface_diamond'
+
+let combined: M2.Combined = <M2.Combined>M2.rawCombined;
+
+combined.addLeft(10);
+assert(combined.left == 11);
+print(combined.left);
+
+combined.addRight(20);
+assert(combined.right == 22);
+print(combined.right);
+
+combined.addCombined(30);
+assert(combined.combined == 33);
+print(combined.combined);
+
+print("done.");

tslang/test/tester/tests/import_object_literal_structural_typed_extends_interface_multilevel.ts

@@ -0,0 +1,31 @@
+import './export_object_literal_structural_typed_extends_interface_multilevel'
+
+let obj: M.C = <M.C>M.raw;
+
+obj.addA(10);
+assert(obj.a == 11);
+print(obj.a);
+
+obj.addB(20);
+assert(obj.b == 22);
+print(obj.b);
+
+obj.addC(30);
+assert(obj.c == 33);
+print(obj.c);
+
+let combined: M.Combined = <M.Combined>M.rawCombined;
+
+combined.addLeft(10);
+assert(combined.left == 11);
+print(combined.left);
+
+combined.addRight(20);
+assert(combined.right == 22);
+print(combined.right);
+
+combined.addCombined(30);
+assert(combined.combined == 33);
+print(combined.combined);
+
+print("done.");