Introducing the mrb_protect_raw() API function

The purpose is two-fold:

 1. to be able to specify a pointer directly when user data is used

    When using `mrb_protect()`, it is necessary to allocate objects by `mrb_obj_cptr()` function when using user data.

    Adding `mrb_protect_raw()` will make it simpler to reimplement `mrbgems/mruby-error`.

 2. to correctly unwind callinfo when an exception is raised from a C function defined as a method (the main topic)

    If a method call is made directly under `mrb_protect()` and a C function is called, control is returned from `mrb_protect()` if an exception occurs there.
    In this case, callinfo is not restored, so it is out of sync.
    Moreover, returning to mruby VM (`mrb_vm_exec()` function) in this state will indicate `ci->pc` of C function which is equal to `NULL`, and subsequent `JUMP` will cause `SIGSEGV`.

    Following is an example that actually causes `SIGSEGV`:

    - `crash.c`

      ```c
      #include <mruby.h>
      #include <mruby/compile.h>
      #include <mruby/error.h>

      static mrb_value
      level1_body(mrb_state *mrb, mrb_value self)
      {
        return mrb_funcall(mrb, self, "level2", 0);
      }

      static mrb_value
      level1(mrb_state *mrb, mrb_value self)
      {
        return mrb_protect(mrb, level1_body, self, NULL);
      }

      static mrb_value
      level2(mrb_state *mrb, mrb_value self)
      {
        mrb_raise(mrb, E_RUNTIME_ERROR, "error!");
        return mrb_nil_value();
      }

      int
      main(int argc, char *argv[])
      {
        mrb_state *mrb = mrb_open();
        mrb_define_method(mrb, mrb->object_class, "level1", level1, MRB_ARGS_NONE());
        mrb_define_method(mrb, mrb->object_class, "level2", level2, MRB_ARGS_NONE());
        mrb_p(mrb, mrb_load_string(mrb, "p level1"));
        mrb_close(mrb);
        return 0;
      }
      ```

    - compile & run

      ```console
      % `bin/mruby-config --cc --cflags --ldflags` crash.c `bin/mruby-config --libs`

      % ./a.out
      zsh: segmentation fault (core dumped)  ./a.out
      ```

    After applying this patch, it will print exception object and exit normally.

The `mrb_protect()`, `mrb_ensure()` and `mrb_rescue_exceptions()` in `mrbgems/mruby-error` have been rewritten using `mrb_protect_raw()`.

include/mruby/error.h

@@ -132,6 +132,14 @@ MRB_API mrb_value mrb_rescue_exceptions(mrb_state *mrb, mrb_func_t body, mrb_val
                                         mrb_func_t rescue, mrb_value r_data,
                                         mrb_int len, struct RClass **classes);
 
+typedef mrb_value mrb_protect_raw_func(mrb_state *mrb, void *userdata);
+
+/**
+ * This API function behaves like `mrb_protect()`.
+ * The advantage is that it avoids objectifying the user data.
+ */
+MRB_API mrb_value mrb_protect_raw(mrb_state *mrb, mrb_protect_raw_func *body, void *userdata, mrb_bool *error);
+
 MRB_END_DECL
 
 #endif  /* MRUBY_ERROR_H */

mrbgems/mruby-error/mrbgem.rake

@@ -2,9 +2,4 @@ MRuby::Gem::Specification.new('mruby-error') do |spec|
   spec.license = 'MIT'
   spec.author  = 'mruby developers'
   spec.summary = 'extensional error handling'
-
-  if build.cxx_exception_enabled?
-    objs << build.compile_as_cxx("#{spec.dir}/src/exception.c")
-    objs.delete_if { |v| v == objfile("#{spec.build_dir}/src/exception") }
-  end
 end

mrbgems/mruby-error/src/exception.c

@@ -1,57 +1,38 @@
 #include <mruby.h>
-#include <mruby/throw.h>
 #include <mruby/error.h>
 
+struct protect_data {
+  mrb_func_t body;
+  mrb_value data;
+};
+
+static mrb_value
+protect_body(mrb_state *mrb, void *p)
+{
+  struct protect_data *dp = (struct protect_data*)p;
+  return dp->body(mrb, dp->data);
+}
+
 MRB_API mrb_value
 mrb_protect(mrb_state *mrb, mrb_func_t body, mrb_value data, mrb_bool *state)
 {
-  struct mrb_jmpbuf *prev_jmp = mrb->jmp;
-  struct mrb_jmpbuf c_jmp;
-  mrb_value result = mrb_nil_value();
-  int ai = mrb_gc_arena_save(mrb);
-
-  if (state) { *state = FALSE; }
-
-  MRB_TRY(&c_jmp) {
-    mrb->jmp = &c_jmp;
-    result = body(mrb, data);
-    mrb->jmp = prev_jmp;
-  } MRB_CATCH(&c_jmp) {
-    mrb->jmp = prev_jmp;
-    result = mrb_obj_value(mrb->exc);
-    mrb->exc = NULL;
-    if (state) { *state = TRUE; }
-  } MRB_END_EXC(&c_jmp);
-
-  mrb_gc_arena_restore(mrb, ai);
-  mrb_gc_protect(mrb, result);
-  return result;
+  struct protect_data protect_data = { body, data };
+  return mrb_protect_raw(mrb, protect_body, &protect_data, state);
 }
 
 MRB_API mrb_value
 mrb_ensure(mrb_state *mrb, mrb_func_t body, mrb_value b_data, mrb_func_t ensure, mrb_value e_data)
 {
-  struct mrb_jmpbuf *prev_jmp = mrb->jmp;
-  struct mrb_jmpbuf c_jmp;
-  mrb_value result;
   int ai = mrb_gc_arena_save(mrb);
-
-  MRB_TRY(&c_jmp) {
-    mrb->jmp = &c_jmp;
-    result = body(mrb, b_data);
-    mrb->jmp = prev_jmp;
-  } MRB_CATCH(&c_jmp) {
-    mrb->jmp = prev_jmp;
-    mrb_gc_arena_restore(mrb, ai);
-    ensure(mrb, e_data);
-    MRB_THROW(mrb->jmp); /* rethrow catched exceptions */
-  } MRB_END_EXC(&c_jmp);
-
-  mrb_gc_arena_restore(mrb, ai);
-  mrb_gc_protect(mrb, result);
+  struct protect_data protect_data = { body, b_data };
+  mrb_bool error;
+  mrb_value result = mrb_protect_raw(mrb, protect_body, &protect_data, &error);
   ensure(mrb, e_data);
   mrb_gc_arena_restore(mrb, ai);
   mrb_gc_protect(mrb, result);
+  if (error) {
+    mrb_exc_raise(mrb, result); /* rethrow catched exceptions */
+  }
   return result;
 }
 
