Port to OpenBSD! #2980

cfallin · 2021-06-12T23:25:35Z

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) because rustup doesn'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 toplevel Cargo.toml to 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, because libc
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-nn by default, because the openvino-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) rustup doesn'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!

cfallin · 2021-06-12T23:28:01Z

I'll also note that this depends on darfink/region-rs#8, which was recently added but has not yet been released yet; I'll ping over there to see if they can release a new version to crates.io.

github-actions · 2021-06-12T23:43:39Z

Subscribe to Label Action

cc @kubkon

This issue or pull request has been labeled: "wasi"

Thus the following users have been cc'd because of the following labels:

kubkon: wasi

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

abrown · 2021-06-14T16:17:40Z

@cfallin, I'm interested in why the openvino-sys crate is failing: the way we build it here, it should have no target-specific logic since it uses libloading to look for the right libraries. Can you send me the failed build output and I'll try to fix upstream?

cfallin · 2021-06-14T16:55:07Z

@cfallin, I'm interested in why the openvino-sys crate is failing: the way we build it here, it should have no target-specific logic since it uses libloading to look for the right libraries. Can you send me the failed build output and I'll try to fix upstream?

It's an issue with some platform-specific constants that are defined with #[cfg(linux)] or similar rather than "all Unix-like platforms", I think:

error[E0425]: cannot find value `ENV_LIBRARY_PATH` in this scope
  --> /home/cfallin/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/openvino-finder-0.3.1/src/lib.rs:44:37
   |
44 |     if let Some(path) = env::var_os(ENV_LIBRARY_PATH) {
   |                                     ^^^^^^^^^^^^^^^^ not found in this scope

error[E0425]: cannot find value `DEFAULT_INSTALLATION_DIRECTORIES` in this scope
  --> /home/cfallin/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/openvino-finder-0.3.1/src/lib.rs:54:24
   |
54 |       for default_dir in DEFAULT_INSTALLATION_DIRECTORIES
   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: a constant with a similar name exists: `KNOWN_INSTALLATION_SUBDIRECTORIES`

sunfishcode · 2021-06-14T22:43:38Z

system-interface 0.6.6 and posish 0.6.3 are now available.

sunfishcode · 2021-06-14T22:45:50Z

crates/runtime/src/traphandlers/unix.rs

+            // This is the kernel-entry register-save frame format and is
+            // *unlikely* to change across OpenBSD versions, but note that
+            // OpenBSD makes no guarantee of ABI compatibility across any
+            // release.


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.

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 :-)

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 -- libc is just a crate, of course, not part of the stdlib distro proper.)

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.

Cargo.toml

cfallin · 2021-06-29T00:32:37Z

@sunfishcode I've updated this now with some spare cycles and I think it might be good for another look, whenever you have the chance! It's still pending a region.rs release but I've pinged over there. I resolved the libc dependency for now with a C helper, which isn't ideal but is better than a hardcoded constant.

There's also some version weirdness where cap-std-* is pulling in two versions of rustc_version (see cargo-deny CI job); not sure what's going on there.

crates/runtime/src/traphandlers/unix.rs

.github/workflows/main.yml

cfallin · 2021-06-29T06:17:42Z

Updated, thanks! This just depends now on (i) a region-rs release, and (ii) resolution of some multi-crate-version weirdness:

@sunfishcode, it seems that we have:

system-interface 0.6.6 -> cap-fs-ext 0.13.7 -> unsafe_io 0.6.12 -> rustc_version 0.3.3
system-interface 0.6.6 -> cap-fs-ext 0.13.7 -> unsafe_io 0.6.12 -> io_lifetimes 0.1.1 -> rustc_version 0.4.0
system-interface 0.6.6 -> cap-fs-ext 0.13.10 -> rustc_version 0.3.3
system-interface 0.6.6 -> rustc_version 0.4.0

in general it seems the various cap-std-related crates have inconsistent dependencies on rustc_version, which is making cargo-deny unhappy -- thoughts? Would it be possible to harmonize everything to 0.4.0?

cfallin · 2021-06-29T07:04:37Z

@alexcrichton PTAL again (with low priority for this unofficial side-project!) re: fibers -- there's some additional flags-setting needed that I hadn't realized before too, as OpenBSD has a special mitigation that requires blessing stack memory in a certain way.

bjorn3 · 2021-06-29T08:54:42Z

crates/fiber/src/unix.rs

    // On OpenBSD, we need to use MAP_STACK to specify that a page can contain a
    // stack; otherwise, we will get a SIGSEGV when it is used as such.
    //
-    // Note also that MAP_STACK must be specified with PROT_READ|PROT_WRITE, and
+    // Note that MAP_STACK must be specified with PROT_READ|PROT_WRITE, and


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

cfallin · 2021-06-29T08:55:55Z

