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

Stack overflow weirdness #92

Closed
andrieshiemstra opened this issue Dec 20, 2020 · 2 comments
Closed

Stack overflow weirdness #92

andrieshiemstra opened this issue Dec 20, 2020 · 2 comments

Comments

@andrieshiemstra
Copy link
Contributor

So i've been busy lately wrapping quickjs-rs in my own project and every now and then i got unexpected stack-overflows which seemed to appear and disappear quite randomly.

Now i've finally reproduced that behaviour in a small program and stuff just got weirder...

my cargo.toml, with a ref to my own quickjs-rs branch which has the latest quickjs version (please note that the 0.8.0 release had the same issues)

[package]
name = "untitled"
version = "0.1.0"
authors = ["Andries Hiemstra <info@hirofa.com>"]
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
libquickjs-sys = {git="https://github.com/HiRoFa/quickjs-rs", branch="issue87"}

main.rs

use libquickjs_sys as q;
use std::ffi::CString;

struct RtWrapper {
    runtime: *mut q::JSRuntime,
}

impl RtWrapper {
    fn new() -> Self {
        println!("creating new QuickJsRuntime");
        let runtime = unsafe { q::JS_NewRuntime() };
        if runtime.is_null() {
            panic!("RuntimeCreationFailed");
        }
        Self { runtime }
    }
}

impl Drop for RtWrapper {
    fn drop(&mut self) {
        println!("before JS_FreeRuntime");
        unsafe { q::JS_FreeRuntime(self.runtime) };
        println!("after JS_FreeRuntime");
    }
}

fn main() {
    unsafe {

        let null = q::JSValue {
            u: q::JSValueUnion { int32: 0 },
            tag: 2,
        };

        let wrapper = RtWrapper::new();

        let runtime = wrapper.runtime;

        let context: *mut q::JSContext = q::JS_NewContext(runtime);

        let script = "(function err(){throw Error('Oh dear, stuff failed');});";
        let script_cstring = CString::new(script).ok().unwrap();
        let filename_cstring = CString::new("test_err.js").ok().unwrap();

        let value_raw = q::JS_Eval(
            context,
            script_cstring.as_ptr(),
            script.len() as _,
            filename_cstring.as_ptr(),
            q::JS_EVAL_TYPE_GLOBAL as i32,
        );

        if q::JS_IsException(value_raw ) {
            panic!("eval returned error");
        }

        // should be an Object (function)
        assert_eq!(value_raw.tag, -1);

        let func_res = q::JS_Call(
            context,
            value_raw,
            null,
            0,
            vec![].as_mut_ptr(),
        );

        if !q::JS_IsException(func_res ) {
            panic!("call did not error");
        }

        let exception_val = q::JS_GetException(context);

        let message_prop_name = CString::new("message").ok().unwrap();
        let exception_msg = q::JS_GetPropertyStr(context, exception_val, message_prop_name.as_ptr());

        let msg = val_to_str(context, exception_msg);

        q::JS_FreeValue(context, value_raw);
        q::JS_FreeValue(context, func_res);
        q::JS_FreeValue(context, exception_val);
        q::JS_FreeValue(context, exception_msg);

        if !msg.contains("stuff failed") {
            println!("err msg was unexpected val of {}", msg);
        } else {
            println!("we got a stuff failed err which is what we wanted");
        }

        run_pending_jobs_if_any(runtime);

        q::JS_FreeContext(context);

        println!("done");

    }
}

unsafe fn run_pending_jobs_if_any(runtime: *mut q::JSRuntime) {
    while q::JS_IsJobPending(runtime) > 0 {
        let mut ctx: *mut q::JSContext = std::ptr::null_mut();
        let res = q::JS_ExecutePendingJob(runtime, &mut ctx);
        if res < 0 {
            panic!("running pending job failed");
        }
    }
}

unsafe fn val_to_str(context: *mut q::JSContext, val: q::JSValue) -> String {
    let ptr = q::JS_ToCStringLen2(context, std::ptr::null_mut(), val, 0);
    let cstr = std::ffi::CStr::from_ptr(ptr);
    let s = cstr.to_str().expect("invalid string").to_string();

    // Free the c string.
    q::JS_FreeCString(context, ptr);
    s
}

In this case i'm throwing an error from a function, please note that i've had the same issue when Promises we're dropped

