Support for wasm32-wasi #1568

JanKaul · 2022-12-16T13:14:19Z

This PR adds support for the wasm32-wasi target.

To get the CI working, i had to use the dtolnay/rust-toolchain@stable action. The toolchain only seems to work when it is installed with rustup, see a similar issue here: https://bugs.archlinux.org/task/75269.

Currently the clippy and audit CI runs are failing. I didn't include them in this PR because I thought it would make it more difficult to see the actual changes. If I should include the clippy and audit changes, please let me know.

I agree to license my contributions to each file under the terms given at the top of each file I changed.

xxchan · 2023-07-20T14:23:51Z

just kindly fwd: @dynamite-bud said this doesn't work #1043 (comment)

WASI_SDK_PATH=/(...)/wasi/wasi-sdk cargo build --target wasm32-wasi                                                              1.71.0-nightly  2.90  4.88G 14:24:35  100%

   Compiling ring v0.17.0-not-released-yet (/(...)/ring)
error: no rules expected the token `unsafe`
  --> src/arithmetic/bigint/bn_mul_mont_fallback.rs:26:5
   |
26 |     unsafe fn bn_mul_mont(
   |     ^^^^^^ no rules expected this token in macro call
   |
  ::: src/prefixed.rs:1:1
   |
1  | macro_rules! prefixed_extern {
   | ---------------------------- when calling this macro
   |
note: while trying to match `fn`
  --> src/prefixed.rs:6:22
   |
6  |             $vis:vis fn $name:ident ( $( $arg_pat:ident : $arg_ty:ty ),* $(,)? )
   |                      ^^

warning: unused macro definition: `prefixed_export`
  --> src/prefixed.rs:44:14
   |
44 | macro_rules! prefixed_export {
   |              ^^^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_macros)]` on by default

warning: unused imports: `Limb`, `MODULUS_MAX_LIMBS`, `N0`, `limbs_from_mont_in_place`, `limbs_mul`
  --> src/arithmetic/bigint/bn_mul_mont_fallback.rs:22:13
   |
22 | use super::{limbs_from_mont_in_place, limbs_mul, Limb, MODULUS_MAX_LIMBS, N0};
   |             ^^^^^^^^^^^^^^^^^^^^^^^^  ^^^^^^^^^  ^^^^  ^^^^^^^^^^^^^^^^^  ^^
   |
   = note: `#[warn(unused_imports)]` on by default

warning: unused import: `crate::c`
  --> src/arithmetic/bigint/bn_mul_mont_fallback.rs:23:5
   |
23 | use crate::c;
   |     ^^^^^^^^

warning: `ring` (lib) generated 3 warnings
error: could not compile `ring` (lib) due to previous error; 3 warnings emitted

JanKaul · 2023-07-25T17:49:57Z

It works with

export WASI_SDK_PATH=/opt/wasi-sdk CC="${WASI_SDK_PATH}/bin/clang --sysroot=${WASI_SDK_PATH}/share/wasi-sysroot"

And the wasi tests in the CI also succeed.

briansmith · 2023-09-19T17:37:03Z

tests/digest_tests.rs

@@ -47,7 +47,7 @@ fn digest_misc() {
 }

 // wasm_bindgen doesn't build this correctly.
-#[cfg(not(target_arch = "wsam32"))]
+#[cfg(not(target_arch = "wasm32"))]


Thanks for pointing out the typo. I fixed this the right way in PR #1647, so you can remove this change.

Okay, I will, Thanks for the review.

briansmith · 2023-09-19T17:38:06Z

build.rs

+            &std::env::var("WASI_SDK_PATH").expect("missing environment variable: WASI_SDK_PATH");
+        c.compiler(format!("{}/bin/clang", wasi_sdk_path).as_str());
+        c.archiver(format!("{}/bin/llvm-ar", wasi_sdk_path).as_str());
+        c.flag(format!("--sysroot={}/share/wasi-sysroot", wasi_sdk_path).as_str());


I feel like these changes belong in cc-rs. We traditionally avoid doing this kind of discovery. I suggest we remove this change from this PR.

briansmith · 2023-09-19T17:40:18Z

.github/workflows/ci.yml

+          target: ${{ matrix.target }}
+          toolchain: ${{ matrix.rust_channel }}
+
+      - name: Install wasi-sdk


This logic ideally belongs in install-build-tools.sh. The idea is that anybody can run mk/install-build-tools.sh --target=wasm32-wasi and the necessary tools installed. Then CI can use mk/install-build-tools.sh too, and then peoples' local build environment matches CI (more or less).

When I install the wasi-sdk with the install-build-tools.sh script, where would you place the install files? On linux you would typically place them in /opt/wasi-sdk but I'm not sure it I can assume that.

briansmith · 2023-09-19T17:42:08Z

.github/workflows/ci.yml

@@ -464,6 +464,69 @@ jobs:
      # TODO: Do this check on Windows too.
      - run: mk/check-symbol-prefixes.sh --target=${{ matrix.target }}

+  # The wasm32-wasi targets have a different set of feature sets.


I do not understand this comment about feature sets, and thus I don't understand why the wasi target needs its own build matrix.

jedisct1 · 2023-09-22T20:32:16Z

You should consider supporting cargo zigbuild instead. This is less painful than wasi-sdk, and the same command can be used to cross-compile to other targets.

Support for WASI using cargo zigbuild was done in https://github.com/jedisct1/ring-wasi

Without supporting pre-compiled files, it doesn't require any changes to the build scripts, nor messing with environment variables.

briansmith · 2023-09-27T01:24:44Z

.github/workflows/ci.yml

+        run: |
+          export WASI_URL=$(curl -s https://api.github.com/repos/WebAssembly/wasi-sdk/releases/latest | jq -r '.assets[] | select(.name | contains("linux.tar.gz")) | .browser_download_url')
+          wget -O ./wasi-sdk-linux.tar.gz $WASI_URL
+          mkdir wasi-sdk


Actually, why do we need the WASI SDK? If you look at the existing wasm32 targets, we manage to build them with standard clang, and I don't see anything WASI-specific here besides the rand.rs, which doesn't add any new C stuff. Maybe we can get rid of this SDK dependency by treating the -wasi target just like the -unknown-unknown target?

That would be even better, as it could run in web browsers without additional modules.

briansmith · 2023-10-14T19:07:52Z

@JanKaul Thanks for your PR. Please see #1745 where I incorporated these changes and added the CI integration. Closing this in favor of that PR.

JanKaul · 2023-10-16T09:10:46Z

Thanks a lot for taking this on. I wasn't entirely sure how to do this.

JanKaul mentioned this pull request Dec 16, 2022

wasm32-wasi doesn't build #1043

Closed

ipuustin mentioned this pull request Apr 25, 2023

Provide a wasm-compatible Rust TLS implementation WasmEdge/WasmEdge#1430

Closed

JanKaul added 12 commits September 8, 2023 12:19

wasi sdk

ef998ef

impl SecureRandom

944f60f

update ci

346aa7f

set wasi-sdk compiler + archiver

5f9aafe

use llvm-ar

7996915

fix test

7eac948

remove wasm_bindgen dev-dependency

6091890

set wasi_sdk_path

0206283

adopt cargo.sh

4e63155

use rustup

961ceff

add cc compiler

02d4a5d

change toolchain

eb40678

briansmith reviewed Sep 19, 2023

View reviewed changes

briansmith reviewed Sep 27, 2023

View reviewed changes

StringKe mentioned this pull request Oct 6, 2023

The ureq dependency in the svg feature does not compile wasm rust-skia/rust-skia#845

Closed

This was referenced Oct 14, 2023

Can't use ring on wasm32-unknown-unknown due to a missing import (for Envoy Proxy) #1453

Closed

Adapt tests to work for wasm32-wasi and add wasm32-wasi to CI. #1745

Merged

briansmith closed this Oct 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for wasm32-wasi #1568

Support for wasm32-wasi #1568

JanKaul commented Dec 16, 2022 •

edited

Loading

xxchan commented Jul 20, 2023

JanKaul commented Jul 25, 2023

briansmith Sep 19, 2023

JanKaul Sep 22, 2023

briansmith Sep 19, 2023

briansmith Sep 19, 2023

JanKaul Sep 22, 2023

briansmith Sep 19, 2023

jedisct1 commented Sep 22, 2023

briansmith Sep 27, 2023

jedisct1 Sep 27, 2023

briansmith commented Oct 14, 2023

JanKaul commented Oct 16, 2023

Support for wasm32-wasi #1568

Support for wasm32-wasi #1568

Conversation

JanKaul commented Dec 16, 2022 • edited Loading

xxchan commented Jul 20, 2023

JanKaul commented Jul 25, 2023

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

jedisct1 commented Sep 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

briansmith commented Oct 14, 2023

JanKaul commented Oct 16, 2023

JanKaul commented Dec 16, 2022 •

edited

Loading