Skip to content

Commit 8edfb2a

Browse files
Fix object literals silently corrupting data with interleaved field/method type annotations (#265)
An object literal's own storage always lays out as "all fields, then all methods" (two separate codegen passes over the properties, by design - a method may reference sibling fields that must already be registered). A type-literal annotation on the receiving var/let, however, preserves verbatim interleaved source declaration order. When these orderings diverge - any annotation with 2+ fields and 2+ methods not grouped field-first - method bodies get compiled against the WRONG byte offsets for the annotated variable's actual memory layout: a `this.field` write can silently land on a neighboring method's function-pointer slot instead, corrupting data with no compile error and no crash. Fix: after all fields/methods/captures are processed but before method bodies are generated, if the receiver type's field order differs from the literal's own accumulated order, permute the literal's field list (and parallel value list) to match it. Field access inside method bodies resolves by name against the storage type's live field list, so this reordering alone is sufficient - no per-method index patching needed. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent eb74fce commit 8edfb2a

5 files changed

Lines changed: 150 additions & 0 deletions

tslang/lib/TypeScript/MLIRGenExpressions.cpp

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1319,6 +1319,59 @@ namespace mlirgen
13191319
return mlir::failure();
13201320
}
13211321

1322+
// If the destination has a known field order (e.g. a type-literal
1323+
// annotation on the receiving var/let), re-lay-out this literal's own
1324+
// storage to match it BEFORE method bodies are generated. Without
1325+
// this, the literal's storage stays in "all fields, then all
1326+
// methods" order (mlirGenObjectLiteralFields/-MethodPrototypes run as
1327+
// two separate passes over the properties, by design - methods may
1328+
// need every field already registered), which only coincidentally
1329+
// matches a declared type whose members interleave fields and
1330+
// methods in a different order. Method bodies compiled against the
1331+
// MISMATCHED order silently read/write the wrong byte offset when
1332+
// later invoked against the (differently laid out) destination
1333+
// memory - no crash, no error, just corrupted data: a `this.field`
1334+
// write can land on a neighboring method's function-pointer slot.
1335+
// Field position resolution inside method bodies is by NAME against
1336+
// objectStorageType's CURRENT field list (TupleFieldType/getIndex,
1337+
// MLIRCodeLogic.h), so reordering here - before
1338+
// mlirGenObjectLiteralMethodBodies runs - is sufficient; no
1339+
// per-method index patching needed. Must run after
1340+
// mlirGenObjectLiteralCaptures (methodInfosWithCaptures already holds
1341+
// pre-reorder indices consumed there) and before the final
1342+
// setFields/method-bodies pass below.
1343+
llvm::SmallVector<mlir_ts::FieldInfo> receiverFields;
1344+
if (oli.receiverType && mlir::succeeded(mth.getFields(oli.receiverType, receiverFields, true)) &&
1345+
receiverFields.size() == oli.fieldInfos.size())
1346+
{
1347+
llvm::SmallVector<mlir::Attribute> reorderedValues;
1348+
llvm::SmallVector<mlir_ts::FieldInfo> reorderedFieldInfos;
1349+
reorderedValues.reserve(oli.fieldInfos.size());
1350+
reorderedFieldInfos.reserve(oli.fieldInfos.size());
1351+
1352+
auto allMatched = true;
1353+
for (auto &receiverField : receiverFields)
1354+
{
1355+
auto foundIt = llvm::find_if(oli.fieldInfos,
1356+
[&](const mlir_ts::FieldInfo &fieldInfo) { return fieldInfo.id == receiverField.id; });
1357+
if (foundIt == oli.fieldInfos.end())
1358+
{
1359+
allMatched = false;
1360+
break;
1361+
}
1362+
1363+
auto index = std::distance(oli.fieldInfos.begin(), foundIt);
1364+
reorderedFieldInfos.push_back(*foundIt);
1365+
reorderedValues.push_back(oli.values[index]);
1366+
}
1367+
1368+
if (allMatched)
1369+
{
1370+
oli.fieldInfos = std::move(reorderedFieldInfos);
1371+
oli.values = std::move(reorderedValues);
1372+
}
1373+
}
1374+
13221375
// final type, update
13231376
objectStorageType.setFields(oli.fieldInfos);
13241377

