Hitting a crash in an assert after lua_equal is called.

[kunitoki]

I have some code that executes a bunch of lua declarations of global variables, then i obtain those as references and then i compare those references (which will invoke lua_equal) and i'm getting an assert hit inside Luau when lua_equal is executed for some of the references.

This is the callstack:

#0	0x0000000100031e91 in void luau_execute<false>(lua_State*) at LuaBridge3/ThirdParty/luau/VM/src/lvmexecute.cpp:308
#1	0x0000000100019931 in luau_execute(lua_State*) at LuaBridge3/ThirdParty/luau/VM/src/lvmexecute.cpp:2858
#2	0x000000010005a344 in luaD_call(lua_State*, lua_TValue*, int) at LuaBridge3/ThirdParty/luau/VM/src/ldo.cpp:237
#3	0x00000001000699db in callTMres(lua_State*, lua_TValue*, lua_TValue const*, lua_TValue const*, lua_TValue const*) at LuaBridge3/ThirdParty/luau/VM/src/lvmutils.cpp:69
#4	0x0000000100052cde in luaV_equalval(lua_State*, lua_TValue const*, lua_TValue const*) at LuaBridge3/ThirdParty/luau/VM/src/lvmutils.cpp:291
#5	0x000000010005255e in lua_equal(lua_State*, int, int) at LuaBridge3/ThirdParty/luau/VM/src/lapi.cpp:310

and this is the assert i'm getting:

template<bool SingleStep>
static void luau_execute(lua_State* L)
{
#if VM_USE_CGOTO
    static const void* kDispatchTable[256] = {VM_DISPATCH_TABLE()};
#endif

    // the critical interpreter state, stored in locals for performance
    // the hope is that these map to registers without spilling (which is not true for x86 :/)
    Closure* cl;
    StkId base;
    TValue* k;
    const Instruction* pc;

    LUAU_ASSERT(isLua(L->ci));
    LUAU_ASSERT(luaC_threadactive(L)); // <<<< Thread 1: EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)
    LUAU_ASSERT(!luaC_threadsleeping(L));

It's interesting because all lua vms i'm testing against with this particular unit test (5.1, 5.2, 5.3, 5.4, Luajit2) they pass it nicely. Anything i should be looking for ? i've tried creating some stack space explicitly but i cannot seems to avoid the crash.

[zeux]

Can you post the code that calls lua_equal?

[kunitoki]

It's related to defining the __eq metamethod for a table, then invoking lua_compare on itself:

TEST_F(LuaRefTests, ComparisonRepro)
{
    runLua(R"(
        t1 = {a = 1}
        setmetatable (t1, {
            __eq = function (l, r) return l.a == r.a end
        })
    )");
    
    lua_getglobal(L, "t1");
    lua_getglobal(L, "t1");

    auto result = lua_compare(L, -2, -1, LUA_OPEQ);
    EXPECT_EQ(1, result);
}

with lua_compare as in 5.3 / 5.4 or on falling back to this in 5.1 / 5.2:

#define LUA_OPEQ 1
#define LUA_OPLT 2
#define LUA_OPLE 3

inline int lua_compare(lua_State* L, int idx1, int idx2, int op)
{
    switch (op)
    {
    case LUA_OPEQ:
        return lua_equal(L, idx1, idx2);
        break;

    case LUA_OPLT:
        return lua_lessthan(L, idx1, idx2);
        break;

    case LUA_OPLE:
        return lua_equal(L, idx1, idx2) || lua_lessthan(L, idx1, idx2);
        break;

    default:
        return 0;
    }
}

[zeux]

I think we might be missing thread wakeup calls on external calls that can invoke Lua (lua_equal, lua_lessthan, maybe others? unsure)...

If you want a quick fix, copying this code from lua_call to lua_equal probably works.

    int wasActive = luaC_threadactive(L);
    l_setbit(L->stackstate, THREAD_ACTIVEBIT);
    luaC_checkthreadsleep(L);

[kunitoki]

Yes it works ! Thanks a bunch 👍🏻

[zeux]

lua_getfield and other functions that can invoke metatables would have the same problem... maybe we should change the placement for wakeup. We'll take a look at how to best fix this so that these issues don't come up, thanks for the report! (we don't really use these functions at Roblox which is why we never noticed the issue)

[kunitoki]

It seems this is the remaining blocker for having LuaBridge3 supporting Luau 🥂

[vegorov-rbx]

A fix for this issue is ready and the PR will be available soon.

[zeux]

This is now fixed, although you need to enable the fix with a flag like this:

#include "Luau/Common.h"

LUAU_FASTFLAG(LuauActivateBeforeExec)

...
FFlag::LuauActivateBeforeExec.value = true;

The flag is going to be removed in the future and this won't be necessary.

[kunitoki]

I don't understand where to apply the fix? I'm just including the VM and running bytecode (i don't have access to the Ast library).

[zeux]

I see... the flag interaction with the OSS world for fixes like this is something we'll need to think about. I'll change the defaults here for now to have things just work.