Skip to content

Commit

Permalink
blake2: fix build + warnings on no_std targets
Browse files Browse the repository at this point in the history
Many of the warnings were occurring because the parallel backend
requires x86 SIMD intrinsics (AVX2 or SSE41) and therefore the parallel
code is dead on other targets.

This commit feature gates all such code, ensuring that everything will
build warning-free on other targets, and that users who attempt to use
the parallel APIs on unsupported targets get compile errors rather than
warnings.
  • Loading branch information
tarcieri committed Aug 29, 2021
1 parent f2caf71 commit 2f45194
Show file tree
Hide file tree
Showing 11 changed files with 77 additions and 56 deletions.
46 changes: 22 additions & 24 deletions .github/workflows/blake2.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,30 +17,28 @@ env:
RUSTFLAGS: "-Dwarnings"

jobs:
# TODO(tarcieri): re-enable these when failures are addressed:
# <https://github.com/RustCrypto/hashes/pull/228/checks?check_run_id=1831937705>
# build:
# runs-on: ubuntu-latest
# strategy:
# matrix:
# rust:
# - 1.41.0 # MSRV
# - stable
# target:
# - thumbv7em-none-eabi
# - wasm32-unknown-unknown
# steps:
# - uses: actions/checkout@v1
# - uses: actions-rs/toolchain@v1
# with:
# profile: minimal
# toolchain: ${{ matrix.rust }}
# target: ${{ matrix.target }}
# override: true
# - run: cargo build --target ${{ matrix.target }} --release --no-default-features
# - run: cargo build --target ${{ matrix.target }} --release --no-default-features --features blake2b
# - run: cargo build --target ${{ matrix.target }} --release --no-default-features --features blake2s
# - run: cargo build --target ${{ matrix.target }} --release --no-default-features --features blake2b,blake2s
build:
runs-on: ubuntu-latest
strategy:
matrix:
rust:
- 1.41.0 # MSRV
- stable
target:
- thumbv7em-none-eabi
- wasm32-unknown-unknown
steps:
- uses: actions/checkout@v1
- uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: ${{ matrix.rust }}
target: ${{ matrix.target }}
override: true
- run: cargo build --target ${{ matrix.target }} --release --no-default-features
- run: cargo build --target ${{ matrix.target }} --release --no-default-features --features blake2b
- run: cargo build --target ${{ matrix.target }} --release --no-default-features --features blake2s
- run: cargo build --target ${{ matrix.target }} --release --no-default-features --features blake2b,blake2s

test:
runs-on: ubuntu-latest
Expand Down
6 changes: 5 additions & 1 deletion blake2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ arrayvec = { version = "0.5", default-features = false }
crypto-mac = "0.8"
digest = "0.9"
opaque-debug = "0.3"
subtle = ">=2, <2.5"
subtle = { version = ">=2, <2.5", default-features = false }

[dev-dependencies]
crypto-mac = { version = "0.8", features = ["dev"] }
Expand All @@ -39,3 +39,7 @@ blake2s = []
# performance. This feature disables some inlining, improving the performance
# of the portable implementation in that specific case.
uninline_portable = []

[package.metadata.docs.rs]
all-features = true
rustdoc-args = ["--cfg", "docsrs"]
9 changes: 6 additions & 3 deletions blake2/src/blake2b.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,12 @@
//! ```

pub(crate) mod backend;
pub mod many;
pub(crate) mod state;

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
#[cfg_attr(docsrs, doc(cfg(any(target_arch = "x86", target_arch = "x86_64"))))]
pub mod many;

