Wasmtime GitHub notifications bot (Jun 12 2021 at 23:25): Wasmtime GitHub notifications bot (Jun 12 2021 at 23:25):cfallin opened PR #2980 from openbsd69 to main:
Verified to work with several simple Wasm modules on an OpenBSD 6.9
x86_64 machine, built with the Rust 1.51.0 included in the packages
collection (pkg_add rust) becauserustupdoesn't support
OpenBSD.This requires a number of PRs to upstream crates to be merged and
released on crates.io first; as a temporary measure, this commit
includes a number of patches in the toplevelCargo.tomlto refer
to git branches. Once those patches are merged and released,
I can update this PR (cc @sunfishcode).The only change to wasmtime code itself is in the trap-handler runtime.
I had to hardcode the offset to RIP in the signal frame, becauselibc
doesn't have a struct definition for OpenBSD. If we think it would be better
to get that upstreamed, I can do that instead. Or, we could go back to a
C helper and just include<sys/signal.h>, but I suspect we don't want to do that.This also disables
wasi-nnby default, because theopenvino-sys
crate fails to build on OpenBSD. Ideally we would either fix
upstream or include some more build configuration to exclude the
feature on our end on OpenBSD; unfortunately it seems (according
to rust-lang/cargo#1197) that target-specific features are not
yet supported by Cargo, so this is not very easy to do. I'd be interested
in @abrown's thoughts on this!Also note that we won't be able to build or test this platform on CI,
unfortunately, for three reasons: (i)rustupdoesn't carry toolchain builds
for OpenBSD, I think because of the OS's "no ABI compatibility across releases"
policy; (ii) GitHub CI Actions doesn't have any runners
for this platform; and (iii) distributing binaries is difficult, for the above
ABI-compatibility reasons. Nevertheless, it would be nice to be able to say
that it has been known to work at one time, and also maybe at some point
in the distant future someone will see fit to include us in the OpenBSD
ports/packages tree.Low priority for review (this was just a fun side-adventure). Thanks!
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
Wasmtime GitHub notifications bot (Jun 12 2021 at 23:25):cfallin requested alexcrichton for a review on PR #2980.
Wasmtime GitHub notifications bot (Jun 12 2021 at 23:25):cfallin requested sunfishcode for a review on PR #2980.
Wasmtime GitHub notifications bot (Jun 15 2021 at 01:53):sunfishcode submitted PR review.
Wasmtime GitHub notifications bot (Jun 15 2021 at 01:53):sunfishcode created PR review comment:
I'd suggest submitting a PR to the libc crate for this. The netbsd version of this is straightforward, so hopefully openbsd's is too.
Wasmtime GitHub notifications bot (Jun 15 2021 at 01:53):sunfishcode submitted PR review.
Wasmtime GitHub notifications bot (Jun 15 2021 at 04:35):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Jun 15 2021 at 04:35):cfallin created PR review comment:
Sure thing -- created rust-lang/libc#2242. It might be a bit until this is easily usable without building rustc from source (the platform doesn't have rustup so it would likely be available with the packages in the next OpenBSD release in October) but then this PR isn't in any hurry either :-)
Wasmtime GitHub notifications bot (Jun 15 2021 at 04:42):cfallin updated PR #2980 from openbsd69 to main.
Wasmtime GitHub notifications bot (Jun 17 2021 at 19:44):abrown submitted PR review.
Wasmtime GitHub notifications bot (Jun 17 2021 at 19:44):abrown created PR review comment:
@cfallin, I fixed the
openvinocrates so the error you saw above should be resolved. I think this line should be unnecessary if you bump theopenvinocrate to v0.3.2.
Wasmtime GitHub notifications bot (Jun 29 2021 at 00:21):cfallin updated PR #2980 from openbsd69 to main.
Wasmtime GitHub notifications bot (Jun 29 2021 at 00:23):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Jun 29 2021 at 00:23):cfallin created PR review comment:
Thanks!
Wasmtime GitHub notifications bot (Jun 29 2021 at 00:25):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Jun 29 2021 at 00:25):cfallin created PR review comment:
So it looks like there was an ongoing PR for this (rust-lang/libc#2189) that is somewhat complex, and stalled. For now I added a C helper and I'm happy to revisit this when Rust support exists, if that seems reasonable to you! (Also I misspoke above w.r.t. waiting for an OS package update --
libcis just a crate, of course, not part of the stdlib distro proper.)
Wasmtime GitHub notifications bot (Jun 29 2021 at 00:30):cfallin updated PR #2980 from openbsd69 to main.
Wasmtime GitHub notifications bot (Jun 29 2021 at 00:30):cfallin has marked PR #2980 as ready for review.
Wasmtime GitHub notifications bot (Jun 29 2021 at 05:03):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jun 29 2021 at 05:03):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jun 29 2021 at 05:03):alexcrichton created PR review comment:
For less cfg could this be defined in the block where it's called below?
Wasmtime GitHub notifications bot (Jun 29 2021 at 05:03):alexcrichton created PR review comment:
There's another nightly in this file elsewhere, mind updating that one too?
Wasmtime GitHub notifications bot (Jun 29 2021 at 06:04):cfallin updated PR #2980 from openbsd69 to main.
Wasmtime GitHub notifications bot (Jun 29 2021 at 06:04):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Jun 29 2021 at 06:04):cfallin created PR review comment:
Done!
Wasmtime GitHub notifications bot (Jun 29 2021 at 06:04):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Jun 29 2021 at 06:04):cfallin created PR review comment:
Ah, yes, I had missed that, thanks!
Wasmtime GitHub notifications bot (Jun 29 2021 at 06:52):cfallin updated PR #2980 from openbsd69 to main.
Wasmtime GitHub notifications bot (Jun 29 2021 at 08:49):cfallin updated PR #2980 from openbsd69 to main.
Wasmtime GitHub notifications bot (Jun 29 2021 at 08:54):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (Jun 29 2021 at 08:54):bjorn3 created PR review comment:
MAP_STACKcan also be used on Linux. It is currently a no-op, but according to the man page it is accepted such that if a future architecture requires special handling for the stack, no program changes are necessary.
Wasmtime GitHub notifications bot (Jun 29 2021 at 08:56):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (Jun 29 2021 at 08:56):bjorn3 created PR review comment:
What kind of limits?
Wasmtime GitHub notifications bot (Jun 29 2021 at 08:58):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Jun 29 2021 at 08:58):cfallin created PR review comment:
MAP_STACK and issues with the way that the pooling allocator deallocs/reallocs to zero; see the commit message on the commit that added this line.
Wasmtime GitHub notifications bot (Jun 29 2021 at 09:00):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Jun 29 2021 at 09:00):cfallin created PR review comment:
To give a little more detail: this check in the OpenBSD kernel's mmap() disallows requesting a fixed address for a MAP_STACK mmap, so one can't use this to get fresh CoW pages in an existing stack area.
Wasmtime GitHub notifications bot (Jun 29 2021 at 14:04):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jun 29 2021 at 14:04):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jun 29 2021 at 14:04):alexcrichton created PR review comment:
Could this use an
ifinstead of a conditional compilation to guard this?
Wasmtime GitHub notifications bot (Jun 29 2021 at 14:04):alexcrichton created PR review comment:
Can this use
skip_pooling_allocator_tests?
Wasmtime GitHub notifications bot (Jun 29 2021 at 14:04):alexcrichton created PR review comment:
Since
mmapis roughly the same could this useif cfg!(...)instead of#[cfg]so we get some type-checking when compiling for other unix platforms? (not changing the functionality here of course, just the structure)
Wasmtime GitHub notifications bot (Jun 29 2021 at 14:04):alexcrichton created PR review comment:
Could this test be changed to use
skip_pooling_allocator_testsinstead of adding a cfg here?
Wasmtime GitHub notifications bot (Jun 29 2021 at 14:04):alexcrichton created PR review comment:
Could this perhaps be extracted to a helper function in this module that tests for arm64 macos and openbsd and skips the tests if that's true?
Wasmtime GitHub notifications bot (Jun 29 2021 at 14:04):alexcrichton created PR review comment:
Similar to above could this be an
if cfg!at the top of the function which panics? (I like to keep#[cfg]to a minimum if we can since we can't type-check anything that's excluded)
Wasmtime GitHub notifications bot (Jun 29 2021 at 14:04):alexcrichton created PR review comment:
Could this (and the above test) use
skip_pooling_allocator_testsinstead?
Wasmtime GitHub notifications bot (Jul 01 2021 at 01:44):sunfishcode submitted PR review.
Wasmtime GitHub notifications bot (Jul 01 2021 at 01:44):sunfishcode created PR review comment:
The C wrapper looks fine to me. I suggest adding a link to rust-lang/libc#2189 in a comment so that people looking to refactor that code can easily check on its status.
Wasmtime GitHub notifications bot (Aug 29 2021 at 18:14):cfallin updated PR #2980 from openbsd69 to main.
Wasmtime GitHub notifications bot (Aug 29 2021 at 18:25):cfallin closed without merge PR #2980.
Last updated: Dec 23 2024 at 12:05 UTC