luau-lang / luau

A fast, small, safe, gradually typed embeddable scripting language derived from Lua
https://luau.org
MIT License
3.95k stars 372 forks source link

luau_getinfo ensure when used immediately after a `LOP_CALL` Instruction #1369

Open kalebs-anotheraxiom opened 4 weeks ago

kalebs-anotheraxiom commented 4 weeks ago

I'm currently in the process of implementing a luau debugger and have ran into one small issue

If you are running in step mode and breaking at every single instruction (The user is wanting to step forward for example), Once a LOP_CALL instruction is reached whenever you next call lua_getinfo a few odd conditions and assumptions are broken

When you call lua_getinfo and inside of the auxgetinfo call during the caculation of the current line

case 'l':
        {
            if (ci)
            {
/*=========>*/  ar->currentline = isLua(ci) ? currentline(L, ci) : -1;
            }
            else
            {
                ar->currentline = f->isC ? -1 : f->l.p->linedefined;
            }

            break;
        }

currentline(L, ci) is called. This is the call that ensures

current line is defined as follows and currentpc is the reason for the ensure

    return luaG_getline(ci_func(ci)->l.p, currentpc(L, ci));

The main issue is that currentpc returns -1 which immediately hits an ensure on the luaG_getline call

There's not much documentation on the very very internals of the lua/Luau engine but after doing some investigation my best guess as to what's happening is that CallInfo->savedpc (Which im guessing stands for procedure call) is exactly equal to the stacks current closure instruction pointer

the currentcp line uses pcRel

#define pcRel(pc, p) ((pc) ? cast_to(int, (pc) - (p)->code) - 1 : 0)

Which just subtracts the CallInfo's Procedure call from the targeted closure (In this case we are targeting level 0 so the top most closure) code pointer and subtracts 1 (Not sure why exactly but I'm guessing its due to things starting at 1) . And because this is Immediately after a LOP_CALL the current closures code start and the call info's pc are equal.

This -1 value then hits the first ENSURE block of luaG_getline, And also returns junk data

I'm not entirely sure the best way to fix this but my suggestion would be to change

int luaG_getline(Proto* p, int pc)
{
    LUAU_ASSERT(pc >= 0 && pc < p->sizecode);

    if (!p->lineinfo)
        return 0;

    return p->abslineinfo[pc >> p->linegaplog2] + p->lineinfo[pc];
}

to

int luaG_getline(Proto* p, int pc)
{
    LUAU_ASSERT(pc >= -1 && pc < p->sizecode);

    if (pc == -1)
        return 0;

    if (!p->lineinfo)
        return 0;

    return p->abslineinfo[pc >> p->linegaplog2] + p->lineinfo[pc];
}

It's not the best solution but it's the one with the least changes needed

vegorov-rbx commented 3 weeks ago

Before we look into this more, keep in mind that when debugstep is called, line information is already provided in lua_Debug structure, so there is no need to call lua_getinfo with 'l'.

kalebs-anotheraxiom commented 3 weeks ago

Thank you and yea that is good to know and i will mess with that to see what i can pull from that information, However the way that I'm currently implementing it is after the debugstep which breaks execution it sends a break event to the Debug Adapter which then causes a "Get Current Stack" request to be sent to my VM, Which then makes the lua_getinfo call on the currently paused VM, Asynchronously. I'm not 100% sure how much information is given from the debugstep info but i dont think its the full stack trace anyways

vegorov-rbx commented 3 weeks ago

debugstep line is only enough to make the decision on whether to break or not, yes.

As a workaround, on a paused VM, we currently achieve debug function calls by executing them inside the luau_callhook context.

luau_callhook(L, [](lua_State* L, lua_Debug* ar) {
        ...
}, /*ar->userdata value*/nullptr);

We are aware that this method is internal and I'm pretty sure I understand where the issue has originated from, so will check that out.

kalebs-anotheraxiom commented 3 weeks ago

Is there a way to know when is the best time to break? Right now i just call lua_break every single time the debugstep hook is called and i skip the first time whenever stepping forward, The issue is that every single "Line" of luau code is made up of a larger amount of operations/Op codes. Some internal and some user code. Is there a way to know if the current instruction is actual user code and is safe to break inside of? I'd prefer to not have to make too many modifications to the luau source and modify it to expose that info but i can if i need too

That could also solve the issue if i just don't break the vm in a weird in-between opcode

kalebs-anotheraxiom commented 3 weeks ago

For what its worth creating a function called

template<typename FunctorType>
void luau_callhookwrapped(lua_State* L, FunctorType&& Functor)
{
    luau_callhook(
        L, [](lua_State* L, lua_Debug* ar) {
            (*((FunctorType*)ar->userdata))(L, ar);
        },
        &Functor);
}

And wrapping all of my stack trace extraction logic in that fixed the issue, So i will go ahead and do that.

Not exactly sure what wrapping the getinfo calls in a lua_hook does internally that makes it different but its behaving perfectly now

vegorov-rbx commented 3 weeks ago

Hopefully you'll be able to remove that once we fix the bug.