Partially allow nested VM operations except for rewinding

kateinoigakukun · kateinoigakukun · commit c7c43a8e06b9 · 2023-05-18T18:42:53.000+09:00
diff --git a/ext/witapi/bindgen/rb-abi-guest.c b/ext/witapi/bindgen/rb-abi-guest.c
@@ -205,3 +205,8 @@ __attribute__((export_name("rb-vm-bugreport: func() -> ()")))
 void __wasm_export_rb_abi_guest_rb_vm_bugreport(void) {
   rb_abi_guest_rb_vm_bugreport();
 }
+__attribute__((export_name("rb-set-should-prohibit-rewind: func(new-value: bool) -> bool")))
+int32_t __wasm_export_rb_abi_guest_rb_set_should_prohibit_rewind(int32_t arg) {
+  bool ret = rb_abi_guest_rb_set_should_prohibit_rewind(arg);
+  return ret;
+}
diff --git a/ext/witapi/bindgen/rb-abi-guest.h b/ext/witapi/bindgen/rb-abi-guest.h
@@ -68,6 +68,7 @@ extern "C"
   void rb_abi_guest_rb_clear_errinfo(void);
   void rb_abi_guest_rstring_ptr(rb_abi_guest_rb_abi_value_t value, rb_abi_guest_string_t *ret0);
   void rb_abi_guest_rb_vm_bugreport(void);
+  bool rb_abi_guest_rb_set_should_prohibit_rewind(bool new_value);
   #ifdef __cplusplus
 }
 #endif
diff --git a/ext/witapi/bindgen/rb-abi-guest.wit b/ext/witapi/bindgen/rb-abi-guest.wit
@@ -18,3 +18,5 @@ rb-clear-errinfo: func()
 rstring-ptr: func(value: rb-abi-value) -> string
 
 rb-vm-bugreport: func()
+
+rb-set-should-prohibit-rewind: func(new-value: bool) -> bool
diff --git a/ext/witapi/witapi-core.c b/ext/witapi/witapi-core.c
@@ -63,6 +63,8 @@ __attribute__((import_module("asyncify"), import_name("stop_unwind"))) void
 asyncify_stop_unwind(void);
 __attribute__((import_module("asyncify"), import_name("start_rewind"))) void
 asyncify_start_rewind(void *buf);
+#define asyncify_start_rewind(buf) \
+  asyncify_start_rewind((buf))
 __attribute__((import_module("asyncify"), import_name("stop_rewind"))) void
 asyncify_stop_rewind(void);
 
