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

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

Open
liuaifu opened this issue Aug 25, 2022 · 6 comments
Open

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

liuaifu opened this issue Aug 25, 2022 · 6 comments

Comments

@liuaifu
Copy link

liuaifu commented Aug 25, 2022

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.

@dmitry-t
Copy link
Collaborator

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.

@stefanfrings
Copy link
Contributor

stefanfrings commented Aug 25, 2022

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.

@dmitry-t
Copy link
Collaborator

dmitry-t commented Aug 26, 2022

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.

@dmitry-t
Copy link
Collaborator

dmitry-t commented Aug 26, 2022

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.

@kunitoki
Copy link

You can support integers across all lua versions, see https://github.com/kunitoki/LuaBridge3/blob/master/Source/LuaBridge/detail/LuaHelpers.h#L90

@kunitoki
Copy link

kunitoki commented Oct 1, 2022

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).

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

No branches or pull requests

4 participants