From 6ca99495d82ca727a3b54d86df1f4e177146561c Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 25 Oct 2023 13:35:58 -0700 Subject: [PATCH 1/5] work --- src/wasm/wasm-validator.cpp | 19 +++++++++++++----- .../closed-world-interface-groups.wast | 20 +++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) create mode 100644 test/lit/validation/closed-world-interface-groups.wast diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index 179235fbcc2..ec09f0e68fd 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -3734,13 +3734,22 @@ 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. - std::unordered_set publicFuncTypes; + // 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 publicRecGroups; ModuleUtils::iterImportedFunctions( - module, [&](Function* func) { publicFuncTypes.insert(func->type); }); + module, [&](Function* func) { publicRecGroups.insert(func->type.getRecGroup()); }); for (auto& ex : module.exports) { if (ex->kind == ExternalKind::Function) { - publicFuncTypes.insert(module.getFunction(ex->value)->type); + publicRecGroups.insert(module.getFunction(ex->value)->type.getRecGroup()); + } + } + + std::unordered_set publicTypes; + for (auto& group : publicRecGroups) { + for (auto type : group) { + publicTypes.insert(type); } } @@ -3749,7 +3758,7 @@ static void validateClosedWorldInterface(Module& module, ValidationInfo& info) { auto ignorable = getIgnorablePublicTypes(); for (auto type : ModuleUtils::getPublicHeapTypes(module)) { - if (!publicFuncTypes.count(type) && !ignorable.count(type)) { + 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(); diff --git a/test/lit/validation/closed-world-interface-groups.wast b/test/lit/validation/closed-world-interface-groups.wast new file mode 100644 index 00000000000..3d41c2a9a0c --- /dev/null +++ b/test/lit/validation/closed-world-interface-groups.wast @@ -0,0 +1,20 @@ +;; Test that we do not error out on rec group members of public function types +;; with --closed-world + +;; RUN: wasm-opt -all --closed-world %s + +(module + (rec + ;; This type is directly public because it is used on an exported function. + (type $0 (func)) + ;; This type is not used anywhere, but it is public because $0 is public, + ;; which makes this entire rec group public. + (type $1 (func)) + ;; The same for a non-function type. + (type $2 (struct)) + ) + (export "export-import-A" (func $0)) + (func $0 (type $0) + (nop) + ) +) From 647e61d94432c0520b05ff919fb576f28d124575 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 25 Oct 2023 13:36:08 -0700 Subject: [PATCH 2/5] format --- src/wasm/wasm-validator.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index ec09f0e68fd..cd90e767eca 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -3738,8 +3738,9 @@ static void validateClosedWorldInterface(Module& module, ValidationInfo& info) { // groups that are used, as if a type if public then all types in its rec // group are as well. std::unordered_set publicRecGroups; - ModuleUtils::iterImportedFunctions( - module, [&](Function* func) { publicRecGroups.insert(func->type.getRecGroup()); }); + 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()); From 5a941ac539c9555d6594f2564d7869c0911198a6 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 26 Oct 2023 13:31:42 -0700 Subject: [PATCH 3/5] fix --- test/lit/validation/closed-world-interface.wast | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/lit/validation/closed-world-interface.wast b/test/lit/validation/closed-world-interface.wast index 06cf1cef715..9241db9f8a0 100644 --- a/test/lit/validation/closed-world-interface.wast +++ b/test/lit/validation/closed-world-interface.wast @@ -3,10 +3,6 @@ ;; RUN: not wasm-opt -all --closed-world %s 2>&1 | filecheck %s -;; This is pulled in because it is part of a rec group with $partial-pair-0. -;; CHECK: publicly exposed type disallowed with a closed world: $partial-pair-1, on -;; CHECK-NEXT: (func) - ;; This is pulled in by a global. ;; CHECK: publicly exposed type disallowed with a closed world: $array, on ;; CHECK-NEXT: (array (mut i32)) @@ -61,7 +57,7 @@ ;; Ok even though it is an import instead of an export. (func $5 (import "env" "test5") (type $exported-pair-1)) - ;; Not ok because another type in the group is not on the boundary. + ;; Ok, and we also allow the other type in the group. (func $6 (export "test6") (type $partial-pair-0) (unreachable) ) From dd042982d894be3aa2aa3abc5578c32a71a5a14c Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 26 Oct 2023 13:33:34 -0700 Subject: [PATCH 4/5] test --- .../closed-world-interface-groups.wast | 20 ------------------- .../validation/closed-world-interface.wast | 5 +++++ 2 files changed, 5 insertions(+), 20 deletions(-) delete mode 100644 test/lit/validation/closed-world-interface-groups.wast diff --git a/test/lit/validation/closed-world-interface-groups.wast b/test/lit/validation/closed-world-interface-groups.wast deleted file mode 100644 index 3d41c2a9a0c..00000000000 --- a/test/lit/validation/closed-world-interface-groups.wast +++ /dev/null @@ -1,20 +0,0 @@ -;; Test that we do not error out on rec group members of public function types -;; with --closed-world - -;; RUN: wasm-opt -all --closed-world %s - -(module - (rec - ;; This type is directly public because it is used on an exported function. - (type $0 (func)) - ;; This type is not used anywhere, but it is public because $0 is public, - ;; which makes this entire rec group public. - (type $1 (func)) - ;; The same for a non-function type. - (type $2 (struct)) - ) - (export "export-import-A" (func $0)) - (func $0 (type $0) - (nop) - ) -) diff --git a/test/lit/validation/closed-world-interface.wast b/test/lit/validation/closed-world-interface.wast index 9241db9f8a0..58cbeda8794 100644 --- a/test/lit/validation/closed-world-interface.wast +++ b/test/lit/validation/closed-world-interface.wast @@ -28,8 +28,13 @@ (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-1 (struct)) ) (type $private (func (param v128))) From 74eeeac7a00ce54a1d4f40f1e1d85a4d10a11b34 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 26 Oct 2023 13:33:58 -0700 Subject: [PATCH 5/5] oops --- test/lit/validation/closed-world-interface.wast | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lit/validation/closed-world-interface.wast b/test/lit/validation/closed-world-interface.wast index 58cbeda8794..daedaf990de 100644 --- a/test/lit/validation/closed-world-interface.wast +++ b/test/lit/validation/closed-world-interface.wast @@ -34,7 +34,7 @@ ;; entire rec group is allowed due to the first. (type $partial-pair-1 (func)) ;; Test a non-function type. - (type $partial-pair-1 (struct)) + (type $partial-pair-2 (struct)) ) (type $private (func (param v128)))