From 60e2c39eb6a39fb4299f781df9edf69311ebbc28 Mon Sep 17 00:00:00 2001
From: Hood Chatham <roberthoodchatham@gmail.com>
Date: Fri, 25 Oct 2024 16:55:12 +0200
Subject: [PATCH] Add comments

---
 .../calculate_wasm_func_nargs_fallback.mjs    | 48 +++++++++++++++++--
 1 file changed, 43 insertions(+), 5 deletions(-)

diff --git a/src/core/stack_switching/calculate_wasm_func_nargs_fallback.mjs b/src/core/stack_switching/calculate_wasm_func_nargs_fallback.mjs
index ea9d02b153f6..42e98a2791e7 100644
--- a/src/core/stack_switching/calculate_wasm_func_nargs_fallback.mjs
+++ b/src/core/stack_switching/calculate_wasm_func_nargs_fallback.mjs
@@ -1,3 +1,33 @@
+// Various function pointer mismatch bugs occur because people declare a
+// `METH_NOARGS` function which should take 2 arguments:
+// ```
+// my_no_arg_meth(PyObject* module, PyObject* always_null);
+// ```
+// but leave off the `always_null` second argument or both arguments. Similar
+// errors occur less frequently with `METH_VARARGS | METH_KWDS` functions. When
+// the interpreter tries to use a call_indirect to invoke these methods, we hit
+// an indirect call signature mismatch fatal error.
+//
+// Traditionally we used a JS trampoline to deal with this, because calls from
+// JS to Wasm don't care if the wrong number of arguments are passed. However,
+// these trampolines do not work with JSPI because we cannot stack switch
+// through JavaScript frames.
+//
+// Originally JSPI implied wasm type reflection, so we could ask JS what the
+// type of the function was and then select a `call_indirect` with the right
+// number of arguments based on this result.
+//
+// The new JSPI does not imply that wasm type reflection exists, so we need a
+// way to handle the case when JSPI is available but wasm type reflection is
+// not. What we do here is make a tiny WebAssembly module that attempts to
+// import a single function. If the signature of the function matches the
+// signature of the import, it will succeed. Otherwise, we will raise a
+// LinkError.
+//
+// We only need to handle functions with four different possible signatures:
+// (n i32's) => i32 where n is between 0 and 3. So we try to link at most 4
+// different Wasm modules and find out the signature.
+
 const checkerModules = [undefined, undefined, undefined, undefined];
 
 function makeCheckerModule(n) {
@@ -8,7 +38,7 @@ function makeCheckerModule(n) {
   // )
   //
   // with n i32's. We'll try to import the function to this module to see if it
-  // has this signature.
+  // has this signature. If not it rasises a link error.
   return new WebAssembly.Module(
     // prettier-ignore
     new Uint8Array([
@@ -41,12 +71,20 @@ function getCheckerModule(n) {
 
 export function calculateWasmFuncNargsFallback(functionPtr) {
   for (let n = 0; n < 4; n++) {
+    const mod = getCheckerModule(n);
+    const imports = {
+      e: { f: wasmTable.get(functionPtr) },
+    };
     try {
-      new WebAssembly.Instance(getCheckerModule(n), {
-        e: { f: wasmTable.get(functionPtr) },
-      });
+      new WebAssembly.Instance(mod, imports);
       return n;
-    } catch (e) {}
+    } catch (e) {
+      // Should be a LinkError, if not we have a logic error. Raise fatal error
+      // so it's noisy.
+      if (!e instanceof WebAssembly.LinkError) {
+        throw e;
+      }
+    }
   }
   return -1;
 }