wabt
wabt copied to clipboard
WebAssembly
Compilation is broken for 32-bit targets for WITH_WASI=ON
Various compilation errors prevent compilation on 32-bit targets (e.g., x86-32). Sure would be handy to be able to build and use this tool on those systems.
To be a little more clear: most of the failures are under WITH_WASI=ON, which has (eg) structs that aren't the sizes that are expected, e.g. static_assert(sizeof(__wasi_fdstat_t) == 24, "witx calculated size"); and many other static_asserts. I'm not quite sure of the proper fix here, since at least some of this code seems to be autogenerated, based on the comments at the top of the file.
(There is also a rogue warning/failure in BinaryReader::ReadAddress in which a format string uses %lu for a size_t value, which of course isn't quite right for 32-bit)
Oh I assumed you had fixed this with #1628, sounds like there are a separate set of issues here?
Strange.. I would have expected the disparity to be on 64-bit hosts because was itself, like wasm, has 32-bit pointers (its ILP32).
Oh I assumed you had fixed this with #1628, sounds like there are a separate set of issues here?
Correct (for #1628 I had not tried building on a 32-bit target)
Strange.. I would have expected the disparity to be on 64-bit hosts because was itself, like wasm, has 32-bit pointers (its ILP32).
Yeah, I can't explain it either, but I didn't spend a lot of time looking into it after seeing that the code was autogenerated-then-copy-pasted.
From a casual inspection, it looks like the issue is just one of struct padding, eg __wasi_fdstat_t is basically { u8; u16; u64; u64; }, which requires less padding on x86-32. I'm just not sure of the proper fix: should we edit these in place? Or should we regenerate the code from whatever generated it?
Yeah, this is puzzling; whatever tool generates this code makes some pretty specific assumptions about struct alignment and padding (apparently, that they always follow rules for x86-86, which seems odd)
I think the layout is determined by that the wasm32 target produces (i.e. its a header mostly designed for code that is running in WebAssembly not the implementation side). At one point I looked into having the generator generate the host-side code for packing / unpacking these structs, but IIRC I never finished it because I got something that worked in the ways that I needed it to.
BTW, what 32-bit target exactly you are interested in (and why)?
I think the layout is determined by that the wasm32 target produces (i.e. its a header mostly designed for code that is running in WebAssembly not the implementation side).
That may be, but this code is definitely being compiled for the host (interp-wasi-cc), not for wasm, so the struct layout constraints are definitely (well, theoretically) going to vary by platform. (Do x64 and aarch64 have effectively-identical struct layout rules these days?)
BTW, what 32-bit target exactly you are interested in (and why)?
I was hoping to remove some of the bespoke runtime cruft we wrote for Halide's wasm "JIT" integration and use wasi instead -- I discovered but the inability to build correctly on x86-32 when trying out these changes on our fleet of buildbots (which include x86-32 and arm32).
TBH, it's probably fine for us to simply declare that we don't support this mode on 32-bit targets -- testing it on x86-64 is almost certainly going to be adequate test coverage for our purposes, and would save buildbot test time -- but I have to admit that seeing non-32-bit-clean code sorta disturbs me on principle. :-)
So, yeah, this isn't at all an essential fix. And there are likely better uses of the team's time than addressing this (especially if this code isn't going to be regularly build/tested on 32-bit hosts). But it might be worth at least documenting that the code is unsupported on 32-bit hosts. (Maybe it already is and I overlooked it? If so, oops...)
...I guess I should ask if relying WITH_WASI=ON is a good idea in any event? Per #1628, it apparently wasn't being built/tested at all by default, but I just noticed that interp-wasi.cc says This is an experiment and currently extremely limited implementation of the WASI syscall API -- if this is experimental code that isn't fully baked or tested, then I should probably just abandon this plan entirely.
Its certainly experimental yes. If you can build with out WITH_WASI=ON you will certainly be in safer territory. What is your use case for wabt and WASI?
What is your use case for wabt and WASI?
TL;DR: we want to use wasm-interp to run code compiled with STANDALONE_WASM=1.
More info:
For historical reasons, many of our correctness tests use Halide's JIT; as a practical matter, running these tests requires linking a wasm engine of some sort into libHalide for these tests. We had initially tried embedding V8 inside Halide, but build and versioning issues both inside and outside Google made this challenging, for reasons that aren't important here. We switched to using wabt for this purposes because it's vastly easier to integrate and keep up to date, and the performance isn't important for these tests, just correctness.
However, we also need to test Halide's AOT wasm generation, which requires a shell or browser environment of some sort. Literally zero of our tests require a browser environment, and we'd prefer to use a simple command-line shell for these tests -- so we are currently using d8. This works, but it's really suboptimal from a testing robustness perspective to use completely different runtime environments for our tests, and (again) performance isn't critical for these, so it would be simpler for us to just use wabt-interp for these, too. But since we build these tests with ENVIRONMENT=shell STANDALONE_WASM=1, we need to have --wasi available.
...does that make sense?
Maybe sense yes. It sounds like to unify your environment we/you would either need to invest in either:
- Creating and embedding of v8 with libhalide support
- Improving/maintaining the experimental wasi support in wabt
I can potentially help with (2) although its been while since I had any time to spend on this. We (my team) can consider re-investing there now that we have know we have at least one major customer for that configuration.
I am skeptical that we're going to want to go back to using v8 for this task; while it's awesome performance, it is a beast to compile and much more complicated to integrate, and being able to use recent V8 versions in google3 in a timely manner has been problematic.