Serious safety hole with error handling

[lilyball]

As talked about at the 1/28/14 rust meetup, there is a serious safety hole. Lua error handling by default uses setjmp/longjmp, which won't fire destructors as it pops the stack. Lua can be compiled with C++, which will make it use throw/catch instead, which will fire Rust destructors, but this still isn't safe. Rust code typically makes the assumption that any premature stack unwinding is due to task failure, and therefore anything on the stack will be thrown away.

Compiling Lua as C++ is significantly better than letting it use setjmp/longjmp. However, Lua is typically only compiled this way when embedded (as source) into a C++ application. Freestanding Lua library compilation, which rust-lua takes advantage of by default, will generally be compiled with the default C behavior. Unfortunately, embedding the Lua source into rust-lua is problematic. Not only does it preclude building a Rust library that is then used in a larger application that itself links against Lua, but also the act of compiling Lua needs some platform-specific knowledge (to turn on a few defines that control behavior).

If we bite the bullet and embed Lua after all, we still have the problem where throwing C++ exceptions through Rust code is not particularly safe. I'm inclined to require Rust functions exposed to Lua to be marked unsafe as a hint that there is an issue here. Beyond that I don't know what to do. Marking every method on State as unsafe is possible, but not something I'd like to do. We also cannot call every Lua C function in protected mode as the act of setting up protected mode may potentially throw an exception.

It's also been pointed out to me that C++ considers it undefined behavior to throw a C++ exception past an extern "C" boundary. This does suggest another alternative, which is to wrap every lua function in a C++ function that catches all exceptions and returns an error code. rust-lua could then call those C++ wrappers (which themselves would be extern "C") and return a Result. This has its own problems, of course, beyond the awkwardness of writing all the C++ wrappers. Not only are there performance implications to consider, but I have no idea how Lua will behave if any Lua functions are called after catching this exception (as this interrupted Lua's expected error handling). Furthermore, there's no good way to re-throw the exception again in order to resume Lua's error handling.

Another option to consider is that we could redefine the macros used to control error handling to invoke whatever code we want. The question is how to get Lua to unwind past its own code, stop when it hits a Rust function, and then resume when the Rust function finishes (and further, how to enforce in the Rust function that it needs to return immediately on any errors).

[lilyball]

It occurs to me that, given that the error handling in Lua is literally a switch between try/catch and setjmp/longjmp, I suspect that calling Lua immediately after catching a C++ exception is probably ok, although I'd want to read through a bunch of Lua source to verify that (and to see what would happen to the stack in this case).

[cbeck88]

Hi, I'm potentially interested in using rust-lua (currently using C++ with lua).

I wanted to ask, even if rust's handling of this is not "safe", what are the substantial consequences of that? Is it actually worse than what C++ affords?

In my project we currently target emscripten, and we compile without support for C++ exceptions, meaning that the compiler does not generate "catch" blocks, and any throw terminates the program. (This makes the whole thing a lot easier to simulate quickly in javascript.)

Because of this I build lua as C, and link with it through extern "C" { ... }, and it uses setjmp and longjmp when it throws errors / I do things with coroutines.

According to C++ standard, if longjmp would cause any destructor to be called were it replaced with throw / catch (as in the lua switch), then the standard says it has undefined behavior, and in practice the dtors are skipped.

To handle this I just make sure there are no automatic objects on the stack at the time that a lua call would do this, and then the behavior is well-defined and it works great in practice.

I would be quite happy to use rust-lua and switch the project to rust if I can do something similar -- but is longjmp into a C library ever okay in rust at all? Sorry I am very much a newbie in rust.

[lilyball]

@cbeck88 I apologize for not responding to this before.

Is it actually worse than what C++ affords?

If you compile Lua for C++, such that it uses C++ exceptions for errors, then yes it is worse than what C++ affords. C++ exceptions unwind the stack properly so any stack values will get cleaned up.

However, if you compile without C++ support, such that Lua uses setjmp/longjmp as it does for C, then that behavior is identical to rust-lua. Or if you've disabled C++ exception support in your project then presumably that behaves the same (I assume it doesn't generate landing pads at all and so doesn't unwind the stack if a library throws an exception, but since you're compiling Lua as C it doesn't throw exceptions to begin with).

To handle this I just make sure there are no automatic objects on the stack at the time that a lua call would do this, and then the behavior is well-defined and it works great in practice.

That's exactly how you handle it in rust-lua as well. As long as there's nothing with a destructor on the stack at the time of the Lua error, then everything's fine.