With latest updates, all tests pass in wasmtime crate. I had to disable the pooling allocator as well, for MAP_STACK-related reasons. Checking all tests (our full CI gamut) would require me to set up a wasm32-wasi toolchain, which means building rustc from source (whee!), so that's the last of my side-project steam for now.

On a slightly less-unofficial note, at some point it would be neat to articulate a tier strategy for platform support; I imagine something like Rust's tier 1/2/3 (tested, compiled, and best-effort respectively?), where OpenBSD is definitely tier 3, but there might be some systems we could include at tier 2 (other BSDs? Linux/musl? macOS/aarch64?) if cross-compilation works.

bjorn3 · 2021-06-29T08:56:40Z

crates/runtime/src/instance/allocator/pooling.rs

+        instance_limits: &InstanceLimits,
+        tunables: &Tunables,
+    ) -> Result<Self> {
+        panic!("Pooling allocator not supported on OpenBSD due to limits in stack mmap'ing.");


What kind of limits?

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.

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.

alexcrichton

Personally I don't mind tier 2 ish platforms so long as they don't impose a maintenance burden, but I personally find it somewhat burdensome to try to deal with #[cfg] in lots of places. I'm a bit worried about the size of directives added to tests and such. Without any CI this is inevitable to regress over time, and I'm not sure if it's really too worthwhile to get the test suite "green" within a snapshot in time, only to have it not green later.

alexcrichton · 2021-06-29T13:58:36Z

crates/fiber/src/unix.rs

+}
+
+/// Maps a new stack with a guard page. Returns (top, len).
+#[cfg(target_os = "openbsd")]


Since mmap is roughly the same could this use if 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)

alexcrichton · 2021-06-29T13:59:32Z

crates/runtime/src/instance/allocator/pooling.rs

@@ -344,6 +349,15 @@ impl InstancePool {
        Ok(pool)
    }

+    #[cfg(target_os = "openbsd")]


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)

alexcrichton · 2021-06-29T14:00:44Z

crates/runtime/src/traphandlers/unix.rs

-            libc::PROT_READ | libc::PROT_WRITE,
-        );
-        assert_eq!(r, 0, "mprotect to configure memory for sigaltstack failed");
+        #[cfg(not(target_os = "openbsd"))]


Could this use an if instead of a conditional compilation to guard this?

alexcrichton · 2021-06-29T14:01:38Z

tests/all/instance.rs

@@ -33,6 +33,7 @@ fn initializes_linear_memory() -> Result<()> {
 }

 #[test]
+#[cfg(not(target_os = "openbsd"))]


Could this test be changed to use skip_pooling_allocator_tests instead of adding a cfg here?

alexcrichton · 2021-06-29T14:01:59Z

tests/all/async_functions.rs

@@ -447,6 +448,7 @@ fn async_with_pooling_stacks() {
 }

 #[test]
+#[cfg(not(target_os = "openbsd"))]


Could this (and the above test) use skip_pooling_allocator_tests instead?

alexcrichton · 2021-06-29T14:02:17Z

tests/all/limits.rs

@@ -220,6 +220,7 @@ fn test_initial_table_limits_exceeded() -> Result<()> {
 }

 #[test]
+#[cfg(not(target_os = "openbsd"))]


Can this use skip_pooling_allocator_tests?

alexcrichton · 2021-06-29T14:02:54Z

tests/all/traps.rs

