diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index f56619ae4..281108181 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -220,7 +220,8 @@ jobs: df -h / sudo rm -rf --one-file-system /usr/local/lib/android df -h / - # `simd` needs nightly, so the matrix runs on the pinned toolchain. + # Pinned toolchain: .cargo/config.toml carries nightly-only rustflags + # (-Zshare-generics) for this target, and this job does not clear them. - uses: dtolnay/rust-toolchain@master with: toolchain: nightly-2026-06-16 @@ -251,7 +252,7 @@ jobs: -p wacore-libsignal -p wacore-noise -p waproto test-stable: - name: Test Stable (no-simd) + name: Test Stable (MSRV) runs-on: ubuntu-latest # .cargo/config.toml sets nightly-only rustflags (-Zshare-generics) for the # x86_64-linux target; a set-but-empty RUSTFLAGS takes precedence over @@ -279,18 +280,16 @@ jobs: uses: Swatinem/rust-cache@v2 with: cache-targets: "true" - - name: Build wacore-binary (stable, no SIMD) - run: cargo build -p wacore-binary --no-default-features --verbose - - name: Test wacore-binary (stable, no SIMD) - run: cargo nextest run --profile ci -p wacore-binary --no-default-features --lib - - name: Build wacore-appstate (stable, no SIMD) - run: cargo build -p wacore-appstate --no-default-features --verbose - - name: Test wacore-appstate (stable, no SIMD) - run: cargo nextest run --profile ci -p wacore-appstate --no-default-features --lib - - name: Build wacore (stable, no SIMD) - run: cargo build -p wacore --no-default-features --verbose - - name: Test wacore (stable, no SIMD) - run: cargo nextest run --profile ci -p wacore --no-default-features --lib + # These three used to be reachable here only with `--no-default-features`, + # because their default `simd` feature pulled in `portable_simd` and so + # needed nightly. There is no SIMD in the tree any more and no feature + # gating it, so this now runs what ships. + - name: Test wacore-binary (stable) + run: cargo nextest run --profile ci -p wacore-binary --lib + - name: Test wacore-appstate (stable) + run: cargo nextest run --profile ci -p wacore-appstate --lib + - name: Test wacore (stable) + run: cargo nextest run --profile ci -p wacore --lib # `rust-version` is published metadata for every member, but only the # three crates above are exercised at that toolchain. This compiles the # rest of the publishable set so the declared floor is a checked promise diff --git a/.github/workflows/miri.yml b/.github/workflows/miri.yml index 7b9c4d23b..4af579c1d 100644 --- a/.github/workflows/miri.yml +++ b/.github/workflows/miri.yml @@ -40,15 +40,14 @@ jobs: # inflate's uninitialized spare capacity in `zlib_pool`. Both are # invisible to clippy and to native tests — nothing observes the # aliasing violation or the uninit read until it miscompiles. + # + # One leg, not two: the crate used to carry portable-SIMD scanners in + # the decoder/encoder alongside scalar fallbacks, so `--no-default- + # features` reached a genuinely different code path. The packed codec + # is table-driven scalar now and there is only one path to check. - name: wacore-binary - cache-key: binary-simd + cache-key: binary args: -p wacore-binary --lib - # The portable-SIMD scanners in the decoder/encoder and their scalar - # fallbacks are separate code paths, and `--no-default-features` is the - # only way to reach the latter. - - name: wacore-binary (no simd) - cache-key: binary-scalar - args: -p wacore-binary --no-default-features --lib # No `unsafe` of its own, but it drives wacore-binary's zero-copy # decode over real Noise frames and pulls the crypto stack # (aes/sha2/curve25519), whose unsafe backends this exercises. diff --git a/Cargo.toml b/Cargo.toml index 92cb2e75e..114a7895c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,7 +48,7 @@ default-members = [ ] # Minimum supported Rust version. Floor is the stable release CI's -# "Test Stable (no-simd)" job pins; raise both together (that job's +# "Test Stable (MSRV)" job pins; raise both together (that job's # `toolchain:` and this value) when a dependency or language feature needs it. [workspace.package] rust-version = "1.94" @@ -161,7 +161,6 @@ tracing-pii = ["wacore/tracing-pii", "wacore-binary/tracing-pii"] danger-skip-tls-verify = ["whatsapp-rust-tokio-transport?/danger-skip-tls-verify"] danger-skip-cert-chain-verify = ["wacore/danger-skip-cert-chain-verify"] default = [ - "simd", "sqlite-storage", "tokio-transport", "tokio-runtime", @@ -169,7 +168,6 @@ default = [ "tokio-native", "signal", ] -simd = ["wacore/simd"] ureq-client = ["dep:whatsapp-rust-ureq-http-client"] tokio-transport = ["dep:whatsapp-rust-tokio-transport"] tokio-runtime = ["dep:tokio"] diff --git a/tests/bench-integration/Cargo.toml b/tests/bench-integration/Cargo.toml index c393f6024..b8c239a44 100644 --- a/tests/bench-integration/Cargo.toml +++ b/tests/bench-integration/Cargo.toml @@ -12,7 +12,6 @@ env_logger = { workspace = true } tokio = { workspace = true, features = ["macros", "rt-multi-thread", "sync", "time"] } whatsapp-rust = { path = "../..", default-features = false, features = [ "danger-skip-tls-verify", - "simd", "tokio-runtime", "tokio-native", "signal", diff --git a/tests/e2e/Cargo.toml b/tests/e2e/Cargo.toml index 523597e00..90155200b 100644 --- a/tests/e2e/Cargo.toml +++ b/tests/e2e/Cargo.toml @@ -23,7 +23,6 @@ wacore-binary = { path = "../../wacore/binary" } whatsapp-rust = { path = "../..", default-features = false, features = [ "danger-skip-cert-chain-verify", "danger-skip-tls-verify", - "simd", "tokio-runtime", "tokio-native", "signal", diff --git a/wacore/Cargo.toml b/wacore/Cargo.toml index 375088036..8029c99e4 100644 --- a/wacore/Cargo.toml +++ b/wacore/Cargo.toml @@ -17,8 +17,6 @@ autobenches = false ignored = ["getrandom"] [features] -default = ["simd"] -simd = ["wacore-appstate/simd"] debug-snapshots = [] # Typed interop with the decoded libsignal SessionRecord v1 model. legacy-session-interop = ["wacore-libsignal/legacy-session-interop"] diff --git a/wacore/appstate/Cargo.toml b/wacore/appstate/Cargo.toml index fccbb0b5b..4617f280a 100644 --- a/wacore/appstate/Cargo.toml +++ b/wacore/appstate/Cargo.toml @@ -11,10 +11,6 @@ description = "Appstate for WhatsApp protocol" [lib] crate-type = ["rlib"] -[features] -default = ["simd"] -simd = ["wacore-binary/simd"] - [dependencies] anyhow = { workspace = true } buffa = { workspace = true } diff --git a/wacore/appstate/src/lib.rs b/wacore/appstate/src/lib.rs index 8bbef2762..1c73d4ace 100644 --- a/wacore/appstate/src/lib.rs +++ b/wacore/appstate/src/lib.rs @@ -1,4 +1,3 @@ -#![cfg_attr(feature = "simd", feature(portable_simd))] pub mod decode; pub mod encode; pub mod errors; diff --git a/wacore/appstate/src/lthash.rs b/wacore/appstate/src/lthash.rs index bfe4ac14b..06ac77b40 100644 --- a/wacore/appstate/src/lthash.rs +++ b/wacore/appstate/src/lthash.rs @@ -1,5 +1,3 @@ -#[cfg(feature = "simd")] -use core::simd::u16x8; use hkdf::Hkdf; use hmac::digest::KeyInit; use hmac::{Hmac, Mac}; @@ -57,59 +55,19 @@ impl LTHash { } } +/// Deliberately scalar. A hand-vectorised version of this loop lived here +/// until it was measured: over an 812-MAC batch it moved the total by 0.28%, +/// because HKDF above it dominates and LLVM already auto-vectorizes this loop +/// about as well as the intrinsics did. fn perform_pointwise_with_overflow(base: &mut [u8], input: &[u8], subtract: bool) { assert_eq!(base.len(), input.len(), "length mismatch"); - // Use `% 2` instead of `.is_multiple_of(2)` for stable Rust compatibility. - #[allow(clippy::manual_is_multiple_of)] - { - assert!(base.len() % 2 == 0, "slice lengths must be even"); - } - - #[allow(unused_mut, unused_assignments)] - let (mut base_remaining, mut input_remaining): (&mut [u8], &[u8]) = (base, input); + assert!(base.len().is_multiple_of(2), "slice lengths must be even"); // WA Web treats the accumulator as little-endian u16 lanes // (`new DataView(...).getUint16(off, true)` in WA/Crypto/LtHash.js). // Snapshot/patch MACs are HMACs over the accumulator bytes, so the lane // endianness is part of the wire spec. - #[cfg(feature = "simd")] - { - let (base_chunks, base_rem) = base_remaining.as_chunks_mut::<16>(); - let (input_chunks, input_rem) = input_remaining.as_chunks::<16>(); - - for (base_chunk, input_chunk) in base_chunks.iter_mut().zip(input_chunks) { - // `from_le_bytes` per lane states the wire endianness directly, so - // the same code is correct on either host; on little-endian it - // lowers to the plain 16-byte load a transmute would have emitted. - let base_arr: [u16; 8] = core::array::from_fn(|i| { - u16::from_le_bytes([base_chunk[2 * i], base_chunk[2 * i + 1]]) - }); - let input_arr: [u16; 8] = core::array::from_fn(|i| { - u16::from_le_bytes([input_chunk[2 * i], input_chunk[2 * i + 1]]) - }); - let base_simd = u16x8::from_array(base_arr); - let input_simd = u16x8::from_array(input_arr); - - let result_simd = if subtract { - base_simd - input_simd - } else { - base_simd + input_simd - }; - - let out = result_simd.to_array(); - for (base_pair, lane) in base_chunk.as_chunks_mut::<2>().0.iter_mut().zip(out) { - *base_pair = lane.to_le_bytes(); - } - } - - base_remaining = base_rem; - input_remaining = input_rem; - } - - for (base_pair, input_pair) in base_remaining - .chunks_exact_mut(2) - .zip(input_remaining.chunks_exact(2)) - { + for (base_pair, input_pair) in base.chunks_exact_mut(2).zip(input.chunks_exact(2)) { let x = u16::from_le_bytes([base_pair[0], base_pair[1]]); let y = u16::from_le_bytes([input_pair[0], input_pair[1]]); @@ -118,9 +76,7 @@ fn perform_pointwise_with_overflow(base: &mut [u8], input: &[u8], subtract: bool } else { x.wrapping_add(y) }; - let bytes = result.to_le_bytes(); - base_pair[0] = bytes[0]; - base_pair[1] = bytes[1]; + base_pair.copy_from_slice(&result.to_le_bytes()); } } @@ -170,27 +126,73 @@ mod tests { } #[test] - fn test_simd_determinism_and_consistency() { + fn add_then_subtract_returns_to_zero_across_sizes() { let test_sizes = [2, 4, 8, 16, 18, 32, 64, 128, 256]; for &size in &test_sizes { - let mut base_simd = vec![0u8; size]; - let mut base_scalar = vec![0u8; size]; + let mut base = vec![0u8; size]; let input = vec![1u8; size]; - perform_pointwise_with_overflow(&mut base_simd, &input, false); - perform_pointwise_with_overflow(&mut base_scalar, &input, false); - assert_eq!(base_simd, base_scalar, "Add failed for size {}", size); - - perform_pointwise_with_overflow(&mut base_simd, &input, true); - perform_pointwise_with_overflow(&mut base_scalar, &input, true); - assert_eq!(base_simd, base_scalar, "Subtract failed for size {}", size); - assert_eq!( - base_simd, - vec![0u8; size], - "Subtract result incorrect for size {}", - size - ); + perform_pointwise_with_overflow(&mut base, &input, false); + perform_pointwise_with_overflow(&mut base, &input, true); + assert_eq!(base, vec![0u8; size], "size {size}"); + } + } + + /// Reference for the test below. It reaches the same answer by a + /// different route than the implementation: lanes are assembled by hand + /// from byte positions and the arithmetic is done in `u32` and masked, so + /// it shares neither `from_le_bytes` nor `wrapping_*` with the code under + /// test. A reference that mirrors the implementation proves nothing. + fn reference_pointwise(base: &mut [u8], input: &[u8], subtract: bool) { + for i in (0..base.len()).step_by(2) { + let x = base[i] as u32 | ((base[i + 1] as u32) << 8); + let y = input[i] as u32 | ((input[i + 1] as u32) << 8); + let r = if subtract { + x.wrapping_sub(y) & 0xFFFF + } else { + (x + y) & 0xFFFF + }; + base[i] = (r & 0xFF) as u8; + base[i + 1] = (r >> 8) as u8; + } + } + + /// Sizes straddle the 16-byte boundary a vectorised implementation would + /// chunk on, so the coverage still holds if one ever comes back. Inputs + /// are seeded onto the wrap boundaries in both directions, which is where + /// a lane-width or endianness mistake shows up rather than in round data. + #[test] + fn pointwise_matches_independent_reference() { + let sizes = [0usize, 2, 14, 16, 18, 32, 34, 128, 130, 256]; + // Deterministic LCG: reproducible failures, no dev-dependency. + let mut seed = 0x2545_F491u32; + let mut next = move || { + seed = seed.wrapping_mul(1_664_525).wrapping_add(1_013_904_223); + (seed >> 24) as u8 + }; + + for size in sizes { + for subtract in [false, true] { + for edge in [0u8, 0xFF, 0x01] { + let base: Vec = (0..size) + .map(|i| if i % 3 == 0 { edge } else { next() }) + .collect(); + let input: Vec = (0..size) + .map(|i| if i % 5 == 0 { edge } else { next() }) + .collect(); + + let mut actual = base.clone(); + let mut expected = base.clone(); + perform_pointwise_with_overflow(&mut actual, &input, subtract); + reference_pointwise(&mut expected, &input, subtract); + + assert_eq!( + actual, expected, + "size {size}, subtract {subtract}, edge {edge:#04x}" + ); + } + } } } diff --git a/wacore/binary/Cargo.toml b/wacore/binary/Cargo.toml index 0531adc77..716daa462 100644 --- a/wacore/binary/Cargo.toml +++ b/wacore/binary/Cargo.toml @@ -12,8 +12,6 @@ description = "Binary data and constants for WhatsApp protocol" crate-type = ["rlib"] [features] -default = ["simd"] -simd = [] serde = ["dep:serde", "compact_str/serde", "smallvec/serde"] # Render raw phone numbers in `Jid::observe()` instead of the redacted `pn#`. # Local debugging only; never enable in production. diff --git a/wacore/binary/src/decoder.rs b/wacore/binary/src/decoder.rs index 24c273fac..0fea535f7 100644 --- a/wacore/binary/src/decoder.rs +++ b/wacore/binary/src/decoder.rs @@ -4,8 +4,6 @@ use crate::node::{AttrsRef, NodeContentRef, NodeRef, NodeStr, ValueRef}; use crate::token; use compact_str::CompactString; use std::borrow::Cow; -#[cfg(feature = "simd")] -use std::simd::{Simd, prelude::*, u8x16}; /// Format a JidRef directly into CompactString using direct push operations, /// bypassing `fmt::Display` and `dyn Write` dispatch entirely. @@ -374,30 +372,16 @@ impl<'a> Decoder<'a> { Ok(CompactString::from(s)) } + // Deliberately scalar. A vectorised version of this loop lived here until + // it was measured against the table: `HEX_PAIRS[byte]` is one 2-byte load + // per input byte, and a shuffle/interleave/store sequence does not beat + // that. Under callgrind the SIMD path cost 4.5% more instructions on a + // 20-character id and 9.7% more on a 32-character one, and enabling real + // `pshufb` (`-Ctarget-cpu=x86-64-v2`) only narrowed the loss to 7.0%. + // Packed payloads are ids and phone numbers, so the loop also needed a + // 31-character string before it engaged at all. #[inline] fn decode_packed_hex(packed_data: &[u8], out: &mut [u8], pos: &mut usize) { - #[cfg(feature = "simd")] - let packed_data = { - const HEX_LOOKUP: [u8; 16] = *b"0123456789ABCDEF"; - let lookup_table = Simd::from_array(HEX_LOOKUP); - let low_mask = Simd::splat(0x0F); - - let (chunks, remainder) = packed_data.as_chunks::<16>(); - for chunk in chunks { - let data = u8x16::from_array(*chunk); - let high_nibbles = (data >> 4) & low_mask; - let low_nibbles = data & low_mask; - let high_chars = lookup_table.swizzle_dyn(high_nibbles); - let low_chars = lookup_table.swizzle_dyn(low_nibbles); - let (lo, hi) = Simd::interleave(high_chars, low_chars); - out[*pos..*pos + 16].copy_from_slice(lo.as_array()); - *pos += 16; - out[*pos..*pos + 16].copy_from_slice(hi.as_array()); - *pos += 16; - } - remainder - }; - let written = packed_data.len() * 2; for (slot, &byte) in out[*pos..*pos + written] .chunks_exact_mut(2) @@ -408,54 +392,12 @@ impl<'a> Decoder<'a> { *pos += written; } + // Scalar for the same reason as `decode_packed_hex`, and more so: the + // vector version had to validate every lane against the two legal + // out-of-range nibbles before it could shuffle, then fall back to this + // loop anyway whenever a lane failed. #[inline] fn decode_packed_nibble(packed_data: &[u8], out: &mut [u8], pos: &mut usize) -> Result<()> { - #[cfg(feature = "simd")] - let packed_data = { - const NIBBLE_LOOKUP: [u8; 16] = *b"0123456789-.\x00\x00\x00\x00"; - let lookup_table = Simd::from_array(NIBBLE_LOOKUP); - let low_mask = Simd::splat(0x0F); - let le11 = Simd::splat(11); - let f15 = Simd::splat(15); - - let (chunks, remainder) = packed_data.as_chunks::<16>(); - for chunk in chunks { - let data = u8x16::from_array(*chunk); - - let high_nibbles = (data >> 4) & low_mask; - let low_nibbles = data & low_mask; - - let hi_valid = high_nibbles.simd_le(le11) | high_nibbles.simd_eq(f15); - let lo_valid = low_nibbles.simd_le(le11) | low_nibbles.simd_eq(f15); - if !(hi_valid & lo_valid).all() { - for byte in *chunk { - let high = (byte & 0xF0) >> 4; - let low = byte & 0x0F; - Self::unpack_nibble(high)?; - Self::unpack_nibble(low)?; - } - for byte in *chunk { - let high = (byte & 0xF0) >> 4; - let low = byte & 0x0F; - out[*pos] = Self::unpack_nibble(high)?; - *pos += 1; - out[*pos] = Self::unpack_nibble(low)?; - *pos += 1; - } - continue; - } - - let high_chars = lookup_table.swizzle_dyn(high_nibbles); - let low_chars = lookup_table.swizzle_dyn(low_nibbles); - let (lo, hi) = Simd::interleave(high_chars, low_chars); - out[*pos..*pos + 16].copy_from_slice(lo.as_array()); - *pos += 16; - out[*pos..*pos + 16].copy_from_slice(hi.as_array()); - *pos += 16; - } - remainder - }; - let written = packed_data.len() * 2; for (slot, &byte) in out[*pos..*pos + written] .chunks_exact_mut(2) diff --git a/wacore/binary/src/encoder.rs b/wacore/binary/src/encoder.rs index a8d78b6e2..39d78c59d 100644 --- a/wacore/binary/src/encoder.rs +++ b/wacore/binary/src/encoder.rs @@ -1,17 +1,47 @@ use std::io::Write; -#[cfg(feature = "simd")] -use core::simd::Select; -#[cfg(feature = "simd")] -use core::simd::prelude::*; -#[cfg(feature = "simd")] -use core::simd::{Simd, u8x16}; - use crate::error::{BinaryError, Result}; use crate::jid::{self, Jid, JidRef}; use crate::node::{Node, NodeContent, NodeContentRef, NodeRef, NodeValue, ValueRef}; use crate::token; +/// Marks a byte no packed encoding accepts. `validate_hex`/`validate_nibble` +/// gate every caller, so a hit means the caller skipped that check. +const PACK_INVALID: u8 = 0xFF; + +/// ASCII to nibble, the inverse of the decoder's `HEX_PAIRS`. Index 0 maps to +/// 15 because that is the pad an odd-length string writes as its second half. +static HEX_ENC: [u8; 256] = { + let mut table = [PACK_INVALID; 256]; + let mut c = b'0'; + while c <= b'9' { + table[c as usize] = c - b'0'; + c += 1; + } + let mut c = b'A'; + while c <= b'F' { + table[c as usize] = 10 + (c - b'A'); + c += 1; + } + table[0] = 15; + table +}; + +/// ASCII to nibble for `NIBBLE_8`: digits plus the two punctuation characters +/// a phone number can carry. +static NIBBLE_ENC: [u8; 256] = { + let mut table = [PACK_INVALID; 256]; + let mut c = b'0'; + while c <= b'9' { + table[c as usize] = c - b'0'; + c += 1; + } + table[b'-' as usize] = 10; + table[b'.' as usize] = 11; + table[0] = 15; + table +}; + pub trait ByteWriter { fn write_u8(&mut self, value: u8) -> Result<()>; fn write_bytes(&mut self, bytes: &[u8]) -> Result<()>; @@ -867,32 +897,6 @@ impl<'a, W: ByteWriter> Encoder<'a, W> { Ok(()) } - #[inline(always)] - fn pack_nibble(value: u8) -> u8 { - match value { - b'-' => 10, - b'.' => 11, - 0 => 15, - c if c.is_ascii_digit() => c - b'0', - _ => panic!("Invalid char for nibble packing: {value}"), - } - } - - #[inline(always)] - fn pack_hex(value: u8) -> u8 { - match value { - c if c.is_ascii_digit() => c - b'0', - c if (b'A'..=b'F').contains(&c) => 10 + (c - b'A'), - 0 => 15, - _ => panic!("Invalid char for hex packing: {value}"), - } - } - - #[inline(always)] - fn pack_byte_pair(packer: fn(u8) -> u8, part1: u8, part2: u8) -> u8 { - (packer(part1) << 4) | packer(part2) - } - fn write_packed_bytes(&mut self, value: &str, data_type: u8) -> Result<()> { if value.len() > token::PACKED_MAX as usize { panic!("String too long to be packed: {}", value.len()); @@ -906,67 +910,49 @@ impl<'a, W: ByteWriter> Encoder<'a, W> { } self.write_u8(rounded_len)?; - #[allow(unused_mut)] - let mut input_bytes = value.as_bytes(); - - if data_type == token::NIBBLE_8 { - #[cfg(feature = "simd")] - { - const NIBBLE_LOOKUP: [u8; 16] = - [10, 11, 255, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 255, 255, 255]; - let lookup = Simd::from_array(NIBBLE_LOOKUP); - let nibble_base = Simd::splat(b'-'); - - while input_bytes.len() >= 16 { - let (chunk, rest) = input_bytes.split_at(16); - let input = u8x16::from_slice(chunk); - let indices = input.saturating_sub(nibble_base); - let nibbles = lookup.swizzle_dyn(indices); - - let (evens, odds) = nibbles.deinterleave(nibbles.rotate_elements_left::<1>()); - let packed: Simd = (evens << Simd::splat(4)) | odds; - let packed_bytes = packed.to_array(); - self.write_raw_bytes(&packed_bytes[..8])?; - - input_bytes = rest; - } - } + let input_bytes = value.as_bytes(); + let table = if data_type == token::NIBBLE_8 { + &NIBBLE_ENC + } else { + &HEX_ENC + }; - let mut bytes_iter = input_bytes.iter().copied(); - while let Some(part1) = bytes_iter.next() { - let part2 = bytes_iter.next().unwrap_or(0); - self.write_u8(Self::pack_byte_pair(Self::pack_nibble, part1, part2))?; - } + // Whole pairs first, so the common even-length case carries no + // per-iteration "is there a second half" branch. `PACKED_MAX` is 127, + // so the buffer covers any string that reaches here. + let mut packed = [0u8; 64]; + let (pairs, tail) = input_bytes.as_chunks::<2>(); + + // The validity test is an OR accumulator checked once below, not a + // branch per pair. Legal table entries are 0..=15 and `PACK_INVALID` + // is 0xFF, so a set high nibble in `seen` means some character was + // rejected. Keeping the branch out is what lets LLVM unroll this. + let mut seen = 0u8; + for (slot, pair) in packed.iter_mut().zip(pairs) { + let hi = table[pair[0] as usize]; + let lo = table[pair[1] as usize]; + seen |= hi | lo; + *slot = (hi << 4) | lo; + } + + // Odd length: the low nibble is the 0 pad, which both tables map to 15. + let odd = if let [last] = tail { + let hi = table[*last as usize]; + let lo = table[0]; + seen |= hi | lo; + Some((hi << 4) | lo) } else { - #[cfg(feature = "simd")] - { - let ascii_0 = Simd::splat(b'0'); - let ascii_a = Simd::splat(b'A'); - let ten = Simd::splat(10); - - while input_bytes.len() >= 16 { - let (chunk, rest) = input_bytes.split_at(16); - let input = u8x16::from_slice(chunk); - - let digit_vals = input - ascii_0; - let letter_vals = input - ascii_a + ten; - let is_letter = input.simd_ge(ascii_a); - let nibbles = is_letter.select(letter_vals, digit_vals); - - let (evens, odds) = nibbles.deinterleave(nibbles.rotate_elements_left::<1>()); - let packed: Simd = (evens << Simd::splat(4)) | odds; - let packed_bytes = packed.to_array(); - self.write_raw_bytes(&packed_bytes[..8])?; - - input_bytes = rest; - } - } + None + }; - let mut bytes_iter = input_bytes.iter().copied(); - while let Some(part1) = bytes_iter.next() { - let part2 = bytes_iter.next().unwrap_or(0); - self.write_u8(Self::pack_byte_pair(Self::pack_hex, part1, part2))?; - } + // Checked before anything reaches the writer. `validate_hex` and + // `validate_nibble` gate every caller, so this is the same unreachable + // case the `match` ladders this replaced used to panic on. + assert!(seen & 0xF0 == 0, "invalid char for packing"); + + self.write_raw_bytes(&packed[..pairs.len()])?; + if let Some(byte) = odd { + self.write_u8(byte)?; } Ok(()) } @@ -1008,6 +994,48 @@ mod tests { type TestResult = Result<()>; + /// The `match` ladders `HEX_ENC`/`NIBBLE_ENC` replaced, kept here as the + /// specification they are checked against. `None` is the case the old code + /// panicked on and the tables mark with `PACK_INVALID`. + fn reference_hex(value: u8) -> Option { + match value { + c if c.is_ascii_digit() => Some(c - b'0'), + c if (b'A'..=b'F').contains(&c) => Some(10 + (c - b'A')), + 0 => Some(15), + _ => None, + } + } + + fn reference_nibble(value: u8) -> Option { + match value { + b'-' => Some(10), + b'.' => Some(11), + 0 => Some(15), + c if c.is_ascii_digit() => Some(c - b'0'), + _ => None, + } + } + + /// Exhaustive over the byte domain, so the tables cannot drift from the + /// ladders they were derived from: same accepted set, same nibble for + /// every accepted byte, same rejected set. + #[test] + fn encode_tables_match_the_ladders_they_replaced() { + for byte in 0u8..=255 { + let i = byte as usize; + assert_eq!( + reference_hex(byte), + (HEX_ENC[i] != PACK_INVALID).then_some(HEX_ENC[i]), + "hex table disagrees at {byte:#04x}" + ); + assert_eq!( + reference_nibble(byte), + (NIBBLE_ENC[i] != PACK_INVALID).then_some(NIBBLE_ENC[i]), + "nibble table disagrees at {byte:#04x}" + ); + } + } + #[test] fn test_encode_node() -> TestResult { let node = Node::new( diff --git a/wacore/binary/src/lib.rs b/wacore/binary/src/lib.rs index 1f6528743..ec0ee3ae5 100644 --- a/wacore/binary/src/lib.rs +++ b/wacore/binary/src/lib.rs @@ -1,5 +1,3 @@ -#![cfg_attr(feature = "simd", feature(portable_simd))] - pub mod attrs; pub mod builder; pub mod consts;