diff --git a/rust/src/zstd_double_fast.rs b/rust/src/zstd_double_fast.rs index 04f307230..4e3c3d4cd 100644 --- a/rust/src/zstd_double_fast.rs +++ b/rust/src/zstd_double_fast.rs @@ -235,7 +235,41 @@ fn lowest_match_index(low_limit: u32, loaded_dict_end: u32, curr: u32, window_lo #[inline] fn index_overlap_check(prefix_lowest_index: u32, rep_index: u32) -> bool { - prefix_lowest_index.wrapping_sub(1).wrapping_sub(rep_index) >= 3 + /* + * Keep the C predicate's two valid coordinate regions without relying on + * unsigned subtraction to classify an already-invalid index. The + * attached-dictionary coordinate guard below additionally bounds prefix + * repcodes by the current position. + */ + rep_index >= prefix_lowest_index + || rep_index + .checked_add(4) + .is_some_and(|rep_end| rep_end <= prefix_lowest_index) +} + +#[inline] +fn attached_rep_index( + prefix_lowest_index: u32, + dict_start_index: u32, + dict_index_delta: u32, + current: u32, + advance: u32, + offset: u32, +) -> Option { + if offset == 0 { + return None; + } + let rep_index = current.checked_add(advance)?.checked_sub(offset)?; + if rep_index > current || !index_overlap_check(prefix_lowest_index, rep_index) { + return None; + } + if rep_index < prefix_lowest_index { + let dict_rep_start = dict_start_index.checked_add(dict_index_delta)?; + if rep_index < dict_rep_start { + return None; + } + } + Some(rep_index) } #[inline] @@ -774,6 +808,7 @@ unsafe fn post_dict_match( base: *const u8, dict_base: *const u8, prefix_lowest_index: u32, + dict_start_index: u32, dict_index_delta: u32, dict_end: *const u8, prefix_lowest: *const u8, @@ -810,17 +845,22 @@ unsafe fn post_dict_match( while ptr_le(ip, ilimit) { let current2 = unsafe { index_from(base, ip) }; - let rep_index2 = current2.wrapping_sub(*offset2); + let Some(rep_index2) = attached_rep_index( + prefix_lowest_index, + dict_start_index, + dict_index_delta, + current2, + 0, + *offset2, + ) else { + break; + }; let rep_match2 = if rep_index2 < prefix_lowest_index { - dict_base - .wrapping_sub(dict_index_delta as usize) - .wrapping_add(rep_index2 as usize) + dict_base.wrapping_add((rep_index2 - dict_index_delta) as usize) } else { base.wrapping_add(rep_index2 as usize) }; - if index_overlap_check(prefix_lowest_index, rep_index2) - && unsafe { read32(rep_match2) == read32(ip) } - { + if unsafe { read32(rep_match2) == read32(ip) } { let rep_end2 = if rep_index2 < prefix_lowest_index { dict_end } else { @@ -929,71 +969,78 @@ unsafe fn compress_block_double_fast_dict_match_state( let mut match_index_small = unsafe { table_get(hash_small, h_small) }; let mut match_long = base.wrapping_add(match_index_long as usize); let mut matched = base.wrapping_add(match_index_small as usize); - let rep_index = current.wrapping_add(1).wrapping_sub(offset1); - let rep_match = if rep_index < prefix_lowest_index { - dict_base.wrapping_add(rep_index.wrapping_sub(dict_index_delta) as usize) - } else { - base.wrapping_add(rep_index as usize) - }; unsafe { table_set(hash_long, h_long, current); table_set(hash_small, h_small, current); } - if index_overlap_check(prefix_lowest_index, rep_index) - && unsafe { read32(rep_match) == read32(ip.wrapping_add(1)) } - { - let rep_match_end = if rep_index < prefix_lowest_index { - dict_end + if let Some(rep_index) = attached_rep_index( + prefix_lowest_index, + dict_start_index, + dict_index_delta, + current, + 1, + offset1, + ) { + let rep_match = if rep_index < prefix_lowest_index { + dict_base.wrapping_add((rep_index - dict_index_delta) as usize) } else { - iend + base.wrapping_add(rep_index as usize) }; - let match_length = unsafe { - count_2segments( - ip.wrapping_add(5), - rep_match.wrapping_add(4), - iend, - rep_match_end, - prefix_lowest, - ) + 4 - }; - ip = ip.wrapping_add(1); - unsafe { - store_seq( - seq_store, - ip.offset_from(anchor) as usize, - anchor, - iend, - REPCODE1_TO_OFFBASE, - match_length, - ) - }; - let (next_ip, next_anchor) = unsafe { - post_dict_match( - hash_long, - hash_small, - base, - dict_base, - prefix_lowest_index, - dict_index_delta, - dict_end, - prefix_lowest, - hash_log, - chain_log, - mls, - seq_store, - &mut offset1, - &mut offset2, - ip, - match_length, - current, - iend, - ilimit, - ) - }; - ip = next_ip; - anchor = next_anchor; - continue; + if unsafe { read32(rep_match) == read32(ip.wrapping_add(1)) } { + let rep_match_end = if rep_index < prefix_lowest_index { + dict_end + } else { + iend + }; + let match_length = unsafe { + count_2segments( + ip.wrapping_add(5), + rep_match.wrapping_add(4), + iend, + rep_match_end, + prefix_lowest, + ) + 4 + }; + ip = ip.wrapping_add(1); + unsafe { + store_seq( + seq_store, + ip.offset_from(anchor) as usize, + anchor, + iend, + REPCODE1_TO_OFFBASE, + match_length, + ) + }; + let (next_ip, next_anchor) = unsafe { + post_dict_match( + hash_long, + hash_small, + base, + dict_base, + prefix_lowest_index, + dict_start_index, + dict_index_delta, + dict_end, + prefix_lowest, + hash_log, + chain_log, + mls, + seq_store, + &mut offset1, + &mut offset2, + ip, + match_length, + current, + iend, + ilimit, + ) + }; + ip = next_ip; + anchor = next_anchor; + continue; + } } if match_index_long >= prefix_lowest_index && unsafe { read64(match_long) == read64(ip) } { @@ -1027,6 +1074,7 @@ unsafe fn compress_block_double_fast_dict_match_state( base, dict_base, prefix_lowest_index, + dict_start_index, dict_index_delta, dict_end, prefix_lowest, @@ -1091,6 +1139,7 @@ unsafe fn compress_block_double_fast_dict_match_state( base, dict_base, prefix_lowest_index, + dict_start_index, dict_index_delta, dict_end, prefix_lowest, @@ -1278,6 +1327,7 @@ unsafe fn compress_block_double_fast_dict_match_state( base, dict_base, prefix_lowest_index, + dict_start_index, dict_index_delta, dict_end, prefix_lowest, @@ -1817,3 +1867,83 @@ pub unsafe extern "C" fn ZSTD_rust_compressBlock_doubleFast_extDict( ) } } + +#[cfg(test)] +mod tests { + use super::{attached_rep_index, index_overlap_check}; + + #[test] + fn index_overlap_check_preserves_dictionary_and_prefix_boundaries() { + assert!(index_overlap_check(16, 12)); + assert!(!index_overlap_check(16, 13)); + assert!(index_overlap_check(16, 16)); + } + + #[test] + fn attached_rep_index_rejects_wrapped_post_match_coordinate() { + // This is the failing shape: 0x1fb - 0x770 wrapped to 0xfffffa8b. + assert_eq!(attached_rep_index(0, 0, 0, 0x1fb, 0, 0x770), None); + } + + #[test] + fn attached_rep_index_checks_dictionary_coordinate_boundaries() { + let prefix_lowest_index = 100; + let dict_start_index = 10; + let dict_index_delta = 20; + let current = 120; + + // The translated dictionary interval is [30, 100), with four bytes + // available at the upper boundary only through index 96. + assert_eq!( + attached_rep_index( + prefix_lowest_index, + dict_start_index, + dict_index_delta, + current, + 0, + 90, + ), + Some(30) + ); + assert_eq!( + attached_rep_index( + prefix_lowest_index, + dict_start_index, + dict_index_delta, + current, + 0, + 91, + ), + None + ); + assert_eq!( + attached_rep_index( + prefix_lowest_index, + dict_start_index, + dict_index_delta, + current, + 0, + 24, + ), + Some(96) + ); + assert_eq!( + attached_rep_index( + prefix_lowest_index, + dict_start_index, + dict_index_delta, + current, + 0, + 23, + ), + None + ); + } + + #[test] + fn attached_rep_index_accepts_prefix_boundary_but_not_zero_offset() { + assert_eq!(attached_rep_index(100, 10, 20, 120, 0, 20), Some(100)); + assert_eq!(attached_rep_index(100, 10, 20, 120, 1, 21), Some(100)); + assert_eq!(attached_rep_index(100, 10, 20, 120, 1, 0), None); + } +} diff --git a/rust/src/zstd_opt.rs b/rust/src/zstd_opt.rs index 52ae4cb73..cd0124946 100644 --- a/rust/src/zstd_opt.rs +++ b/rust/src/zstd_opt.rs @@ -363,8 +363,14 @@ fn lowest_match_index(state: &ZSTD_RustOptState, current: u32) -> u32 { } #[inline] -fn index_overlap_check(prefix_lowest_index: u32, rep_index: u32) -> bool { - prefix_lowest_index.wrapping_sub(1).wrapping_sub(rep_index) >= 3 +fn index_overlap_check(prefix_lowest_index: u32, rep_index: u32, current: u32) -> bool { + if rep_index > current { + return false; + } + rep_index >= prefix_lowest_index + || rep_index + .checked_add(4) + .is_some_and(|rep_end| rep_end <= prefix_lowest_index) } #[inline] @@ -1006,7 +1012,7 @@ unsafe fn insert_bt_and_get_all_matches( }; if dict_mode == DICT_EXT && rep_offset.wrapping_sub(1) < current.wrapping_sub(window_low) - && index_overlap_check(dict_limit, rep_index) + && index_overlap_check(dict_limit, rep_index, current) && unsafe { read_min_match(ip, min_match) } == unsafe { read_min_match(rep_match, min_match) } { @@ -1023,7 +1029,7 @@ unsafe fn insert_bt_and_get_all_matches( if dict_mode == DICT_MATCH_STATE && rep_offset.wrapping_sub(1) < current.wrapping_sub(dms_low_limit.wrapping_add(dms_index_delta)) - && index_overlap_check(dict_limit, rep_index) + && index_overlap_check(dict_limit, rep_index, current) && unsafe { read_min_match(ip, min_match) } == unsafe { read_min_match(rep_match, min_match) } { @@ -1849,9 +1855,25 @@ pub unsafe extern "C" fn ZSTD_rust_compressBlock_btultra2( #[cfg(test)] mod tests { - use super::{downscale_stats, refresh_window_projection, ZSTD_RustOptState}; + use super::{ + downscale_stats, index_overlap_check, refresh_window_projection, ZSTD_RustOptState, + }; use std::ptr; + #[test] + fn index_overlap_check_accepts_exact_non_overlapping_boundary() { + assert!(index_overlap_check(16, 12, 16)); + assert!(!index_overlap_check(16, 13, 16)); + assert!(index_overlap_check(16, 16, 16)); + } + + #[test] + fn index_overlap_check_rejects_wrapped_indices() { + assert!(!index_overlap_check(0, u32::MAX - 3, 0x1fb)); + assert!(!index_overlap_check(3, u32::MAX, 2)); + assert!(index_overlap_check(u32::MAX, u32::MAX, u32::MAX)); + } + #[test] fn ultra2_window_projection_tracks_the_second_pass_window() { let mut base = 0x2000usize as *const u8;