mod hash;
mod params;
#[cfg(test)]
Expand Down Expand Up @@ -280,10 +283,10 @@ pub(crate) fn paint_test_input(buf: &mut [u8]) {

// This module is pub for internal benchmarks only. Please don't use it.
#[doc(hidden)]
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub mod benchmarks {
use crate::blake2bp;

use super::*;
use crate::blake2bp;

pub fn force_portable(params: &mut Params) {
params.implementation = backend::Implementation::portable();
Expand Down
15 changes: 7 additions & 8 deletions blake2/src/blake2b/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ use core::cmp;
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub const MAX_DEGREE: usize = 4;

#[cfg(not(any(target_arch = "x86", target_arch = "x86_64")))]
pub const MAX_DEGREE: usize = 1;

/// Variants other than Portable are unreachable in no_std, unless CPU features
/// are explicitly enabled for the build with e.g. RUSTFLAGS="-C target-feature=avx2".
/// This might change in the future if is_x86_feature_detected moves into libcore.
Expand Down Expand Up @@ -91,11 +88,10 @@ impl Implementation {
None
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub fn degree(&self) -> usize {
match self.0 {
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
Platform::Avx2 => avx2::DEGREE,
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
Platform::Sse41 => sse41::DEGREE,
Platform::Portable => 1,
}
Expand Down Expand Up @@ -123,19 +119,19 @@ impl Implementation {
}
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub fn compress2_loop(&self, jobs: &mut [Job<'_, '_>; 2], finalize: Finalize, stride: Stride) {
match self.0 {
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
Platform::Avx2 | Platform::Sse41 => unsafe {
sse41::compress2_loop(jobs, finalize, stride)
},
_ => panic!("unsupported"),
}
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub fn compress4_loop(&self, jobs: &mut [Job<'_, '_>; 4], finalize: Finalize, stride: Stride) {
match self.0 {
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
Platform::Avx2 => unsafe { avx2::compress4_loop(jobs, finalize, stride) },
_ => panic!("unsupported"),
}
Expand Down Expand Up @@ -196,14 +192,16 @@ impl LastNode {

#[derive(Clone, Copy, Debug)]
pub enum Stride {
Serial, // BLAKE2b/BLAKE2s
Serial, // BLAKE2b/BLAKE2s
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
Parallel, // BLAKE2bp/BLAKE2sp
}

impl Stride {
pub fn padded_blockbytes(&self) -> usize {
match self {
Stride::Serial => BLOCKBYTES,
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
Stride::Parallel => crate::blake2bp::DEGREE * BLOCKBYTES,
}
}
Expand All @@ -217,6 +215,7 @@ pub(crate) fn count_high(count: Count) -> Word {
(count >> (8 * size_of::<Word>())) as Word
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub(crate) fn assemble_count(low: Word, high: Word) -> Count {
low as Count + ((high as Count) << (8 * size_of::<Word>()))
}
Expand Down
6 changes: 6 additions & 0 deletions blake2/src/blake2b/test.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use super::*;

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
use crate::blake2bp;

const EMPTY_HASH: &str = "786a02f742015903c6c6fd852552d272912f4740e15847618a86e217f71f5419\
Expand Down Expand Up @@ -120,6 +122,7 @@ fn test_all_parameters() {
);
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
#[test]
fn test_all_parameters_blake2bp() {
let mut params = crate::blake2bp::Params::new();
Expand Down Expand Up @@ -183,18 +186,21 @@ fn test_long_inner_hash_length_panics() {
Params::new().inner_hash_length(OUTBYTES + 1);
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
#[test]
#[should_panic]
fn test_blake2bp_short_hash_length_panics() {
blake2bp::Params::new().hash_length(0);
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
#[test]
#[should_panic]
fn test_blake2bp_long_hash_length_panics() {
blake2bp::Params::new().hash_length(OUTBYTES + 1);
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
#[test]
#[should_panic]
fn test_blake2bp_long_key_panics() {
Expand Down
3 changes: 0 additions & 3 deletions blake2/src/blake2bp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ use crate::blake2b::{
};
use core::{cmp, fmt, mem::size_of};

#[cfg(feature = "std")]
use std;

pub(crate) const DEGREE: usize = 4;

/// Compute the BLAKE2bp hash of a slice of bytes all at once, using default
Expand Down
8 changes: 6 additions & 2 deletions blake2/src/blake2s.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,19 @@
//! ```

pub(crate) mod backend;
pub mod many;
pub(crate) mod state;

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
#[cfg_attr(docsrs, doc(cfg(any(target_arch = "x86", target_arch = "x86_64"))))]
pub mod many;

mod hash;
mod params;
#[cfg(test)]
mod test;

pub use self::{hash::Hash, params::Params, state::State};

use crate::blake2sp;
use core::{fmt, mem::size_of};
use crypto_mac::{InvalidKeyLength, Mac, NewMac};
use digest::{
Expand Down Expand Up @@ -270,8 +272,10 @@ pub(crate) fn paint_test_input(buf: &mut [u8]) {

// This module is pub for internal benchmarks only. Please don't use it.
#[doc(hidden)]
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub mod benchmarks {
use super::*;
use crate::blake2sp;

pub fn force_portable(params: &mut Params) {
params.implementation = backend::Implementation::portable();
Expand Down
15 changes: 7 additions & 8 deletions blake2/src/blake2s/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ use core::cmp;
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub const MAX_DEGREE: usize = 8;

#[cfg(not(any(target_arch = "x86", target_arch = "x86_64")))]
pub const MAX_DEGREE: usize = 1;

/// Variants other than Portable are unreachable in no_std, unless CPU features
/// are explicitly enabled for the build with e.g. RUSTFLAGS="-C target-feature=avx2".
/// This might change in the future if is_x86_feature_detected moves into libcore.
Expand Down Expand Up @@ -91,11 +88,10 @@ impl Implementation {
None
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub fn degree(&self) -> usize {
match self.0 {
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
Platform::Avx2 => avx2::DEGREE,
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
Platform::Sse41 => sse41::DEGREE,
Platform::Portable => 1,
}
Expand All @@ -121,19 +117,19 @@ impl Implementation {
}
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub fn compress4_loop(&self, jobs: &mut [Job<'_, '_>; 4], finalize: Finalize, stride: Stride) {
match self.0 {
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
Platform::Avx2 | Platform::Sse41 => unsafe {
sse41::compress4_loop(jobs, finalize, stride)
},
_ => panic!("unsupported"),
}
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub fn compress8_loop(&self, jobs: &mut [Job<'_, '_>; 8], finalize: Finalize, stride: Stride) {
match self.0 {
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
Platform::Avx2 => unsafe { avx2::compress8_loop(jobs, finalize, stride) },
_ => panic!("unsupported"),
}
Expand Down Expand Up @@ -194,14 +190,16 @@ impl LastNode {

#[derive(Clone, Copy, Debug)]
pub enum Stride {
Serial, // BLAKE2b/BLAKE2s
Serial, // BLAKE2b/BLAKE2s
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
Parallel, // BLAKE2bp/BLAKE2sp
}

impl Stride {
pub fn padded_blockbytes(&self) -> usize {
match self {
Stride::Serial => BLOCKBYTES,
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
Stride::Parallel => crate::blake2sp::DEGREE * BLOCKBYTES,
}
}
Expand All @@ -215,6 +213,7 @@ pub(crate) fn count_high(count: Count) -> Word {
(count >> (8 * size_of::<Word>())) as Word
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub(crate) fn assemble_count(low: Word, high: Word) -> Count {
low as Count + ((high as Count) << (8 * size_of::<Word>()))
}
Expand Down
9 changes: 9 additions & 0 deletions blake2/src/blake2s/test.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use super::*;

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
use crate::blake2sp;

const EMPTY_HASH: &str = "69217a3079908094e11121d042354a7c1f55b6482ca1a51e1b250dfd1ed0eef9";
const ABC_HASH: &str = "508c5e8c327c14e2e1a72ba34eeb452f37458b209ed63a294d999b4c86675982";
const ONE_BLOCK_HASH: &str = "ae09db7cd54f42b490ef09b6bc541af688e4959bb8c53f359a6f56e38ab454a3";
Expand Down Expand Up @@ -115,6 +118,7 @@ fn test_all_parameters() {
);
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
#[test]
fn test_all_parameters_blake2sp() {
let mut params = blake2sp::Params::new();
Expand Down Expand Up @@ -178,29 +182,34 @@ fn test_long_inner_hash_length_panics() {
Params::new().inner_hash_length(OUTBYTES + 1);
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
#[test]
#[should_panic]
fn test_blake2sp_short_hash_length_panics() {
blake2sp::Params::new().hash_length(0);
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
#[test]
#[should_panic]
fn test_blake2sp_long_hash_length_panics() {
blake2sp::Params::new().hash_length(OUTBYTES + 1);
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
#[test]
#[should_panic]
fn test_blake2sp_long_key_panics() {
blake2sp::Params::new().key(&[0; KEYBYTES + 1]);
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
#[test]
fn test_blake2sp_max_offset_ok() {
Params::new().node_offset((1 << 48) - 1);
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
#[test]
#[should_panic]
fn test_blake2sp_offset_too_large_panics() {
Expand Down
3 changes: 0 additions & 3 deletions blake2/src/blake2sp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ use crate::blake2s::{
};
use core::{cmp, fmt, mem::size_of};

#[cfg(feature = "std")]
use std;

pub(crate) const DEGREE: usize = 8;

/// Compute the BLAKE2sp hash of a slice of bytes all at once, using default
Expand Down

0 comments on commit 2f45194

Please sign in to comment.