Don't use Uint8Array when not necessary

rust-random · Dec 6, 2024 · 20885b4 · 20885b4
1 parent 7adc287
commit 20885b4
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 35 deletions.
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -229,13 +229,29 @@ jobs:
   #         run: cargo test
 
   web:
-    name: Web
+    name: Web ${{ matrix.rust.description }}
     runs-on: ubuntu-24.04
+    strategy:
+      fail-fast: false
+      matrix:
+        rust:
+          - { version: stable, atomics: false }
+          - {
+              description: with Atomics,
+              atomics: true,
+              version: nightly,
+              components: rust-src,
+              flags: '-Ctarget-feature=+atomics,+bulk-memory',
+              args: '-Zbuild-std=panic_abort,std',
+            }
     env:
       CARGO_BUILD_TARGET: wasm32-unknown-unknown
     steps:
       - uses: actions/checkout@v4
-      - uses: dtolnay/rust-toolchain@stable
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ matrix.rust.version }}
+          components: ${{ matrix.rust.components }}
       # - name: Install precompiled wasm-bindgen
       #   shell: bash
       #   run: |
@@ -251,39 +267,39 @@ jobs:
       - uses: Swatinem/rust-cache@v2
       - name: Test (Node)
         env:
-          RUSTFLAGS: -Dwarnings --cfg getrandom_backend="wasm_js"
-        run: cargo test --features std
+          RUSTFLAGS: -Dwarnings --cfg getrandom_backend="wasm_js" ${{ matrix.rust.flags }}
+        run: cargo test --features std ${{ matrix.rust.args }}
       - name: Test (Firefox)
         env:
           GECKODRIVER: geckodriver
           WASM_BINDGEN_USE_BROWSER: 1
-          RUSTFLAGS: -Dwarnings --cfg getrandom_backend="wasm_js"
-        run: cargo test --features std
+          RUSTFLAGS: -Dwarnings --cfg getrandom_backend="wasm_js" ${{ matrix.rust.flags }}
+        run: cargo test --features std ${{ matrix.rust.args }}
       - name: Test (Chrome)
         env:
           CHROMEDRIVER: chromedriver
           WASM_BINDGEN_USE_BROWSER: 1
-          RUSTFLAGS: -Dwarnings --cfg getrandom_backend="wasm_js"
-        run: cargo test --features std
+          RUSTFLAGS: -Dwarnings --cfg getrandom_backend="wasm_js" ${{ matrix.rust.flags }}
+        run: cargo test --features std ${{ matrix.rust.args }}
       - name: Test (dedicated worker)
         env:
           GECKODRIVER: geckodriver
           WASM_BINDGEN_USE_DEDICATED_WORKER: 1
-          RUSTFLAGS: -Dwarnings --cfg getrandom_backend="wasm_js"
-        run: cargo test --features std
+          RUSTFLAGS: -Dwarnings --cfg getrandom_backend="wasm_js" ${{ matrix.rust.flags }}
+        run: cargo test --features std ${{ matrix.rust.args }}
       - name: Test (shared worker)
         env:
           GECKODRIVER: geckodriver
           WASM_BINDGEN_USE_SHARED_WORKER: 1
-          RUSTFLAGS: -Dwarnings --cfg getrandom_backend="wasm_js"
-        run: cargo test --features std
+          RUSTFLAGS: -Dwarnings --cfg getrandom_backend="wasm_js" ${{ matrix.rust.flags }}
+        run: cargo test --features std ${{ matrix.rust.args }}
       - name: Test (service worker)
         env:
           # Firefox doesn't support module service workers and therefor can't import scripts
           CHROMEDRIVER: chromedriver
           WASM_BINDGEN_USE_SERVICE_WORKER: 1
-          RUSTFLAGS: -Dwarnings --cfg getrandom_backend="wasm_js"
-        run: cargo test --features std
+          RUSTFLAGS: -Dwarnings --cfg getrandom_backend="wasm_js" ${{ matrix.rust.flags }}
+        run: cargo test --features std ${{ matrix.rust.args }}
 
   wasi:
     name: WASI

