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

Luau assert failure in lua_xmove when an interrupt handler returns Err in an empty stack frame, but only when a wrapped failure has already been cached #153

Closed
Y0SH1M4S73R opened this issue Apr 26, 2022 · 2 comments

Comments

@Y0SH1M4S73R
Copy link

Y0SH1M4S73R commented Apr 26, 2022

When acquiring a cached failure using the get_wrapped_failure closure, callback_error_ext does not account for the possibility that the state corresponding to the current scope has an empty stack. (or whatever is implied when state->ci->top == state->top). This results in a failure of the following assertion in lua_xmove:

api_check(from, to->ci->top - to->top >= n);

At the bottom of this issue report is a test that demonstrates the described behavior. Creating userdata with a modified metatable was the most readily apparent method of producing a wrapped failure to cache. Like the previous issue I reported, this may be an underlying issue within luau, but I believed it would be more appropriate to report it here.

#![cfg(feature = "luau")]

use mlua::{Error, Lua, Result, UserData, VmState, UserDataFields};

thread_local! {
    /// A CPU usage limit in milliseconds for each run of lua code
    pub static START: std::cell::RefCell<std::time::Instant> = std::cell::RefCell::new(std::time::Instant::now());
}

#[test]
fn enigmatic_assertion_failure() -> Result<()> {
    let lua = Lua::new();

    struct ExampleStruct(i32);

    impl UserData for ExampleStruct {
        fn add_fields<'lua, F: UserDataFields<'lua, Self>>(fields: &mut F) {
            fields.add_field_method_get("foo", |_, this| {
                Ok("bar")
            });
        }
    }

    lua.set_interrupt(|| {
        START.with(|start| {
            if start.borrow().elapsed().as_millis() > 100 {
                Err(Error::RuntimeError("error from interrupt".into()))
            } else {
                Ok(VmState::Continue)
            }
        })
    });

    lua.globals().set("something", ExampleStruct(1));
    let uh_oh = lua.load(r#"
    while true do end
    "#)
    .into_function()?;
    START.with(|start| *start.borrow_mut() = std::time::Instant::now());
    match uh_oh.call::<_, ()>(()) {
        Err(Error::CallbackError { cause, .. }) => match *cause {
            Error::RuntimeError(ref m) if m == "error from interrupt" => {}
            ref e => panic!("expected RuntimeError with a specific message, got {:?}", e),
        },
        r => panic!("expected RuntimeError with a specific message, got {:?}", r)
    }
    lua.remove_interrupt();

    Ok(())
}
@Y0SH1M4S73R Y0SH1M4S73R changed the title Luau assert failure in lua_xmove when an interrupt handler returns Err in a function with no arguments, but only when a wrapped failure has already been cached Luau assert failure in lua_xmove when an interrupt handler returns Err in an empty stack frame, but only when a wrapped failure has already been cached Apr 26, 2022
khvzak added a commit that referenced this issue Apr 26, 2022
@khvzak
Copy link
Member

khvzak commented Apr 26, 2022

Thanks!
This is still related to a Luau bug luau-lang/luau#446 and my workaround in 70d287c

I hope the new fix solves it. It's bad that I cannot check stack in advance.

@khvzak
Copy link
Member

khvzak commented May 27, 2022

I believe it's fixed in beta5

@khvzak khvzak closed this as completed May 27, 2022
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

2 participants