@@ -66,36 +47,26 @@ MRB_API mrb_value
 mrb_rescue_exceptions(mrb_state *mrb, mrb_func_t body, mrb_value b_data, mrb_func_t rescue, mrb_value r_data,
                       mrb_int len, struct RClass **classes)
 {
-  struct mrb_jmpbuf *prev_jmp = mrb->jmp;
-  struct mrb_jmpbuf c_jmp;
-  mrb_value result;
-  mrb_bool error_matched = FALSE;
-  mrb_int i;
   int ai = mrb_gc_arena_save(mrb);
-
-  MRB_TRY(&c_jmp) {
-    mrb->jmp = &c_jmp;
-    result = body(mrb, b_data);
-    mrb->jmp = prev_jmp;
-  } MRB_CATCH(&c_jmp) {
-    mrb->jmp = prev_jmp;
-
-    for (i = 0; i < len; ++i) {
-      if (mrb_obj_is_kind_of(mrb, mrb_obj_value(mrb->exc), classes[i])) {
+  struct protect_data protect_data = { body, b_data };
+  mrb_bool error;
+  mrb_value result = mrb_protect_raw(mrb, protect_body, &protect_data, &error);
+  if (error) {
+    mrb_bool error_matched = FALSE;
+    for (mrb_int i = 0; i < len; ++i) {
+      if (mrb_obj_is_kind_of(mrb, result, classes[i])) {
         error_matched = TRUE;
         break;
       }
     }
 
-    if (!error_matched) { MRB_THROW(mrb->jmp); }
+    if (!error_matched) { mrb_exc_raise(mrb, result); }
 
     mrb->exc = NULL;
-    mrb_gc_arena_restore(mrb, ai);
     result = rescue(mrb, r_data);
-  } MRB_END_EXC(&c_jmp);
-
-  mrb_gc_arena_restore(mrb, ai);
-  mrb_gc_protect(mrb, result);
+    mrb_gc_arena_restore(mrb, ai);
+    mrb_gc_protect(mrb, result);
+  }
   return result;
 }
 

src/vm.c

@@ -306,6 +306,49 @@ cipop(mrb_state *mrb)
   return c->ci;
 }
 
+MRB_API mrb_value
+mrb_protect_raw(mrb_state *mrb, mrb_protect_raw_func *body, void *userdata, mrb_bool *error)
+{
+  struct mrb_jmpbuf *prev_jmp = mrb->jmp;
+  struct mrb_jmpbuf c_jmp;
+  mrb_value result = mrb_nil_value();
+  int ai = mrb_gc_arena_save(mrb);
+  const struct mrb_context *c = mrb->c;
+  int ci_index = c->ci - c->cibase;
+
+  if (error) { *error = FALSE; }
+
+  MRB_TRY(&c_jmp) {
+    mrb->jmp = &c_jmp;
+    result = body(mrb, userdata);
+    mrb->jmp = prev_jmp;
+  }
+  MRB_CATCH(&c_jmp) {
+    mrb->jmp = prev_jmp;
+    result = mrb_obj_value(mrb->exc);
+    mrb->exc = NULL;
+    if (error) { *error = TRUE; }
+    if (mrb->c == c) {
+      while (c->ci - c->cibase > ci_index) {
+        cipop(mrb);
+      }
+    }
+    else {
+      // It was probably switched by mrb_fiber_resume().
+      // Simply destroy all successive CI_ACC_DIRECTs once the fiber has been switched.
+      c = mrb->c;
+      while (c->ci > c->cibase && c->ci->acc == CI_ACC_DIRECT) {
+        cipop(mrb);
+      }
+    }
+  }
+  MRB_END_EXC(&c_jmp);
+
+  mrb_gc_arena_restore(mrb, ai);
+  mrb_gc_protect(mrb, result);
+  return result;
+}
+
 void mrb_exc_set(mrb_state *mrb, mrb_value exc);
 static mrb_value mrb_run(mrb_state *mrb, const struct RProc* proc, mrb_value self);