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

--invoke does not call invoked function #1024

Closed
robinvanemden opened this issue Dec 1, 2019 · 4 comments · Fixed by #1036
Closed

--invoke does not call invoked function #1024

robinvanemden opened this issue Dec 1, 2019 · 4 comments · Fixed by #1036
Assignees
Labels
bug Something isn't working

Comments

@robinvanemden
Copy link

robinvanemden commented Dec 1, 2019

To run our C++ based framework, we need to invoke an exported function pred() in a WASI standalone Webassembly binary compiled using Emscripten. The main() and pred() functions are defined as follows:

extern "C" {
    int pred() __attribute__((used));
    int pred() {
        printf("Running pred()\n");
        double *prediction = cpwbart();
        double result = prediction[0] + mu;
        return (0);
    }
}

int main() {
    printf("Running main()\n");
    double *prediction = cpwbart();
    double result = prediction[0] + mu;
    return (0);
}

Running WAVM works fine:

> wavm run --nocache --function=pred bart.wasm 

Running pred()

But running the same using Wasmer results in it calling the default main() function:

> wasmer run --disable-cache bart.wasm --invoke pred

Running main()

Just to make sure, we also tested:

> wasmer run --disable-cache --invoke pred bart.wasm 

Running main()

But that also resulted in calling main.

See also the possibly related issue here: wasmerio/wasmer-go#91

@robinvanemden robinvanemden added the bug Something isn't working label Dec 1, 2019
@robinvanemden robinvanemden changed the title Invoke does not seem to call invoked function --invoke does not call invoked function Dec 1, 2019
@MarkMcCaskey MarkMcCaskey self-assigned this Dec 2, 2019
@MarkMcCaskey
Copy link
Contributor

It looks like we only run invoke on no-ABI modules right now. It's easy to fix, but it will do the wrong thing with WASI a lot of the time. WASI currently only supports being run as an executable which means we have to execute _start which calls main. Without that, most file system operations won't work.

I'll make it work with WASI too and add a warning saying as much!

@robinvanemden
Copy link
Author

robinvanemden commented Dec 3, 2019

Thanks for making this work! So, when intending to invoke other functions in WASI modules, I presume is generally wise to first invoke _start() yourself. While keeping in mind _start() will itself call main(), of course.

@MarkMcCaskey
Copy link
Contributor

MarkMcCaskey commented Dec 3, 2019

Yeah, that's correct: currently to fully use WASI as a library you have to call main. If you can trust main to return then that's the easiest solution. I believe it should be possible to use metering here too, but I don't think we have a standardized, ready to use solution for this use case. It would be nice to have these types of things provided by use directly.

I think this will be added to WASI somewhat soon I'd imagine -- explicit setting up of the guest-side filesystem data structures without invoking main I mean.

edit: to clarify what I mean about metering is that you have to set up metering yourself; we don't have a single function or bool you can pass somewhere and have it just work yet for this specific use case which is unfortunate.

@robinvanemden
Copy link
Author

robinvanemden commented Dec 4, 2019

Thanks for the solution and for the background information - both very useful. The PR does indeed fix calling functions in WASI ABI modules:

> wasmer run --disable-cache wasimodule.wasm --invoke pred

WARNING: Invoking a function with WASI is not officially supported in the WASI standard yet.  Use this feature at your own risk, things may not work!
Running pred()
pred returned [I32(0)]

@bors bors bot closed this as completed in b336726 Dec 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants