vinniefalco/LuaBridge

Crash in luabridge::Stack<int>::get

liuaifu opened this issue · 6 comments

The type of the value at index -1 is double.

auto is_int = luabridge::Stack<int>::isInstance(L, -1);    // true
auto value = luabridge::Stack<int>::get(L, -1);            // crash

I'm using Lua v5.4.4 and LuaBridge v2.7.207 on Windows 10.

This is not clearly stated in the documentation but Lua 5.3 and 5.4 are unsupported.
At least without any 5.3 and 5.4 tests we cannot proof they are supported.

I can reproduce the issue in my environment as well.

  • most recent LuaBridge
  • Lua 5.4.2
  • g++ 10.3.0 on Windows 10 with msys2
lua_State* lua = luaL_newstate();

luabridge::Stack<double>::push(lua, 23.24);

// returns true
std::cout << luabridge::Stack<int>::isInstance(lua, -1) << std::endl;
std::cout << luabridge::Stack<unsigned int>::isInstance(lua, -1) << std::endl;
std::cout << luabridge::Stack<float>::isInstance(lua, -1) << std::endl;
std::cout << luabridge::Stack<double>::isInstance(lua, -1) << std::endl;

// returns false
std::cout << luabridge::Stack<char>::isInstance(lua, -1) << std::endl;
std::cout << luabridge::Stack<std::optional<int>>::isInstance(lua, -1) << std::endl;
std::cout << luabridge::Stack<std::string>::isInstance(lua, -1) << std::endl;

lua_close(lua);

The return value of isInstance shall only be true if get can be executed successful. It is somehow related to the issue #251 (comment).

Unfortunately the issue can't be solved because the affected isInstance functions make use of lua_type which can't distinguish between different number types. The check for all affected types is as following.
return lua_type(L, index) == LUA_TNUMBER;

This issue applies to different Lua versions. I've explicitly checked Lua 5.2 and 5.4.

image
https://www.lua.org/manual/5.2/manual.html
https://www.lua.org/manual/5.4/manual.html

The issue can be solved with Lua 5.2+. There are the functions lua_tointegerx and lua_tonumberx available.

So I've checked the behavior.
The isInstance<int>() returns true while it should return false.
The Stack<int>::get() crash would basically be expected if it returned false.

Please take a look at the #292.
An interesting fact that in Lua 5.1 and Lua 5.2 we cannot quickly check if the value on the stack is integer, however a float value is successfully converted into an integer.
This is what I mentioned earlier: if isInstance says true, then get should work.

Stack::get cannot be safely called from C++ as it's raising lua_error (it calls the lua panic handler which will throw with exceptions on but will std::abort without exceptions, which is not acceptable). Because of that we would need to use a safe get method not calling lua_error but returning something like c++23 std::expected (which in turn can be handled to call lua_error only when invoked from lua).