WebAssembly / WASI

WebAssembly System Interface
Other
4.81k stars 249 forks source link

Environ (& args) API questions and clarifications #396

Closed MaulingMonkey closed 3 years ago

MaulingMonkey commented 3 years ago

Are the name/value pairs retrieved by environ_get / environ_sizes_get expected to be an interleaved array - each pair contributing 2 to the total count and separated by \0s - or are those pairs expected to be joined by =s and contribute only 1 each to the total count?

Rust's stdlib currently uses libc's environ emulation without using these API calls. Would it be appropriate to have rust initialize the environ based on these calls, or should that logic be pushed into libc itself?

Based on Rust's use of args_get / args_sizes_get, I'm assuming each individual entry should be \0-terminated, and that the final buffer size can/should include all \0s - it might be worth noting this in the documentation for both sets of APIs as well.

Thanks!

(I've thrown together a tool to bundle Rust apps into standalone HTML files, cargo html, and I'm creating my own implementation of WASI for the browser, which I'm starting to fill out.)

devsnek commented 3 years ago

count is the number of kv pairs. size is the number of bytes taken by adding up all the k=v\0 pairs.

caspervonb commented 3 years ago

Indeed the values are \0 terminated.

In case its useful, here's my implementation for Deno which is written in fairly plain TypeScript: https://deno.land/std@0.88.0/wasi/snapshot_preview1.ts

Warning tho there are some subtle bugs in there that I intend to write tests for here.

  "environ_sizes_get": syscall((
        environcOffset: number,
        environBufferSizeOffset: number,
      ): number => {
        const entries = Object.entries(this.#env);
        const textEncoder = new TextEncoder();
        const memoryView = new DataView(this.#memory.buffer);

        memoryView.setUint32(environcOffset, entries.length, true);
        memoryView.setUint32(
          environBufferSizeOffset,
          entries.reduce(function (acc, [key, value]) {
            return acc + textEncoder.encode(`${key}=${value}\0`).byteLength;
          }, 0),
          true,
        );

        return ERRNO_SUCCESS;
      }),

Also have a web implementation but because atomics weren't shipping for the longest time it isn't fully featured. Emscripten also implements parts of it.

MaulingMonkey commented 3 years ago

Perfect! I've managed to get my own implementation tested, bugfixed, and working.

To answer my own question RE: libc's environ: wasi-libc already contains code to query WASI to initialize it's mutable copy of the ambient environment. For whatever reason, __wasilibc_environ is getting zero-initialized, and __wasilibc_ensure_environ skips initialization as a result. I've got a dodgy workaround for my example code, and I'm updating my copy of rust-lang/rust to maybe poke at that.

MaulingMonkey commented 3 years ago

I was using the wrong entry point for my wasm32-wasi binaries (main, when _start should be used.) With that fixed, I'm successfully using these syscalls. Linked PR attempts to document these answers, which should resolve this issue. Thanks again!