Skip to content

Make zerocopy optional #253

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Make zerocopy optional #253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aaea41a
Make zerocopy optional
mitsuhiko Feb 5, 2025
aa95323
CI with zerocopy
mitsuhiko Feb 5, 2025
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
34 changes: 18 additions & 16 deletions .github/workflows/rust.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,17 @@ jobs:
toolchain: stable
components: clippy
- name: check nostd
run: cargo check --no-default-features,zerocopy
- name: check nostd dist
run: cargo check --no-default-features
- name: test nostd
run: cargo test --no-default-features
run: cargo test --no-default-features,zerocopy
- name: check constrandom
run: cargo check --no-default-features --features compile-time-rng
run: cargo check --no-default-features --features compile-time-rng,zerocopy
- name: test constrandom
run: cargo test --no-default-features --features compile-time-rng
run: cargo test --no-default-features --features compile-time-rng,zerocopy
- name: check fixed-seed
run: cargo check --no-default-features --features std
run: cargo check --no-default-features --features std,zerocopy
- name: check
run: cargo check
- name: test
Expand All @@ -41,11 +43,11 @@ jobs:
- name: check nightly
run: cargo check -Z msrv-policy
- name: test nightly
run: cargo test
run: cargo test --features=zerocopy
- name: check serde
run: cargo check --features serde
run: cargo check --features serde,zerocopy
- name: test serde
run: cargo test --features serde
run: cargo test --features serde,zerocopy
linux_arm7:
name: Linux ARMv7
runs-on: ubuntu-latest
Expand All @@ -71,9 +73,9 @@ jobs:
with:
toolchain: stable
targets: aarch64-apple-darwin
- run: cargo check --target aarch64-apple-darwin
- run: cargo test
- run: cargo test --no-default-features --features compile-time-rng
- run: cargo check --target aarch64-apple-darwin --features=zerocopy
- run: cargo test --features=zerocopy
- run: cargo test --no-default-features --features compile-time-rng,zerocopy
- name: Install 1.72.0
uses: dtolnay/rust-toolchain@master
with:
Expand All @@ -94,15 +96,15 @@ jobs:
- run: cargo check --target i686-unknown-linux-gnu
- run: cargo test --target i686-unknown-linux-gnu
- name: check constrandom
run: cargo check --no-default-features --features compile-time-rng --target i686-unknown-linux-gnu
run: cargo check --no-default-features --features compile-time-rng,zerocopy --target i686-unknown-linux-gnu
- name: Install 1.72.0
uses: dtolnay/rust-toolchain@master
with:
toolchain: 1.72.0
targets: i686-unknown-linux-gnu
- run: cargo +1.72.0 check --target i686-unknown-linux-gnu
- name: check constrandom
run: cargo +1.72.0 check --no-default-features --features compile-time-rng --target i686-unknown-linux-gnu
run: cargo +1.72.0 check --no-default-features --features compile-time-rng,zerocopy --target i686-unknown-linux-gnu
x86_64-unknown-linux-gnu:
name: Linux x86_64
runs-on: ubuntu-latest
Expand All @@ -117,14 +119,14 @@ jobs:
- run: cargo check --target x86_64-unknown-linux-gnu
- run: cargo test --target x86_64-unknown-linux-gnu
- name: check constrandom
run: cargo check --no-default-features --features compile-time-rng --target x86_64-unknown-linux-gnu
run: cargo check --no-default-features --features compile-time-rng,zerocopy --target x86_64-unknown-linux-gnu
- name: Install 1.72.0
uses: dtolnay/rust-toolchain@master
with:
toolchain: 1.72.0
- run: cargo +1.72.0 check --target x86_64-unknown-linux-gnu
- name: check constrandom
run: cargo +1.72.0 check --no-default-features --features compile-time-rng --target x86_64-unknown-linux-gnu
run: cargo +1.72.0 check --no-default-features --features compile-time-rng,zerocopy --target x86_64-unknown-linux-gnu
thumbv6m:
name: thumbv6m
runs-on: ubuntu-latest
Expand All @@ -134,7 +136,7 @@ jobs:
with:
toolchain: stable
targets: thumbv6m-none-eabi
- run: cargo check --target thumbv6m-none-eabi --no-default-features
- run: cargo check --target thumbv6m-none-eabi --no-default-features --features=zerocopy
wasm32-unknown-unknown:
name: wasm
runs-on: ubuntu-latest
Expand All @@ -144,7 +146,7 @@ jobs:
with:
toolchain: stable
targets: wasm32-unknown-unknown
- run: cargo check --target wasm32-unknown-unknown --no-default-features
- run: cargo check --target wasm32-unknown-unknown --no-default-features --features=zerocopy
no_std:
name: no-std build
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ serde = { version = "1.0.117", optional = true }
cfg-if = "1.0"
portable-atomic = { version = "1.0.0", optional = true }
getrandom = { version = "0.2.7", optional = true }
zerocopy = { version = "0.7.31", default-features = false, features = ["simd"] }
zerocopy = { version = "0.7.31", default-features = false, features = ["simd"], optional = true }

