diff --git a/zstd/src/decoding/block_decoder.rs b/zstd/src/decoding/block_decoder.rs index 4e9bae5dc..9684f98b4 100644 --- a/zstd/src/decoding/block_decoder.rs +++ b/zstd/src/decoding/block_decoder.rs @@ -237,7 +237,6 @@ impl BlockDecoder { parts.buffer, parts.offset_hist, parts.literals_buffer, - parts.sequences, raw, ) } @@ -276,7 +275,6 @@ impl BlockDecoder { parts.buffer, parts.offset_hist, parts.literals_buffer, - parts.sequences, raw, ) } @@ -298,7 +296,6 @@ impl BlockDecoder { buffer: &mut crate::decoding::decode_buffer::DecodeBuffer, offset_hist: &mut [u32; 3], literals_buffer: &mut alloc::vec::Vec, - sequences: &mut alloc::vec::Vec, raw: &[u8], ) -> Result<(), DecompressBlockError> { let mut section = LiteralsSection::new(); @@ -374,9 +371,10 @@ impl BlockDecoder { if seq_section.num_sequences != 0 { // Fused decode + execute: avoids the Vec round-trip // and inlines the per-iter execute_one_sequence work next to - // the FSE state advance. Falls back to the legacy two-pass - // pipeline internally when any of LL/ML/OF is in RLE mode. - // Pass field-level borrows from the WorkspaceRef so `raw` + // the FSE state advance. RLE-mode axes are handled in the same + // fused loop via a degenerate single-state FSE table built by + // `maybe_update_fse_tables`, so there is no separate two-pass + // path. Pass field-level borrows from the WorkspaceRef so `raw` // (immutable view into block_content_buffer) can coexist // with the mutable borrows on the FSE / decode-buffer / // offset-hist fields. @@ -387,7 +385,6 @@ impl BlockDecoder { buffer, offset_hist, literals_view, - sequences, )?; } else { if !raw.is_empty() { @@ -398,7 +395,6 @@ impl BlockDecoder { )); } buffer.push(literals_view); - sequences.clear(); } Ok(()) @@ -638,14 +634,12 @@ mod tests { let mut fse = FSEScratch::new(); let mut offset_hist = [1u32, 4, 8]; let mut literals_buffer = alloc::vec::Vec::new(); - let mut sequences = alloc::vec::Vec::new(); let mut block_content_buffer = alloc::vec::Vec::new(); let mut direct = DirectScratch { huf: &mut huf, fse: &mut fse, offset_hist: &mut offset_hist, literals_buffer: &mut literals_buffer, - sequences: &mut sequences, block_content_buffer: &mut block_content_buffer, buffer, }; @@ -686,14 +680,12 @@ mod tests { let mut fse = FSEScratch::new(); let mut offset_hist = [1u32, 4, 8]; let mut literals_buffer = alloc::vec::Vec::new(); - let mut sequences = alloc::vec::Vec::new(); let mut block_content_buffer = alloc::vec::Vec::new(); let mut direct = DirectScratch { huf: &mut huf, fse: &mut fse, offset_hist: &mut offset_hist, literals_buffer: &mut literals_buffer, - sequences: &mut sequences, block_content_buffer: &mut block_content_buffer, buffer, }; diff --git a/zstd/src/decoding/errors.rs b/zstd/src/decoding/errors.rs index 01351ae47..d9fcc1819 100644 --- a/zstd/src/decoding/errors.rs +++ b/zstd/src/decoding/errors.rs @@ -951,15 +951,28 @@ pub enum DecodeSequenceError { GetBitsError(GetBitsError), FSEDecoderError(FSEDecoderError), FSETableError(FSETableError), - ExtraPadding { skipped_bits: i32 }, - UnsupportedOffset { offset_code: u8 }, + ExtraPadding { + skipped_bits: i32, + }, + UnsupportedOffset { + offset_code: u8, + }, ZeroOffset, NotEnoughBytesForNumSequences, - ExtraBits { bits_remaining: isize }, + ExtraBits { + bits_remaining: isize, + }, MissingCompressionMode, MissingByteForRleLlTable, MissingByteForRleOfTable, MissingByteForRleMlTable, + /// An RLE-mode sequence table's single symbol code is out of range + /// for its axis (LL/ML/OF). `axis` names the axis; `code` is the + /// offending value read from the stream. + InvalidRleCode { + axis: &'static str, + code: u8, + }, } #[cfg(feature = "std")] @@ -1014,6 +1027,9 @@ impl core::fmt::Display for DecodeSequenceError { DecodeSequenceError::MissingByteForRleMlTable => { write!(f, "Need a byte to read for RLE ml table") } + DecodeSequenceError::InvalidRleCode { axis, code } => { + write!(f, "RLE {axis} table code {code} is out of range") + } } } } diff --git a/zstd/src/decoding/frame_decoder.rs b/zstd/src/decoding/frame_decoder.rs index 446bc3a14..93dc557f8 100644 --- a/zstd/src/decoding/frame_decoder.rs +++ b/zstd/src/decoding/frame_decoder.rs @@ -1627,14 +1627,13 @@ impl FrameDecoder { // fields; only `buffer` differs and we don't use that here. // Macro-style binding avoids the closure / generic // gymnastics of returning multiple `&mut` from a match arm. - let (huf, fse, offset_hist, literals_buffer, sequences, block_content_buffer, window_size) = + let (huf, fse, offset_hist, literals_buffer, block_content_buffer, window_size) = match &mut state.decoder_scratch { DecoderScratchKind::Flat(s) => ( &mut s.huf, &mut s.fse, &mut s.offset_hist, &mut s.literals_buffer, - &mut s.sequences, &mut s.block_content_buffer, s.buffer.window_size, ), @@ -1643,7 +1642,6 @@ impl FrameDecoder { &mut s.fse, &mut s.offset_hist, &mut s.literals_buffer, - &mut s.sequences, &mut s.block_content_buffer, s.buffer.window_size, ), @@ -1655,7 +1653,6 @@ impl FrameDecoder { fse, offset_hist, literals_buffer, - sequences, block_content_buffer, buffer, }; diff --git a/zstd/src/decoding/scratch.rs b/zstd/src/decoding/scratch.rs index f47ed4b5f..8a454e98d 100644 --- a/zstd/src/decoding/scratch.rs +++ b/zstd/src/decoding/scratch.rs @@ -1,6 +1,5 @@ //! Structures that wrap around various decoders to make decoding easier. -use super::super::blocks::sequence_section::Sequence; use super::buffer_backend::BufferBackend; use super::decode_buffer::DecodeBuffer; use super::ringbuffer::RingBuffer; @@ -30,7 +29,6 @@ pub struct DecoderScratch { pub offset_hist: [u32; 3], pub literals_buffer: Vec, - pub sequences: Vec, pub block_content_buffer: Vec, } @@ -52,7 +50,6 @@ pub struct WorkspaceRef<'a, B: BufferBackend> { pub buffer: &'a mut DecodeBuffer, pub offset_hist: &'a mut [u32; 3], pub literals_buffer: &'a mut Vec, - pub sequences: &'a mut Vec, pub block_content_buffer: &'a mut Vec, } @@ -83,7 +80,6 @@ impl Workspace for DecoderScratch { buffer: &mut self.buffer, offset_hist: &mut self.offset_hist, literals_buffer: &mut self.literals_buffer, - sequences: &mut self.sequences, block_content_buffer: &mut self.block_content_buffer, } } @@ -115,7 +111,6 @@ pub struct DirectScratch<'o, 'p> { pub buffer: DecodeBuffer>, pub offset_hist: &'p mut [u32; 3], pub literals_buffer: &'p mut Vec, - pub sequences: &'p mut Vec, pub block_content_buffer: &'p mut Vec, } @@ -133,7 +128,6 @@ impl<'o, 'p> Workspace for DirectScratch<'o, 'p> { buffer: &mut self.buffer, offset_hist: &mut *self.offset_hist, literals_buffer: &mut *self.literals_buffer, - sequences: &mut *self.sequences, block_content_buffer: &mut *self.block_content_buffer, } } @@ -147,11 +141,8 @@ impl DecoderScratch { }, fse: FSEScratch { offsets: AlignedFSETable::new(MAX_OFFSET_CODE), - of_rle: None, literal_lengths: AlignedFSETable::new(MAX_LITERAL_LENGTH_CODE), - ll_rle: None, match_lengths: AlignedFSETable::new(MAX_MATCH_LENGTH_CODE), - ml_rle: None, offsets_long_share: 0, ddict_is_cold: false, }, @@ -160,14 +151,12 @@ impl DecoderScratch { block_content_buffer: Vec::new(), literals_buffer: Vec::new(), - sequences: Vec::new(), } } pub fn reset(&mut self, window_size: usize) { self.offset_hist = [1, 4, 8]; self.literals_buffer.clear(); - self.sequences.clear(); self.block_content_buffer.clear(); // Pre-allocate the per-block scratch Vecs to `min(window_size, @@ -214,9 +203,6 @@ impl DecoderScratch { self.fse.literal_lengths.reset(); self.fse.match_lengths.reset(); self.fse.offsets.reset(); - self.fse.ll_rle = None; - self.fse.ml_rle = None; - self.fse.of_rle = None; // Reset the cached pipeline-gate signal alongside the FSE // table reset — otherwise scratch reuse across frames could // engage the long pipeline on a new frame's Repeat-mode @@ -274,11 +260,8 @@ impl Default for HuffmanScratch { pub struct FSEScratch { pub offsets: AlignedFSETable, - pub of_rle: Option, pub literal_lengths: AlignedFSETable, - pub ll_rle: Option, pub match_lengths: AlignedFSETable, - pub ml_rle: Option, /// Cached "share of offset codes strictly > LONG_OFFSET_CODE_THRESHOLD /// (i.e. codes ≥ 23 when the threshold is 22)" scaled to donor's /// `OffFSELog = 8` (256-entry reference). @@ -308,11 +291,8 @@ impl FSEScratch { pub fn new() -> FSEScratch { FSEScratch { offsets: AlignedFSETable::new(MAX_OFFSET_CODE), - of_rle: None, literal_lengths: AlignedFSETable::new(MAX_LITERAL_LENGTH_CODE), - ll_rle: None, match_lengths: AlignedFSETable::new(MAX_MATCH_LENGTH_CODE), - ml_rle: None, offsets_long_share: 0, ddict_is_cold: false, } @@ -322,9 +302,6 @@ impl FSEScratch { self.offsets.reinit_from(&other.offsets); self.literal_lengths.reinit_from(&other.literal_lengths); self.match_lengths.reinit_from(&other.match_lengths); - self.of_rle = other.of_rle; - self.ll_rle = other.ll_rle; - self.ml_rle = other.ml_rle; // Recompute the share from the just-copied offsets table // rather than trusting `other.offsets_long_share`. Two source // shapes produce a populated `offsets` table but a still-zero diff --git a/zstd/src/decoding/seq_decoder_avx2.rs b/zstd/src/decoding/seq_decoder_avx2.rs index d8ce293b1..81f890c95 100644 --- a/zstd/src/decoding/seq_decoder_avx2.rs +++ b/zstd/src/decoding/seq_decoder_avx2.rs @@ -20,17 +20,15 @@ use super::buffer_backend::BufferBackend; use super::decode_buffer::DecodeBuffer; use super::scratch::FSEScratch; use super::sequence_section_decoder::{ - ADVANCE, ADVANCE_MASK, ExecSeq, compute_use_long_pipeline, decode_sequences_with_rle, - maybe_update_fse_tables, + ADVANCE, ADVANCE_MASK, ExecSeq, compute_use_long_pipeline, maybe_update_fse_tables, }; use crate::bit_io::BitReaderReversed; use crate::blocks::sequence_section::{MAX_OFFSET_CODE, Sequence, SequencesHeader}; use crate::common::MAX_BLOCK_SIZE; use crate::cpu_kernel::Avx2Kernel; use crate::decoding::errors::{DecodeSequenceError, DecompressBlockError, ExecuteSequencesError}; -use crate::decoding::sequence_execution::{do_offset_history, execute_sequences_fields}; +use crate::decoding::sequence_execution::do_offset_history; use crate::fse::SeqFSEDecoder; -use alloc::vec::Vec; /// Textual expansion of per-sequence decode. Reads LL/ML/OF state, /// performs triple-bit extract via `peek_bits_triple_bmi2` (`_pext_u64` @@ -194,10 +192,7 @@ pub(crate) unsafe fn decode_and_execute_sequences_avx2( buffer: &mut DecodeBuffer, offset_hist: &mut [u32; 3], literals_buffer: &[u8], - rle_fallback_sequences: &mut Vec, ) -> Result<(), DecompressBlockError> { - rle_fallback_sequences.clear(); - let ddict_is_cold = fse.ddict_is_cold; fse.ddict_is_cold = false; @@ -217,12 +212,6 @@ pub(crate) unsafe fn decode_and_execute_sequences_avx2( return Err(DecodeSequenceError::ExtraPadding { skipped_bits }.into()); } - if fse.ll_rle.is_some() || fse.ml_rle.is_some() || fse.of_rle.is_some() { - decode_sequences_with_rle(section, &mut br, fse, rle_fallback_sequences)?; - execute_sequences_fields(buffer, literals_buffer, offset_hist, rle_fallback_sequences)?; - return Ok(()); - } - let mut ll_dec = SeqFSEDecoder::new(&fse.literal_lengths); let mut ml_dec = SeqFSEDecoder::new(&fse.match_lengths); let mut of_dec = SeqFSEDecoder::new(&fse.offsets); diff --git a/zstd/src/decoding/seq_decoder_bmi2.rs b/zstd/src/decoding/seq_decoder_bmi2.rs index 1a0386e81..9234ffcee 100644 --- a/zstd/src/decoding/seq_decoder_bmi2.rs +++ b/zstd/src/decoding/seq_decoder_bmi2.rs @@ -11,17 +11,15 @@ use super::buffer_backend::BufferBackend; use super::decode_buffer::DecodeBuffer; use super::scratch::FSEScratch; use super::sequence_section_decoder::{ - ADVANCE, ADVANCE_MASK, ExecSeq, compute_use_long_pipeline, decode_sequences_with_rle, - maybe_update_fse_tables, + ADVANCE, ADVANCE_MASK, ExecSeq, compute_use_long_pipeline, maybe_update_fse_tables, }; use crate::bit_io::BitReaderReversed; use crate::blocks::sequence_section::{MAX_OFFSET_CODE, Sequence, SequencesHeader}; use crate::common::MAX_BLOCK_SIZE; use crate::cpu_kernel::Bmi2Kernel; use crate::decoding::errors::{DecodeSequenceError, DecompressBlockError, ExecuteSequencesError}; -use crate::decoding::sequence_execution::{do_offset_history, execute_sequences_fields}; +use crate::decoding::sequence_execution::do_offset_history; use crate::fse::SeqFSEDecoder; -use alloc::vec::Vec; /// Textual decode-one body. PEXT-direct via `peek_bits_triple_bmi2` /// when vendor cache enables it. @@ -166,10 +164,7 @@ pub(crate) unsafe fn decode_and_execute_sequences_bmi2( buffer: &mut DecodeBuffer, offset_hist: &mut [u32; 3], literals_buffer: &[u8], - rle_fallback_sequences: &mut Vec, ) -> Result<(), DecompressBlockError> { - rle_fallback_sequences.clear(); - let ddict_is_cold = fse.ddict_is_cold; fse.ddict_is_cold = false; @@ -189,12 +184,6 @@ pub(crate) unsafe fn decode_and_execute_sequences_bmi2( return Err(DecodeSequenceError::ExtraPadding { skipped_bits }.into()); } - if fse.ll_rle.is_some() || fse.ml_rle.is_some() || fse.of_rle.is_some() { - decode_sequences_with_rle(section, &mut br, fse, rle_fallback_sequences)?; - execute_sequences_fields(buffer, literals_buffer, offset_hist, rle_fallback_sequences)?; - return Ok(()); - } - let mut ll_dec = SeqFSEDecoder::new(&fse.literal_lengths); let mut ml_dec = SeqFSEDecoder::new(&fse.match_lengths); let mut of_dec = SeqFSEDecoder::new(&fse.offsets); diff --git a/zstd/src/decoding/seq_decoder_scalar.rs b/zstd/src/decoding/seq_decoder_scalar.rs index 49ae436e4..5c278a037 100644 --- a/zstd/src/decoding/seq_decoder_scalar.rs +++ b/zstd/src/decoding/seq_decoder_scalar.rs @@ -9,17 +9,15 @@ use super::buffer_backend::BufferBackend; use super::decode_buffer::DecodeBuffer; use super::scratch::FSEScratch; use super::sequence_section_decoder::{ - ADVANCE, ADVANCE_MASK, ExecSeq, compute_use_long_pipeline, decode_sequences_with_rle, - maybe_update_fse_tables, + ADVANCE, ADVANCE_MASK, ExecSeq, compute_use_long_pipeline, maybe_update_fse_tables, }; use crate::bit_io::BitReaderReversed; use crate::blocks::sequence_section::{MAX_OFFSET_CODE, Sequence, SequencesHeader}; use crate::common::MAX_BLOCK_SIZE; use crate::cpu_kernel::ScalarKernel; use crate::decoding::errors::{DecodeSequenceError, DecompressBlockError, ExecuteSequencesError}; -use crate::decoding::sequence_execution::{do_offset_history, execute_sequences_fields}; +use crate::decoding::sequence_execution::do_offset_history; use crate::fse::SeqFSEDecoder; -use alloc::vec::Vec; macro_rules! decode_one_body { ($ll_dec:expr, $ml_dec:expr, $of_dec:expr, $br:expr) => {{ @@ -99,10 +97,7 @@ pub(crate) fn decode_and_execute_sequences_scalar( buffer: &mut DecodeBuffer, offset_hist: &mut [u32; 3], literals_buffer: &[u8], - rle_fallback_sequences: &mut Vec, ) -> Result<(), DecompressBlockError> { - rle_fallback_sequences.clear(); - let ddict_is_cold = fse.ddict_is_cold; fse.ddict_is_cold = false; @@ -122,12 +117,6 @@ pub(crate) fn decode_and_execute_sequences_scalar( return Err(DecodeSequenceError::ExtraPadding { skipped_bits }.into()); } - if fse.ll_rle.is_some() || fse.ml_rle.is_some() || fse.of_rle.is_some() { - decode_sequences_with_rle(section, &mut br, fse, rle_fallback_sequences)?; - execute_sequences_fields(buffer, literals_buffer, offset_hist, rle_fallback_sequences)?; - return Ok(()); - } - let mut ll_dec = SeqFSEDecoder::new(&fse.literal_lengths); let mut ml_dec = SeqFSEDecoder::new(&fse.match_lengths); let mut of_dec = SeqFSEDecoder::new(&fse.offsets); diff --git a/zstd/src/decoding/seq_decoder_vbmi2.rs b/zstd/src/decoding/seq_decoder_vbmi2.rs index 468dd25fa..1d04aa062 100644 --- a/zstd/src/decoding/seq_decoder_vbmi2.rs +++ b/zstd/src/decoding/seq_decoder_vbmi2.rs @@ -11,17 +11,15 @@ use super::buffer_backend::BufferBackend; use super::decode_buffer::DecodeBuffer; use super::scratch::FSEScratch; use super::sequence_section_decoder::{ - ADVANCE, ADVANCE_MASK, ExecSeq, compute_use_long_pipeline, decode_sequences_with_rle, - maybe_update_fse_tables, + ADVANCE, ADVANCE_MASK, ExecSeq, compute_use_long_pipeline, maybe_update_fse_tables, }; use crate::bit_io::BitReaderReversed; use crate::blocks::sequence_section::{MAX_OFFSET_CODE, Sequence, SequencesHeader}; use crate::common::MAX_BLOCK_SIZE; use crate::cpu_kernel::Vbmi2Kernel; use crate::decoding::errors::{DecodeSequenceError, DecompressBlockError, ExecuteSequencesError}; -use crate::decoding::sequence_execution::{do_offset_history, execute_sequences_fields}; +use crate::decoding::sequence_execution::do_offset_history; use crate::fse::SeqFSEDecoder; -use alloc::vec::Vec; macro_rules! decode_one_body { ($ll_dec:expr, $ml_dec:expr, $of_dec:expr, $br:expr) => {{ @@ -162,10 +160,7 @@ pub(crate) unsafe fn decode_and_execute_sequences_vbmi2( buffer: &mut DecodeBuffer, offset_hist: &mut [u32; 3], literals_buffer: &[u8], - rle_fallback_sequences: &mut Vec, ) -> Result<(), DecompressBlockError> { - rle_fallback_sequences.clear(); - let ddict_is_cold = fse.ddict_is_cold; fse.ddict_is_cold = false; @@ -185,12 +180,6 @@ pub(crate) unsafe fn decode_and_execute_sequences_vbmi2( return Err(DecodeSequenceError::ExtraPadding { skipped_bits }.into()); } - if fse.ll_rle.is_some() || fse.ml_rle.is_some() || fse.of_rle.is_some() { - decode_sequences_with_rle(section, &mut br, fse, rle_fallback_sequences)?; - execute_sequences_fields(buffer, literals_buffer, offset_hist, rle_fallback_sequences)?; - return Ok(()); - } - let mut ll_dec = SeqFSEDecoder::new(&fse.literal_lengths); let mut ml_dec = SeqFSEDecoder::new(&fse.match_lengths); let mut of_dec = SeqFSEDecoder::new(&fse.offsets); diff --git a/zstd/src/decoding/sequence_execution.rs b/zstd/src/decoding/sequence_execution.rs index 5f1a70422..b07801f18 100644 --- a/zstd/src/decoding/sequence_execution.rs +++ b/zstd/src/decoding/sequence_execution.rs @@ -1,68 +1,3 @@ -use super::decode_buffer::DecodeBuffer; -use crate::blocks::sequence_section::Sequence; -use crate::common::MAX_BLOCK_SIZE; -use crate::decoding::errors::ExecuteSequencesError; - -/// Field-level variant of [`execute_sequences`] used when the caller cannot -/// hand over a full `&mut DecoderScratch` (e.g. another field is immutably -/// borrowed at the call site). Takes the same subset of fields the workspace -/// version touches and runs the identical execution loop. Used by the -/// fused `decode_and_execute_sequences` for its RLE-mode fallback. -pub(crate) fn execute_sequences_fields( - buffer: &mut DecodeBuffer, - literals_buffer: &[u8], - offset_hist: &mut [u32; 3], - sequences: &[Sequence], -) -> Result<(), ExecuteSequencesError> { - let mut literals_copy_counter: usize = 0; - let old_buffer_size = buffer.len(); - let mut seq_sum: u32 = 0; - - buffer.reserve(MAX_BLOCK_SIZE as usize); - - let literals_buffer_len = literals_buffer.len(); - for seq in sequences { - let seq = *seq; - let high = literals_copy_counter + seq.ll as usize; - if high > literals_buffer_len { - return Err(ExecuteSequencesError::NotEnoughBytesForSequence { - wanted: high, - have: literals_buffer_len, - }); - } - // SAFETY: literals_copy_counter <= high <= literals_buffer_len. - let literals = unsafe { literals_buffer.get_unchecked(literals_copy_counter..high) }; - literals_copy_counter = high; - // `try_push` routes a fixed-capacity backend overshoot - // (UserSliceBackend) into a structured - // `ExecuteSequencesError::OutputBufferOverflow` instead of - // panicking via the per-call `assert!` inside - // `BufferBackend::extend`. Growable backends accept the write - // infallibly via the default trait impl. - buffer.try_push(literals)?; - - let actual_offset = do_offset_history(seq.of, seq.ll, offset_hist); - if actual_offset == 0 { - return Err(ExecuteSequencesError::ZeroOffset); - } - buffer.repeat(actual_offset as usize, seq.ml as usize)?; - - seq_sum = seq_sum.wrapping_add(seq.ml).wrapping_add(seq.ll); - } - if literals_copy_counter < literals_buffer_len { - let rest_literals = &literals_buffer[literals_copy_counter..]; - buffer.try_push(rest_literals)?; - seq_sum = seq_sum.wrapping_add(rest_literals.len() as u32); - } - - let diff = buffer.len() - old_buffer_size; - debug_assert_eq!( - seq_sum as usize, diff, - "seq_sum {seq_sum} != buffer growth {diff}" - ); - Ok(()) -} - /// Update the most recently used offsets to reflect the provided offset value, and return the /// "actual" offset needed because offsets are not stored in a raw way, some transformations are needed /// before you get a functional number. diff --git a/zstd/src/decoding/sequence_section_decoder.rs b/zstd/src/decoding/sequence_section_decoder.rs index 607361b2c..fd411b463 100644 --- a/zstd/src/decoding/sequence_section_decoder.rs +++ b/zstd/src/decoding/sequence_section_decoder.rs @@ -8,8 +8,9 @@ use crate::blocks::sequence_section::{ }; use crate::common::MAX_BLOCK_SIZE; use crate::decoding::errors::{DecodeSequenceError, DecompressBlockError, ExecuteSequencesError}; -use crate::decoding::sequence_execution::{do_offset_history, execute_sequences_fields}; +use crate::decoding::sequence_execution::do_offset_history; use crate::fse::SeqFSEDecoder; +#[cfg(test)] use alloc::vec::Vec; // 8-slot software pipeline mirroring donor @@ -105,7 +106,6 @@ pub fn decode_and_execute_sequences( buffer: &mut super::decode_buffer::DecodeBuffer, offset_hist: &mut [u32; 3], literals_buffer: &[u8], - rle_fallback_sequences: &mut Vec, ) -> Result<(), DecompressBlockError> { #[cfg(all(target_arch = "aarch64", feature = "kernel_neon"))] use crate::cpu_kernel::NeonKernel; @@ -126,7 +126,6 @@ pub fn decode_and_execute_sequences( buffer, offset_hist, literals_buffer, - rle_fallback_sequences, ) } #[cfg(all(target_arch = "x86_64", feature = "kernel_sse2"))] @@ -143,7 +142,6 @@ pub fn decode_and_execute_sequences( buffer, offset_hist, literals_buffer, - rle_fallback_sequences, ) } #[cfg(all(target_arch = "x86_64", feature = "kernel_bmi2"))] @@ -162,7 +160,6 @@ pub fn decode_and_execute_sequences( buffer, offset_hist, literals_buffer, - rle_fallback_sequences, ) } } @@ -177,7 +174,6 @@ pub fn decode_and_execute_sequences( buffer, offset_hist, literals_buffer, - rle_fallback_sequences, ) } } @@ -193,7 +189,6 @@ pub fn decode_and_execute_sequences( buffer, offset_hist, literals_buffer, - rle_fallback_sequences, ) } } @@ -205,7 +200,6 @@ pub fn decode_and_execute_sequences( buffer, offset_hist, literals_buffer, - rle_fallback_sequences, ), #[cfg(all( target_arch = "aarch64", @@ -219,7 +213,6 @@ pub fn decode_and_execute_sequences( buffer, offset_hist, literals_buffer, - rle_fallback_sequences, ), } } @@ -245,23 +238,16 @@ pub(crate) fn decode_and_execute_sequences_impl< buffer: &mut super::decode_buffer::DecodeBuffer, offset_hist: &mut [u32; 3], literals_buffer: &[u8], - rle_fallback_sequences: &mut Vec, ) -> Result<(), DecompressBlockError> { - // Reset the fallback sequences vec on entry. The non-RLE fast path - // never writes to it, so without this clear it would carry whatever - // entries the previous block left behind — a stale-data hazard for - // any external caller that inspects scratch.sequences after decode. - rle_fallback_sequences.clear(); - // Consume the one-shot `ddict_is_cold` flag at function entry, - // BEFORE any early returns (RLE-mode fallback below, padding-bit - // validation). Donor `ZSTD_decompressBlock_internal` clears + // BEFORE any early returns (padding-bit validation). + // Donor `ZSTD_decompressBlock_internal` clears // `dctx->ddictIsCold = 0` unconditionally after the // sequence-section dispatch decision; if the early-return paths // left the flag set, a later block's gate would mis-apply the // cold-dict signal that no longer holds (FSE/HUF tables are now - // warm regardless of whether the previous block decoded - // sequences or fell back to RLE). + // warm regardless of how the previous block decoded its sequences, + // including RLE-mode axes handled in-line via the fused table). let ddict_is_cold = fse.ddict_is_cold; fse.ddict_is_cold = false; @@ -285,23 +271,9 @@ pub(crate) fn decode_and_execute_sequences_impl< return Err(DecodeSequenceError::ExtraPadding { skipped_bits }.into()); } - // RLE-mode blocks: fall back to the legacy two-pass pipeline. These - // are uncommon in real-world corpora; fusing them too would double - // the source maintenance for zero observed wins. - if fse.ll_rle.is_some() || fse.ml_rle.is_some() || fse.of_rle.is_some() { - decode_sequences_with_rle(section, &mut br, fse, rle_fallback_sequences)?; - // `execute_sequences_fields` routes literal-pushes through - // `DecodeBuffer::try_push` (and match-repeats through - // `BufferBackend::try_reserve` inside `repeat_inner`), so a - // malformed RLE-driven sequence stream whose literal or match - // length overshoots a fixed-capacity backend (UserSliceBackend) - // surfaces as `ExecuteSequencesError::OutputBufferOverflow` - // rather than panicking via UserSliceBackend::extend's - // release-mode `assert!`. - execute_sequences_fields(buffer, literals_buffer, offset_hist, rle_fallback_sequences)?; - return Ok(()); - } - + // RLE-mode axes are handled uniformly: `maybe_update_fse_tables` + // builds a degenerate single-state table for them, so the fused + // decode below reads every axis the same way (no separate fallback). let mut ll_dec = SeqFSEDecoder::new(&fse.literal_lengths); let mut ml_dec = SeqFSEDecoder::new(&fse.match_lengths); let mut of_dec = SeqFSEDecoder::new(&fse.offsets); @@ -1238,116 +1210,6 @@ fn decode_one_sequence_inline( } } -pub(crate) fn decode_sequences_with_rle( - section: &SequencesHeader, - br: &mut BitReaderReversed<'_, K>, - scratch: &FSEScratch, - target: &mut Vec, -) -> Result<(), DecodeSequenceError> { - let mut ll_dec = SeqFSEDecoder::new(&scratch.literal_lengths); - let mut ml_dec = SeqFSEDecoder::new(&scratch.match_lengths); - let mut of_dec = SeqFSEDecoder::new(&scratch.offsets); - - if scratch.ll_rle.is_none() { - ll_dec.init_state(br)?; - } - if scratch.of_rle.is_none() { - of_dec.init_state(br)?; - } - if scratch.ml_rle.is_none() { - ml_dec.init_state(br)?; - } - - target.clear(); - target.reserve(section.num_sequences as usize); - - // Only non-RLE decoders need state updates; compute their combined worst-case. - let max_update_bits = if scratch.ll_rle.is_none() { - scratch.literal_lengths.accuracy_log - } else { - 0 - } + if scratch.ml_rle.is_none() { - scratch.match_lengths.accuracy_log - } else { - 0 - } + if scratch.of_rle.is_none() { - scratch.offsets.accuracy_log - } else { - 0 - }; - debug_assert!( - max_update_bits <= 56, - "sequence section update bits exceed 56-bit budget" - ); - - for _seq_idx in 0..section.num_sequences { - // RLE-mode tables don't carry an enriched FSE state — fall - // back to `lookup_ll_code` / `lookup_ml_code` on the RLE byte - // for LL / ML, and the closed-form `(1 << code, code)` shape - // for OF. FSE-mode tables read base / extra-bits directly - // off the active state's enriched `SeqSymbol`. `SeqSymbol` - // has no `symbol` byte; OF code is recoverable from - // `num_additional_bits` (== code for code < 32) but the hot - // path uses `base_value` / `num_additional_bits` directly. - let (ll_value, ll_num_bits) = if let Some(ll_rle) = scratch.ll_rle { - lookup_ll_code(ll_rle) - } else { - (ll_dec.state.base_value, ll_dec.state.num_additional_bits) - }; - let (ml_value, ml_num_bits) = if let Some(ml_rle) = scratch.ml_rle { - lookup_ml_code(ml_rle) - } else { - (ml_dec.state.base_value, ml_dec.state.num_additional_bits) - }; - let (of_value, of_num_bits) = if let Some(of_rle) = scratch.of_rle { - (1u32 << of_rle, of_rle) - } else { - (of_dec.state.base_value, of_dec.state.num_additional_bits) - }; - - debug_assert!(of_num_bits <= MAX_OFFSET_CODE); - - let (obits, ml_add, ll_add) = br.get_bits_triple(of_num_bits, ml_num_bits, ll_num_bits); - let offset = obits as u32 + of_value; - - debug_assert_ne!(offset, 0); - - target.push(Sequence { - ll: ll_value + ll_add as u32, - ml: ml_value + ml_add as u32, - of: offset, - }); - - if target.len() < section.num_sequences as usize { - // One refill check for all non-RLE state updates (batched fast path). - if max_update_bits > 0 { - br.ensure_bits(max_update_bits); - } - if scratch.ll_rle.is_none() { - ll_dec.update_state_fast(br); - } - if scratch.ml_rle.is_none() { - ml_dec.update_state_fast(br); - } - if scratch.of_rle.is_none() { - of_dec.update_state_fast(br); - } - } - - if br.bits_remaining() < 0 { - return Err(DecodeSequenceError::NotEnoughBytesForNumSequences); - } - } - - if br.bits_remaining() > 0 { - Err(DecodeSequenceError::ExtraBits { - bits_remaining: br.bits_remaining(), - }) - } else { - Ok(()) - } -} - /// Packed (baseline, extra_bits) pairs for literal-length codes. /// Donor parity: `LL_base` + `LL_bits` from the zstd reference /// (`zstd_compress_internal.h`). Per Zstandard format §3.1.1.3.2.1.1.1, @@ -1412,55 +1274,6 @@ const fn pack_code_meta(bases: &[u32; N], extra_bits: &[u8; N]) out } -/// Unpack the (baseline, extra_bits) tuple from a packed [`LL_META`] / -/// [`ML_META`] entry. Inlined so the shift+mask collapses to ALU ops -/// with no cross-function call overhead on the hot path. -#[inline(always)] -const fn unpack_code_meta(meta: u32) -> (u32, u8) { - (meta & 0x00FF_FFFF, (meta >> 24) as u8) -} - -/// Look up the provided state value from a literal length table predefined -/// by the Zstandard reference document. Returns a tuple of (value, number of bits). -/// -/// -#[inline(always)] -fn lookup_ll_code(code: u8) -> (u32, u8) { - // The FSE LL table is constructed with `max_symbol = - // MAX_LITERAL_LENGTH_CODE` (35); `build_decoding_table` returns - // `FSETableError::TooManySymbols` if `read_probabilities` produces - // more entries than that, and the RLE byte path is range-checked - // in `maybe_update_fse_tables`. So a `code` reaching this lookup - // is invariant 0..=35. Keep the `debug_assert` as a tripwire in - // case a future caller forgets one of those validations; drop the - // release-mode `assert!` so the hot path takes a single - // `get_unchecked` instead of a bounds-checked indexed load. - let idx = code as usize; - debug_assert!( - idx < LL_META.len(), - "Illegal literal length code was: {code}" - ); - // SAFETY: idx < LL_META.len() == 36 per the FSE table - // construction invariant documented above. - unpack_code_meta(unsafe { *LL_META.get_unchecked(idx) }) -} - -/// Look up the provided state value from a match length table predefined -/// by the Zstandard reference document. Returns a tuple of (value, number of bits). -/// -/// -#[inline(always)] -fn lookup_ml_code(code: u8) -> (u32, u8) { - // Same invariant as `lookup_ll_code`: the ML FSE table is built - // with `max_symbol = MAX_MATCH_LENGTH_CODE` (52) and the RLE byte - // is range-checked, so `code` reaching this lookup is 0..=52. - let idx = code as usize; - debug_assert!(idx < ML_META.len(), "Illegal match length code was: {code}"); - // SAFETY: idx < ML_META.len() == 53 per the FSE table - // construction invariant. - unpack_code_meta(unsafe { *ML_META.get_unchecked(idx) }) -} - // This info is buried in the symbol compression mode table /// "The maximum allowed accuracy log for literals length and match length tables is 9" pub const LL_MAX_LOG: u8 = 9; @@ -1520,7 +1333,6 @@ pub(crate) fn maybe_update_fse_tables( vprintln!("Updating ll table"); vprintln!("Used bytes: {}", bytes); - scratch.ll_rle = None; } ModeType::RLE => { vprintln!("Use RLE ll table"); @@ -1529,9 +1341,15 @@ pub(crate) fn maybe_update_fse_tables( } bytes_read += 1; if source[0] > MAX_LITERAL_LENGTH_CODE { - return Err(DecodeSequenceError::MissingByteForRleMlTable); + return Err(DecodeSequenceError::InvalidRleCode { + axis: "LL", + code: source[0], + }); } - scratch.ll_rle = Some(source[0]); + scratch.literal_lengths.build_rle(source[0]); + scratch + .literal_lengths + .enrich_with_packed_seq_meta(&LL_META); } ModeType::Predefined => { vprintln!("Use predefined ll table"); @@ -1550,7 +1368,6 @@ pub(crate) fn maybe_update_fse_tables( .literal_lengths .enrich_with_packed_seq_meta(&LL_META); } - scratch.ll_rle = None; } ModeType::Repeat => { vprintln!("Repeat ll table"); @@ -1567,7 +1384,6 @@ pub(crate) fn maybe_update_fse_tables( vprintln!("Updating of table"); vprintln!("Used bytes: {}", bytes); bytes_read += bytes; - scratch.of_rle = None; scratch.offsets_long_share = compute_offsets_long_share(&scratch.offsets); } ModeType::RLE => { @@ -1577,9 +1393,16 @@ pub(crate) fn maybe_update_fse_tables( } bytes_read += 1; if of_source[0] > MAX_OFFSET_CODE { - return Err(DecodeSequenceError::MissingByteForRleMlTable); + return Err(DecodeSequenceError::InvalidRleCode { + axis: "OF", + code: of_source[0], + }); } - scratch.of_rle = Some(of_source[0]); + // Build a degenerate 1-state table so the fused decode path + // handles this axis uniformly (no separate RLE fallback). + scratch.offsets.build_rle(of_source[0]); + scratch.offsets.enrich_for_offsets(); + scratch.offsets_long_share = compute_offsets_long_share(&scratch.offsets); } ModeType::Predefined => { vprintln!("Use predefined of table"); @@ -1598,7 +1421,6 @@ pub(crate) fn maybe_update_fse_tables( scratch.offsets.enrich_for_offsets(); scratch.offsets_long_share = compute_offsets_long_share(&scratch.offsets); } - scratch.of_rle = None; } ModeType::Repeat => { vprintln!("Repeat of table"); @@ -1615,7 +1437,6 @@ pub(crate) fn maybe_update_fse_tables( bytes_read += bytes; vprintln!("Updating ml table"); vprintln!("Used bytes: {}", bytes); - scratch.ml_rle = None; } ModeType::RLE => { vprintln!("Use RLE ml table"); @@ -1624,9 +1445,13 @@ pub(crate) fn maybe_update_fse_tables( } bytes_read += 1; if ml_source[0] > MAX_MATCH_LENGTH_CODE { - return Err(DecodeSequenceError::MissingByteForRleMlTable); + return Err(DecodeSequenceError::InvalidRleCode { + axis: "ML", + code: ml_source[0], + }); } - scratch.ml_rle = Some(ml_source[0]); + scratch.match_lengths.build_rle(ml_source[0]); + scratch.match_lengths.enrich_with_packed_seq_meta(&ML_META); } ModeType::Predefined => { vprintln!("Use predefined ml table"); @@ -1643,7 +1468,6 @@ pub(crate) fn maybe_update_fse_tables( )?; scratch.match_lengths.enrich_with_packed_seq_meta(&ML_META); } - scratch.ml_rle = None; } ModeType::Repeat => { vprintln!("Repeat ml table"); diff --git a/zstd/src/fse/fse_decoder.rs b/zstd/src/fse/fse_decoder.rs index 842d96744..2fb0d6a45 100644 --- a/zstd/src/fse/fse_decoder.rs +++ b/zstd/src/fse/fse_decoder.rs @@ -47,7 +47,13 @@ impl<'t, E: FseEntry> FSEDecoderImpl<'t, E> { &mut self, bits: &mut BitReaderReversed<'_, K>, ) -> Result<(), FSEDecoderError> { - if self.table.accuracy_log == 0 { + // Uninitialised = no decode entries. (Previously this checked + // `accuracy_log == 0`, but a valid RLE table has accuracy_log 0 + // with a single entry — donor's RLE DTable. An empty `decode` + // vec is the real "never built" signal; the `InvalidTableShape` + // check below still enforces `decode.len() == 1 << accuracy_log`, + // which holds for the 1-entry RLE table since `1 << 0 == 1`.) + if self.table.decode.is_empty() { return Err(FSEDecoderError::TableIsUninitialized); } // Defense-in-depth internal-invariant guard: in normal builds @@ -788,6 +794,35 @@ impl FSETableImpl { } } } + + /// Build a degenerate single-state RLE table for a sequence axis + /// whose Compression_Mode is RLE: exactly one symbol, decoded every + /// sequence with no FSE state-transition bits. Mirrors the donor + /// RLE DTable (`accuracy_log = 0`, one entry, `new_state = 0`, + /// `num_bits = 0`). The caller runs the usual `enrich_with_packed_seq_meta` + /// (LL/ML) or `enrich_for_offsets` (OF) pass afterward to fill + /// `base_value` / `num_additional_bits` from `symbol`, so the fused + /// per-sequence loop reads this axis uniformly with the FSE axes + /// (init reads 0 state bits, every `update_state` keeps state 0). + pub(crate) fn build_rle(&mut self, symbol: u8) { + self.reset(); + // NB: do NOT shrink `max_symbol` to `symbol` — the scratch table + // is reused across blocks, and a later FSE-mode block's + // `build_decoder` validates its symbol count against `max_symbol`. + // Setting it to the single RLE symbol would reject any subsequent + // table with more symbols (`TooManySymbols`). `reset` leaves + // `max_symbol` at the axis maximum, which is correct here. + // Spread buffer drives the enrich pass (symbol per slot); one slot. + self.symbol_spread_buffer.push(symbol); + self.decode.push(SeqSymbol { + new_state: 0, + num_bits: 0, + num_additional_bits: 0, + base_value: 0, + }); + // accuracy_log stays 0 (donor RLE DTable tableLog); init_state + // reads 0 state bits and update_state keeps the single state. + } } /// Sequence-section decoder alias: reads 8-byte [`SeqSymbol`] entries. diff --git a/zstd/tests/cross_validation.rs b/zstd/tests/cross_validation.rs index 8889b1f82..2b28f5b77 100644 --- a/zstd/tests/cross_validation.rs +++ b/zstd/tests/cross_validation.rs @@ -360,3 +360,70 @@ fn level22_stays_within_ffi_level22_on_corpus_proxy() { ffi_level22.len() ); } + +/// RLE-mode sequence tables: C zstd emits Compression_Mode = RLE for an +/// LL/ML/OF axis when a block's sequences all share one code (uniform / +/// highly-repetitive data). This exercises the fused RLE decode path — +/// the degenerate 1-state FSE table built by `FSETableImpl::build_rle`. +/// All-same-byte input is the canonical RLE producer: C encodes it as a +/// single long match, so every sequence axis has exactly one code and +/// C switches the table to RLE mode. A wrong RLE table build or a wrong +/// 1-state decode would corrupt the output here. +#[test] +fn cross_ffi_compress_rust_decompress_rle_mode_tables() { + let mut period4: Vec = Vec::with_capacity(8192); + while period4.len() < 8192 { + period4.extend_from_slice(b"abcd"); + } + let mut two_runs: Vec = vec![0x11u8; 4096]; + two_runs.extend_from_slice(&[0x22u8; 4096]); + + // Periodic unit = a fixed 15-byte pattern that matches the previous + // unit (offset 16) + one varying literal byte. Every unit yields the + // SAME (lit_len, match_len, offset) sequence, so the block has many + // sequences all sharing one code per axis → RLE-mode sequence tables + // with MULTI-sequence blocks (exercises update_state transitions on + // the 1-state table, unlike the single-match inputs above). + // 9000 units * 16 B = 144 KiB → spans more than one 128 KiB block, + // every block RLE-mode. + let mut periodic: Vec = Vec::with_capacity(16 * 9000); + for i in 0..9000u32 { + periodic.extend_from_slice(&[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]); + periodic.push((i & 0xFF) as u8); + } + // RLE blocks followed by FSE blocks in ONE frame: the scratch FSE + // tables are reused across blocks, so a stale RLE-table field leaking + // into the next FSE-mode block's build is caught here (regression for + // the max_symbol-clobber bug). + let mut mixed: Vec = periodic.clone(); + mixed.extend_from_slice(include_bytes!("../decodecorpus_files/z000033")); + + let inputs: Vec> = vec![ + periodic.clone(), // periodic units → multi-sequence RLE tables + mixed, // RLE blocks → FSE blocks in one frame + vec![0x5Au8; 4096], // all-same byte → single long match → RLE axes + vec![0u8; 70_000], // multi-block uniform + period4, // period-4 repeat + two_runs, // two long single-byte runs + // Decode corpus at negative/low levels produces MULTI-sequence + // RLE blocks (many sequences sharing one code), so the 1-state + // table's `update_state` IS exercised between sequences — the + // single-sequence inputs above leave that transition untested. + include_bytes!("../decodecorpus_files/z000033").to_vec(), + ]; + for level in [-6i32, -1, 1, 3, 9, 19] { + for (idx, data) in inputs.iter().enumerate() { + let compressed = zstd::encode_all(&data[..], level).unwrap(); + let mut decoder = StreamingDecoder::new(compressed.as_slice()).unwrap(); + let mut result = Vec::new(); + decoder.read_to_end(&mut result).unwrap(); + // `assert_eq!(*data, result, ...)` borrows both operands + // (`match (&*data, &result)`), so dereferencing the + // `&Vec` from `inputs.iter()` does not move out of it. + assert_eq!( + *data, result, + "RLE-mode ffi→rust decode mismatch (input {idx}, level {level})", + ); + } + } +}