Skip to content

Commit

Permalink
Allow rec groups of public function types in closed world (WebAssembl…
Browse files Browse the repository at this point in the history
…y#6053)

Closed-world mode allows function types to escape if they are on exported functions,
because that has been possible since wasm MVP and cannot be avoided. But we need to
also allow all types in those type's rec groups as well. Consider this case:

(module
 (rec
  (type $0 (func))
  (type $1 (func))
 )
 (func "0" (type $0)
  (nop)
 )
 (func "1" (type $1)
  (nop)
 )
)

The two exported functions make the two types public, so this module validates in
closed world mode. Now imagine that metadce removes one export:

(module
 (rec
  (type $0 (func))
  (type $1 (func))
 )
 (func "0" (type $0)
  (nop)
 )
 ;; The export "1" is gone.
)

Before this PR that no longer validates, because it only marks the type $0 as public.
But when a type is public that makes its entire rec group public, so $1 is errored on.

To fix that, this PR allows all types in a rec group of an exported function's type, which
makes that last module validate.
  • Loading branch information
kripken authored and radekdoulik committed Jul 12, 2024
1 parent 8d0c015 commit ec9feee
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 11 deletions.
22 changes: 16 additions & 6 deletions src/wasm/wasm-validator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3734,13 +3734,23 @@ 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<HeapType> publicFuncTypes;
ModuleUtils::iterImportedFunctions(
module, [&](Function* func) { publicFuncTypes.insert(func->type); });
// 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) {
publicFuncTypes.insert(module.getFunction(ex->value)->type);
publicRecGroups.insert(module.getFunction(ex->value)->type.getRecGroup());
}
}

std::unordered_set<HeapType> publicTypes;
for (auto& group : publicRecGroups) {
for (auto type : group) {
publicTypes.insert(type);
}
}

Expand All @@ -3749,7 +3759,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();
Expand Down
11 changes: 6 additions & 5 deletions test/lit/validation/closed-world-interface.wast
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -32,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-2 (struct))
)

(type $private (func (param v128)))
Expand Down Expand Up @@ -61,7 +62,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)
)
Expand Down

0 comments on commit ec9feee

Please sign in to comment.