8000 luau_getinfo ensure when used immediately after a `LOP_CALL` Instruction · Issue #1369 · luau-lang/luau · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

luau_getinfo ensure when used immediately after a LOP_CALL Instruction #1369

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

Closed
kalebs-anotheraxiom opened this issue Aug 17, 2024 · 6 comments · Fixed by #1832
Closed

luau_getinfo ensure when used immediately after a LOP_CALL Instruction #1369

kalebs-anotheraxiom opened this issue Aug 17, 2024 · 6 comments · Fixed by #1832
Labels
bug Something isn't working

Comments

@kalebs-anotheraxiom
Copy link
kalebs-anotheraxiom commented Aug 17, 2024

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

@kalebs-anotheraxiom kalebs-anotheraxiom added the bug Something isn't working label Aug 17, 2024
@vegorov-rbx
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Author
kalebs-anotheraxiom commented Aug 19, 2024

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
Copy link
Author
kalebs-anotheraxiom commented Aug 19, 2024

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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

2 participants
0