From c8d870fe52b043828f1f59b8976b4d7c55865289 Mon Sep 17 00:00:00 2001 From: Elliot Gorokhovsky Date: Mon, 21 Nov 2022 15:39:18 -0500 Subject: [PATCH 1/3] Improve LDM cparam validation logic --- lib/compress/zstd_compress.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index adf1f6e7a..c2926ab85 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -265,9 +265,9 @@ static int ZSTD_allocateChainTable(const ZSTD_strategy strategy, return forDDSDict || ((strategy != ZSTD_fast) && !ZSTD_rowMatchFinderUsed(strategy, useRowMatchFinder)); } -/* Returns 1 if compression parameters are such that we should +/* Returns ZSTD_ps_enable if compression parameters are such that we should * enable long distance matching (wlog >= 27, strategy >= btopt). - * Returns 0 otherwise. + * Returns ZSTD_ps_disable otherwise. */ static ZSTD_paramSwitch_e ZSTD_resolveEnableLdm(ZSTD_paramSwitch_e mode, const ZSTD_compressionParameters* const cParams) { @@ -482,8 +482,8 @@ ZSTD_bounds ZSTD_cParam_getBounds(ZSTD_cParameter param) return bounds; case ZSTD_c_enableLongDistanceMatching: - bounds.lowerBound = 0; - bounds.upperBound = 1; + bounds.lowerBound = (int)ZSTD_ps_auto; + bounds.upperBound = (int)ZSTD_ps_disable; return bounds; case ZSTD_c_ldmHashLog: @@ -854,6 +854,7 @@ size_t ZSTD_CCtxParams_setParameter(ZSTD_CCtx_params* CCtxParams, return (size_t)CCtxParams->enableDedicatedDictSearch; case ZSTD_c_enableLongDistanceMatching : + BOUNDCHECK(ZSTD_c_enableLongDistanceMatching, value); CCtxParams->ldmParams.enableLdm = (ZSTD_paramSwitch_e)value; return CCtxParams->ldmParams.enableLdm; From 3720910d060a42f53f72252ecc188c9f9b33740e Mon Sep 17 00:00:00 2001 From: Elliot Gorokhovsky Date: Mon, 21 Nov 2022 16:09:04 -0500 Subject: [PATCH 2/3] Fix fuzzer failure --- tests/zstreamtest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index ce9020f12..348f72ed4 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -2281,7 +2281,7 @@ static int fuzzerTests_newAPI(U32 seed, int nbTests, int startTest, /* mess with long distance matching parameters */ if (bigTests) { - if (FUZ_rand(&lseed) & 1) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_c_enableLongDistanceMatching, FUZ_rand(&lseed) & 63, opaqueAPI) ); + if (FUZ_rand(&lseed) & 1) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_c_enableLongDistanceMatching, FUZ_randomClampedLength(&lseed, ZSTD_ps_auto, ZSTD_ps_disable), opaqueAPI) ); if (FUZ_rand(&lseed) & 3) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_c_ldmHashLog, FUZ_randomClampedLength(&lseed, ZSTD_HASHLOG_MIN, 23), opaqueAPI) ); if (FUZ_rand(&lseed) & 3) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_c_ldmMinMatch, FUZ_randomClampedLength(&lseed, ZSTD_LDM_MINMATCH_MIN, ZSTD_LDM_MINMATCH_MAX), opaqueAPI) ); if (FUZ_rand(&lseed) & 3) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_c_ldmBucketSizeLog, FUZ_randomClampedLength(&lseed, ZSTD_LDM_BUCKETSIZELOG_MIN, ZSTD_LDM_BUCKETSIZELOG_MAX), opaqueAPI) ); From bb3c01c8539ecc85f0884dd709767de915fb0d72 Mon Sep 17 00:00:00 2001 From: Elliot Gorokhovsky Date: Mon, 21 Nov 2022 16:20:38 -0500 Subject: [PATCH 3/3] Migrate other test usages of boolean LDM flag to paramSwitch enum --- tests/fuzz/zstd_helpers.c | 2 +- tests/fuzzer.c | 22 +++++++++++----------- tests/regression/config.c | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/fuzz/zstd_helpers.c b/tests/fuzz/zstd_helpers.c index b4a6509db..08ce70dd7 100644 --- a/tests/fuzz/zstd_helpers.c +++ b/tests/fuzz/zstd_helpers.c @@ -80,7 +80,7 @@ void FUZZ_setRandomParameters(ZSTD_CCtx *cctx, size_t srcSize, FUZZ_dataProducer setRand(cctx, ZSTD_c_checksumFlag, 0, 1, producer); setRand(cctx, ZSTD_c_dictIDFlag, 0, 1, producer); /* Select long distance matching parameters */ - setRand(cctx, ZSTD_c_enableLongDistanceMatching, 0, 1, producer); + setRand(cctx, ZSTD_c_enableLongDistanceMatching, ZSTD_ps_auto, ZSTD_ps_disable, producer); setRand(cctx, ZSTD_c_ldmHashLog, ZSTD_HASHLOG_MIN, 16, producer); setRand(cctx, ZSTD_c_ldmMinMatch, ZSTD_LDM_MINMATCH_MIN, ZSTD_LDM_MINMATCH_MAX, producer); diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 81c2d9dba..879e537bc 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -851,7 +851,7 @@ static int basicUnitTests(U32 const seed, double compressibility) RDG_genBuffer(dict, size, 0.5, 0.5, seed); RDG_genBuffer(src, size, 0.5, 0.5, seed); - CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, 1)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, ZSTD_ps_enable)); assert(!ZSTD_isError(ZSTD_compress_usingDict(cctx, dst, dstCapacity, src, size, dict, size, 3))); ZSTD_freeCCtx(cctx); @@ -875,7 +875,7 @@ static int basicUnitTests(U32 const seed, double compressibility) CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_nbWorkers, nbWorkers)); CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_checksumFlag, 1)); CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_forceMaxWindow, 1)); - CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, 1)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, ZSTD_ps_enable)); CHECK_Z(ZSTD_CCtx_refPrefix(cctx, dict, CNBuffSize)); cSize = ZSTD_compress2(cctx, compressedBuffer, compressedBufferSize, CNBuffer, CNBuffSize); CHECK_Z(cSize); @@ -900,7 +900,7 @@ static int basicUnitTests(U32 const seed, double compressibility) RDG_genBuffer(CNBuffer, testSize, 0.6, 0.6, seed); memcpy(dict + testSize, CNBuffer, testSize); for (level = 1; level <= 5; ++level) { - for (ldmEnabled = 0; ldmEnabled <= 1; ++ldmEnabled) { + for (ldmEnabled = ZSTD_ps_enable; ldmEnabled <= ZSTD_ps_disable; ++ldmEnabled) { size_t cSize0; XXH64_hash_t compressedChecksum0; @@ -956,7 +956,7 @@ static int basicUnitTests(U32 const seed, double compressibility) /* Enable MT, LDM, and opt parser */ CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_nbWorkers, 1)); - CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, 1)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, ZSTD_ps_enable)); CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_checksumFlag, 1)); CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_compressionLevel, 19)); @@ -995,7 +995,7 @@ static int basicUnitTests(U32 const seed, double compressibility) /* Disable content size to skip single-pass decompression. */ CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_contentSizeFlag, 0)); CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_windowLog, (int)kWindowLog)); - CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, 1)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, ZSTD_ps_enable)); CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_ldmMinMatch, 32)); CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_ldmHashRateLog, 1)); CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_ldmHashLog, 16)); @@ -1092,7 +1092,7 @@ static int basicUnitTests(U32 const seed, double compressibility) /* compress on level 1 using refPrefix and ldm */ ZSTD_CCtx_refPrefix(cctx, dict, size);; - CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, 1)) + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, ZSTD_ps_enable)) refPrefixLdmCompressedSize = ZSTD_compress2(cctx, dst, dstSize, src, size); assert(!ZSTD_isError(refPrefixLdmCompressedSize)); @@ -2820,7 +2820,7 @@ static int basicUnitTests(U32 const seed, double compressibility) DISPLAYLEVEL(3, "test%3i : parameters in order : ", testNb++); assert(cctx != NULL); CHECK( ZSTD_CCtx_setParameter(cctx, ZSTD_c_compressionLevel, 2) ); - CHECK( ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, 1) ); + CHECK( ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, ZSTD_ps_enable) ); CHECK( ZSTD_CCtx_setParameter(cctx, ZSTD_c_windowLog, 18) ); { size_t const compressedSize = ZSTD_compress2(cctx, compressedBuffer, ZSTD_compressBound(inputSize), @@ -2836,7 +2836,7 @@ static int basicUnitTests(U32 const seed, double compressibility) { ZSTD_CCtx* cctx = ZSTD_createCCtx(); DISPLAYLEVEL(3, "test%3i : parameters disordered : ", testNb++); CHECK( ZSTD_CCtx_setParameter(cctx, ZSTD_c_windowLog, 18) ); - CHECK( ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, 1) ); + CHECK( ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, ZSTD_ps_enable) ); CHECK( ZSTD_CCtx_setParameter(cctx, ZSTD_c_compressionLevel, 2) ); { size_t const result = ZSTD_compress2(cctx, compressedBuffer, ZSTD_compressBound(inputSize), @@ -3492,7 +3492,7 @@ static int basicUnitTests(U32 const seed, double compressibility) /* Enable MT, LDM, and use refPrefix() for a small dict */ CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_nbWorkers, 2)); - CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, 1)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, ZSTD_ps_enable)); CHECK_Z(ZSTD_CCtx_refPrefix(cctx, dict, dictSize)); CHECK_Z(ZSTD_compress2(cctx, dst, dstSize, src, srcSize)); @@ -3686,7 +3686,7 @@ static int longUnitTests(U32 const seed, double compressibility) /* Enable checksum to verify round trip. */ CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_checksumFlag, 1)); - CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, 1)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, ZSTD_ps_enable)); CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_compressionLevel, 19)); /* Round trip once with ldm. */ @@ -3696,7 +3696,7 @@ static int longUnitTests(U32 const seed, double compressibility) ZSTD_CCtx_reset(cctx, ZSTD_reset_session_and_parameters); CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_checksumFlag, 1)); - CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, 0)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_enableLongDistanceMatching, ZSTD_ps_disable)); CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_compressionLevel, 19)); /* Round trip once without ldm. */ diff --git a/tests/regression/config.c b/tests/regression/config.c index 57cd110c6..30d0ca5e2 100644 --- a/tests/regression/config.c +++ b/tests/regression/config.c @@ -183,7 +183,7 @@ static config_t no_pledged_src_size_with_dict = { }; static param_value_t const ldm_param_values[] = { - {.param = ZSTD_c_enableLongDistanceMatching, .value = 1}, + {.param = ZSTD_c_enableLongDistanceMatching, .value = ZSTD_ps_enable}, }; static config_t ldm = { @@ -204,7 +204,7 @@ static config_t mt = { static param_value_t const mt_ldm_param_values[] = { {.param = ZSTD_c_nbWorkers, .value = 2}, - {.param = ZSTD_c_enableLongDistanceMatching, .value = 1}, + {.param = ZSTD_c_enableLongDistanceMatching, .value = ZSTD_ps_enable}, }; static config_t mt_ldm = {