diff --git a/src/passes/MergeSimilarFunctions.cpp b/src/passes/MergeSimilarFunctions.cpp index 6b6aeca09d1..a4632f5fe77 100644 --- a/src/passes/MergeSimilarFunctions.cpp +++ b/src/passes/MergeSimilarFunctions.cpp @@ -79,6 +79,7 @@ #include "ir/manipulation.h" #include "ir/module-utils.h" #include "ir/names.h" +#include "ir/public-type-validator.h" #include "ir/utils.h" #include "opt-utils.h" #include "pass.h" @@ -248,6 +249,18 @@ bool MergeSimilarFunctions::areInEquvalentClass(Function* lhs, if (lhsCallee->type != rhsCallee->type) { return false; } + // Parameterizing direct calls to different functions requires creating + // `ref.func` and `call_ref` instructions for them. In an open world, this + // can cause a previously private function signature to become public (for + // instance, if `funcref` is publicly exposed). Do not parameterize the + // call if the callee's signature is not a valid public type (e.g., if it + // contains an exact reference when custom descriptors are disabled). + if (lhsCallee != rhsCallee && + getPassOptions().worldMode == WorldMode::Open && + !PublicTypeValidator(module->features) + .isValidPublicType(lhsCallee->type.getHeapType())) { + return false; + } // Arguments operands should be also equivalent ignoring constants. for (Index i = 0; i < lhsCast->operands.size(); i++) { diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index 42da1dfb6c4..03a445d33bd 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -250,6 +250,10 @@ void validateExactReferences(Module& module, ValidationInfo& info) { return; } + // TODO: This only checks directly exposed root types. To catch all invalid + // public exact references (such as types reachable from exposed types or + // subtypes of exposed `funcref` in open-world mode), we should check all + // public heap types if we can do so without making validation too expensive. for (auto& [type, _] : ModuleUtils::getExposedPublicHeapTypes(module)) { for (auto child : type.getTypeChildren()) { if (child.isExact()) { diff --git a/test/lit/passes/dae-merge-similar-functions-exact.wast b/test/lit/passes/dae-merge-similar-functions-exact.wast new file mode 100644 index 00000000000..c102a35e277 --- /dev/null +++ b/test/lit/passes/dae-merge-similar-functions-exact.wast @@ -0,0 +1,221 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. + +;; RUN: wasm-opt %s --dae --merge-similar-functions --minimize-rec-groups \ +;; RUN: -all --disable-custom-descriptors -S -o - | filecheck %s + +;; RUN: wasm-opt %s --dae --merge-similar-functions --minimize-rec-groups \ +;; RUN: -all --disable-custom-descriptors --closed-world -S -o - \ +;; RUN: | filecheck %s --check-prefix=CLOSD + +;; Regression test for a bug where MergeSimilarFunctions parameterized direct +;; calls to unreferenced functions ($callee1 and $callee2) whose private +;; signature ($priv) contained an exact reference, making $priv public when +;; custom descriptors were disabled and causing it to collide with the existing +;; inexact public signature ($pub) in MinimizeRecGroups. + +(module + ;; CHECK: (type $0 (func)) + ;; CLOSD: (type $0 (func)) + (type $0 (func)) + ;; CHECK: (type $pub (func (result (ref $0)))) + ;; CLOSD: (type $pub (func (result (ref $0)))) + (type $pub (func (result (ref $0)))) + + ;; Exporting a funcref table makes all referenced (non-private) function + ;; signatures public in open-world mode. + ;; CHECK: (rec + ;; CHECK-NEXT: (type $2 (struct)) + + ;; CHECK: (type $3 (func (result (ref (exact $0))))) + + ;; CHECK: (table $t 1 1 funcref) + ;; CLOSD: (rec + ;; CLOSD-NEXT: (type $2 (struct)) + + ;; CLOSD: (type $3 (func (result (ref (exact $0))))) + + ;; CLOSD: (type $4 (func (param (ref $3)) (result (ref $0)))) + + ;; CLOSD: (table $t 1 1 funcref) + (table $t (export "t") 1 1 funcref) + ;; Referencing $pub-fn in an element segment makes $pub a public signature. + ;; CHECK: (elem $e (i32.const 0) $pub-fn) + ;; CLOSD: (elem $e (i32.const 0) $pub-fn) + (elem $e (i32.const 0) $pub-fn) + + ;; CHECK: (elem declare func $target) + + ;; CHECK: (export "t" (table $t)) + + ;; CHECK: (export "caller1" (func $caller1)) + + ;; CHECK: (export "caller2" (func $caller2)) + + ;; CHECK: (func $target (type $0) + ;; CHECK-NEXT: ) + ;; CLOSD: (elem declare func $callee1 $callee2 $target) + + ;; CLOSD: (export "t" (table $t)) + + ;; CLOSD: (export "caller1" (func $caller1)) + + ;; CLOSD: (export "caller2" (func $caller2)) + + ;; CLOSD: (func $target (type $0) + ;; CLOSD-NEXT: ) + (func $target (type $0)) + + ;; CHECK: (func $pub-fn (type $pub) (result (ref $0)) + ;; CHECK-NEXT: (ref.func $target) + ;; CHECK-NEXT: ) + ;; CLOSD: (func $pub-fn (type $pub) (result (ref $0)) + ;; CLOSD-NEXT: (ref.func $target) + ;; CLOSD-NEXT: ) + (func $pub-fn (type $pub) (result (ref $0)) + ;; Referenced in $e, so its signature $pub is public from the start. + (ref.func $target) + ) + + ;; CHECK: (func $callee1 (type $3) (result (ref (exact $0))) + ;; CHECK-NEXT: (ref.func $target) + ;; CHECK-NEXT: ) + ;; CLOSD: (func $callee1 (type $3) (result (ref (exact $0))) + ;; CLOSD-NEXT: (ref.func $target) + ;; CLOSD-NEXT: ) + (func $callee1 (result (ref $0)) + ;; Unreferenced helper. DAE refines its return type to `(ref (exact $0))`. + ;; Without the fix in MergeSimilarFunctions, MergeSimilarFunctions would + ;; take `(ref.func $callee1)` in a thunk for $caller1 and call it via + ;; `call_ref` in a shared helper, promoting its refined signature to a + ;; public type that collides with $pub in MinimizeRecGroups once + ;; --disable-custom-descriptors erases `exact`. + (ref.func $target) + ) + + ;; CHECK: (func $callee2 (type $3) (result (ref (exact $0))) + ;; CHECK-NEXT: (ref.func $target) + ;; CHECK-NEXT: ) + ;; CLOSD: (func $callee2 (type $3) (result (ref (exact $0))) + ;; CLOSD-NEXT: (ref.func $target) + ;; CLOSD-NEXT: ) + (func $callee2 (result (ref $0)) + ;; Second unreferenced helper whose return type is refined to + ;; `(ref (exact $0))` by DAE. + (ref.func $target) + ) + + ;; CHECK: (func $caller1 (type $pub) (result (ref $0)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (call $callee1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (call $callee1) + ;; CHECK-NEXT: ) + ;; CLOSD: (func $caller1 (type $pub) (result (ref $0)) + ;; CLOSD-NEXT: (return_call $byn$mgfn-shared$caller1 + ;; CLOSD-NEXT: (ref.func $callee1) + ;; CLOSD-NEXT: ) + ;; CLOSD-NEXT: ) + (func $caller1 (export "caller1") (result (ref $0)) + ;; Wrapping the first call in `ref.as_non_null` ensures DAE does not treat + ;; the call as dropped and instead refines $callee1's return type. + ;; Without the fix, MergeSimilarFunctions would merge $caller1 and $caller2 + ;; into a shared function taking a parameter of $callee1's refined signature + ;; and replace $caller1's body with a thunk passing `(ref.func $callee1)`. + ;; With the fix, MergeSimilarFunctions sees that the refined signature is + ;; not a valid public type without custom descriptors and leaves $caller1 + ;; unmerged. The nops are so MergeSimilarFunctions would think this function + ;; and $caller2 are otherwise profitable to merge. + ;; + ;; We can still optimize with --closed-world. + (drop (ref.as_non_null (call $callee1))) + (nop) (nop) (nop) (nop) (nop) (nop) (nop) (nop) + (nop) (nop) (nop) (nop) (nop) (nop) (nop) (nop) + (call $callee1) + ) + + ;; CHECK: (func $caller2 (type $pub) (result (ref $0)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (call $callee2) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (call $callee2) + ;; CHECK-NEXT: ) + ;; CLOSD: (func $caller2 (type $pub) (result (ref $0)) + ;; CLOSD-NEXT: (return_call $byn$mgfn-shared$caller1 + ;; CLOSD-NEXT: (ref.func $callee2) + ;; CLOSD-NEXT: ) + ;; CLOSD-NEXT: ) + (func $caller2 (export "caller2") (result (ref $0)) + ;; Identical to $caller1 except for calling $callee2 instead of $callee1. + ;; Without the fix, MergeSimilarFunctions would replace $caller2's body + ;; with a thunk passing `(ref.func $callee2)`. With the fix, $caller2 is + ;; left unmerged so the refined signature remains private. + (drop (ref.as_non_null (call $callee2))) + (nop) (nop) (nop) (nop) (nop) (nop) (nop) (nop) + (nop) (nop) (nop) (nop) (nop) (nop) (nop) (nop) + (call $callee2) + ) +) +;; CLOSD: (func $byn$mgfn-shared$caller1 (type $4) (param $0 (ref $3)) (result (ref $0)) +;; CLOSD-NEXT: (drop +;; CLOSD-NEXT: (ref.as_non_null +;; CLOSD-NEXT: (call_ref $3 +;; CLOSD-NEXT: (local.get $0) +;; CLOSD-NEXT: ) +;; CLOSD-NEXT: ) +;; CLOSD-NEXT: ) +;; CLOSD-NEXT: (nop) +;; CLOSD-NEXT: (nop) +;; CLOSD-NEXT: (nop) +;; CLOSD-NEXT: (nop) +;; CLOSD-NEXT: (nop) +;; CLOSD-NEXT: (nop) +;; CLOSD-NEXT: (nop) +;; CLOSD-NEXT: (nop) +;; CLOSD-NEXT: (nop) +;; CLOSD-NEXT: (nop) +;; CLOSD-NEXT: (nop) +;; CLOSD-NEXT: (nop) +;; CLOSD-NEXT: (nop) +;; CLOSD-NEXT: (nop) +;; CLOSD-NEXT: (nop) +;; CLOSD-NEXT: (nop) +;; CLOSD-NEXT: (call_ref $3 +;; CLOSD-NEXT: (local.get $0) +;; CLOSD-NEXT: ) +;; CLOSD-NEXT: )