When running this with cargo run i get the following output

creating new QuickJsRuntime
err msg was unexpected val of stack overflow
done
before JS_FreeRuntime
after JS_FreeRuntime

Process finished with exit code 0

So the JS_Call throws a Exception as expected bu the message of the exception is stack overflow

Now for the weird, if i run the same code with cargo run --release i get the error and output i expected

we got a stuff failed err which is what we wanted
done
before JS_FreeRuntime
after JS_FreeRuntime

Process finished with exit code 0

And now for the really weird, if i remove the println!("creating new QuickJsRuntime"); line before creating the runtime i also get the output i expected

we got a stuff failed err which is what we wanted
done
before JS_FreeRuntime
after JS_FreeRuntime

Process finished with exit code 0

Anyone out there having similar issues or have any idea what's going on here?

@andrieshiemstra
Copy link
Contributor Author

Ok so the problem has to do with the stack_size checking of quickjs, this happens in quickjs.c in the js_check_stack_overflow method

it used the js_get_stack_pointer method to check the current stack pointer and compares that to the pointer stored in JSRuntime which also uses the js_get_stack_pointer method when it is initialized.

at a certain moment the js_get_stack_pointer method returns a number lower then what is was when the runtime was initialized and because it's a unsigned int size = rt->stack_top - js_get_stack_pointer(); becomes a very large number leading quickjs to believe it's stack has overflowed..

i've added a printf to do some debugging

static inline BOOL js_check_stack_overflow(JSRuntime *rt, size_t alloca_size)
{
    // return FALSE;
    size_t size;
    size = rt->stack_top - js_get_stack_pointer();
    printf("js_check_stack_overflow rt_stack_top:%u js_get_stack_pointer:%u alloca_size:%u size:%u rt_size:%u\n ", rt->stack_top, js_get_stack_pointer(), alloca_size, size, rt->stack_size);
    return unlikely((size + alloca_size) > rt->stack_size);
}

this outputs the following

creating new QuickJsRuntime
err msg was unexpected val of stack overflow
done
before JS_FreeRuntime
after JS_FreeRuntime
js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464356016 alloca_size:0 size:208 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464354080 alloca_size:0 size:2144 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464354080 alloca_size:0 size:2144 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464354080 alloca_size:0 size:2144 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464354080 alloca_size:0 size:2144 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464354080 alloca_size:0 size:2144 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464354080 alloca_size:0 size:2144 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464354080 alloca_size:0 size:2144 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464354080 alloca_size:0 size:2144 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464354080 alloca_size:0 size:2144 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464354080 alloca_size:0 size:2144 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464354080 alloca_size:0 size:2144 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464354080 alloca_size:0 size:2144 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464354080 alloca_size:0 size:2144 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464354080 alloca_size:0 size:2144 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464354080 alloca_size:0 size:2144 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464354400 alloca_size:0 size:1824 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464352944 alloca_size:0 size:3280 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464352944 alloca_size:0 size:3280 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464352944 alloca_size:0 size:3280 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464352944 alloca_size:0 size:3280 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464352944 alloca_size:0 size:3280 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464352592 alloca_size:0 size:3632 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464351360 alloca_size:0 size:4864 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464351360 alloca_size:0 size:4864 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464350176 alloca_size:0 size:6048 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464351360 alloca_size:0 size:4864 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464352592 alloca_size:0 size:3632 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464352944 alloca_size:0 size:3280 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464354400 alloca_size:0 size:1824 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464355664 alloca_size:0 size:560 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464355840 alloca_size:32 size:384 rt_size:262144
 js_check_stack_overflow rt_stack_top:3464356224 js_get_stack_pointer:3464356272 alloca_size:32 size:4294967248 rt_size:262144
 invoke stackOverflow from JS_CallInternal()
 JS_ThrowStackOverflow invoked

and now.. .to figure out how to fix it.. :)

@andrieshiemstra
Copy link
Contributor Author

figured it out

after reading up on some basic stack vs heap knowledge (https://doc.rust-lang.org/1.22.0/book/first-edition/the-stack-and-the-heap.html)

it seems initializing a runtime in a function is a bad idea because in the init method, and certainly after doing a pintln!("sdgf") the current stack frame is higher then what it's going to be when calling a method later.. so initializing a JSRuntime should be the first thing to do when starting a thread which acts as your event queue...

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

1 participant