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

[bug] Calling wasi_snapshot_preview1 path_open without "extra steps" returns BADF (error 8) #161

Open
glebbash opened this issue Apr 8, 2024 · 3 comments

Comments

@glebbash
Copy link

glebbash commented Apr 8, 2024

When the following is compiled with WASI SDK to test.wasm:

#include <stdio.h>

int main(int argc, const char **argv)
{
    FILE *file = fopen("test.txt", "r");
    if (file == NULL)
    {
        return 1;
    }

    char buffer[1024];
    while (fgets(buffer, sizeof(buffer), file) != NULL)
    {
        fputs(buffer, stdout);
    }
    fclose(file);

    return 0;
}

and executed using this setup:

import { Wasm } from "@vscode/wasm-wasi";

// ...
const wasm: Wasm = await Wasm.api();
const pty = wasm.createPseudoterminal();
const terminal = vscode.window.createTerminal({
    name: "test",
    pty,
    isTransient: true,
});
terminal.show(true);

try {
    const process = await wasm.createProcess(
        "test.wasm",
        currentCompilerModule,
        {
            stdio: pty.stdio,
            mountPoints: [
                {
                    kind: "vscodeFileSystem",
                    uri: workspaceUri,
                    mountPoint: "/",
                },
            ],
        }
    );

    const exitCode = await process.run();
    if (exitCode !== 0) {
        vscode.window.showErrorMessage(
            `Process exited with code: ${exitCode}`
        );
    }
} catch (error) {
    vscode.window.showErrorMessage((error as Error).message);
    terminal.dispose();
}

VSCode properly reads test.txt file from the current workspace and print it to created terminal.

However:

Trying to call wasi_snapshot_preview1 path_open function with fd=3, path="test.txt" directly always returns EBADF (code 8).

Minimal reproduction:

(; repro.wat ;)
(module
  (type (;0;) (func (param i32 i32 i32 i32 i32 i64 i64 i32 i32) (result i32)))
  (type (;1;) (func (param i32)))
  (type (;2;) (func))
  (import "wasi_snapshot_preview1" "path_open" (func (;0;) (type 0)))
  (import "wasi_snapshot_preview1" "proc_exit" (func (;1;) (type 1)))
  (func $_start (type 2)
    (local i32)
    i32.const 3
    i32.const 1
    i32.const 68314
    i32.const 8
    i32.const 0
    i64.const 264240830
    i64.const 268435455
    i32.const 0
    i32.const 67188
    call 0
    local.set 0
    local.get 0
    call 1)
  (memory (;0;) 40)
  (export "memory" (memory 0))
  (export "_start" (func $_start))
  (data (;0;) (i32.const 68314) "test.txt"))

P.S: Arguments to the function were extracted by using node:wasi and logging calls of all import functions.

Running repro.wat works fine in both node:wasi and wasmtime.

@glebbash
Copy link
Author

glebbash commented Apr 10, 2024

Found the issue using trace flag in the options.

Broken:

2024-04-11 00:51:54.148 [info] path_open(fd: 3 => fd: 3, dirflags: symlink_follow, path: test.txt, oflags: none, fdflags: none) => [result: badf] (0ms)

Working:

2024-04-11 02:33:01.964 [info] fd_prestat_get(fd: 3) => [prestat: {"$ptr":67192,"preopentype":0,"len":1}, result: success] (1ms)
2024-04-11 02:33:01.965 [info] fd_prestat_dir_name(fd: 3) => [path: /, result: success] (1ms)
2024-04-11 02:33:01.966 [info] fd_prestat_get(fd: 4) => [result: badf] (0ms)
2024-04-11 02:33:01.966 [info] fd_fdstat_get(fd: 3 => /) => [fdstat: directory}, result: success] (1ms)
2024-04-11 02:33:01.976 [warning] path_open(fd: 3 => /, dirflags: symlink_follow, path: test.txt, oflags: none, fdflags: none) => [fd: 4, result: success] (9ms)

For some reason @vscode/wasm-wasi implementation requires you to:

  1. call fd_prestat_get to get prestat.u.dir.pr_name_len and allocate a string of this size
  2. call fd_prestat_dir_name with the reference to that string
  3. call fd_prestat_get with fd + 1 or something ???
  4. call fd_fdstat_get with base fd ???
  5. finally actually be able to use the base fd for opening files

I've never seen this behavior (steps 1-4), both node:wasi and wasmtime don't require you to do anything before opening a file so it looks like a bug.

@glebbash glebbash changed the title Calling pure wasi_snapshot_preview1 path_open always returns BADF (error 8) [bug] Calling wasi_snapshot_preview1 path_open without "warming up base fd" returns BADF (error 8) Apr 10, 2024
@glebbash glebbash changed the title [bug] Calling wasi_snapshot_preview1 path_open without "warming up base fd" returns BADF (error 8) [bug] Calling wasi_snapshot_preview1 path_open without "knowing base fd name" returns BADF (error 8) Apr 10, 2024
@glebbash glebbash changed the title [bug] Calling wasi_snapshot_preview1 path_open without "knowing base fd name" returns BADF (error 8) [bug] Calling wasi_snapshot_preview1 path_open without "extra steps" returns BADF (error 8) Apr 10, 2024
@dbaeumer
Copy link
Member

Sorry for the long delay but somehow I overlooked the issue.

This is a little tricky and I need to see why node and wasmtime is not failing. The fd's for the pre-opened directories are actually determined by the wasm and not the host. So from a spec perspective they could start at 1000. Without the fd_prestat_get call it will be a guess which fd to use for the pre-opens.

I could assume they always start at 3 and throw later on if someone calls fd_prestat_get with a different fd.

@peter-jerry-ye
Copy link

I came across this issue when I tried to call path_open but getting inval until I first called the fd_prestat_get.

May I ask why it is said that

The fd's for the pre-opened directories are actually determined by the wasm and not the host. So from a spec perspective they could start at 1000. Without the fd_prestat_get call it will be a guess which fd to use for the pre-opens.

The spec of fd_prestat_get only says

Return a description of the given preopened file descriptor.

I understand the given as predefined by the environment, and by preopened file descriptor I understand as it's assigned a fd that won't be changed. Though it is a bit tricky since there's no behavior specification...

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

3 participants