@@ -79,6 +81,23 @@ void *rb_wasm_handle_fiber_unwind(void (**new_fiber_entry)(void *, void *),
 #  define RB_WASM_DEBUG_LOG(...) (void)0
 #endif
 
+static bool rb_should_prohibit_rewind = false;
+
+__attribute__((import_module("rb-js-abi-host"),
+               import_name("rb_wasm_throw_prohibit_rewind_exception")))
+__attribute__((noreturn)) void
+rb_wasm_throw_prohibit_rewind_exception(const char *c_msg, size_t msg_len);
+
+#define RB_WASM_CHECK_REWIND_PROHIBITED(msg)                                   \
+  /*
+    If the unwond source and rewinding destination are same, it's acceptable
+    to rewind even under nested VM operations.
+   */                                                                          \
+  if (rb_should_prohibit_rewind &&                                             \
+      (asyncify_buf != asyncify_unwound_buf || fiber_entry_point)) {           \
+    rb_wasm_throw_prohibit_rewind_exception(msg, sizeof(msg) - 1);             \
+  }
+
 #define RB_WASM_LIB_RT(MAIN_ENTRY)                                             \
   {                                                                            \
                                                                                \
@@ -93,24 +112,31 @@ void *rb_wasm_handle_fiber_unwind(void (**new_fiber_entry)(void *, void *),
       }                                                                        \
                                                                                \
       bool new_fiber_started = false;                                          \
-      void *asyncify_buf;                                                      \
+      void *asyncify_buf = NULL;                                               \
+      extern void *rb_asyncify_unwind_buf;                                     \
+      void *asyncify_unwound_buf = rb_asyncify_unwind_buf;                     \
       asyncify_stop_unwind();                                                  \
                                                                                \
       if ((asyncify_buf = rb_wasm_handle_jmp_unwind()) != NULL) {              \
+        RB_WASM_CHECK_REWIND_PROHIBITED("rb_wasm_handle_jmp_unwind")           \
         asyncify_start_rewind(asyncify_buf);                                   \
         continue;                                                              \
       }                                                                        \
       if ((asyncify_buf = rb_wasm_handle_scan_unwind()) != NULL) {             \
+        RB_WASM_CHECK_REWIND_PROHIBITED("rb_wasm_handle_scan_unwind")          \
         asyncify_start_rewind(asyncify_buf);                                   \
         continue;                                                              \
       }                                                                        \
                                                                                \
       asyncify_buf = rb_wasm_handle_fiber_unwind(&fiber_entry_point, &arg0,    \
                                                  &arg1, &new_fiber_started);   \
       if (asyncify_buf) {                                                      \
+        RB_WASM_CHECK_REWIND_PROHIBITED("rb_wasm_handle_fiber_unwind")         \
         asyncify_start_rewind(asyncify_buf);                                   \
         continue;                                                              \
       } else if (new_fiber_started) {                                          \
+        RB_WASM_CHECK_REWIND_PROHIBITED(                                       \
+            "rb_wasm_handle_fiber_unwind but new fiber");                      \
         continue;                                                              \
       }                                                                        \
                                                                                \
@@ -303,4 +329,10 @@ void rb_vm_bugreport(const void *);
 
 void rb_abi_guest_rb_vm_bugreport(void) { rb_vm_bugreport(NULL); }
 
+bool rb_abi_guest_rb_set_should_prohibit_rewind(bool value) {
+  bool old = rb_should_prohibit_rewind;
+  rb_should_prohibit_rewind = value;
+  return old;
+}
+
 void Init_witapi(void) {}
diff --git a/packages/npm-packages/ruby-wasm-wasi/src/bindgen/rb-abi-guest.d.ts b/packages/npm-packages/ruby-wasm-wasi/src/bindgen/rb-abi-guest.d.ts
@@ -80,6 +80,7 @@ export class RbAbiGuest {
   rbClearErrinfo(): void;
   rstringPtr(value: RbAbiValue): string;
   rbVmBugreport(): void;
+  rbSetShouldProhibitRewind(newValue: boolean): boolean;
 }
 
 export class RbIseq {
diff --git a/packages/npm-packages/ruby-wasm-wasi/src/bindgen/rb-abi-guest.js b/packages/npm-packages/ruby-wasm-wasi/src/bindgen/rb-abi-guest.js
@@ -1,4 +1,4 @@
-import { data_view, to_uint32, UTF8_DECODER, utf8_encode, UTF8_ENCODED_LEN, Slab } from './intrinsics.js';
+import { data_view, to_uint32, UTF8_DECODER, utf8_encode, UTF8_ENCODED_LEN, Slab, throw_invalid_bool } from './intrinsics.js';
 export class RbAbiGuest {
   constructor() {
     this._resource0_slab = new Slab();
@@ -161,6 +161,11 @@ export class RbAbiGuest {
   rbVmBugreport() {
     this._exports['rb-vm-bugreport: func() -> ()']();
   }
+  rbSetShouldProhibitRewind(arg0) {
+    const ret = this._exports['rb-set-should-prohibit-rewind: func(new-value: bool) -> bool'](arg0 ? 1 : 0);
+    const bool0 = ret;
+    return bool0 == 0 ? false : (bool0 == 1 ? true : throw_invalid_bool());
+  }
 }
 
 export class RbIseq {
diff --git a/packages/npm-packages/ruby-wasm-wasi/src/index.ts b/packages/npm-packages/ruby-wasm-wasi/src/index.ts
@@ -38,7 +38,7 @@ export class RubyVM {
     // Wrap exported functions from Ruby VM to prohibit nested VM operation
     // if the call stack has sandwitched JS frames like JS -> Ruby -> JS -> Ruby.
     const proxyExports = (exports: RbAbi.RbAbiGuest) => {
-      const excludedMethods: (keyof RbAbi.RbAbiGuest)[] = ["addToImports", "instantiate"];
+      const excludedMethods: (keyof RbAbi.RbAbiGuest)[] = ["addToImports", "instantiate", "rbSetShouldProhibitRewind"];
       const excluded = ["constructor"].concat(excludedMethods);
       // wrap all methods in RbAbi.RbAbiGuest class
       for (const key of Object.getOwnPropertyNames(RbAbi.RbAbiGuest.prototype)) {
@@ -49,10 +49,15 @@ export class RubyVM {
         if (typeof value === "function") {
           exports[key] = (...args: any[]) => {
             const isNestedVMCall = this.interfaceState.hasJSFrameAfterRbFrame;
+            let oldValue = false;
             if (isNestedVMCall) {
-              throw new Error("Nested VM operation is prohibited. If you are trying to call RubyVM API environment through Ruby, please call it from JS environment directly.");
+              oldValue = this.guest.rbSetShouldProhibitRewind(true)
             }
-            return Reflect.apply(value, exports, args)
+            const result = Reflect.apply(value, exports, args)
+            if (isNestedVMCall) {
+              this.guest.rbSetShouldProhibitRewind(oldValue)
+            }
+            return result;
           }
         }
       }
@@ -103,10 +108,35 @@ export class RubyVM {
         try {
           return { tag: "success", val: f(...args) };
         } catch (e) {
+          if (e instanceof RbFatalError) {
+            // RbFatalError should not be caught by Ruby because it Ruby VM
+            // can be already in an inconsistent state.
+            throw e;
+          }
           return { tag: "failure", val: e };
         }
       };
     }
+    imports["rb-js-abi-host"] = {
+      rb_wasm_throw_prohibit_rewind_exception: (messagePtr: number, messageLen: number) => {
+        const memory = this.instance.exports.memory as WebAssembly.Memory;
+        const str = new TextDecoder().decode(
+          new Uint8Array(memory.buffer, messagePtr, messageLen)
+        );
+        throw new RbFatalError(
+          "Ruby APIs that may rewind the VM stack are prohibited under nested VM operation " + `(${str})\n`
+          + "Nested VM operation means that the call stack has sandwitched JS frames like JS -> Ruby -> JS -> Ruby "
+          + "caused by something like `window.rubyVM.eval(\"JS.global[:rubyVM].eval('Fiber.yield')\")`\n"
+          + "\n"
+          + "Please check your call stack and make sure that you are **not** doing any of the following inside the nested Ruby frame:\n"
+          + "  1. Switching fibers (e.g. Fiber#resume, Fiber.yield, and Fiber#transfer)\n"
+          + "     Note that JS::Object#await switches fibers internally\n"
+          + "  2. Raising uncaught exceptions\n"
+          + "     Please catch all exceptions inside the nested operation\n"
+          + "  3. Calling Continuation APIs\n"
+        );
+      }
+    }
     // NOTE: The GC may collect objects that are still referenced by Wasm
     // locals because Asyncify cannot scan the Wasm stack above the JS frame.
     // So we need to keep track whether the JS frame is sandwitched by Ruby
@@ -621,3 +651,16 @@ export class RbError extends Error {
     super(message);
   }
 }
+
+/**
+ * Error class thrown by Ruby execution when it is not possible to recover.
+ * This is usually caused when Ruby VM is in an inconsistent state.
+ */
+export class RbFatalError extends RbError {
+  /**
+   * @hideconstructor
+   */
+  constructor(message: string) {
+    super("Ruby Fatal Error: " + message);
+  }
+}
diff --git a/packages/npm-packages/ruby-wasm-wasi/test/vm.test.ts b/packages/npm-packages/ruby-wasm-wasi/test/vm.test.ts
@@ -120,29 +120,32 @@ eval:11:in \`<main>'`);
     expect(vm.eval(`__ENCODING__.name`).toString()).toBe("UTF-8");
   });
 
-  // FIXME: The below two tests are now failing because we cannot make it safe
-  // to call eval within eval.
-  // See https://github.com/ruby/ruby.wasm/issues/219#issuecomment-1551758711
-  test.failing("Nested Fiber switch", async () => {
-    const vm = await initRubyVM();
-    const setVM = vm.eval(`proc { |vm| JS::RubyVM = vm }`)
-    setVM.call("call", vm.wrap(vm))
-    vm.eval(`
+  test.each([
+    `JS::RubyVM.eval('Fiber.yield')`,
+    `
     $f0 = Fiber.new {
       JS::RubyVM.eval("$f1.resume")
     }
     $f1 = Fiber.new {}
     $f0.resume
-    `)
-    vm.eval(`puts $q.inspect`)
+    `,
+    `JS::RubyVM.eval("raise 'Exception from nested eval'")`
+  ])
+  ("nested VM rewinding operation should throw fatal error", async (code) => {
+    const vm = await initRubyVM();
+    const setVM = vm.eval(`proc { |vm| JS::RubyVM = vm }`)
+    setVM.call("call", vm.wrap(vm))
+    expect(() => {
+      vm.eval(code)
+    }).toThrowError("Ruby APIs that may rewind the VM stack are prohibited")
   })
 
-  test.failing("raise in nested eval", async () => {
+  test("caught raise in nested eval is ok", async () => {
     const vm = await initRubyVM();
     const setVM = vm.eval(`proc { |vm| JS::RubyVM = vm }`)
     setVM.call("call", vm.wrap(vm))
     expect(() => {
-      vm.eval(`JS::RubyVM.eval("raise 'Exception from nested eval'")`)
-    }).toThrowError("Exception from nested eval")
+      vm.eval(`JS::RubyVM.eval("begin; raise 'Exception from nested eval'; rescue; end")`)
+    }).not.toThrowError()
   })
 });

Original file line number	Diff line number	Diff line change
`@@ -205,3 +205,8 @@ __attribute__((export_name("rb-vm-bugreport: func() -> ()")))`
`205`	`205`	`void __wasm_export_rb_abi_guest_rb_vm_bugreport(void) {`
`206`	`206`	`rb_abi_guest_rb_vm_bugreport();`
`207`	`207`	`}`
	`208`	`+__attribute__((export_name("rb-set-should-prohibit-rewind: func(new-value: bool) -> bool")))`
	`209`	`+int32_t __wasm_export_rb_abi_guest_rb_set_should_prohibit_rewind(int32_t arg) {`
	`210`	`+ bool ret = rb_abi_guest_rb_set_should_prohibit_rewind(arg);`
	`211`	`+ return ret;`
	`212`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -68,6 +68,7 @@ extern "C"`
`68`	`68`	`void rb_abi_guest_rb_clear_errinfo(void);`
`69`	`69`	`void rb_abi_guest_rstring_ptr(rb_abi_guest_rb_abi_value_t value, rb_abi_guest_string_t *ret0);`
`70`	`70`	`void rb_abi_guest_rb_vm_bugreport(void);`
	`71`	`+ bool rb_abi_guest_rb_set_should_prohibit_rewind(bool new_value);`
`71`	`72`	`#ifdef __cplusplus`
`72`	`73`	`}`
`73`	`74`	`#endif`
Original file line number	Diff line number	Diff line change
`@@ -80,6 +80,7 @@ export class RbAbiGuest {`
`80`	`80`	`rbClearErrinfo(): void;`
`81`	`81`	`rstringPtr(value: RbAbiValue): string;`
`82`	`82`	`rbVmBugreport(): void;`
	`83`	`+ rbSetShouldProhibitRewind(newValue: boolean): boolean;`
`83`	`84`	`}`
`84`	`85`
`85`	`86`	`export class RbIseq {`