From cb2c1aede14ab7ae4d802307bbafda37eaf8bca4 Mon Sep 17 00:00:00 2001 From: ddidderr Date: Mon, 20 Jul 2026 16:25:52 +0200 Subject: [PATCH] feat(cdict): move static workspace sizing policy to Rust ZSTD_initStaticCDict previously resolved automatic row matching and computed the minimum workspace in C before handing the result to the Rust construction orchestrator. That left the public static-CDict sizing branch split from the Rust validation and callback failure policy. Make the Rust projection carry dictionary size, load method, compression parameters, and the C-owned layout sizing inputs. Rust now resolves automatic row matching, computes the dedicated-search workspace requirement, validates workspace capacity, and passes the resolved mode into the initializer while preserving create/reserve/move/init order. C retains opaque private cwksp and CDict operations, allocator/layout behavior, and dictionary-content loading. Add focused tests for computed sizing policy, capacity boundaries, callback order, reservation and initialization failures, and null or unaligned workspaces. Test Plan: - `rustfmt --edition 2021 --check rust/src/zstd_compress_dictionary.rs` -- passed - GCC `-fsyntax-only` checks with multithreaded, non-multithreaded, `ZSTD_DISABLE_ASM=1`, and `ZSTD_ADDRESS_SANITIZER=1` configurations -- passed - Clang `-fsyntax-only` check with `ZSTD_MULTITHREAD` -- passed - `git diff --check` and `git diff --cached --check` -- passed - Cargo, make, native builds, and large tests were not run per task constraints --- lib/compress/zstd_compress.c | 50 ++++---- rust/src/zstd_compress_dictionary.rs | 170 +++++++++++++++++++++------ 2 files changed, 164 insertions(+), 56 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 5e0e1d024..295fe3420 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -462,14 +462,17 @@ typedef void* (*ZSTD_rust_initStaticCDictReserveObject_f)(void* context); typedef void (*ZSTD_rust_initStaticCDictMoveWorkspace_f)( void* context, void* cdict); typedef void (*ZSTD_rust_initStaticCDictInitialize_f)( - void* context, void* cdict); + void* context, void* cdict, int useRowMatchFinder); typedef void* (*ZSTD_rust_initStaticCDictInit_f)( void* context, void* cdict); typedef struct { void* callbackContext; void* workspace; size_t workspaceSize; - size_t neededSize; + size_t dictSize; + size_t dictLoadMethod; + const ZSTD_compressionParameters* cParams; + const void* sizing; ZSTD_rust_initStaticCDictCreateWorkspace_f createWorkspace; ZSTD_rust_initStaticCDictReserveObject_f reserveObject; ZSTD_rust_initStaticCDictMoveWorkspace_f moveWorkspace; @@ -483,19 +486,25 @@ typedef char ZSTD_rust_init_static_cdict_state_layout[ == sizeof(void*) && offsetof(ZSTD_rust_initStaticCDictState, workspaceSize) == 2 * sizeof(void*) - && offsetof(ZSTD_rust_initStaticCDictState, neededSize) + && offsetof(ZSTD_rust_initStaticCDictState, dictSize) == 3 * sizeof(void*) - && offsetof(ZSTD_rust_initStaticCDictState, createWorkspace) + && offsetof(ZSTD_rust_initStaticCDictState, dictLoadMethod) == 4 * sizeof(void*) - && offsetof(ZSTD_rust_initStaticCDictState, reserveObject) + && offsetof(ZSTD_rust_initStaticCDictState, cParams) == 5 * sizeof(void*) - && offsetof(ZSTD_rust_initStaticCDictState, moveWorkspace) + && offsetof(ZSTD_rust_initStaticCDictState, sizing) == 6 * sizeof(void*) - && offsetof(ZSTD_rust_initStaticCDictState, initialize) + && offsetof(ZSTD_rust_initStaticCDictState, createWorkspace) == 7 * sizeof(void*) - && offsetof(ZSTD_rust_initStaticCDictState, init) + && offsetof(ZSTD_rust_initStaticCDictState, reserveObject) == 8 * sizeof(void*) - && sizeof(ZSTD_rust_initStaticCDictState) == 9 * sizeof(void*)) + && offsetof(ZSTD_rust_initStaticCDictState, moveWorkspace) + == 9 * sizeof(void*) + && offsetof(ZSTD_rust_initStaticCDictState, initialize) + == 10 * sizeof(void*) + && offsetof(ZSTD_rust_initStaticCDictState, init) + == 11 * sizeof(void*) + && sizeof(ZSTD_rust_initStaticCDictState) == 12 * sizeof(void*)) ? 1 : -1]; typedef void (*ZSTD_rust_clearAllDictsCallback_f)(void* context); typedef struct { @@ -7252,7 +7261,6 @@ typedef struct { ZSTD_dictLoadMethod_e dictLoadMethod; ZSTD_dictContentType_e dictContentType; ZSTD_compressionParameters cParams; - ZSTD_ParamSwitch_e useRowMatchFinder; ZSTD_cwksp workspaceState; ZSTD_CCtx_params params; } ZSTD_rust_initStaticCDictContext; @@ -7280,7 +7288,8 @@ static void ZSTD_rust_initStaticCDict_moveWorkspace( ZSTD_cwksp_move(&((ZSTD_CDict*)cdict)->workspace, &context->workspaceState); } -static void ZSTD_rust_initStaticCDict_initialize(void* opaque, void* cdict) +static void ZSTD_rust_initStaticCDict_initialize( + void* opaque, void* cdict, int useRowMatchFinder) { ZSTD_rust_initStaticCDictContext* const context = (ZSTD_rust_initStaticCDictContext*)opaque; @@ -7289,8 +7298,8 @@ static void ZSTD_rust_initStaticCDict_initialize(void* opaque, void* cdict) ZSTD_CCtxParams_init(params, 0); params->cParams = context->cParams; - params->useRowMatchFinder = context->useRowMatchFinder; - dictionary->useRowMatchFinder = context->useRowMatchFinder; + params->useRowMatchFinder = (ZSTD_ParamSwitch_e)useRowMatchFinder; + dictionary->useRowMatchFinder = (ZSTD_ParamSwitch_e)useRowMatchFinder; dictionary->compressionLevel = ZSTD_NO_CLEVEL; } @@ -7313,8 +7322,6 @@ const ZSTD_CDict* ZSTD_initStaticCDict( ZSTD_dictContentType_e dictContentType, ZSTD_compressionParameters cParams) { - ZSTD_ParamSwitch_e const useRowMatchFinder = (ZSTD_ParamSwitch_e) - ZSTD_resolveRowMatchFinderMode((int)ZSTD_ps_auto, cParams); ZSTD_rustCDictSizing const sizing = { sizeof(ZSTD_CDict), HUF_WORKSPACE_SIZE, @@ -7323,15 +7330,12 @@ const ZSTD_CDict* ZSTD_initStaticCDict( sizeof(ZSTD_optimal_t), ZSTD_RUST_ASAN_REDZONE_SIZE }; - /* Dedicated search keeps the match state large enough for a later DDS - * plus row-hash use of this static CDict. */ - size_t const neededSize = ZSTD_rust_params_estimateCDictWorkspaceSize( - dictSize, cParams, (int)dictLoadMethod, - useRowMatchFinder, 1, &sizing); ZSTD_rust_initStaticCDictContext context; ZSTD_rust_initStaticCDictState state; DEBUGLOG(4, "ZSTD_initStaticCDict (dictSize==%u)", (unsigned)dictSize); + /* Rust resolves row matching and computes the required workspace size; + * these callbacks retain only private workspace and CDict operations. */ context.workspace = workspace; context.workspaceSize = workspaceSize; context.dict = dict; @@ -7339,11 +7343,13 @@ const ZSTD_CDict* ZSTD_initStaticCDict( context.dictLoadMethod = dictLoadMethod; context.dictContentType = dictContentType; context.cParams = cParams; - context.useRowMatchFinder = useRowMatchFinder; state.callbackContext = &context; state.workspace = workspace; state.workspaceSize = workspaceSize; - state.neededSize = neededSize; + state.dictSize = dictSize; + state.dictLoadMethod = (size_t)dictLoadMethod; + state.cParams = &cParams; + state.sizing = &sizing; state.createWorkspace = ZSTD_rust_initStaticCDict_createWorkspace; state.reserveObject = ZSTD_rust_initStaticCDict_reserveObject; state.moveWorkspace = ZSTD_rust_initStaticCDict_moveWorkspace; diff --git a/rust/src/zstd_compress_dictionary.rs b/rust/src/zstd_compress_dictionary.rs index 53253ea5a..a31efc197 100644 --- a/rust/src/zstd_compress_dictionary.rs +++ b/rust/src/zstd_compress_dictionary.rs @@ -19,9 +19,10 @@ use crate::zstd_compress_params::{ ZSTD_compressionParameters, ZSTD_frameParameters, ZSTD_parameters, ZSTD_rustCDictSizing, ZSTD_rust_params_allocateChainTable, ZSTD_rust_params_defaultCLevel, ZSTD_rust_params_estimateCDictWorkspaceSize, ZSTD_rust_params_getCParams, - ZSTD_rust_params_getParamsInternal, ZSTD_rust_params_rowMatchFinderUsed, - ZSTD_rust_params_shouldAttachDict, ZSTD_CONTENTSIZE_UNKNOWN, ZSTD_RUST_CPM_CREATE_CDICT, - ZSTD_RUST_CPM_NO_ATTACH_DICT, + ZSTD_rust_params_getParamsInternal, ZSTD_rust_params_resolveRowMatchFinderMode, + ZSTD_rust_params_rowMatchFinderUsed, ZSTD_rust_params_shouldAttachDict, + ZSTD_CONTENTSIZE_UNKNOWN, ZSTD_RUST_CPM_CREATE_CDICT, ZSTD_RUST_CPM_NO_ATTACH_DICT, + ZSTD_RUST_PS_AUTO, }; use crate::zstd_compress_params_api::ZSTD_CCtx_params; use crate::zstd_compress_stats::{ @@ -1081,20 +1082,23 @@ pub unsafe extern "C" fn ZSTD_rust_freeCDict(state: *const ZSTD_rust_freeCDictSt type InitStaticCDictCreateWorkspaceFn = unsafe extern "C" fn(*mut c_void); type InitStaticCDictReserveObjectFn = unsafe extern "C" fn(*mut c_void) -> *mut c_void; type InitStaticCDictMoveWorkspaceFn = unsafe extern "C" fn(*mut c_void, *mut c_void); -type InitStaticCDictInitializeFn = unsafe extern "C" fn(*mut c_void, *mut c_void); +type InitStaticCDictInitializeFn = unsafe extern "C" fn(*mut c_void, *mut c_void, c_int); type InitStaticCDictFn = unsafe extern "C" fn(*mut c_void, *mut c_void) -> *mut c_void; /// Explicit projection for the public `ZSTD_initStaticCDict` wrapper. /// -/// Rust owns workspace pointer/alignment validation, minimum-size validation, -/// and callback ordering. C retains the private static-workspace layout and -/// dictionary initialization behind narrow callbacks. +/// Rust owns row-match resolution, workspace-size policy, workspace +/// validation, and callback ordering. C retains the private static-workspace +/// layout and dictionary initialization behind narrow callbacks. #[repr(C)] pub struct ZSTD_rust_initStaticCDictState { callback_context: *mut c_void, workspace: *mut c_void, workspace_size: usize, - needed_size: usize, + dict_size: usize, + dict_load_method: usize, + c_params: *const ZSTD_compressionParameters, + sizing: *const ZSTD_rustCDictSizing, create_workspace: InitStaticCDictCreateWorkspaceFn, reserve_object: InitStaticCDictReserveObjectFn, move_workspace: InitStaticCDictMoveWorkspaceFn, @@ -1106,13 +1110,16 @@ const _: () = { assert!(offset_of!(ZSTD_rust_initStaticCDictState, callback_context) == 0); assert!(offset_of!(ZSTD_rust_initStaticCDictState, workspace) == size_of::()); assert!(offset_of!(ZSTD_rust_initStaticCDictState, workspace_size) == 2 * size_of::()); - assert!(offset_of!(ZSTD_rust_initStaticCDictState, needed_size) == 3 * size_of::()); - assert!(offset_of!(ZSTD_rust_initStaticCDictState, create_workspace) == 4 * size_of::()); - assert!(offset_of!(ZSTD_rust_initStaticCDictState, reserve_object) == 5 * size_of::()); - assert!(offset_of!(ZSTD_rust_initStaticCDictState, move_workspace) == 6 * size_of::()); - assert!(offset_of!(ZSTD_rust_initStaticCDictState, initialize) == 7 * size_of::()); - assert!(offset_of!(ZSTD_rust_initStaticCDictState, init) == usize::BITS as usize); - assert!(size_of::() == size_of::<[usize; 9]>()); + assert!(offset_of!(ZSTD_rust_initStaticCDictState, dict_size) == 3 * size_of::()); + assert!(offset_of!(ZSTD_rust_initStaticCDictState, dict_load_method) == 4 * size_of::()); + assert!(offset_of!(ZSTD_rust_initStaticCDictState, c_params) == 5 * size_of::()); + assert!(offset_of!(ZSTD_rust_initStaticCDictState, sizing) == 6 * size_of::()); + assert!(offset_of!(ZSTD_rust_initStaticCDictState, create_workspace) == 7 * size_of::()); + assert!(offset_of!(ZSTD_rust_initStaticCDictState, reserve_object) == 8 * size_of::()); + assert!(offset_of!(ZSTD_rust_initStaticCDictState, move_workspace) == 9 * size_of::()); + assert!(offset_of!(ZSTD_rust_initStaticCDictState, initialize) == 10 * size_of::()); + assert!(offset_of!(ZSTD_rust_initStaticCDictState, init) == 11 * size_of::()); + assert!(size_of::() == size_of::<[usize; 12]>()); }; /// Validate static CDict workspace ownership before entering the C leaf. @@ -1126,11 +1133,28 @@ pub unsafe extern "C" fn ZSTD_rust_initStaticCDict( let state = unsafe { &*state }; if state.callback_context.is_null() || state.workspace.is_null() + || state.c_params.is_null() + || state.sizing.is_null() || (state.workspace as usize) & (ZSTD_STATIC_WORKSPACE_ALIGNMENT - 1) != 0 - || state.workspace_size < state.needed_size { return ptr::null_mut(); } + let c_params = unsafe { *state.c_params }; + let use_row_match_finder = + ZSTD_rust_params_resolveRowMatchFinderMode(ZSTD_RUST_PS_AUTO, c_params); + let needed_size = unsafe { + ZSTD_rust_params_estimateCDictWorkspaceSize( + state.dict_size, + c_params, + state.dict_load_method as c_int, + use_row_match_finder, + 1, + state.sizing, + ) + }; + if state.workspace_size < needed_size { + return ptr::null_mut(); + } unsafe { (state.create_workspace)(state.callback_context); } @@ -1140,7 +1164,7 @@ pub unsafe extern "C" fn ZSTD_rust_initStaticCDict( } unsafe { (state.move_workspace)(state.callback_context, cdict); - (state.initialize)(state.callback_context, cdict); + (state.initialize)(state.callback_context, cdict, use_row_match_finder); (state.init)(state.callback_context, cdict) } } @@ -5134,6 +5158,7 @@ mod tests { moved_cdict: *mut c_void, initialized_cdict: *mut c_void, init_cdict: *mut c_void, + use_row_match_finder: c_int, initialized: bool, result: *mut c_void, } @@ -5163,10 +5188,12 @@ mod tests { unsafe extern "C" fn init_static_cdict_test_initialize( context: *mut c_void, cdict: *mut c_void, + use_row_match_finder: c_int, ) { let probe = unsafe { &mut *context.cast::() }; probe.events.push("initialize"); probe.initialized_cdict = cdict; + probe.use_row_match_finder = use_row_match_finder; probe.initialized = true; } @@ -5184,13 +5211,19 @@ mod tests { probe: &mut InitStaticCDictProbe, workspace: *mut c_void, workspace_size: usize, - needed_size: usize, + dict_size: usize, + dict_load_method: c_int, + c_params: &ZSTD_compressionParameters, + sizing: *const ZSTD_rustCDictSizing, ) -> ZSTD_rust_initStaticCDictState { ZSTD_rust_initStaticCDictState { callback_context: (probe as *mut InitStaticCDictProbe).cast(), workspace, workspace_size, - needed_size, + dict_size, + dict_load_method: dict_load_method as usize, + c_params, + sizing, create_workspace: init_static_cdict_test_create_workspace, reserve_object: init_static_cdict_test_reserve_object, move_workspace: init_static_cdict_test_move_workspace, @@ -5199,6 +5232,37 @@ mod tests { } } + fn init_static_cdict_test_cparams() -> ZSTD_compressionParameters { + ZSTD_compressionParameters { + windowLog: 10, + chainLog: 10, + hashLog: 10, + searchLog: 1, + minMatch: 4, + targetLength: 0, + strategy: ZSTD_FAST_STRATEGY as c_int, + } + } + + fn init_static_cdict_expected_workspace_size( + dict_size: usize, + dict_load_method: c_int, + c_params: ZSTD_compressionParameters, + ) -> usize { + let use_row_match_finder = + ZSTD_rust_params_resolveRowMatchFinderMode(ZSTD_RUST_PS_AUTO, c_params); + unsafe { + ZSTD_rust_params_estimateCDictWorkspaceSize( + dict_size, + c_params, + dict_load_method, + use_row_match_finder, + 1, + &CREATE_CDICT_ADVANCED_TEST_SIZING, + ) + } + } + #[test] fn init_static_cdict_rejects_an_unaligned_workspace_before_callback() { let mut probe = InitStaticCDictProbe { @@ -5207,7 +5271,16 @@ mod tests { }; let mut workspace = [0usize; 2]; let unaligned_workspace = workspace.as_mut_ptr().cast::().wrapping_add(1).cast(); - let state = init_static_cdict_test_state(&mut probe, unaligned_workspace, usize::MAX, 0); + let c_params = init_static_cdict_test_cparams(); + let state = init_static_cdict_test_state( + &mut probe, + unaligned_workspace, + usize::MAX, + 0, + ZSTD_DLM_BY_REF, + &c_params, + &CREATE_CDICT_ADVANCED_TEST_SIZING, + ); let result = unsafe { ZSTD_rust_initStaticCDict(&state) }; @@ -5222,11 +5295,15 @@ mod tests { ..Default::default() }; let mut workspace = [0usize; 1]; + let c_params = init_static_cdict_test_cparams(); let state = init_static_cdict_test_state( &mut probe, workspace.as_mut_ptr().cast(), size_of_val(&workspace), - size_of_val(&workspace) + 1, + 0, + ZSTD_DLM_BY_REF, + &c_params, + &CREATE_CDICT_ADVANCED_TEST_SIZING, ); let result = unsafe { ZSTD_rust_initStaticCDict(&state) }; @@ -5236,19 +5313,23 @@ mod tests { } #[test] - fn init_static_cdict_calls_the_initializer_after_workspace_validation() { + fn init_static_cdict_computes_workspace_policy_before_initialization() { let cdict = ptr::dangling_mut::(); let mut probe = InitStaticCDictProbe { reserved_cdict: cdict, result: ptr::dangling_mut(), ..Default::default() }; - let mut workspace = [0usize; 1]; + let c_params = init_static_cdict_test_cparams(); + let needed_size = init_static_cdict_expected_workspace_size(0, ZSTD_DLM_BY_REF, c_params); let state = init_static_cdict_test_state( &mut probe, - workspace.as_mut_ptr().cast(), - size_of_val(&workspace), - size_of_val(&workspace), + ptr::dangling_mut(), + needed_size, + 0, + ZSTD_DLM_BY_REF, + &c_params, + &CREATE_CDICT_ADVANCED_TEST_SIZING, ); let result = unsafe { ZSTD_rust_initStaticCDict(&state) }; @@ -5267,18 +5348,26 @@ mod tests { assert_eq!(probe.moved_cdict, cdict); assert_eq!(probe.initialized_cdict, cdict); assert_eq!(probe.init_cdict, cdict); + assert_eq!( + probe.use_row_match_finder, + ZSTD_rust_params_resolveRowMatchFinderMode(ZSTD_RUST_PS_AUTO, c_params) + ); assert!(probe.initialized); } #[test] fn init_static_cdict_stops_after_object_reservation_failure() { let mut probe = InitStaticCDictProbe::default(); - let mut workspace = [0usize; 1]; + let c_params = init_static_cdict_test_cparams(); + let needed_size = init_static_cdict_expected_workspace_size(0, ZSTD_DLM_BY_REF, c_params); let state = init_static_cdict_test_state( &mut probe, - workspace.as_mut_ptr().cast(), - size_of_val(&workspace), + ptr::dangling_mut(), + needed_size, 0, + ZSTD_DLM_BY_REF, + &c_params, + &CREATE_CDICT_ADVANCED_TEST_SIZING, ); let result = unsafe { ZSTD_rust_initStaticCDict(&state) }; @@ -5297,12 +5386,16 @@ mod tests { result: ptr::null_mut(), ..Default::default() }; - let mut workspace = [0usize; 1]; + let c_params = init_static_cdict_test_cparams(); + let needed_size = init_static_cdict_expected_workspace_size(0, ZSTD_DLM_BY_REF, c_params); let state = init_static_cdict_test_state( &mut probe, - workspace.as_mut_ptr().cast(), - size_of_val(&workspace), - size_of_val(&workspace), + ptr::dangling_mut(), + needed_size, + 0, + ZSTD_DLM_BY_REF, + &c_params, + &CREATE_CDICT_ADVANCED_TEST_SIZING, ); let result = unsafe { ZSTD_rust_initStaticCDict(&state) }; @@ -5328,7 +5421,16 @@ mod tests { result: ptr::dangling_mut(), ..Default::default() }; - let state = init_static_cdict_test_state(&mut probe, ptr::null_mut(), usize::MAX, 0); + let c_params = init_static_cdict_test_cparams(); + let state = init_static_cdict_test_state( + &mut probe, + ptr::null_mut(), + usize::MAX, + 0, + ZSTD_DLM_BY_REF, + &c_params, + &CREATE_CDICT_ADVANCED_TEST_SIZING, + ); let result = unsafe { ZSTD_rust_initStaticCDict(&state) };