tslang/test/tester/CMakeLists.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,7 @@ add_test(NAME test-compile-00-interface-function-typed-field COMMAND test-runner
289289
add_test(NAME test-compile-00-interface-captures COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_captures.ts")
290290
add_test(NAME test-compile-00-object-annotated-method COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00object_annotated_method.ts")
291291
add_test(NAME test-compile-00-object-annotated-method-params COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00object_annotated_method_params.ts")
292+
add_test(NAME test-compile-00-object-annotated-method-interleaved COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00object_annotated_method_interleaved.ts")
292293
add_test(NAME test-compile-00-interface-object-array COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_object_array.ts")
293294
add_test(NAME test-compile-00-interface-generic COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_generic.ts")
294295
add_test(NAME test-compile-00-interface-new COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_new.ts")
@@ -645,6 +646,7 @@ add_test(NAME test-jit-00-interface-function-typed-field COMMAND test-runner -ji
645646
add_test(NAME test-jit-00-interface-captures COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_captures.ts")
646647
add_test(NAME test-jit-00-object-annotated-method COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00object_annotated_method.ts")
647648
add_test(NAME test-jit-00-object-annotated-method-params COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00object_annotated_method_params.ts")
649+
add_test(NAME test-jit-00-object-annotated-method-interleaved COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00object_annotated_method_interleaved.ts")
648650
add_test(NAME test-jit-00-interface-object-array COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_object_array.ts")
649651
add_test(NAME test-jit-00-interface-generic COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_generic.ts")
650652
add_test(NAME test-jit-00-interface-new COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_new.ts")
@@ -858,6 +860,7 @@ add_test(NAME test-compile-shared-export-import-object-literal-untyped-multi-met
858860
add_test(NAME test-compile-shared-export-import-object-literal-structural-typed COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed.ts")
859861
add_test(NAME test-compile-shared-export-import-object-literal-structural-typed-params COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_params.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_params.ts")
860862
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")
863+
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")
861864
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")
862865
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")
863866
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")
@@ -878,6 +881,7 @@ add_test(NAME test-jit-shared-export-import-object-literal-untyped-multi-method
878881
add_test(NAME test-jit-shared-export-import-object-literal-structural-typed COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed.ts")
879882
add_test(NAME test-jit-shared-export-import-object-literal-structural-typed-params COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_params.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_params.ts")
880883
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")
884+
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")
881885
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")
882886
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")
883887
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")
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// Extends 00object_annotated_method_params.ts's coverage: that test's
2+
// type-literal annotations always declare all fields before all methods
3+
// (total; add(); addTwice()). Here fields and methods INTERLEAVE in
4+
// declaration order (base; addBase(); total; add(); ...) - this used to
5+
// silently corrupt data: the object literal's own storage laid out as
6+
// "fields, then methods" regardless of the annotation's order, so a method
7+
// compiled against that layout read/wrote the wrong byte offset once
8+
// invoked against the annotation-ordered variable, up to overwriting a
9+
// sibling method's function pointer with a field's value. See
10+
// docs/object-literal-boxing-design.md-adjacent history; root-caused and
11+
// fixed via a field/value reorder pass in mlirGen(ObjectLiteralExpression).
12+
13+
function main() {
14+
let acc: { base: number; addBase(n: number): void; total: number; add(n: number): void; addTwice(n: number): void; scaled(factor: number): number } = {
15+
base: 100,
16+
addBase(n: number) { this.base = this.base + n; },
17+
total: 0,
18+
add(n: number) { this.total = this.total + n; },
19+
addTwice(n: number) { this.add(n); this.add(n); },
20+
scaled(factor: number) { return this.total * factor; },
21+
};
22+
23+
acc.addBase(5);
24+
assert(acc.base == 105);
25+
print(acc.base);
26+
27+
acc.add(3);
28+
assert(acc.total == 3);
29+
print(acc.total);
30+
31+
acc.addTwice(2);
32+
assert(acc.total == 7);
33+
print(acc.total);
34+
35+
const result = acc.scaled(2);
36+
assert(result == 14);
37+
print(result);
38+
39+
print("done.");
40+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
namespace A {
2+
3+
export interface Accumulator {
4+
base: number;
5+
addBase(n: number): void;
6+
total: number;
7+
add(n: number): void;
8+
addTwice(n: number): void;
9+
scaled(factor: number): number;
10+
}
11+
12+
// extends export_object_literal_structural_typed_multi_method.ts's
13+
// coverage: that test's methods are all grouped after every field
14+
// (total; add(); addTwice(); scaled()). Here fields and methods
15+
// INTERLEAVE in declaration order (base; addBase(); total; add(); ...) -
16+
// see 00object_annotated_method_interleaved.ts for the same-module
17+
// version of this coverage and the mechanism this used to break.
18+
export var acc: {
19+
base: number;
20+
addBase(n: number): void;
21+
total: number;
22+
add(n: number): void;
23+
addTwice(n: number): void;
24+
scaled(factor: number): number;
25+
} = {
26+
base: 100.0,
27+
addBase(n: number) { this.base = this.base + n; },
28+
total: 0.0,
29+
add(n: number) { this.total = this.total + n; },
30+
addTwice(n: number) { this.add(n); this.add(n); },
31+
scaled(factor: number) { return this.total * factor; },
32+
};
33+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import './export_object_literal_structural_typed_interleaved'
2+
3+
var acc: A.Accumulator = <A.Accumulator>A.acc;
4+
5+
acc.addBase(5);
6+
print(acc.base);
7+
assert(acc.base == 105);
8+
9+
acc.add(3);
10+
print(acc.total);
11+
assert(acc.total == 3);
12+
13+
acc.addTwice(2);
14+
print(acc.total);
15+
assert(acc.total == 7);
16+
17+
print(acc.scaled(2));
18+
assert(acc.scaled(2) == 14);
19+
20+
print("done.");

0 commit comments

Comments
 (0)