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

[glebbash]

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]

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.

[dbaeumer]

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]

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...

[konsumer]

I think the idea here is that wasm loops through fds (0=stdout, 1=stderr, 2=/, so it starts on 3) calling fd_prestat_get on them all until it doesn't return ERRNO_SUCCESS. fds should include all loaded dirs & files. I came across this issue because I am implementing my own browser-wasi.

pseudo-code:

function fdIsGood(fd:number): boolean {
  // is this a valid fd, in my own records?
  return false
}

function doPrestatInfo(fd:number, bufP:number):void {
  // copy prestat struct for fd into bufP
}

function fd_prestat_get (fd:number, bufP:number):void {
  if (fdIsGood(fd) {
    doPrestatInfo(fd, bufP)
    return ERRNO_SUCCESS
  }
  return ERRNO_BADF
}

with my simple wasm example, you can see it calling them 1-by-1 (this happened when I did fopen in C wasi-sdk wasm, and does not run if I don't fopen), so I just correlate fd to an array I keep of preloaded-files:

For /home (fd:8), I think bufP should look like this, in wasm-mem, since len is 6 (don't forget trailing \0) and it's a dir (type:3):

You can get this view by logging new Uint8Array(interface.exports.memory.buffer, bufP, 5)

You can set it like this:

const view = new DataView(interface.exports.memory.buffer)
view.setUint8(bufP, 3) // directory
view.setUint32(bufP + 1, '/home'.length + 1)

I like to use class-helpers for this sort of thing, like

export class WasiPrestat {
  constructor (value = {}, view, address) {
    this._mem = view
    this._address = address // || malloc(5)
    for (const k of Object.keys(value)) {
      this[k] = value[k]
    }
  }

  get type () {
    return this._mem.getUint8(this._address)
  }

  set type (value) {
    this._mem.setUint8(this._address, value)
  }

  get name_len () {
    return this._mem.getUint32(this._address + 1, true)
  }

  set name_len (value) {
    this._mem.setUint32(this._address + 1, value, true)
  }
}