@@ -539,6 +547,7 @@ fn parse_dwarf_info() -> Result<()> {

 #[test]
 #[cfg_attr(all(target_os = "macos", target_arch = "aarch64"), ignore)] // TODO #2808 system libunwind is broken on aarch64
+#[cfg_attr(target_os = "openbsd", ignore)] // backtraces are broken on OpenBSD


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?

sunfishcode · 2021-07-01T01:56:08Z

@cfallin #3049 updates almost everything to rustc_version 0.4.0, except wasmtime still depends on criterion which depends on cast which depends on rustc_version 0.3.3. I've now submitted japaric/cast.rs#34 to update cast.

Verified to work with several simple Wasm modules on an OpenBSD 6.9 x86_64 machine, built with the Rust included in the packages collection (`pkg_add rust`) because `rustup` doesn't support OpenBSD. This depends on a not-yet-released version of region-rs; this commit will be updated once a new release is made.

OpenBSD >= 6.4 requires a special `MAP_STACK` flag to `mmap()` when allocating any memory that will be used as a stack. This mapping attribute is checked whenever the kernel returns to userspace, according to the stack pointer's current value. If SP points to other memory, the process will receive SIGSEGV.

Also disable the pooling allocator and associated tests on OpenBSD; its method of zeroing the stack (unmapping and remapping at the same fixed address) is not compatible with MAP_STACK, which forbids fixed mapping requests.

cfallin · 2021-08-29T18:25:32Z

I just rebased onto latest main and updated to region-rs 3.0.0, which includes a fix for OpenBSD.

Unfortunately it looks like new build failures have cropped up in the meantime, this time in rsix (bytecodealliance/rustix#32). I also had to upgrade my test VM to OpenBSD-current to get Rust 1.54.0 because -stable (currently 6.9) has Rust 1.51.0, which fails to build some other things. (Rustup doesn't support OpenBSD unfortunately so one must use the system package or build from source.)

Given the moving target, the fact that we probably won't have a reasonable way to test this on CI, and the fact that we (or at least I) don't really have the time to support another platform beyond "one weekend morning once every few months" (this PR so far), I think I'll go ahead and close this PR. We should definitely reconsider this if/when OpenBSD graduates from tier3 and Rustup adds binaries, and if/when we have a way of adding native GitHub CI runners, at which point someone could run OpenBSD CI.

Until then I'll keep the wasmtime binary on my little router as a keepsake from that one golden moment when this PR compiled cleanly. Thanks for the patience, all!

cfallin requested review from alexcrichton and sunfishcode June 12, 2021 23:25

github-actions bot added the wasi Issues pertaining to WASI label Jun 12, 2021

sunfishcode reviewed Jun 15, 2021

View reviewed changes

abrown reviewed Jun 17, 2021

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

cfallin force-pushed the openbsd69 branch from 701fd3d to 5bdfb1c Compare June 29, 2021 00:21

cfallin marked this pull request as ready for review June 29, 2021 00:30

alexcrichton approved these changes Jun 29, 2021

View reviewed changes

crates/runtime/src/traphandlers/unix.rs Outdated Show resolved Hide resolved

.github/workflows/main.yml Outdated Show resolved Hide resolved

cfallin force-pushed the openbsd69 branch from e7f300b to e6aff7e Compare June 29, 2021 06:04

bjorn3 reviewed Jun 29, 2021

View reviewed changes

alexcrichton reviewed Jun 29, 2021

View reviewed changes

cratelyn mentioned this pull request Jul 12, 2021

cannot find function get_region in module os fastly/Viceroy#4

Closed

cfallin added 4 commits August 29, 2021 10:48

Fix up remaining stack allocs on OpenBSD to use MAP_STACK.

bb1ec52

Also disable the pooling allocator and associated tests on OpenBSD; its method of zeroing the stack (unmapping and remapping at the same fixed address) is not compatible with MAP_STACK, which forbids fixed mapping requests.

Update to region-rs 3.0.0.

c1f7038

cfallin force-pushed the openbsd69 branch from f78a59d to c1f7038 Compare August 29, 2021 18:14

cfallin closed this Aug 29, 2021

cfallin mentioned this pull request Sep 12, 2023

NetBSD/OpenBSD support #6962

Open

cfallin mentioned this pull request May 14, 2024

Added OpenBSD support to wasmtime's signals.rs #8613

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port to OpenBSD! #2980

Port to OpenBSD! #2980

cfallin commented Jun 12, 2021

cfallin commented Jun 12, 2021

github-actions bot commented Jun 12, 2021

abrown commented Jun 14, 2021

cfallin commented Jun 14, 2021

sunfishcode commented Jun 14, 2021

sunfishcode Jun 14, 2021

cfallin Jun 15, 2021

cfallin Jun 29, 2021

sunfishcode Jul 1, 2021

cfallin commented Jun 29, 2021

cfallin commented Jun 29, 2021

cfallin commented Jun 29, 2021

bjorn3 Jun 29, 2021

cfallin commented Jun 29, 2021 •

edited

Loading

bjorn3 Jun 29, 2021

cfallin Jun 29, 2021

cfallin Jun 29, 2021

alexcrichton left a comment

alexcrichton Jun 29, 2021

alexcrichton Jun 29, 2021

alexcrichton Jun 29, 2021

alexcrichton Jun 29, 2021

alexcrichton Jun 29, 2021

alexcrichton Jun 29, 2021

alexcrichton Jun 29, 2021

sunfishcode commented Jul 1, 2021

cfallin commented Aug 29, 2021

Port to OpenBSD! #2980

Port to OpenBSD! #2980

Conversation

cfallin commented Jun 12, 2021

cfallin commented Jun 12, 2021

github-actions bot commented Jun 12, 2021

Subscribe to Label Action

abrown commented Jun 14, 2021

cfallin commented Jun 14, 2021

sunfishcode commented Jun 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cfallin commented Jun 29, 2021

cfallin commented Jun 29, 2021

cfallin commented Jun 29, 2021

Choose a reason for hiding this comment

cfallin commented Jun 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexcrichton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunfishcode commented Jul 1, 2021

cfallin commented Aug 29, 2021

cfallin commented Jun 29, 2021 •

edited

Loading