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

Add fizzy-wasi tool #329

Merged
merged 8 commits into from
Oct 1, 2020
Merged

Add fizzy-wasi tool #329

merged 8 commits into from
Oct 1, 2020

Conversation

axic
Copy link
Member

@axic axic commented May 22, 2020

TODO:

  • fix ProjectUVWASI because it doesn't properly link in libuv (on macos at least)
  • add FIZZY_WASI cmake option
  • implement fd_write (add memory translation helpers)
  • maybe restructure to make error handling nicer
  • change from wasi_unstable to wasi_snapshot_preview1
  • add CI task running helloworld.wasm and expect "hello world\n" on cout

Optional: support helloworld from rust

  • wasi_snapshot_preview1::fd_prestat_dir_name (dummy)
  • wasi_snapshot_preview1::fd_prestat_get
  • wasi_snapshot_preview1::environ_sizes_get
  • wasi_snapshot_preview1::environ_get (dummy)

Use hello world from https://github.com/bytecodealliance/wasmtime/blob/master/docs/WASI-tutorial.md#web-assembly-text-example as test.

Also supports running hello world generated by Rust (using wasilibc).

Spec explanation: https://github.com/WebAssembly/WASI/blob/master/phases/README.md

test/wasi/wasi.cpp Outdated Show resolved Hide resolved
test/wasi/wasi.cpp Outdated Show resolved Hide resolved
@axic
Copy link
Member Author

axic commented May 25, 2020

@chfast @gumb0 so should we create a wasi directory or a tools/wasi directory? (Anticipating there may be another runner tool in the future.)

@axic
Copy link
Member Author

axic commented May 25, 2020

There seems to be a linking error:

/usr/bin/ld: cannot find -luv

@axic axic force-pushed the wasi branch 5 times, most recently from c626416 to cb723fb Compare May 25, 2020 17:25
@axic axic marked this pull request as ready for review May 25, 2020 17:25
@axic axic requested review from chfast and gumb0 May 25, 2020 17:25
uvwasi_errno_t ret = uvwasi_serdes_readv_ciovec_t(instance.memory->data(),
instance.memory->size(), iovptr, iovs.data(), static_cast<uvwasi_size_t>(iovs.size()));
if (ret != UVWASI_ESUCCESS)
return {false, {ret}};
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to double check the spec whether we return error code here or trpa.

tools/wasi/wasi.cpp Outdated Show resolved Hide resolved
tools/wasi/wasi.cpp Outdated Show resolved Hide resolved
tools/wasi/wasi.cpp Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
tools/wasi/wasi.cpp Outdated Show resolved Hide resolved
cmake/ProjectUVWASI.cmake Outdated Show resolved Hide resolved
tools/wasi/wasi.cpp Outdated Show resolved Hide resolved
// Global state.
// NOTE: we do not care about uvwasi_destroy(), because it only clears file descriptors (currently)
// and we are a single-run tool. This may change in the future and should reevaluate.
uvwasi_t state;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
uvwasi_t state;
uvwasi_t state;
std::unique_ptr<uvwasi_t, decltype(&uvwasi_destroy)> state_destroyer{&state, &uvwasi_destroy};

@chfast
Copy link
Collaborator

chfast commented Sep 30, 2020

The idea how to test at least some aspects of WASI.
In uvwasi_options_t you can set file descriptions to previously created memory buffers (probably requires POSIX/OS-specific API). This allows providing input and checkout output in unittests.

Although, I'm still ok with merging this in the current from, and leave more extensive testing to some other day.

@axic
Copy link
Member Author

axic commented Sep 30, 2020

@chfast your magic does not compile:

FAILED: tools/wasi/CMakeFiles/fizzy-wasi.dir/wasi.cpp.o 
/usr/bin/cmake -E __run_co_compile --tidy="clang-tidy;--extra-arg-before=--driver-mode=g++" --source=/home/builder/project/tools/wasi/wasi.cpp -- /usr/bin/clang++  -I/home/builder/project/lib/fizzy -I/home/builder/project/include -isystem _deps/src/uvwasi/include -isystem _deps/src/uvwasi-build -g -Wpedantic -Werror -Wall -Wextra -Wshadow -Wconversion -Wsign-conversion -Wno-unknown-pragmas -fstack-protector-strong -Wimplicit-fallthrough -Wcast-qual -Wcast-align -Wmissing-declarations -Wextra-semi -Wold-style-cast -Wfinal-dtor-non-final-class -Wnewline-eof -Wunreachable-code-break -Wduplicate-enum -Weverything -Wno-c++98-compat -Wno-c++98-compat-pedantic -Wno-covered-switch-default -Wno-double-promotion -Wno-float-equal -Wno-padded -Wno-switch-enum -std=c++17 -MD -MT tools/wasi/CMakeFiles/fizzy-wasi.dir/wasi.cpp.o -MF tools/wasi/CMakeFiles/fizzy-wasi.dir/wasi.cpp.o.d -o tools/wasi/CMakeFiles/fizzy-wasi.dir/wasi.cpp.o -c /home/builder/project/tools/wasi/wasi.cpp
/home/builder/project/tools/wasi/wasi.cpp:18:54: error: declaration requires an exit-time destructor [clang-diagnostic-exit-time-destructors,-warnings-as-errors]
std::unique_ptr<uvwasi_t, decltype(&uvwasi_destroy)> state_destroyer{&state, &uvwasi_destroy};
                                                     ^
/home/builder/project/tools/wasi/wasi.cpp:18:54: error: declaration requires a global destructor [clang-diagnostic-global-constructors,-warnings-as-errors]
1135 warnings generated.
Suppressed 1133 warnings (1133 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
2 warnings treated as errors

@axic axic force-pushed the wasi branch 3 times, most recently from a2a3e19 to c573a92 Compare September 30, 2020 17:26
@@ -132,7 +132,7 @@ bool run(int argc, const char** argv)
nullptr, // NOTE: no special allocator
};

uvwasi_errno_t err = uvwasi_init(&state, &options);
const uvwasi_errno_t err = uvwasi_init(&state, &options);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this const stuff so important on this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The const should be the default.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should protest C++ to follow Rust with making const-by-default and have a mutable specifier on others :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We both know this is not possible.

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

Successfully merging this pull request may close these issues.

3 participants