is longjmp into a C library ever okay in rust at all?

It's no worse than it is in C++. I didn't realize longjmp in C++ was actually officially declared as undefined behavior if it skips a destructor (although that makes sense; if nothing else, the optimizer will assume the object is gone when code resumes after the longjmp but this may actually be an unsafe assumption beyond just leaking memory, e.g. if it skipped over a thread::join). AFAIK Rust doesn't say anything at all about longjmp but it's probably safe to assume that it has the same potential for badness if bypassing a destructor.

[DemiMarie]

Worse, LuaJIT does not support C++ exception interoperability on all platforms.

[MasonRemaley]

I believe I have a similar issue in my own project, and wrapping my Lua calls in C++ try/catch and then reporting them back to Rust safely turned out to be a non-solution. I haven't found any documentation on this so it's possible I've made some sort of mistake, but it seems that Lua will just panic instead of throwing exceptions unless you're already in a protected call.

[lilyball]

I admit I haven't looked at the Lua error-handling implementation in detail, but it's certainly very plausible that it only throws a C++ exception if it's inside a protected call (e.g. so it knows the exception will be caught). After all, it certainly has to have this requirement for the longjmp mechanism, because it has to know where to jump to, so it's quite reasonable to have the same behavior with C++ exceptions.

[MasonRemaley]

Yup I think you're right. After reading Lua's docs a bit more I do think this problem has a solution though--you can write C wrappers for each Lua call that call setjmp prior to the real call, and set a panic function that longjmps back into the wrapper where you can then handle the error.

[lilyball]

Or you could just use a protected call wrapper around each call to a Lua function that could throw an error.

I considered doing that automatically, but I was concerned about the performance of making every single Lua C call into a protected call.

[MasonRemaley]

I think you're right about the performance of doing it with protected calls, they only seem to be a little slower than unprotected calls on my machine but normally you wouldn't need the additional call at all. Passing arguments into the protected call without risking a panic in the process is also a little tricky, I suppose you could save the args in a global variable from the wrapper and then read them back in the protected call.

Programming in Lua mentions the possibility of wrapping the main of your program in a protected call like the interpreter does which isn't really an option here, but the manual mentions the possibility of calling longjmp from the panic function to avoid the abort so I think this is something the Lua devs expect people to do. I went ahead and tried this out on my project and it's working well so far, though I still need to figure out whether or not there's a way to get the error type that would have been reported if it was in a protected call.

[MasonRemaley]

Hm, turns out doing it with setjmp/longjmp isn't actually that simple. I realized you can get the status of a thread using lua_status, but I don't think you can reset it upon recovering and I'm not sure but I'm not sure whether using a thread with an error status is defined behavior. If it isn't you can create a new Lua thread beforehand, but looking at the source it seems like that call can error due to memory or due to a bad gc metamethod. You can pause the garbage collector to avoid any gc metamethods from throwing and have Rust just check and panic if the main thread ends up with a bad status since it will be due to memory running out anyway, but now things have gotten complicated again...

[DemiMarie]

Using a panicked lua_State is undefined behavior.

The approach I came up with is this:

1. Use lua_cpcall to create a protected environment without risking a Lua error
2. From this environment, call lua_pushcclosure to create the various native-implemented Lua functions. Place them in the Lua registry using lightuserdata (corresponding to the addresses of static C/Rust functions) as keys.
3. For each Lua API function that may throw an error, place a pointer to the arguments in thread-local storage, then use lua_pcall to call the appropriate function pushed in step 2. Alternatively, the function itself could also be in TLS, so only one trampoline would be needed.

The trampoline can be placed in the Lua registry lazily, since using lua_rawget of nonexistent value in the Lua registry returns nil and that can be easily checked for.

This part is completely independent of Rust. I personally believe that it should be implemented in C, so that it can be used by projects such as HsLua and ocaml-lua that do not want a build dependency on rustc and a runtime dependency on libstd, and be split into its own project.

[MasonRemaley]

I like that idea a lot, I'm sure it'd make a lot of people's lives easier who are trying to use Lua from languages other than C.

For what it's worth I just did a quick profile of my setjmp/longjmp solution vs a solution similar to yours. It turns out that in my application setjmp/longjmp is actually slower than the pcall version anyway--even without a new Lua thread which you'd need to keep the main thread from panicking.

[DemiMarie]

Here is a first attempt at such a library: safer-lua-api.

It still needs a lot of work, and has no tests at all (other than just compiling), but it is a start.