From 608f1bfc4cf4eb92470cb8c31373effd511d0db9 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 11 May 2020 18:16:38 -0700 Subject: [PATCH 1/5] fixed context downsize with initStatic When context is created using initStatic, no resize is possible. fix : only bump oversizeDuration when !initStatic --- lib/compress/zstd_compress.c | 21 +++++++++------------ tests/fuzzer.c | 22 ++++++++++++++++++---- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 538f8e9e..8776c5cd 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -15,7 +15,7 @@ #include /* memset */ #include "../common/cpu.h" #include "../common/mem.h" -#include "hist.h" /* HIST_countFast_wksp */ +#include "hist.h" /* HIST_countFast_wksp */ #define FSE_STATIC_LINKING_ONLY /* FSE_encodeSymbol */ #include "../common/fse.h" #define HUF_STATIC_LINKING_ONLY @@ -90,7 +90,7 @@ ZSTD_CCtx* ZSTD_createCCtx_advanced(ZSTD_customMem customMem) } } -ZSTD_CCtx* ZSTD_initStaticCCtx(void *workspace, size_t workspaceSize) +ZSTD_CCtx* ZSTD_initStaticCCtx(void* workspace, size_t workspaceSize) { ZSTD_cwksp ws; ZSTD_CCtx* cctx; @@ -99,9 +99,8 @@ ZSTD_CCtx* ZSTD_initStaticCCtx(void *workspace, size_t workspaceSize) ZSTD_cwksp_init(&ws, workspace, workspaceSize); cctx = (ZSTD_CCtx*)ZSTD_cwksp_reserve_object(&ws, sizeof(ZSTD_CCtx)); - if (cctx == NULL) { - return NULL; - } + if (cctx == NULL) return NULL; + memset(cctx, 0, sizeof(ZSTD_CCtx)); ZSTD_cwksp_move(&cctx->workspace, &ws); cctx->staticSize = workspaceSize; @@ -110,8 +109,7 @@ ZSTD_CCtx* ZSTD_initStaticCCtx(void *workspace, size_t workspaceSize) if (!ZSTD_cwksp_check_available(&cctx->workspace, HUF_WORKSPACE_SIZE + 2 * sizeof(ZSTD_compressedBlockState_t))) return NULL; cctx->blockState.prevCBlock = (ZSTD_compressedBlockState_t*)ZSTD_cwksp_reserve_object(&cctx->workspace, sizeof(ZSTD_compressedBlockState_t)); cctx->blockState.nextCBlock = (ZSTD_compressedBlockState_t*)ZSTD_cwksp_reserve_object(&cctx->workspace, sizeof(ZSTD_compressedBlockState_t)); - cctx->entropyWorkspace = (U32*)ZSTD_cwksp_reserve_object( - &cctx->workspace, HUF_WORKSPACE_SIZE); + cctx->entropyWorkspace = (U32*)ZSTD_cwksp_reserve_object(&cctx->workspace, HUF_WORKSPACE_SIZE); cctx->bmi2 = ZSTD_cpuid_bmi2(ZSTD_cpuid()); return cctx; } @@ -421,9 +419,8 @@ ZSTD_bounds ZSTD_cParam_getBounds(ZSTD_cParameter param) return bounds; default: - { ZSTD_bounds const boundError = { ERROR(parameter_unsupported), 0, 0 }; - return boundError; - } + bounds.error = ERROR(parameter_unsupported); + return bounds; } } @@ -1458,7 +1455,7 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, needsIndexReset = ZSTDirp_reset; } - ZSTD_cwksp_bump_oversized_duration(ws, 0); + if (!zc->staticSize) ZSTD_cwksp_bump_oversized_duration(ws, 0); /* Check if workspace is large enough, alloc a new one if needed */ { size_t const cctxSpace = zc->staticSize ? ZSTD_cwksp_alloc_size(sizeof(ZSTD_CCtx)) : 0; @@ -1774,7 +1771,7 @@ static size_t ZSTD_copyCCtx_internal(ZSTD_CCtx* dstCCtx, ZSTD_buffered_policy_e zbuff) { DEBUGLOG(5, "ZSTD_copyCCtx_internal"); - RETURN_ERROR_IF(srcCCtx->stage!=ZSTDcs_init, stage_wrong, + RETURN_ERROR_IF(srcCCtx->stage!=ZSTDcs_init, stage_wrong, "Can't copy a ctx that's not in init stage."); memcpy(&dstCCtx->customMem, &srcCCtx->customMem, sizeof(ZSTD_customMem)); diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 1983ae14..4efaf519 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -32,6 +32,7 @@ #include "fse.h" #include "zstd.h" /* ZSTD_VERSION_STRING */ #include "zstd_errors.h" /* ZSTD_getErrorCode */ +#include "zstd_internal.h" /* ZSTD_WORKSPACETOOLARGE_MAXDURATION */ #include "zstdmt_compress.h" #define ZDICT_STATIC_LINKING_ONLY #include "zdict.h" /* ZDICT_trainFromBuffer */ @@ -1065,7 +1066,7 @@ static int basicUnitTests(U32 const seed, double compressibility) DISPLAYLEVEL(3, "OK \n"); /* Static CCtx tests */ -#define STATIC_CCTX_LEVEL 3 +#define STATIC_CCTX_LEVEL 4 DISPLAYLEVEL(3, "test%3i : create static CCtx for level %u : ", testNb++, STATIC_CCTX_LEVEL); { size_t const staticCStreamSize = ZSTD_estimateCStreamSize(STATIC_CCTX_LEVEL); void* const staticCCtxBuffer = malloc(staticCStreamSize); @@ -1079,19 +1080,31 @@ static int basicUnitTests(U32 const seed, double compressibility) testResult = 1; goto _end; } - { size_t const staticCCtxSize = ZSTD_estimateCCtxSize(STATIC_CCTX_LEVEL); + { size_t const smallInSize = 32 KB; + size_t const staticCCtxSize = ZSTD_estimateCCtxSize(STATIC_CCTX_LEVEL); ZSTD_CCtx* staticCCtx = ZSTD_initStaticCCtx(staticCCtxBuffer, staticCCtxSize); ZSTD_DCtx* const staticDCtx = ZSTD_initStaticDCtx(staticDCtxBuffer, staticDCtxSize); - if ((staticCCtx==NULL) || (staticDCtx==NULL)) goto _output_error; DISPLAYLEVEL(4, "CCtx size = %u, ", (U32)staticCCtxSize); + if ((staticCCtx==NULL) || (staticDCtx==NULL)) goto _output_error; DISPLAYLEVEL(3, "OK \n"); - + /* DISPLAYLEVEL(3, "test%3i : compress immediately with static CCtx : ", testNb++); CHECK_VAR(cSize, ZSTD_compressCCtx(staticCCtx, compressedBuffer, compressedBufferSize, CNBuffer, CNBuffSize, STATIC_CCTX_LEVEL) ); DISPLAYLEVEL(3, "OK (%u bytes : %.2f%%)\n", (unsigned)cSize, (double)cSize/CNBuffSize*100); + */ + DISPLAYLEVEL(3, "test%3i : compress small input often enough to trigger context reduce : ", testNb++); + { int nbc; + assert(CNBuffSize > smallInSize + ZSTD_WORKSPACETOOLARGE_MAXDURATION + 3); + for (nbc=0; nbc Date: Mon, 11 May 2020 18:50:10 -0700 Subject: [PATCH 2/5] updated initStatic tests differentiate small CCtx for small inputs from full CCtx from CStream contexts. Ensure allocation & resize tests are more precise. --- tests/fuzzer.c | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 4efaf519..0202b331 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -32,7 +32,7 @@ #include "fse.h" #include "zstd.h" /* ZSTD_VERSION_STRING */ #include "zstd_errors.h" /* ZSTD_getErrorCode */ -#include "zstd_internal.h" /* ZSTD_WORKSPACETOOLARGE_MAXDURATION */ +#include "zstd_internal.h" /* ZSTD_WORKSPACETOOLARGE_MAXDURATION, ZSTD_WORKSPACETOOLARGE_FACTOR, KB, MB */ #include "zstdmt_compress.h" #define ZDICT_STATIC_LINKING_ONLY #include "zdict.h" /* ZDICT_trainFromBuffer */ @@ -47,8 +47,6 @@ /*-************************************ * Constants **************************************/ -#define KB *(1U<<10) -#define MB *(1U<<20) #define GB *(1U<<30) static const int FUZ_compressibility_default = 50; @@ -1081,20 +1079,44 @@ static int basicUnitTests(U32 const seed, double compressibility) goto _end; } { size_t const smallInSize = 32 KB; + ZSTD_compressionParameters const cparams_small = ZSTD_getCParams(STATIC_CCTX_LEVEL, smallInSize, 0); + size_t const smallCCtxSize = ZSTD_estimateCCtxSize_usingCParams(cparams_small); size_t const staticCCtxSize = ZSTD_estimateCCtxSize(STATIC_CCTX_LEVEL); - ZSTD_CCtx* staticCCtx = ZSTD_initStaticCCtx(staticCCtxBuffer, staticCCtxSize); + ZSTD_CCtx* staticCCtx = ZSTD_initStaticCCtx(staticCCtxBuffer, smallCCtxSize); ZSTD_DCtx* const staticDCtx = ZSTD_initStaticDCtx(staticDCtxBuffer, staticDCtxSize); - DISPLAYLEVEL(4, "CCtx size = %u, ", (U32)staticCCtxSize); + DISPLAYLEVEL(4, "Full CCtx size = %u, ", (U32)staticCCtxSize); + DISPLAYLEVEL(4, "CCtx for 32 KB = %u, ", (U32)smallCCtxSize); + assert(staticCCtxSize > smallCCtxSize * ZSTD_WORKSPACETOOLARGE_FACTOR); /* ensure size down scenario */ if ((staticCCtx==NULL) || (staticDCtx==NULL)) goto _output_error; DISPLAYLEVEL(3, "OK \n"); - /* - DISPLAYLEVEL(3, "test%3i : compress immediately with static CCtx : ", testNb++); + + DISPLAYLEVEL(3, "test%3i : compress small input with small static CCtx : ", testNb++); + CHECK_VAR(cSize, ZSTD_compressCCtx(staticCCtx, + compressedBuffer, compressedBufferSize, + CNBuffer, smallInSize, STATIC_CCTX_LEVEL) ); + DISPLAYLEVEL(3, "OK (%u bytes : %.2f%%)\n", + (unsigned)cSize, (double)cSize/smallInSize*100); + + DISPLAYLEVEL(3, "test%3i : compress large input with small static CCtx (must fail) : ", testNb++); + { size_t const r = ZSTD_compressCCtx(staticCCtx, + compressedBuffer, compressedBufferSize, + CNBuffer, CNBuffSize, STATIC_CCTX_LEVEL); + if (ZSTD_getErrorCode((size_t)r) != ZSTD_error_memory_allocation) goto _output_error; + } + DISPLAYLEVEL(3, "OK \n"); + + DISPLAYLEVEL(3, "test%3i : resize context to full CCtx size : ", testNb++); + staticCCtx = ZSTD_initStaticCStream(staticCCtxBuffer, staticCCtxSize); + if ((void*)staticCCtx != staticCCtxBuffer) goto _output_error; + DISPLAYLEVEL(3, "OK \n"); + + DISPLAYLEVEL(3, "test%3i : compress large input with static CCtx : ", testNb++); CHECK_VAR(cSize, ZSTD_compressCCtx(staticCCtx, compressedBuffer, compressedBufferSize, CNBuffer, CNBuffSize, STATIC_CCTX_LEVEL) ); DISPLAYLEVEL(3, "OK (%u bytes : %.2f%%)\n", (unsigned)cSize, (double)cSize/CNBuffSize*100); - */ + DISPLAYLEVEL(3, "test%3i : compress small input often enough to trigger context reduce : ", testNb++); { int nbc; assert(CNBuffSize > smallInSize + ZSTD_WORKSPACETOOLARGE_MAXDURATION + 3); @@ -1148,6 +1170,7 @@ static int basicUnitTests(U32 const seed, double compressibility) DISPLAYLEVEL(3, "test%3i : resize context to CStream size, then stream compress : ", testNb++); staticCCtx = ZSTD_initStaticCStream(staticCCtxBuffer, staticCStreamSize); + assert(staticCCtx != NULL); CHECK_Z( ZSTD_initCStream(staticCCtx, STATIC_CCTX_LEVEL) ); /* note : doesn't allocate */ { ZSTD_outBuffer output = { compressedBuffer, compressedBufferSize, 0 }; ZSTD_inBuffer input = { CNBuffer, CNBuffSize, 0 }; From 76e726e3beb054a0e6d02d3314418277881a19f7 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 11 May 2020 19:21:50 -0700 Subject: [PATCH 3/5] updated documentation for ZSTD_estimate*() make it clearer that tighter memory estimation can be provided using advanced functions on the condition of a defined input size bound. --- lib/zstd.h | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/lib/zstd.h b/lib/zstd.h index 18c20c87..8c6fc6ae 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -274,9 +274,9 @@ typedef enum { * Default level is ZSTD_CLEVEL_DEFAULT==3. * Special: value 0 means default, which is controlled by ZSTD_CLEVEL_DEFAULT. * Note 1 : it's possible to pass a negative compression level. - * Note 2 : setting a level does not automatically set all other compression parameters - * to default. Setting this will however eventually dynamically impact the compression - * parameters which have not been manually set. The manually set + * Note 2 : setting a level does not automatically set all other compression parameters + * to default. Setting this will however eventually dynamically impact the compression + * parameters which have not been manually set. The manually set * ones will 'stick'. */ /* Advanced compression parameters : * It's possible to pin down compression parameters to some specific values. @@ -1268,23 +1268,28 @@ ZSTDLIB_API size_t ZSTD_getSequences(ZSTD_CCtx* zc, ZSTD_Sequence* outSeqs, ***************************************/ /*! ZSTD_estimate*() : - * These functions make it possible to estimate memory usage of a future - * {D,C}Ctx, before its creation. + * These functions make it possible to estimate memory usage + * of a future {D,C}Ctx, before its creation. * - * ZSTD_estimateCCtxSize() will provide a budget large enough for any - * compression level up to selected one. Unlike ZSTD_estimateCStreamSize*(), - * this estimate does not include space for a window buffer, so this estimate - * is guaranteed to be enough for single-shot compressions, but not streaming - * compressions. It will however assume the input may be arbitrarily large, - * which is the worst case. If srcSize is known to always be small, - * ZSTD_estimateCCtxSize_usingCParams() can provide a tighter estimation. - * ZSTD_estimateCCtxSize_usingCParams() can be used in tandem with - * ZSTD_getCParams() to create cParams from compressionLevel. - * ZSTD_estimateCCtxSize_usingCCtxParams() can be used in tandem with - * ZSTD_CCtxParams_setParameter(). + * ZSTD_estimateCCtxSize() will provide a memory budget large enough + * for any compression level up to selected one. + * Note : Unlike ZSTD_estimateCStreamSize*(), this estimate + * does not include space for a window buffer. + * Therefore, the estimation is only guaranteed for single-shot compressions, not streaming. + * The estimate will assume the input may be arbitrarily large, + * which is the worst case. * - * Note: only single-threaded compression is supported. This function will - * return an error code if ZSTD_c_nbWorkers is >= 1. */ + * When srcSize can be bound by a known and rather "small" value, + * this fact can be used to provide a tighter estimation + * because the CCtx compression context will need less memory. + * This tighter estimation can be provided by more advanced functions + * ZSTD_estimateCCtxSize_usingCParams(), which can be used in tandem with ZSTD_getCParams(), + * and ZSTD_estimateCCtxSize_usingCCtxParams(), which can be used in tandem with ZSTD_CCtxParams_setParameter(). + * Both can be used to estimate memory using custom compression parameters and arbitrary srcSize limits. + * + * Note 2 : only single-threaded compression is supported. + * ZSTD_estimateCCtxSize_usingCCtxParams() will return an error code if ZSTD_c_nbWorkers is >= 1. + */ ZSTDLIB_API size_t ZSTD_estimateCCtxSize(int compressionLevel); ZSTDLIB_API size_t ZSTD_estimateCCtxSize_usingCParams(ZSTD_compressionParameters cParams); ZSTDLIB_API size_t ZSTD_estimateCCtxSize_usingCCtxParams(const ZSTD_CCtx_params* params); From 20bd24604538fa3ac3c7147e54016c0e34a51267 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 11 May 2020 19:29:36 -0700 Subject: [PATCH 4/5] blindfix for VS macro redefinition --- lib/common/error_private.h | 2 +- tests/fuzzer.c | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/common/error_private.h b/lib/common/error_private.h index ced1a3ba..982cf8e9 100644 --- a/lib/common/error_private.h +++ b/lib/common/error_private.h @@ -49,7 +49,7 @@ typedef ZSTD_ErrorCode ERR_enum; /*-**************************************** * Error codes handling ******************************************/ -#undef ERROR /* reported already defined on VS 2015 (Rich Geldreich) */ +#undef ERROR /* already defined on Visual Studio */ #define ERROR(name) ZSTD_ERROR(name) #define ZSTD_ERROR(name) ((size_t)-PREFIX(name)) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 0202b331..8bc27c0b 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -32,16 +32,17 @@ #include "fse.h" #include "zstd.h" /* ZSTD_VERSION_STRING */ #include "zstd_errors.h" /* ZSTD_getErrorCode */ -#include "zstd_internal.h" /* ZSTD_WORKSPACETOOLARGE_MAXDURATION, ZSTD_WORKSPACETOOLARGE_FACTOR, KB, MB */ #include "zstdmt_compress.h" #define ZDICT_STATIC_LINKING_ONLY #include "zdict.h" /* ZDICT_trainFromBuffer */ -#include "datagen.h" /* RDG_genBuffer */ #include "mem.h" +#include "datagen.h" /* RDG_genBuffer */ #define XXH_STATIC_LINKING_ONLY /* XXH64_state_t */ #include "xxhash.h" /* XXH64 */ #include "util.h" #include "timefn.h" /* SEC_TO_MICRO, UTIL_time_t, UTIL_TIME_INITIALIZER, UTIL_clockSpanMicro, UTIL_getTime */ +/* must be included after util.h, due to ERROR macro redefinition issue on Visual Studio */ +#include "zstd_internal.h" /* ZSTD_WORKSPACETOOLARGE_MAXDURATION, ZSTD_WORKSPACETOOLARGE_FACTOR, KB, MB */ /*-************************************ From e001715b3d3e4c8bb8a99b928b9e8614d4b63f42 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 11 May 2020 20:35:47 -0700 Subject: [PATCH 5/5] fixed asan test --- tests/fuzzer.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 8bc27c0b..9b01bc94 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -1087,7 +1087,6 @@ static int basicUnitTests(U32 const seed, double compressibility) ZSTD_DCtx* const staticDCtx = ZSTD_initStaticDCtx(staticDCtxBuffer, staticDCtxSize); DISPLAYLEVEL(4, "Full CCtx size = %u, ", (U32)staticCCtxSize); DISPLAYLEVEL(4, "CCtx for 32 KB = %u, ", (U32)smallCCtxSize); - assert(staticCCtxSize > smallCCtxSize * ZSTD_WORKSPACETOOLARGE_FACTOR); /* ensure size down scenario */ if ((staticCCtx==NULL) || (staticDCtx==NULL)) goto _output_error; DISPLAYLEVEL(3, "OK \n"); @@ -1108,7 +1107,8 @@ static int basicUnitTests(U32 const seed, double compressibility) DISPLAYLEVEL(3, "test%3i : resize context to full CCtx size : ", testNb++); staticCCtx = ZSTD_initStaticCStream(staticCCtxBuffer, staticCCtxSize); - if ((void*)staticCCtx != staticCCtxBuffer) goto _output_error; + DISPLAYLEVEL(4, "staticCCtxBuffer = %p, staticCCtx = %p , ", staticCCtxBuffer, staticCCtx); + if (staticCCtx == NULL) goto _output_error; DISPLAYLEVEL(3, "OK \n"); DISPLAYLEVEL(3, "test%3i : compress large input with static CCtx : ", testNb++); @@ -1120,6 +1120,7 @@ static int basicUnitTests(U32 const seed, double compressibility) DISPLAYLEVEL(3, "test%3i : compress small input often enough to trigger context reduce : ", testNb++); { int nbc; + assert(staticCCtxSize > smallCCtxSize * ZSTD_WORKSPACETOOLARGE_FACTOR); /* ensure size down scenario */ assert(CNBuffSize > smallInSize + ZSTD_WORKSPACETOOLARGE_MAXDURATION + 3); for (nbc=0; nbc