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

LuaJIT 5.1: Exceptions are not rethrown through sol::error #28

Closed
Nava2 opened this Issue Mar 3, 2016 · 25 comments

Comments

Projects
None yet
4 participants
@Nava2
Contributor

Nava2 commented Mar 3, 2016

In my code, I'm trying to force some sandboxing, one part of this is setting the require function to nil.

To test this, I create an "library script":

local M = {}
function M.foo()
    return "foo"
end
return M

I store this into a file in the test dir, m.lua, and then load it via:

local M= require('m')
assert(M.foo() == "foo");

On Lua 5.2, this throws a sol::error claiming the require function is nil. This is the expected behaviour, however, luajit exceptions are never caught and it throws an error unrecognized.

@Nava2

This comment has been minimized.

Contributor

Nava2 commented Mar 3, 2016

With a bit more testing, it seems to actually segfault the entire thread, which is odd.

What's happening is that it is throwing ... and sol isn't catching that (source).

try 
{
    lua.script(script);
} 
catch (...)
{
    std::cout << "Caught (...)" << std::endl;
    FAIL("wat");
}

It fails with wat.

There's a few options here:

  1. Use LuaJIT's C API to catch the exceptions (link)
    • Since this is a one-time overhead turn on/off, perhaps, if on LuaJIT, create a function to accept the function? Just thinking out loud
    • Turn it on by default but have a compiler flag to turn it off and by default wrap the sol::error? I'm unsure what the performance ramifications are, though.
  2. Accept that this is how LuaJIT works vs. Lua and ignore it 👎

I tested this on Linux for gcc-4.9 and clang36, didn't check other platforms.

@ThePhD

This comment has been minimized.

Owner

ThePhD commented Mar 3, 2016

We do (1) already in the latest. The only place we actually throw a sol::error is during the atpanic function. If luajit is failing internally and not properly calling it there's really not anything we can do.

@ThePhD

This comment has been minimized.

Owner

ThePhD commented Mar 3, 2016

It.. seems to work properly, but I'm using Windows x64. I assume it works properly on other x64-platforms. Support for exceptions, as the tables on the page for extensions/exceptions displays, depends on your platform. I have made it so every function bound through sol has an exception-catching trampoline placed under it that will translate things to a lua error. protected_function also goes to great lengths to ensure that exceptions are caught (as best as they can be through the lua code) and then translated into lua errors onto the stack (which also get filtered by a handler function, if there is one).

The exception wrapping can be turned off with SOL_NO_EXCEPTIONS being defined, but I only just added it and I'm still writing the docs.

See?

@ThePhD

This comment has been minimized.

Owner

ThePhD commented Mar 3, 2016

If you mean that we should be catching ALL errors, including ones that we can't catch with const char*, sol::error or std::exception, then the answer there is going to have to be "we shouldn't", but even in that case for the c-function trampolines we do anyways. If luajit is directly throwing something as it executes the script and not properly calling atpanic, there's very little we can do to reconcile that.

@Nava2

This comment has been minimized.

Contributor

Nava2 commented Mar 3, 2016

So this was fixed in the latest edition? That's awesome, let me try it out for myself.

I wasn't sure if it was something you were investigating with your recent changes to exceptions.

@ThePhD

This comment has been minimized.

Owner

ThePhD commented Mar 3, 2016

I'm going to assume this is fixed and closed it now. If something else goes entirely derelict, re-open it ASAP and I'll get on it (maybe even get a few Vagrant files to spin things up).

@ThePhD ThePhD closed this Mar 3, 2016

Nava2 added a commit to Nava2/sol2 that referenced this issue Mar 3, 2016

@Nava2

This comment has been minimized.

Contributor

Nava2 commented Mar 3, 2016

I actually can't verify that it's working. 👎

sol::state lua;
                // should error
                // REQUIRE_THROWS_AS(lua.script(script), sol::error);
                try 
                {
                    lua.script("local M = require(\"M\")\n"
                               "assert(M.foo() == \"foo\")\n");
                } catch (sol::error& e)
                {
                    std::cout << "caught sol::error: " << e.what() << std::endl;
                }
                catch (std::runtime_error& e)
                {
                    FAIL("caught std::runtime_error: " << e.what());
                }
                catch (std::exception& e)
                {
                    FAIL("caught std::exception: " << e.what());
                }
                catch (const char* e)
                {
                    FAIL("caught char*: " << e);
                }
                catch (...)
                {
                    try {
                        auto eptr = std::current_exception();
                        if (eptr)
                            std::rethrow_exception(eptr);
                        else 
                            FAIL("caught(...)");
                    } catch (std::exception& e)
                    {
                        FAIL("caught (...): " << e.what());
                    }
                }

It fails with caught (...) (no extra message).

SOL_LUA_VERSION=501

@Nava2

This comment has been minimized.

Contributor

Nava2 commented Mar 3, 2016

@ThePhD

This comment has been minimized.

Owner

ThePhD commented Mar 3, 2016

