Stream: git-wasmtime

Topic: wasmtime / PR #3355 Use rsix to make system calls in Wasm...

Wasmtime GitHub notifications bot (Sep 15 2021 at 23:10):

sunfishcode opened PR #3355 from sunfishcode/use-rsix to main:

rsix is a system call wrapper crate that we use in wasi-common,
which can provide the following advantages in the rest of Wasmtime:

• It eliminates some unsafe blocks in Wasmtime's code. There's
  still an unsafe block in the library, but this way, the unsafe
  is factored out and clearly scoped.

• And, it makes error handling more consistent, factoring out code for
  checking return values and io::Error::last_os_error(), and code that
  does errno::set_errno(0).

This doesn't cover all system calls; rsix doesn't implement
signal-handling APIs, and this doesn't cover calls made through std or
crates like userfaultfd, rand, and region.

<!--

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 (Sep 15 2021 at 23:38):

sunfishcode updated PR #3355 from sunfishcode/use-rsix to main.

Wasmtime GitHub notifications bot (Sep 15 2021 at 23:51):

sunfishcode updated PR #3355 from sunfishcode/use-rsix to main.

Wasmtime GitHub notifications bot (Sep 15 2021 at 23:54):

sunfishcode updated PR #3355 from sunfishcode/use-rsix to main.

Wasmtime GitHub notifications bot (Sep 16 2021 at 00:11):

sunfishcode updated PR #3355 from sunfishcode/use-rsix to main.

Wasmtime GitHub notifications bot (Sep 16 2021 at 00:16):

sunfishcode updated PR #3355 from sunfishcode/use-rsix to main.

Wasmtime GitHub notifications bot (Sep 16 2021 at 00:38):

sunfishcode updated PR #3355 from sunfishcode/use-rsix to main.

Wasmtime GitHub notifications bot (Sep 16 2021 at 00:49):

sunfishcode updated PR #3355 from sunfishcode/use-rsix to main.

Wasmtime GitHub notifications bot (Sep 16 2021 at 01:07):

sunfishcode updated PR #3355 from sunfishcode/use-rsix to main.

Wasmtime GitHub notifications bot (Sep 16 2021 at 01:28):

sunfishcode updated PR #3355 from sunfishcode/use-rsix to main.

Wasmtime GitHub notifications bot (Sep 16 2021 at 02:00):

sunfishcode updated PR #3355 from sunfishcode/use-rsix to main.

Wasmtime GitHub notifications bot (Sep 16 2021 at 07:08):

sunfishcode updated PR #3355 from sunfishcode/use-rsix to main.

Wasmtime GitHub notifications bot (Sep 16 2021 at 14:54):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Sep 16 2021 at 14:54):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Sep 16 2021 at 14:54):

alexcrichton created PR review comment:

How come this was needed with this change? I would have figured that rsix was already part of wasi-common's dep tree to have this required already...

Wasmtime GitHub notifications bot (Sep 16 2021 at 15:01):

sunfishcode submitted PR review.

Wasmtime GitHub notifications bot (Sep 16 2021 at 15:01):

sunfishcode created PR review comment:

It's actually an unrelated change; a contributor recently added 32-bit ARM support in rsix, so it now needs cross tools to be cross-compiled.

Wasmtime GitHub notifications bot (Sep 16 2021 at 15:10):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Sep 16 2021 at 15:10):

alexcrichton created PR review comment:

How come this is required at cargo check-time though? Did rsix previously not have C code to build and now it does?

Wasmtime GitHub notifications bot (Sep 16 2021 at 15:13):

bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (Sep 16 2021 at 15:13):

bjorn3 created PR review comment:

Rsix has two backends. The libc backend and the raw linux syscall backend. The latter requires compiling external assembly and is only used on supported linux targets. Previously 32-bit ARM was not one of such targets and thus would use the libc backend AFAIU.

Wasmtime GitHub notifications bot (Sep 17 2021 at 21:32):

sunfishcode updated PR #3355 from sunfishcode/use-rsix to main.

Wasmtime GitHub notifications bot (Sep 17 2021 at 22:28):

sunfishcode merged PR #3355.

Wasmtime GitHub notifications bot (Sep 21 2021 at 17:26):

bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (Sep 21 2021 at 17:26):

bjorn3 created PR review comment:

This adds a lot of new dependencies to cg_clif:

      Adding errno v0.2.7
      Adding errno-dragonfly v0.1.1
      Adding gcc v0.3.55
      Adding io-lifetimes v0.3.1
      Adding itoa v0.4.8
      Adding linux-raw-sys v0.0.28
      Adding rsix v0.23.5
      Adding rustc_version v0.4.0
      Adding semver v1.0.4

This all to prevent a single libc call is not worth it IMO. Before this change cg_clif didn't depend on a C compiler on any target, now it does on some targets. It also bloats the dylib size as unused functions are still exported from the dylib and thus can't be gc'ed and increases compilation time.

Wasmtime GitHub notifications bot (Sep 26 2021 at 12:58):

sunfishcode created PR review comment:

Makes sense. rsix is only used for one thing in Cranelift here, so I'll submit a PR to revert that to just using libc.

Wasmtime GitHub notifications bot (Sep 26 2021 at 12:58):

sunfishcode submitted PR review.

Wasmtime GitHub notifications bot (Sep 26 2021 at 13:01):

bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (Sep 26 2021 at 13:01):

bjorn3 created PR review comment:

Thanks!

Wasmtime GitHub notifications bot (Oct 22 2021 at 23:08):

sunfishcode submitted PR review.

Wasmtime GitHub notifications bot (Oct 22 2021 at 23:08):

sunfishcode created PR review comment:

This part of the patch is now reverted, so it's not relevant at this time, but in case anyone is reading this is curious, the situation is better now:

New versions of rsix no longer build-depend on gcc or any C compiler, and no longer depend on itoa by default.

Also, it's worth noting that rustc_version and semver are build-time dependencies that aren't linked into the output, and linux-raw-sys doesn't contain any code.

Last updated: Oct 23 2024 at 20:03 UTC