diff --git a/.github/workflows/workspace.yml b/.github/workflows/workspace.yml
@@ -49,6 +49,10 @@ jobs:
       env:
         RUSTFLAGS: -Dwarnings --cfg getrandom_backend="wasm_js"
       run: cargo clippy -Zbuild-std --target wasm32-unknown-unknown
+    - name: Web WASM with atomics (wasm_js.rs)
+      env:
+        RUSTFLAGS: -Dwarnings --cfg getrandom_backend="wasm_js" -Ctarget-feature=+atomics,+bulk-memory
+      run: cargo clippy -Zbuild-std --target wasm32-unknown-unknown
     - name: Linux (linux_android.rs)
       env:
         RUSTFLAGS: -Dwarnings --cfg getrandom_backend="linux_getrandom"

diff --git a/src/backends/wasm_js.rs b/src/backends/wasm_js.rs
@@ -19,28 +19,36 @@ pub fn fill_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
     CRYPTO.with(|crypto| {
         let crypto = crypto.as_ref().ok_or(Error::WEB_CRYPTO)?;
 
-        // getRandomValues does not work with all types of WASM memory,
-        // so we initially write to browser memory to avoid exceptions.
-        let buf = Uint8Array::new_with_length(CRYPTO_BUFFER_SIZE.into());
-        for chunk in dest.chunks_mut(CRYPTO_BUFFER_SIZE.into()) {
-            let chunk_len: u32 = chunk
-                .len()
-                .try_into()
-                .expect("chunk length is bounded by CRYPTO_BUFFER_SIZE");
-            // The chunk can be smaller than buf's length, so we call to
-            // JS to create a smaller view of buf without allocation.
-            let sub_buf = if chunk_len == u32::from(CRYPTO_BUFFER_SIZE) {
-                buf.clone()
-            } else {
-                buf.subarray(0, chunk_len)
-            };
-
-            if crypto.get_random_values(&sub_buf).is_err() {
-                return Err(Error::WEB_GET_RANDOM_VALUES);
-            }
+        if wasm_bindgen::is_memory_shared() {
+            // getRandomValues does not work with all types of WASM memory,
+            // so we initially write to browser memory to avoid exceptions.
+            let buf = Uint8Array::new_with_length(CRYPTO_BUFFER_SIZE.into());
+            for chunk in dest.chunks_mut(CRYPTO_BUFFER_SIZE.into()) {
+                let chunk_len: u32 = chunk
+                    .len()
+                    .try_into()
+                    .expect("chunk length is bounded by CRYPTO_BUFFER_SIZE");
+                // The chunk can be smaller than buf's length, so we call to
+                // JS to create a smaller view of buf without allocation.
+                let sub_buf = if chunk_len == u32::from(CRYPTO_BUFFER_SIZE) {
+                    buf.clone()
+                } else {
+                    buf.subarray(0, chunk_len)
+                };
+
+                if crypto.get_random_values(&sub_buf).is_err() {
+                    return Err(Error::WEB_GET_RANDOM_VALUES);
+                }
 
-            // SAFETY: `sub_buf`'s length is the same length as `chunk`
-            unsafe { sub_buf.raw_copy_to_ptr(chunk.as_mut_ptr().cast::<u8>()) };
+                // SAFETY: `sub_buf`'s length is the same length as `chunk`
+                unsafe { sub_buf.raw_copy_to_ptr(chunk.as_mut_ptr().cast::<u8>()) };
+            }
+        } else {
+            for chunk in dest.chunks_mut(CRYPTO_BUFFER_SIZE.into()) {
+                if crypto.get_random_values_ref(chunk).is_err() {
+                    return Err(Error::WEB_GET_RANDOM_VALUES);
+                }
+            }
         }
         Ok(())
     })
@@ -56,4 +64,6 @@ extern "C" {
     // Crypto.getRandomValues()
     #[wasm_bindgen(method, js_name = getRandomValues, catch)]
     fn get_random_values(this: &Crypto, buf: &Uint8Array) -> Result<(), JsValue>;
+    #[wasm_bindgen(method, js_name = getRandomValues, catch)]
+    fn get_random_values_ref(this: &Crypto, buf: &mut [MaybeUninit<u8>]) -> Result<(), JsValue>;
 }