RIP, and it fails on a regular linux container too. I'm not really running a linux build, just using g++/clang++ on a Windows machine... buh. I'll have to install an actual 64-bit linux at some point.

@ThePhD ThePhD reopened this Mar 3, 2016

@Nava2

This comment has been minimized.

Contributor

Nava2 commented Mar 3, 2016

I've been doing development against all three, but recently, since making this Vagrantfile, I've been using the gcc49 machine more and more.

@ThePhD

This comment has been minimized.

Owner

ThePhD commented Mar 4, 2016

So, this is a tad troubling, but I'm not sure what I can do about it.

I've wrapped every function call in a trampoline, and even protected_function features additional exception catching guarantees to ensure that luajit doesn't behave like this. The problem is, however, there's no way for me to force luajit to play nice: even after by-default assigning an exception-trapping handler using luajit's API and throwing through the panic function, luajit just doesn't rightly care on certain platforms (like Windows x86 and friends).

This is not something that's really within our control, unfortunately. Unless you can spot out any bright ideas, this seems like one of those "Well, it's your burden for using luajit."

@Nava2

This comment has been minimized.

Contributor

Nava2 commented Mar 4, 2016

Does the error reproduce on newer version so LuaJIT? I haven't tried, but I will now.

I fear it might come down to, as you said, "known problems." I'm surprised about this since the site does say that it should work as we expect on Linux. It's supposed to have full interoperability with C++ exceptions on Linux.

@ThePhD

This comment has been minimized.

Owner

ThePhD commented Mar 4, 2016

I'm using 2.0.4, which last I checked is the latest.

@Nava2

This comment has been minimized.

Contributor

Nava2 commented Mar 4, 2016

Yeah, I just verified as well. I'm not sure what to say about that. It stinks, but it is definitely able to be written around.

@ThePhD

This comment has been minimized.

Owner

ThePhD commented Mar 4, 2016

Guess we just file bugs in luajit and pray for the best?

@Nava2

This comment has been minimized.

Contributor

Nava2 commented Mar 4, 2016

I think it's more of a "report to mailing list," but yeah. I think there needs to be a test case created without sol first, though.

@ThePhD

This comment has been minimized.

Owner

ThePhD commented Mar 5, 2016

Unfortunately, we'll have to close this. :( I'll be sure to throw over a issue on the LuaJit Github, which is apparently the active place of development.

@ThePhD ThePhD closed this Mar 5, 2016

@Nava2

This comment has been minimized.

Contributor

Nava2 commented Mar 5, 2016

Sounds good to me.. unfortunately. Where is the repository?

E: Nevermind.. https://github.com/LuaJIT/LuaJIT

@DemiMarie

This comment has been minimized.

DemiMarie commented Mar 28, 2016

One solution is to have exception-throwing C++ functions wrapped in functions that store std::current_exception() in a userdata and throw it as a Lua error, which the Sol API special-cases.

@DemiMarie

This comment has been minimized.

DemiMarie commented Mar 28, 2016

The reason LuaJIT has issues is that, unlike PUC-Lua, LuaJIT is not written in the common subset of C and C++, but rather in C + assembler. LuaJIT must therefore handle unwinding manually, hence the lack of portability.

@ThePhD

This comment has been minimized.

Owner

ThePhD commented Mar 28, 2016

There's nothing we can do about this. If LuaJIT hits its own while performing basic VM tasks (not withing a C Function, we trampoline those appropriately), errors and calls atpanic and the atpanic throws, LuaJIT will not respect the thrown error on most platforms. If we simply return the error message or the exception in a userdata, it doesn't matter because LuaJIT -- like vanilla Lua -- will call abort once the panic function finishes and the user does not lngjmp or throw their way out of it.

I am not interested in re-inventing exceptions with some weird longjmp bullshit because LuaJIT does not keep its promises on 64-bit Linux.

@espkk

This comment has been minimized.

espkk commented Jul 22, 2018

Is there a way to catch lua exceptions using sol on luajit x86 (windows x64)? Simple lua.script("foo.bar=42"); throws SEH through RaiseException and couldn't be caught with neither try nor __try (for some reason)

@ThePhD

This comment has been minimized.

Owner

ThePhD commented Jul 22, 2018

@espkk

  1. Please open a new issue.
  2. Your question has already been answered in the documentation and in LuaJIT's documentation. Please read: http://sol2.readthedocs.io/en/latest/exceptions.html#luajit-and-exceptions Specifically note that LuaJIT x86 cannot handle exceptions properly, no matter if you run it on a 64-bit or 32-bit machine.
@espkk

This comment has been minimized.

espkk commented Jul 22, 2018

@ThePhD
Thank you for your quick reply. I read both LuaJIT and sol documentation, but I had a hope that I misunderstood.

@ThePhD

This comment has been minimized.

Owner

ThePhD commented Jul 22, 2018

For what it's worth, you can define SOL_NO_EXCEPTIONS 1 and just let your exceptions tear through the stack without any interference from sol. If it works out, well, have fun.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment