Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

zeroize: add support for wasm's v128 SIMD register #968

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

zeroize: add support for wasm's v128 SIMD register #968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions zeroize/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,8 @@ pub use zeroize_derive::{Zeroize, ZeroizeOnDrop};

#[cfg(all(feature = "aarch64", target_arch = "aarch64"))]
mod aarch64;
#[cfg(all(target_arch = "wasm32", target_family = "wasm"))]
mod wasm32;
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
mod x86;

Expand Down
22 changes: 22 additions & 0 deletions zeroize/src/wasm32.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
//! [`Zeroize`] impls for WASM SIMD registers.

use crate::{atomic_fence, volatile_write, Zeroize};

use core::arch::wasm32::v128;

macro_rules! impl_zeroize_for_simd_register {
($($type:ty),* $(,)?) => {
$(
#[cfg_attr(docsrs, doc(cfg(target_arch = "wasm32", target_family = "wasm")))]
impl Zeroize for $type {
#[inline]
fn zeroize(&mut self) {
volatile_write(self, unsafe { core::mem::zeroed() });
atomic_fence();
}
}
)+
};
}

impl_zeroize_for_simd_register!(v128);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With only one register type, the macro is probably overkill, unless you plan to add more

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, I was just adhering to the other implementations as I wasn't sure which style would be preferrential - bad decision on my part (even if it is just be abstracted away once compiled).

After looking further into it, wasm has only a generic v128 type, and no other SIMD registers available (I believe the internal types are defined here though).

I'll scrap the macro as it's definitely overkill for the single addressible register type, although I do wonder how the compiler even handles a type this generic.

On another note, should there be platform-specific tests for (maybe all?) SIMD registers, so that we can validate that they're correctly zeroed out? I'm not too sure I've seen any within the codebase, but it might be good for additional validation/reassurance as even just this single type does seem rather internally-complex. I wouldn't mind giving it a go and opening a separate PR if I can get them working and ensure their correctness.

The tests might be unfeasable for many register types though, given that some are gated behind specific CPU features/extensions - I tried to add (feature gated) avx512 register support, but that's both far too recent (MSRV of 1.73) and I couldn't even get it working on my 7950x, which has more than enough avx512 support available. I probably spent way too long throwing flags at the compiler there haha