diff options
| -rw-r--r-- | src/pass.h | 8 | ||||
| -rw-r--r-- | src/passes/AbstractTypeRefining.cpp | 10 | ||||
| -rw-r--r-- | src/passes/GlobalTypeOptimization.cpp | 14 | ||||
| -rw-r--r-- | src/wasm/wasm-validator.cpp | 51 | ||||
| -rw-r--r-- | test/lit/passes/abstract-type-refining.wast | 52 | ||||
| -rw-r--r-- | test/lit/passes/gto-mutability.wast | 36 | ||||
| -rw-r--r-- | test/lit/validation/closed-world-interface-intrinsics.wast | 46 | ||||
| -rw-r--r-- | test/lit/validation/closed-world-interface.wast | 79 |
8 files changed, 113 insertions, 183 deletions
diff --git a/src/pass.h b/src/pass.h index 8d81fce8e..daed8b6ab 100644 --- a/src/pass.h +++ b/src/pass.h @@ -212,14 +212,6 @@ struct PassOptions { // but we also want to keep types of things on the boundary unchanged. For // example, we should not change an exported function's signature, as the // outside may need that type to properly call the export. - // - // * Since the goal of closedWorld is to optimize types aggressively but - // types on the module boundary cannot be changed, we assume the producer - // has made a mistake and we consider it a validation error if any user - // defined types besides the types of imported or exported functions - // themselves appear on the module boundary. For example, no user defined - // struct type may be a parameter or result of an exported function. This - // error may be relaxed or made more configurable in the future. bool closedWorld = false; // Whether to try to preserve debug info through, which are special calls. bool debugInfo = false; diff --git a/src/passes/AbstractTypeRefining.cpp b/src/passes/AbstractTypeRefining.cpp index 313ad5f4d..509448851 100644 --- a/src/passes/AbstractTypeRefining.cpp +++ b/src/passes/AbstractTypeRefining.cpp @@ -106,6 +106,16 @@ struct AbstractTypeRefining : public Pass { } } + // Assume all public types are created, which makes them non-abstract and + // hence ignored below. + // TODO: In principle we could assume such types are not created outside the + // module, given closed world, but we'd also need to make sure that + // we don't need to make any changes to public types that refer to + // them. + for (auto type : ModuleUtils::getPublicHeapTypes(*module)) { + createdTypes.insert(type); + } + SubTypes subTypes(*module); // Compute createdTypesOrSubTypes by starting with the created types and diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 1f0da2595..74107f9cd 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -198,6 +198,20 @@ struct GlobalTypeOptimization : public Pass { continue; } + // The propagation analysis ensures we update immutability in all + // supers and subs in concert, but it does not take into account + // visibility, so do that here: we can only become immutable if the + // parent can as well. + auto super = type.getDeclaredSuperType(); + if (super && !canBecomeImmutable.count(*super)) { + // No entry in canBecomeImmutable means nothing in the parent can + // become immutable. We don't need to check the specific field index, + // because visibility affects them all equally (i.e., if it is public + // then no field can be changed, and if it is private then this field + // can be changed, and perhaps more). + continue; + } + // No set exists. Mark it as something we can make immutable. auto& vec = canBecomeImmutable[type]; vec.resize(i + 1); diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index 0184e3284..d1ca5220c 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -63,7 +63,6 @@ struct ValidationInfo { bool validateWeb; bool validateGlobally; bool quiet; - bool closedWorld; std::atomic<bool> valid; @@ -4135,46 +4134,6 @@ static void validateFeatures(Module& module, ValidationInfo& info) { } } -static void validateClosedWorldInterface(Module& module, ValidationInfo& info) { - // Error if there are any publicly exposed heap types beyond the types of - // publicly exposed functions. Note that we must include all types in the rec - // groups that are used, as if a type if public then all types in its rec - // group are as well. - std::unordered_set<RecGroup> publicRecGroups; - ModuleUtils::iterImportedFunctions(module, [&](Function* func) { - publicRecGroups.insert(func->type.getRecGroup()); - }); - for (auto& ex : module.exports) { - if (ex->kind == ExternalKind::Function) { - publicRecGroups.insert(module.getFunction(ex->value)->type.getRecGroup()); - } - } - - std::unordered_set<HeapType> publicTypes; - for (auto& group : publicRecGroups) { - for (auto type : group) { - publicTypes.insert(type); - } - } - - // Ignorable public types are public, but we can ignore them for purposes of - // erroring here: It is always ok that they are public. - auto ignorable = getIgnorablePublicTypes(); - - for (auto type : ModuleUtils::getPublicHeapTypes(module)) { - if (!publicTypes.count(type) && !ignorable.count(type)) { - auto name = type.toString(); - if (auto it = module.typeNames.find(type); it != module.typeNames.end()) { - name = it->second.name.toString(); - } - info.fail("publicly exposed type disallowed with a closed world: $" + - name, - type, - nullptr); - } - } -} - // TODO: If we want the validator to be part of libwasm rather than libpasses, // then Using PassRunner::getPassDebug causes a circular dependence. We should // fix that, perhaps by moving some of the pass infrastructure into libsupport. @@ -4183,7 +4142,6 @@ bool WasmValidator::validate(Module& module, Flags flags) { info.validateWeb = (flags & Web) != 0; info.validateGlobally = (flags & Globally) != 0; info.quiet = (flags & Quiet) != 0; - info.closedWorld = (flags & ClosedWorld) != 0; // Parallel function validation. PassRunner runner(&module); @@ -4210,9 +4168,6 @@ bool WasmValidator::validate(Module& module, Flags flags) { validateStart(module, info); validateModuleMaps(module, info); validateFeatures(module, info); - if (info.closedWorld) { - validateClosedWorldInterface(module, info); - } } // Validate additional internal IR details when in pass-debug mode. @@ -4231,11 +4186,7 @@ bool WasmValidator::validate(Module& module, Flags flags) { } bool WasmValidator::validate(Module& module, const PassOptions& options) { - Flags flags = options.validateGlobally ? Globally : Minimal; - if (options.closedWorld) { - flags |= ClosedWorld; - } - return validate(module, flags); + return validate(module, options.validateGlobally ? Globally : Minimal); } bool WasmValidator::validate(Function* func, Module& module, Flags flags) { diff --git a/test/lit/passes/abstract-type-refining.wast b/test/lit/passes/abstract-type-refining.wast index 814f5c1f0..e887894ea 100644 --- a/test/lit/passes/abstract-type-refining.wast +++ b/test/lit/passes/abstract-type-refining.wast @@ -1304,3 +1304,55 @@ ) ) ) + +;; $A is never created, but $B is, so all appearances of $A, like in the cast +;; and the struct field, could be replaced by $B, except that $A is a public type, +;; so might be created outside the module. +(module + (rec + ;; YESTNH: (rec + ;; YESTNH-NEXT: (type $A (sub (struct (field (ref null $A))))) + ;; NO_TNH: (rec + ;; NO_TNH-NEXT: (type $A (sub (struct (field (ref null $A))))) + (type $A (sub (struct (field (ref null $A))))) + + ;; YESTNH: (type $B (sub $A (struct (field (ref null $A))))) + ;; NO_TNH: (type $B (sub $A (struct (field (ref null $A))))) + (type $B (sub $A (struct (field (ref null $A))))) + ) + + ;; YESTNH: (type $2 (func (param anyref))) + + ;; YESTNH: (global $global (ref $B) (struct.new_default $B)) + ;; NO_TNH: (type $2 (func (param anyref))) + + ;; NO_TNH: (global $global (ref $B) (struct.new_default $B)) + (global $global (ref $B) (struct.new_default $B)) + + ;; YESTNH: (export "global" (global $global)) + ;; NO_TNH: (export "global" (global $global)) + (export "global" (global $global)) + + ;; YESTNH: (func $new (type $2) (param $x anyref) + ;; YESTNH-NEXT: (drop + ;; YESTNH-NEXT: (ref.cast (ref $A) + ;; YESTNH-NEXT: (local.get $x) + ;; YESTNH-NEXT: ) + ;; YESTNH-NEXT: ) + ;; YESTNH-NEXT: ) + ;; NO_TNH: (func $new (type $2) (param $x anyref) + ;; NO_TNH-NEXT: (drop + ;; NO_TNH-NEXT: (ref.cast (ref $A) + ;; NO_TNH-NEXT: (local.get $x) + ;; NO_TNH-NEXT: ) + ;; NO_TNH-NEXT: ) + ;; NO_TNH-NEXT: ) + (func $new (param $x anyref) + (drop + (ref.cast (ref $A) + (local.get $x) + ) + ) + ) +) + diff --git a/test/lit/passes/gto-mutability.wast b/test/lit/passes/gto-mutability.wast index 13b416b1b..3b01572c8 100644 --- a/test/lit/passes/gto-mutability.wast +++ b/test/lit/passes/gto-mutability.wast @@ -652,3 +652,39 @@ ) ) ) + +;; The parent is public, which prevents us from making any field immutable in +;; the child. +(module + ;; CHECK: (type $parent (sub (struct (field (mut i32))))) + (type $parent (sub (struct (field (mut i32))))) + ;; CHECK: (type $1 (func)) + + ;; CHECK: (type $child (sub $parent (struct (field (mut i32))))) + (type $child (sub $parent (struct (field (mut i32))))) + + ;; CHECK: (global $global (ref $parent) (struct.new $parent + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: )) + (global $global (ref $parent) (struct.new $parent + (i32.const 0) + )) + + ;; Make the parent public by exporting the global. + ;; CHECK: (export "global" (global $global)) + (export "global" (global $global)) + + ;; CHECK: (func $func (type $1) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (struct.new_default $child) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $func + ;; Create the child so the type is used. No sets to the fields exist, so + ;; in theory all fields could be immutable. + (drop + (struct.new_default $child) + ) + ) +) + diff --git a/test/lit/validation/closed-world-interface-intrinsics.wast b/test/lit/validation/closed-world-interface-intrinsics.wast deleted file mode 100644 index 50151b8c0..000000000 --- a/test/lit/validation/closed-world-interface-intrinsics.wast +++ /dev/null @@ -1,46 +0,0 @@ -;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. - -;; RUN: foreach %s %t wasm-opt -all --closed-world -S -o - | filecheck %s - -;; Test that we do not error on call.without.effects despite it being an import. -;; call.without.effects does not make the types in it public, and so it can -;; validate with --closed-world. - -(module - ;; CHECK: (type $struct (struct (field i32))) - (type $struct (struct i32)) - - ;; CHECK: (type $1 (func (param (ref $struct) funcref))) - - ;; CHECK: (type $2 (func)) - - ;; CHECK: (type $3 (func (param (ref $struct)))) - - ;; CHECK: (import "binaryen-intrinsics" "call.without.effects" (func $cwe (type $1) (param (ref $struct) funcref))) - (import "binaryen-intrinsics" "call.without.effects" (func $cwe (param (ref $struct)) (param funcref))) - - ;; CHECK: (elem declare func $func) - - ;; CHECK: (func $test (type $2) - ;; CHECK-NEXT: (call $cwe - ;; CHECK-NEXT: (struct.new $struct - ;; CHECK-NEXT: (i32.const 100) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (ref.func $func) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - (func $test - (call $cwe - (struct.new $struct - (i32.const 100) - ) - (ref.func $func) - ) - ) - - ;; CHECK: (func $func (type $3) (param $ref (ref $struct)) - ;; CHECK-NEXT: (nop) - ;; CHECK-NEXT: ) - (func $func (param $ref (ref $struct)) - ) -) diff --git a/test/lit/validation/closed-world-interface.wast b/test/lit/validation/closed-world-interface.wast deleted file mode 100644 index b4fe965cf..000000000 --- a/test/lit/validation/closed-world-interface.wast +++ /dev/null @@ -1,79 +0,0 @@ -;; Test that we error out on nontrivial public types with --closed-world - -;; RUN: not wasm-opt -all --closed-world %s 2>&1 | filecheck %s - - -;; This is pulled in by a global. -;; CHECK: publicly exposed type disallowed with a closed world: $array, on -;; CHECK-NEXT: (array (mut i32)) - -;; This is pulled in only by a global, so it is disallowed even though it is a function type. -;; CHECK: publicly exposed type disallowed with a closed world: $private, on -;; CHECK-NEXT: (func (param v128)) - -;; This is referred to by the type of a function export, but is still not allowed. -;; CHECK: publicly exposed type disallowed with a closed world: $struct, on -;; CHECK-NEXT: (struct) - -(module - (type $struct (struct)) - (type $array (array (mut i32))) - - (type $void (func)) - (type $abstract (func (param anyref))) - (type $concrete (func (param (ref null $struct)))) - - (rec - (type $exported-pair-0 (func (param (ref $exported-pair-1)))) - (type $exported-pair-1 (func (param (ref $exported-pair-0)))) - ) - (rec - ;; This is on an exported function. - (type $partial-pair-0 (func)) - ;; The latter type types are not public, but allowed to be because the - ;; entire rec group is allowed due to the first. - (type $partial-pair-1 (func)) - ;; Test a non-function type. - (type $partial-pair-2 (struct)) - ) - - (type $private (func (param v128))) - - ;; Ok even though it is an import instead of an export. - (func $1 (import "env" "test5") (type $exported-pair-1)) - - (func $2 (export "test1") (type $void) - (unreachable) - ) - - ;; Ok because it only refers to basic heap types - (func $3 (export "test2") (type $abstract) - (unreachable) - ) - - ;; Not ok because it refers to $struct. - (func $4 (export "test3") (type $concrete) - (unreachable) - ) - - ;; Ok even though it is in a rec group because the rest of the group and the - ;; types this refers to are on the boundary as well. - (func $5 (export "test4") (type $exported-pair-0) - (unreachable) - ) - - ;; Ok, and we also allow the other type in the group. - (func $6 (export "test6") (type $partial-pair-0) - (unreachable) - ) - - ;; Not ok. - (global $1 (export "g1") (ref null $array) (ref.null none)) - - ;; Ok because this type is on the boundary anyway. - (global $2 (export "g2") (ref null $void) (ref.null func)) - - ;; Not ok even though it is a function type because it is not otherwise on the - ;; boundary. - (global $3 (export "g3") (ref null $private) (ref.null func)) -) |