[target.'cfg(not(all(target_arch = "arm", target_os = "none")))'.dependencies]
once_cell = { version = "1.18.0", default-features = false, features = ["alloc"] }
Expand Down
17 changes: 15 additions & 2 deletions src/convert.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
macro_rules! safer_transmute {
($e:expr) => {{
#[cfg(feature = "zerocopy")]
{
zerocopy::transmute!($e)
}
#[cfg(not(feature = "zerocopy"))]
{
::core::mem::transmute($e)
}
}}
}

pub(crate) trait Convert<To> {
fn convert(self) -> To;
}
Expand All @@ -7,13 +20,13 @@ macro_rules! convert {
impl Convert<$b> for $a {
#[inline(always)]
fn convert(self) -> $b {
zerocopy::transmute!(self)
unsafe { safer_transmute!(self) }
Copy link
Contributor

Choose a reason for hiding this comment

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

This still suffers from the fact that the unsafe is hidden in a macro, and also there is no SAFETY comment. I think those were the two problems that the PR that added zerocopy was trying to address.

If it is important to use transmute for performance or other reasons, then it would be better to just manually copy/paste the necessary implementations so that all the unsafe are moved out of the macro.

Copy link
Author

Choose a reason for hiding this comment

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

Agreed. This would need to be addressed. I just saw that you pushed PRs up that I think in general are a good thing to land if possible. So maybe that should take priority.

I general though I think all of this is stalled for the moment until a maintainer shows up. I'm not sure who has permissions to merge or approve things here.

}
}
impl Convert<$a> for $b {
#[inline(always)]
fn convert(self) -> $a {
zerocopy::transmute!(self)
unsafe { safer_transmute!(self) }
}
}
};
Expand Down
15 changes: 12 additions & 3 deletions src/hash_quality_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,12 @@ fn test_no_full_collisions<T: Hasher>(gen_hash: impl Fn() -> T) {
gen_combinations(&options, 7, Vec::new(), &mut combinations);
let mut map: HashMap<u64, Vec<u8>> = HashMap::new();
for combination in combinations {
use zerocopy::AsBytes;
let array = combination.as_slice().as_bytes().to_vec();
let array = unsafe {
let (begin, middle, end) = combination.align_to::<u8>();
assert_eq!(0, begin.len());
assert_eq!(0, end.len());
middle.to_vec()
};
let mut hasher = gen_hash();
hasher.write(&array);
let hash = hasher.finish();
Expand Down Expand Up @@ -441,7 +445,12 @@ mod fallback_tests {
///Basic sanity tests of the cypto properties of aHash.
#[cfg(any(
all(any(target_arch = "x86", target_arch = "x86_64"), target_feature = "aes", not(miri)),
all(feature = "nightly-arm-aes", target_arch = "aarch64", target_feature = "aes", not(miri)),
all(
feature = "nightly-arm-aes",
target_arch = "aarch64",
target_feature = "aes",
not(miri)
),
all(feature = "nightly-arm-aes", target_arch = "arm", target_feature = "aes", not(miri)),
))]
#[cfg(test)]
Expand Down
47 changes: 30 additions & 17 deletions src/operations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use crate::convert::*;
#[allow(unused)]
use zerocopy::transmute;

