Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix exceptions for visualcpp #2747

Merged
merged 1 commit into from
Mar 19, 2015
Merged

Fix exceptions for visualcpp #2747

merged 1 commit into from
Mar 19, 2015

Conversation

jbreeden
Copy link
Contributor

Exception handling is broken for visualcpp toolchain, if using C++ source files.

The issue is the the /EHsc cxx flag. From MSDN /EH docs, this 'tells the compiler to assume that functions declared as extern "C" never throw a C++ exception.'

The problem with that is that all of the functions in vm.c are extern "C". See this block from mrb_funcall_with_block

   MRB_TRY(&c_jmp) {
      mrb->jmp = &c_jmp;
      /* recursive call */
      val = mrb_funcall_with_block(mrb, self, mid, argc, argv, blk);
      mrb->jmp = 0;
    }
    MRB_CATCH(&c_jmp) { /* error */
      while (old_ci != mrb->c->ci) {
        mrb->c->stack = mrb->c->ci->stackent;
        cipop(mrb);
      }
      mrb->jmp = 0;
      val = mrb_obj_value(mrb->exc);
    }
    MRB_END_EXC(&c_jmp);

cl.exe (Microsoft C/C++ Version 18.00.31101 for x86) looks at this block, sees that mrb_funcall_with_block is extern "C" and assumes - like it's been told - that this function cannot throw a C++ exception. The exception handler is then removed from the generated code.

However, MRB_THROW is defined to throw an exception with the cxx_abi is enabled. The effect is that at runtime an exception is thrown out of this block. The stack is not processed, and mrb->jmp is not cleared.

Changing the flag to /EHs allows the exception handling to work as intended.

I have run ruby ./minirake test with both GCC & VisualCPP, and all tests pass with this change.
I don't have a test written for this change, but you can see my usage here:

  • lamina commit e9dec24 changes the toolchain flag
    • mruby-cef (used by lamina) commit d25e436 shows how I am able to handle exceptions with the change
    • In this commit, you can see I was catching the exceptions before, that should never have been making it out to my code.
    • Now, I'm able to check the return value (or mrb->exc) as expected.

@jbreeden
Copy link
Contributor Author

As a note, I'm building against CEF3's binary distribution on Windows, which requires linking against the static multithreaded runtime via /MT.

This is different than the default /Md runtime selected by the visualcpp toolchain. I don't imagine this affects the exception handling mechanics, but it may be useful if trying to recreate the exact conditions this bug was identified in.

matz added a commit that referenced this pull request Mar 19, 2015
@matz matz merged commit dc757f7 into mruby:master Mar 19, 2015
@matz
Copy link
Member

matz commented Mar 19, 2015

merged. If you see any problem, let us know.

@jbreeden jbreeden deleted the vcpp_exceptions branch September 2, 2015 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants