Do not create invalid public types in MergeSimilarFunctions

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 (lhsCast->target != rhsCast->target &&
+          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++) {

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()) {

test/lit/passes/dae-merge-similar-functions-exact.wast

@@ -0,0 +1,131 @@
+;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
+;; RUN: foreach %s %t wasm-opt --merge-similar-functions --minimize-rec-groups -all --disable-custom-descriptors -S -o - | filecheck %s
+
+;; 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 $pub (func (result (ref $0))))
+  (type $0 (func))
+  (type $pub (func (result (ref $0))))
+  ;; $priv is only used by unreferenced functions ($callee1 and $callee2), so it
+  ;; is initially a private type and is allowed to contain `(ref (exact $0))` in
+  ;; the IR even with --disable-custom-descriptors. Because it stays private,
+  ;; MinimizeRecGroups rewrites it to the equivalent inexact public type $pub.
+  (type $priv (func (result (ref (exact $0)))))
+
+  ;; CHECK:      (type $0 (func))
+
+  ;; Exporting a funcref table makes all referenced (non-private) function
+  ;; signatures public in open-world mode.
+  ;; CHECK:      (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)
+  (elem $e (i32.const 0) (ref.func $pub-fn))
+
+  ;; CHECK:      (export "t" (table $t))
+
+  ;; CHECK:      (export "caller1" (func $caller1))
+
+  ;; CHECK:      (export "caller2" (func $caller2))
+
+  ;; CHECK:      (func $target (type $0)
+  ;; CHECK-NEXT: )
+  (func $target (type $0))
+
+  ;; CHECK:      (func $pub-fn (type $pub) (result (ref $0))
+  ;; CHECK-NEXT:  (ref.func $target)
+  ;; CHECK-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 $pub) (result (ref $0))
+  ;; CHECK-NEXT:  (ref.func $target)
+  ;; CHECK-NEXT: )
+  (func $callee1 (type $priv) (result (ref (exact $0)))
+    ;; Unreferenced helper with private signature $priv. Without the fix,
+    ;; MergeSimilarFunctions would take `(ref.func $callee1)` in a thunk for
+    ;; $caller1 and call it via `(call_ref $priv)` in a shared helper function,
+    ;; promoting $priv from a private type to a public type that collides with
+    ;; $pub in MinimizeRecGroups once --disable-custom-descriptors erases
+    ;; `exact`.
+    (ref.func $target)
+  )
+
+  ;; CHECK:      (func $callee2 (type $pub) (result (ref $0))
+  ;; CHECK-NEXT:  (ref.func $target)
+  ;; CHECK-NEXT: )
+  (func $callee2 (type $priv) (result (ref (exact $0)))
+    ;; Second unreferenced helper with private signature $priv. Without the fix,
+    ;; MergeSimilarFunctions would likewise take `(ref.func $callee2)` in a
+    ;; thunk for $caller2 and pass it to the shared helper function.
+    (ref.func $target)
+  )
+
+  ;; CHECK:      (func $caller1 (type $pub) (result (ref $0))
+  ;; 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: )
+  (func $caller1 (export "caller1") (result (ref $0))
+    ;; Identical to $caller2 except for calling $callee1 instead of $callee2.
+    ;; Without the fix, MergeSimilarFunctions would merge $caller1 and $caller2
+    ;; into a shared function taking `(param (ref $priv))` and replace
+    ;; $caller1's body with `(return_call $byn$mgfn-shared$caller1 (ref.func
+    ;; $callee1))`. With the fix, MergeSimilarFunctions sees that $priv is not a
+    ;; valid public type without custom descriptors and leaves $caller1
+    ;; unmerged.
+    (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:  (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: )
+  (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
+    ;; `(return_call $byn$mgfn-shared$caller1 (ref.func $callee2))`. With the
+    ;; fix, $caller2 is left unmerged so $priv remains private.
+    (nop) (nop) (nop) (nop) (nop) (nop) (nop) (nop)
+    (nop) (nop) (nop) (nop) (nop) (nop) (nop) (nop)
+    (call $callee2)
+  )
+)