///This constant comes from Kunth's prng (Empirically it works better than those from splitmix32).
pub(crate) const MULTIPLE: u64 = 6364136223846793005;
Expand Down Expand Up @@ -57,7 +55,12 @@ pub(crate) fn shuffle(a: u128) -> u128 {
use core::arch::x86::*;
#[cfg(target_arch = "x86_64")]
use core::arch::x86_64::*;
unsafe { transmute!(_mm_shuffle_epi8(transmute!(a), transmute!(SHUFFLE_MASK))) }
unsafe {
checked_transmute!(_mm_shuffle_epi8(
checked_transmute!(a),
checked_transmute!(SHUFFLE_MASK)
))
}
}
#[cfg(not(all(target_feature = "ssse3", not(miri))))]
{
Expand Down Expand Up @@ -87,7 +90,7 @@ pub(crate) fn add_by_64s(a: [u64; 2], b: [u64; 2]) -> [u64; 2] {
use core::arch::x86::*;
#[cfg(target_arch = "x86_64")]
use core::arch::x86_64::*;
transmute!(_mm_add_epi64(transmute!(a), transmute!(b)))
checked_transmute!(_mm_add_epi64(checked_transmute!(a), checked_transmute!(b)))
}
}

Expand All @@ -106,13 +109,18 @@ pub(crate) fn aesenc(value: u128, xor: u128) -> u128 {
#[cfg(target_arch = "x86_64")]
use core::arch::x86_64::*;
unsafe {
let value = transmute!(value);
transmute!(_mm_aesenc_si128(value, transmute!(xor)))
let value = checked_transmute!(value);
checked_transmute!(_mm_aesenc_si128(value, checked_transmute!(xor)))
}
}

#[cfg(any(
all(feature = "nightly-arm-aes", target_arch = "aarch64", target_feature = "aes", not(miri)),
all(
feature = "nightly-arm-aes",
target_arch = "aarch64",
target_feature = "aes",
not(miri)
),
all(feature = "nightly-arm-aes", target_arch = "arm", target_feature = "aes", not(miri)),
))]
#[allow(unused)]
Expand All @@ -123,7 +131,7 @@ pub(crate) fn aesenc(value: u128, xor: u128) -> u128 {
#[cfg(target_arch = "arm")]
use core::arch::arm::*;
let res = unsafe { vaesmcq_u8(vaeseq_u8(transmute!(value), transmute!(0u128))) };
let value: u128 = transmute!(res);
let value: u128 = checked_transmute!(res);
xor ^ value
}

Expand All @@ -136,13 +144,18 @@ pub(crate) fn aesdec(value: u128, xor: u128) -> u128 {
#[cfg(target_arch = "x86_64")]
use core::arch::x86_64::*;
unsafe {
let value = transmute!(value);
transmute!(_mm_aesdec_si128(value, transmute!(xor)))
let value = checked_transmute!(value);
checked_transmute!(_mm_aesdec_si128(value, checked_transmute!(xor)))
}
}

#[cfg(any(
all(feature = "nightly-arm-aes", target_arch = "aarch64", target_feature = "aes", not(miri)),
all(
feature = "nightly-arm-aes",
target_arch = "aarch64",
target_feature = "aes",
not(miri)
),
all(feature = "nightly-arm-aes", target_arch = "arm", target_feature = "aes", not(miri)),
))]
#[allow(unused)]
Expand All @@ -152,8 +165,8 @@ pub(crate) fn aesdec(value: u128, xor: u128) -> u128 {
use core::arch::aarch64::*;
#[cfg(target_arch = "arm")]
use core::arch::arm::*;
let res = unsafe { vaesimcq_u8(vaesdq_u8(transmute!(value), transmute!(0u128))) };
let value: u128 = transmute!(res);
let res = unsafe { vaesimcq_u8(vaesdq_u8(checked_transmute!(value), checked_transmute!(0u128))) };
let value: u128 = checked_transmute!(res);
xor ^ value
}

Expand Down Expand Up @@ -196,7 +209,7 @@ mod test {
// #[cfg(target_arch = "x86_64")]
// use core::arch::x86_64::*;
// MASK.with(|mask| {
// unsafe { transmute!(_mm_shuffle_epi8(transmute!(a), transmute!(mask.get()))) }
// unsafe { safer_transmute!(_mm_shuffle_epi8(safer_transmute!(a), safer_transmute!(mask.get()))) }
// })
// }
//
Expand Down Expand Up @@ -364,10 +377,10 @@ mod test {

#[test]
fn test_add_length() {
let enc : [u64; 2] = [50, u64::MAX];
let mut enc : u128 = enc.convert();
let enc: [u64; 2] = [50, u64::MAX];
let mut enc: u128 = enc.convert();
add_in_length(&mut enc, u64::MAX);
let enc : [u64; 2] = enc.convert();
let enc: [u64; 2] = enc.convert();
assert_eq!(enc[1], u64::MAX);
assert_eq!(enc[0], 49);
}
Expand Down