From c76b77c7e3b4eca24fde81b6e35e843cff85bc3a Mon Sep 17 00:00:00 2001 From: dcode Date: Wed, 19 Oct 2022 23:12:10 +0200 Subject: [PATCH] Make field types invariant for soundness --- src/resolver.ts | 11 +- tests/compiler/duplicate-field-errors.json | 3 +- tests/compiler/duplicate-field-errors.ts | 8 ++ tests/compiler/duplicate-fields.debug.wat | 113 +++++---------- tests/compiler/duplicate-fields.release.wat | 146 ++++++++------------ tests/compiler/duplicate-fields.ts | 8 +- 6 files changed, 109 insertions(+), 180 deletions(-) diff --git a/src/resolver.ts b/src/resolver.ts index 9297041b2a..b9415f3133 100644 --- a/src/resolver.ts +++ b/src/resolver.ts @@ -3331,8 +3331,15 @@ export class Resolver extends DiagnosticEmitter { } } - // assignability - if (!fieldType.isStrictlyAssignableTo(existingField.type)) { + // assignability (to guarantee soundness, field types must be invariant) + // see also Wasm GC, where mutable fields are invariant for this reason + // + // class Animal { sibling: Animal; } + // class Cat extends Animal { sibling: Cat; } // covariance + // class Dog extends Animal { sibling: Dog; } // is unsound + // (new Cat()).sibling = new Dog(); // → Cat with Dog sibling + // + if (fieldType != existingField.type) { this.errorRelated( DiagnosticCode.Property_0_in_type_1_is_not_assignable_to_the_same_property_in_base_type_2, fieldPrototype.identifierNode.range, existingField.identifierNode.range, diff --git a/tests/compiler/duplicate-field-errors.json b/tests/compiler/duplicate-field-errors.json index b32e0e0cc0..c6bec26ad5 100644 --- a/tests/compiler/duplicate-field-errors.json +++ b/tests/compiler/duplicate-field-errors.json @@ -8,6 +8,7 @@ "TS2325: Property 'protPriv' is private in type 'duplicate-field-errors/B' but not in type 'duplicate-field-errors/A'.", "TS2325: Property 'pubPriv' is private in type 'duplicate-field-errors/B' but not in type 'duplicate-field-errors/A'.", "TS2444: Property 'pubProt' is protected in type 'duplicate-field-errors/B' but public in type 'duplicate-field-errors/A'.", - "TS2300: Duplicate identifier 'method'." + "TS2300: Duplicate identifier 'method'.", + "Property 'sibling' in type 'duplicate-field-errors/Cat' is not assignable to the same property in base type 'duplicate-field-errors/Animal'." ] } diff --git a/tests/compiler/duplicate-field-errors.ts b/tests/compiler/duplicate-field-errors.ts index 61979a4166..dd5bc58cac 100644 --- a/tests/compiler/duplicate-field-errors.ts +++ b/tests/compiler/duplicate-field-errors.ts @@ -36,7 +36,15 @@ class B extends A { public method: i32; } +class Animal { + sibling: Animal | null; +} +class Cat extends Animal { + sibling: Cat | null; // covariance is unsound +} + export function test(): void { new A(); new B(); + new Cat(); } diff --git a/tests/compiler/duplicate-fields.debug.wat b/tests/compiler/duplicate-fields.debug.wat index 76211402a2..1859cf8420 100644 --- a/tests/compiler/duplicate-fields.debug.wat +++ b/tests/compiler/duplicate-fields.debug.wat @@ -5,8 +5,8 @@ (type $i32_=>_none (func (param i32))) (type $none_=>_none (func)) (type $i32_i32_i32_=>_none (func (param i32 i32 i32))) - (type $i32_i32_i32_=>_i32 (func (param i32 i32 i32) (result i32))) (type $i32_i32_i32_i32_=>_none (func (param i32 i32 i32 i32))) + (type $i32_i32_i32_=>_i32 (func (param i32 i32 i32) (result i32))) (type $none_=>_i32 (func (result i32))) (import "env" "abort" (func $~lib/builtins/abort (param i32 i32 i32 i32))) (global $~lib/rt/itcms/total (mut i32) (i32.const 0)) @@ -26,9 +26,9 @@ (global $duplicate-fields/foo (mut i32) (i32.const 0)) (global $duplicate-fields/raz (mut i32) (i32.const 0)) (global $~lib/rt/__rtti_base i32 (i32.const 480)) - (global $~lib/memory/__data_end i32 (i32.const 572)) - (global $~lib/memory/__stack_pointer (mut i32) (i32.const 16956)) - (global $~lib/memory/__heap_base i32 (i32.const 16956)) + (global $~lib/memory/__data_end i32 (i32.const 564)) + (global $~lib/memory/__stack_pointer (mut i32) (i32.const 16948)) + (global $~lib/memory/__heap_base i32 (i32.const 16948)) (memory $0 1) (data (i32.const 12) "<\00\00\00\00\00\00\00\00\00\00\00\01\00\00\00(\00\00\00A\00l\00l\00o\00c\00a\00t\00i\00o\00n\00 \00t\00o\00o\00 \00l\00a\00r\00g\00e\00\00\00\00\00") (data (i32.const 76) "<\00\00\00\00\00\00\00\00\00\00\00\01\00\00\00 \00\00\00~\00l\00i\00b\00/\00r\00t\00/\00i\00t\00c\00m\00s\00.\00t\00s\00\00\00\00\00\00\00\00\00\00\00\00\00") @@ -39,7 +39,7 @@ (data (i32.const 320) "\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00") (data (i32.const 348) "<\00\00\00\00\00\00\00\00\00\00\00\01\00\00\00\1e\00\00\00~\00l\00i\00b\00/\00r\00t\00/\00t\00l\00s\00f\00.\00t\00s\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00") (data (i32.const 412) "<\00\00\00\00\00\00\00\00\00\00\00\01\00\00\00&\00\00\00d\00u\00p\00l\00i\00c\00a\00t\00e\00-\00f\00i\00e\00l\00d\00s\00.\00t\00s\00\00\00\00\00\00\00") - (data (i32.const 480) "\0b\00\00\00 \00\00\00\00\00\00\00 \00\00\00\00\00\00\00\00\00\00\00\00\00\00\00 \00\00\00\00\00\00\00 \00\00\00\03\00\00\00\00\00\00\00\00\00\00\00 \00\00\00\00\00\00\00\00\00\00\00\05\00\00\00 \00\00\00\06\00\00\00 \00\00\00\n\00\00\00 \00\00\00\00\00\00\00") + (data (i32.const 480) "\n\00\00\00 \00\00\00\00\00\00\00 \00\00\00\00\00\00\00\00\00\00\00\00\00\00\00 \00\00\00\00\00\00\00 \00\00\00\03\00\00\00\00\00\00\00\00\00\00\00 \00\00\00\00\00\00\00\00\00\00\00\05\00\00\00 \00\00\00\t\00\00\00 \00\00\00\00\00\00\00") (table $0 1 1 funcref) (elem $0 (i32.const 1)) (export "memory" (memory $0)) @@ -2217,11 +2217,6 @@ local.get $1 i32.store $0 ) - (func $duplicate-fields/Bar#set:bar (param $0 i32) (param $1 i32) - local.get $0 - local.get $1 - i32.store $0 offset=4 - ) (func $duplicate-fields/A3#set:protProt (param $0 i32) (param $1 i32) local.get $0 local.get $1 @@ -2315,46 +2310,43 @@ block $invalid block $duplicate-fields/A3 block $duplicate-fields/B3 - block $duplicate-fields/Bar - block $duplicate-fields/B2 - block $duplicate-fields/Foo - block $duplicate-fields/A2 - block $duplicate-fields/B - block $duplicate-fields/A - block $~lib/arraybuffer/ArrayBufferView - block $~lib/string/String - block $~lib/arraybuffer/ArrayBuffer - local.get $0 - i32.const 8 - i32.sub - i32.load $0 - br_table $~lib/arraybuffer/ArrayBuffer $~lib/string/String $~lib/arraybuffer/ArrayBufferView $duplicate-fields/A $duplicate-fields/B $duplicate-fields/A2 $duplicate-fields/Foo $duplicate-fields/B2 $duplicate-fields/Bar $duplicate-fields/B3 $duplicate-fields/A3 $invalid - end - return + block $duplicate-fields/B2 + block $duplicate-fields/Foo + block $duplicate-fields/A2 + block $duplicate-fields/B + block $duplicate-fields/A + block $~lib/arraybuffer/ArrayBufferView + block $~lib/string/String + block $~lib/arraybuffer/ArrayBuffer + local.get $0 + i32.const 8 + i32.sub + i32.load $0 + br_table $~lib/arraybuffer/ArrayBuffer $~lib/string/String $~lib/arraybuffer/ArrayBufferView $duplicate-fields/A $duplicate-fields/B $duplicate-fields/A2 $duplicate-fields/Foo $duplicate-fields/B2 $duplicate-fields/B3 $duplicate-fields/A3 $invalid end return end - local.get $0 - local.get $1 - call $~lib/arraybuffer/ArrayBufferView~visit return end + local.get $0 + local.get $1 + call $~lib/arraybuffer/ArrayBufferView~visit return end return end - local.get $0 - local.get $1 - call $duplicate-fields/A2~visit return end + local.get $0 + local.get $1 + call $duplicate-fields/A2~visit return end - local.get $0 - local.get $1 - call $duplicate-fields/B2~visit return end + local.get $0 + local.get $1 + call $duplicate-fields/B2~visit return end return @@ -2442,8 +2434,7 @@ i32.const 0 i32.const 0 i32.const 1 - i32.const 2 - call $duplicate-fields/Bar#constructor + call $duplicate-fields/Foo#constructor local.set $0 global.get $~lib/memory/__stack_pointer local.get $0 @@ -2453,8 +2444,8 @@ global.set $duplicate-fields/raz global.get $duplicate-fields/raz i32.load $0 - i32.load $0 offset=4 - i32.const 2 + i32.load $0 + i32.const 1 i32.eq i32.eqz if @@ -2655,46 +2646,6 @@ global.set $~lib/memory/__stack_pointer local.get $2 ) - (func $duplicate-fields/Bar#constructor (param $this i32) (param $foo i32) (param $bar i32) (result i32) - (local $3 i32) - global.get $~lib/memory/__stack_pointer - i32.const 4 - i32.sub - global.set $~lib/memory/__stack_pointer - call $~stack_check - global.get $~lib/memory/__stack_pointer - i32.const 0 - i32.store $0 - local.get $this - i32.eqz - if - global.get $~lib/memory/__stack_pointer - i32.const 8 - i32.const 8 - call $~lib/rt/itcms/__new - local.tee $this - i32.store $0 - end - local.get $this - i32.const 0 - call $duplicate-fields/Bar#set:bar - global.get $~lib/memory/__stack_pointer - local.get $this - local.get $foo - call $duplicate-fields/Foo#constructor - local.tee $this - i32.store $0 - local.get $this - local.get $bar - call $duplicate-fields/Bar#set:bar - local.get $this - local.set $3 - global.get $~lib/memory/__stack_pointer - i32.const 4 - i32.add - global.set $~lib/memory/__stack_pointer - local.get $3 - ) (func $duplicate-fields/A3#constructor (param $this i32) (result i32) (local $1 i32) global.get $~lib/memory/__stack_pointer @@ -2710,7 +2661,7 @@ if global.get $~lib/memory/__stack_pointer i32.const 12 - i32.const 10 + i32.const 9 call $~lib/rt/itcms/__new local.tee $this i32.store $0 @@ -2747,7 +2698,7 @@ if global.get $~lib/memory/__stack_pointer i32.const 12 - i32.const 9 + i32.const 8 call $~lib/rt/itcms/__new local.tee $this i32.store $0 diff --git a/tests/compiler/duplicate-fields.release.wat b/tests/compiler/duplicate-fields.release.wat index 844660772a..ff88449e1e 100644 --- a/tests/compiler/duplicate-fields.release.wat +++ b/tests/compiler/duplicate-fields.release.wat @@ -19,7 +19,7 @@ (global $~lib/rt/tlsf/ROOT (mut i32) (i32.const 0)) (global $duplicate-fields/foo (mut i32) (i32.const 0)) (global $duplicate-fields/raz (mut i32) (i32.const 0)) - (global $~lib/memory/__stack_pointer (mut i32) (i32.const 17980)) + (global $~lib/memory/__stack_pointer (mut i32) (i32.const 17972)) (memory $0 1) (data (i32.const 1036) "<") (data (i32.const 1048) "\01\00\00\00(\00\00\00A\00l\00l\00o\00c\00a\00t\00i\00o\00n\00 \00t\00o\00o\00 \00l\00a\00r\00g\00e") @@ -33,10 +33,10 @@ (data (i32.const 1384) "\01\00\00\00\1e\00\00\00~\00l\00i\00b\00/\00r\00t\00/\00t\00l\00s\00f\00.\00t\00s") (data (i32.const 1436) "<") (data (i32.const 1448) "\01\00\00\00&\00\00\00d\00u\00p\00l\00i\00c\00a\00t\00e\00-\00f\00i\00e\00l\00d\00s\00.\00t\00s") - (data (i32.const 1504) "\0b\00\00\00 \00\00\00\00\00\00\00 ") + (data (i32.const 1504) "\n\00\00\00 \00\00\00\00\00\00\00 ") (data (i32.const 1532) " \00\00\00\00\00\00\00 \00\00\00\03") (data (i32.const 1556) " ") - (data (i32.const 1568) "\05\00\00\00 \00\00\00\06\00\00\00 \00\00\00\n\00\00\00 ") + (data (i32.const 1568) "\05\00\00\00 \00\00\00\t\00\00\00 ") (export "memory" (memory $0)) (start $~start) (func $~lib/rt/itcms/visitRoots @@ -131,7 +131,7 @@ i32.load $0 offset=8 i32.eqz local.get $0 - i32.const 17980 + i32.const 17972 i32.lt_u i32.and i32.eqz @@ -900,7 +900,7 @@ local.set $0 loop $while-continue|0 local.get $0 - i32.const 17980 + i32.const 17972 i32.lt_u if local.get $0 @@ -1000,7 +1000,7 @@ unreachable end local.get $0 - i32.const 17980 + i32.const 17972 i32.lt_u if local.get $0 @@ -1023,7 +1023,7 @@ i32.const 4 i32.add local.tee $0 - i32.const 17980 + i32.const 17972 i32.ge_u if global.get $~lib/rt/tlsf/ROOT @@ -1523,20 +1523,17 @@ block $invalid block $duplicate-fields/A3 block $duplicate-fields/B3 - block $duplicate-fields/Bar - block $duplicate-fields/B2 - block $duplicate-fields/Foo - block $duplicate-fields/B - block $duplicate-fields/A - block $~lib/string/String - block $~lib/arraybuffer/ArrayBuffer - local.get $0 - i32.const 8 - i32.sub - i32.load $0 - br_table $~lib/arraybuffer/ArrayBuffer $~lib/string/String $folding-inner0 $duplicate-fields/A $duplicate-fields/B $folding-inner0 $duplicate-fields/Foo $duplicate-fields/B2 $duplicate-fields/Bar $duplicate-fields/B3 $duplicate-fields/A3 $invalid - end - return + block $duplicate-fields/B2 + block $duplicate-fields/Foo + block $duplicate-fields/B + block $duplicate-fields/A + block $~lib/string/String + block $~lib/arraybuffer/ArrayBuffer + local.get $0 + i32.const 8 + i32.sub + i32.load $0 + br_table $~lib/arraybuffer/ArrayBuffer $~lib/string/String $folding-inner0 $duplicate-fields/A $duplicate-fields/B $folding-inner0 $duplicate-fields/Foo $duplicate-fields/B2 $duplicate-fields/B3 $duplicate-fields/A3 $invalid end return end @@ -1546,16 +1543,16 @@ end return end - local.get $0 - i32.load $0 - local.tee $1 - if - local.get $1 - call $byn-split-outlined-A$~lib/rt/itcms/__visit - end - br $folding-inner0 + return + end + local.get $0 + i32.load $0 + local.tee $1 + if + local.get $1 + call $byn-split-outlined-A$~lib/rt/itcms/__visit end - return + br $folding-inner0 end return end @@ -1581,7 +1578,7 @@ global.set $~lib/memory/__stack_pointer block $folding-inner0 global.get $~lib/memory/__stack_pointer - i32.const 1596 + i32.const 1588 i32.lt_s br_if $folding-inner0 global.get $~lib/memory/__stack_pointer @@ -1591,7 +1588,7 @@ memory.size $0 i32.const 16 i32.shl - i32.const 17980 + i32.const 17972 i32.sub i32.const 1 i32.shr_u @@ -1625,7 +1622,7 @@ i32.sub global.set $~lib/memory/__stack_pointer global.get $~lib/memory/__stack_pointer - i32.const 1596 + i32.const 1588 i32.lt_s br_if $folding-inner0 global.get $~lib/memory/__stack_pointer @@ -1647,7 +1644,7 @@ i32.sub global.set $~lib/memory/__stack_pointer global.get $~lib/memory/__stack_pointer - i32.const 1596 + i32.const 1588 i32.lt_s br_if $folding-inner0 global.get $~lib/memory/__stack_pointer @@ -1702,7 +1699,7 @@ i32.sub global.set $~lib/memory/__stack_pointer global.get $~lib/memory/__stack_pointer - i32.const 1596 + i32.const 1588 i32.lt_s br_if $folding-inner0 global.get $~lib/memory/__stack_pointer @@ -1710,65 +1707,30 @@ i32.const 0 i32.store $0 local.get $0 - i32.const 8 - i32.const 8 - call $~lib/rt/itcms/__new - local.tee $1 - i32.store $0 - local.get $1 - i32.const 0 - i32.store $0 offset=4 - global.get $~lib/memory/__stack_pointer - local.tee $0 i32.const 4 - i32.sub - global.set $~lib/memory/__stack_pointer - global.get $~lib/memory/__stack_pointer - i32.const 1596 - i32.lt_s - br_if $folding-inner0 - global.get $~lib/memory/__stack_pointer - i32.const 0 + i32.const 6 + call $~lib/rt/itcms/__new + local.tee $2 i32.store $0 - local.get $1 - i32.eqz - if - global.get $~lib/memory/__stack_pointer - i32.const 4 - i32.const 6 - call $~lib/rt/itcms/__new - local.tee $1 - i32.store $0 - end - local.get $1 + local.get $2 i32.const 0 i32.store $0 - local.get $1 + local.get $2 i32.const 1 i32.store $0 global.get $~lib/memory/__stack_pointer i32.const 4 i32.add global.set $~lib/memory/__stack_pointer - local.get $0 - local.get $1 - i32.store $0 - local.get $1 - i32.const 2 - i32.store $0 offset=4 - global.get $~lib/memory/__stack_pointer - i32.const 4 - i32.add - global.set $~lib/memory/__stack_pointer global.get $~lib/memory/__stack_pointer - local.get $1 + local.get $2 i32.store $0 global.get $~lib/memory/__stack_pointer i32.const 4 i32.sub global.set $~lib/memory/__stack_pointer global.get $~lib/memory/__stack_pointer - i32.const 1596 + i32.const 1588 i32.lt_s br_if $folding-inner0 global.get $~lib/memory/__stack_pointer @@ -1785,12 +1747,12 @@ i32.const 0 i32.store $0 global.get $~lib/memory/__stack_pointer - local.tee $2 + local.tee $1 i32.const 4 i32.sub global.set $~lib/memory/__stack_pointer global.get $~lib/memory/__stack_pointer - i32.const 1596 + i32.const 1588 i32.lt_s br_if $folding-inner0 global.get $~lib/memory/__stack_pointer @@ -1810,28 +1772,28 @@ i32.const 0 i32.store $0 local.get $0 - local.get $1 + local.get $2 i32.store $0 - local.get $1 + local.get $2 if local.get $0 - local.get $1 + local.get $2 call $byn-split-outlined-A$~lib/rt/itcms/__link end global.get $~lib/memory/__stack_pointer i32.const 4 i32.add global.set $~lib/memory/__stack_pointer - local.get $2 + local.get $1 local.get $0 i32.store $0 local.get $0 - local.get $1 + local.get $2 i32.store $0 - local.get $1 + local.get $2 if local.get $0 - local.get $1 + local.get $2 call $byn-split-outlined-A$~lib/rt/itcms/__link end global.get $~lib/memory/__stack_pointer @@ -1842,8 +1804,8 @@ global.set $duplicate-fields/raz global.get $duplicate-fields/raz i32.load $0 - i32.load $0 offset=4 - i32.const 2 + i32.load $0 + i32.const 1 i32.ne if i32.const 0 @@ -1858,7 +1820,7 @@ i32.sub global.set $~lib/memory/__stack_pointer global.get $~lib/memory/__stack_pointer - i32.const 1596 + i32.const 1588 i32.lt_s br_if $folding-inner0 global.get $~lib/memory/__stack_pointer @@ -1867,7 +1829,7 @@ i32.store $0 local.get $0 i32.const 12 - i32.const 9 + i32.const 8 call $~lib/rt/itcms/__new local.tee $0 i32.store $0 @@ -1877,7 +1839,7 @@ i32.sub global.set $~lib/memory/__stack_pointer global.get $~lib/memory/__stack_pointer - i32.const 1596 + i32.const 1588 i32.lt_s br_if $folding-inner0 global.get $~lib/memory/__stack_pointer @@ -1888,7 +1850,7 @@ if global.get $~lib/memory/__stack_pointer i32.const 12 - i32.const 10 + i32.const 9 call $~lib/rt/itcms/__new local.tee $0 i32.store $0 diff --git a/tests/compiler/duplicate-fields.ts b/tests/compiler/duplicate-fields.ts index fce4ff6855..c11f2b5d96 100644 --- a/tests/compiler/duplicate-fields.ts +++ b/tests/compiler/duplicate-fields.ts @@ -32,15 +32,15 @@ class A2 { } class B2 extends A2 { - bar: Bar; - constructor(bar: Bar) { super(bar); this.bar = bar; } + bar: Foo; // must be invariant + constructor(bar: Foo) { super(bar); this.bar = bar; } } assert(offsetof("bar") == 0); assert(offsetof("bar") == 0); -const raz = new B2(new Bar(1, 2)); -assert(raz.bar.bar == 2); +const raz = new B2(new Foo(1)); +assert(raz.bar.foo == 1); // make sure visibility checks allow these