From 5a4e6c9f3dfc3930455f9e6abbe68c80703a5cbc Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Tue, 28 Aug 2018 13:20:37 -0700 Subject: [PATCH 1/4] [fuzzer] Test growing the seqStore_t --- tests/fuzzer.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 66b33c5e..fd7ccff7 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -1375,6 +1375,24 @@ static int basicUnitTests(U32 seed, double compressibility) ((BYTE*)CNBuffer)[i+1] = _3BytesSeqs[id][1]; ((BYTE*)CNBuffer)[i+2] = _3BytesSeqs[id][2]; } } } + DISPLAYLEVEL(3, "test%3i : growing nbSeq : ", testNb++); + { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); + size_t const maxNbSeq = _3BYTESTESTLENGTH / 3; + size_t const bound = ZSTD_compressBound(_3BYTESTESTLENGTH); + size_t nbSeq = 1; + while (nbSeq <= maxNbSeq) { + CHECK(ZSTD_compressCCtx(cctx, compressedBuffer, bound, CNBuffer, nbSeq * 3, 3)); + /* Check every sequence for the first 100, then skip more rapidly. */ + if (nbSeq < 100) { + ++nbSeq; + } else { + nbSeq += (nbSeq >> 2); + } + } + ZSTD_freeCCtx(cctx); + } + DISPLAYLEVEL(3, "OK \n"); + DISPLAYLEVEL(3, "test%3i : compress lots 3-bytes sequences : ", testNb++); { CHECK_V(r, ZSTD_compress(compressedBuffer, ZSTD_compressBound(_3BYTESTESTLENGTH), CNBuffer, _3BYTESTESTLENGTH, 19) ); @@ -1386,8 +1404,26 @@ static int basicUnitTests(U32 seed, double compressibility) if (r != _3BYTESTESTLENGTH) goto _output_error; } DISPLAYLEVEL(3, "OK \n"); + DISPLAYLEVEL(3, "test%3i : incompressible data and ill suited dictionary : ", testNb++); RDG_genBuffer(CNBuffer, CNBuffSize, 0.0, 0.1, seed); + { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); + size_t const bound = ZSTD_compressBound(CNBuffSize); + size_t size = 1; + while (size <= CNBuffSize) { + CHECK(ZSTD_compressCCtx(cctx, compressedBuffer, bound, CNBuffer, size, 3)); + /* Check every size for the first 100, then skip more rapidly. */ + if (size < 100) { + ++size; + } else { + size += (size >> 2); + } + } + ZSTD_freeCCtx(cctx); + } + DISPLAYLEVEL(3, "OK \n"); + + DISPLAYLEVEL(3, "test%3i : incompressible data and ill suited dictionary : ", testNb++); { /* Train a dictionary on low characters */ size_t dictSize = 16 KB; void* const dictBuffer = malloc(dictSize); From 5e580de6da0861e40303e1227381905f107435f3 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Tue, 28 Aug 2018 13:24:44 -0700 Subject: [PATCH 2/4] [zstd] Fix seqStore growth We could undersize the literals buffer by up to 11 bytes, due to a combination of 2 bugs: * The literals buffer didn't have `WILDCOPY_OVERLENGTH` extra space, like it is supposed to. * We didn't check the literals buffer size in `ZSTD_sufficientBuff()`. --- lib/common/zstd_internal.h | 1 + lib/compress/zstd_compress.c | 28 +++++++++++++++++++-------- lib/compress/zstd_compress_internal.h | 3 ++- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/lib/common/zstd_internal.h b/lib/common/zstd_internal.h index 778d72ef..e75adfa6 100644 --- a/lib/common/zstd_internal.h +++ b/lib/common/zstd_internal.h @@ -193,6 +193,7 @@ typedef struct { BYTE* mlCode; BYTE* ofCode; size_t maxNbSeq; + size_t maxNbLit; U32 longLengthID; /* 0 == no longLength; 1 == Lit.longLength; 2 == Match.longLength; */ U32 longLengthPos; } seqStore_t; diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 76517bb0..5a7b7605 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -805,7 +805,7 @@ size_t ZSTD_estimateCCtxSize_usingCCtxParams(const ZSTD_CCtx_params* params) size_t const blockSize = MIN(ZSTD_BLOCKSIZE_MAX, (size_t)1 << cParams.windowLog); U32 const divider = (cParams.searchLength==3) ? 3 : 4; size_t const maxNbSeq = blockSize / divider; - size_t const tokenSpace = blockSize + 11*maxNbSeq; + size_t const tokenSpace = WILDCOPY_OVERLENGTH + blockSize + 11*maxNbSeq; size_t const entropySpace = HUF_WORKSPACE_SIZE; size_t const blockStateSpace = 2 * sizeof(ZSTD_compressedBlockState_t); size_t const matchStateSize = ZSTD_sizeof_matchState(&cParams, /* forCCtx */ 1); @@ -932,6 +932,7 @@ typedef enum { ZSTDb_not_buffered, ZSTDb_buffered } ZSTD_buffered_policy_e; * check internal buffers exist for streaming if buffPol == ZSTDb_buffered . * Note : they are assumed to be correctly sized if ZSTD_equivalentCParams()==1 */ static U32 ZSTD_sufficientBuff(size_t bufferSize1, size_t maxNbSeq1, + size_t maxNbLit1, ZSTD_buffered_policy_e buffPol2, ZSTD_compressionParameters cParams2, U64 pledgedSrcSize) @@ -939,18 +940,24 @@ static U32 ZSTD_sufficientBuff(size_t bufferSize1, size_t maxNbSeq1, size_t const windowSize2 = MAX(1, (size_t)MIN(((U64)1 << cParams2.windowLog), pledgedSrcSize)); size_t const blockSize2 = MIN(ZSTD_BLOCKSIZE_MAX, windowSize2); size_t const maxNbSeq2 = blockSize2 / ((cParams2.searchLength == 3) ? 3 : 4); + size_t const maxNbLit2 = blockSize2; size_t const neededBufferSize2 = (buffPol2==ZSTDb_buffered) ? windowSize2 + blockSize2 : 0; DEBUGLOG(4, "ZSTD_sufficientBuff: is neededBufferSize2=%u <= bufferSize1=%u", (U32)neededBufferSize2, (U32)bufferSize1); DEBUGLOG(4, "ZSTD_sufficientBuff: is maxNbSeq2=%u <= maxNbSeq1=%u", (U32)maxNbSeq2, (U32)maxNbSeq1); - return (maxNbSeq2 <= maxNbSeq1) & (neededBufferSize2 <= bufferSize1); + DEBUGLOG(4, "ZSTD_sufficientBuff: is maxNbLit2=%u <= maxNbLit1=%u", + (U32)maxNbLit2, (U32)maxNbLit1); + return (maxNbLit2 <= maxNbLit1) + & (maxNbSeq2 <= maxNbSeq1) + & (neededBufferSize2 <= bufferSize1); } /** Equivalence for resetCCtx purposes */ static U32 ZSTD_equivalentParams(ZSTD_CCtx_params params1, ZSTD_CCtx_params params2, - size_t buffSize1, size_t maxNbSeq1, + size_t buffSize1, + size_t maxNbSeq1, size_t maxNbLit1, ZSTD_buffered_policy_e buffPol2, U64 pledgedSrcSize) { @@ -963,8 +970,8 @@ static U32 ZSTD_equivalentParams(ZSTD_CCtx_params params1, DEBUGLOG(4, "ZSTD_equivalentLdmParams() == 0"); return 0; } - if (!ZSTD_sufficientBuff(buffSize1, maxNbSeq1, buffPol2, params2.cParams, - pledgedSrcSize)) { + if (!ZSTD_sufficientBuff(buffSize1, maxNbSeq1, maxNbLit1, buffPol2, + params2.cParams, pledgedSrcSize)) { DEBUGLOG(4, "ZSTD_sufficientBuff() == 0"); return 0; } @@ -1096,7 +1103,8 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, if (crp == ZSTDcrp_continue) { if (ZSTD_equivalentParams(zc->appliedParams, params, - zc->inBuffSize, zc->seqStore.maxNbSeq, + zc->inBuffSize, + zc->seqStore.maxNbSeq, zc->seqStore.maxNbLit, zbuff, pledgedSrcSize)) { DEBUGLOG(4, "ZSTD_equivalentParams()==1 -> continue mode (wLog1=%u, blockSize1=%zu)", zc->appliedParams.cParams.windowLog, zc->blockSize); @@ -1118,7 +1126,7 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, size_t const blockSize = MIN(ZSTD_BLOCKSIZE_MAX, windowSize); U32 const divider = (params.cParams.searchLength==3) ? 3 : 4; size_t const maxNbSeq = blockSize / divider; - size_t const tokenSpace = blockSize + 11*maxNbSeq; + size_t const tokenSpace = WILDCOPY_OVERLENGTH + blockSize + 11*maxNbSeq; size_t const buffOutSize = (zbuff==ZSTDb_buffered) ? ZSTD_compressBound(blockSize)+1 : 0; size_t const buffInSize = (zbuff==ZSTDb_buffered) ? windowSize + blockSize : 0; size_t const matchStateSize = ZSTD_sizeof_matchState(¶ms.cParams, /* forCCtx */ 1); @@ -1215,7 +1223,11 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, zc->seqStore.mlCode = zc->seqStore.llCode + maxNbSeq; zc->seqStore.ofCode = zc->seqStore.mlCode + maxNbSeq; zc->seqStore.litStart = zc->seqStore.ofCode + maxNbSeq; - ptr = zc->seqStore.litStart + blockSize; + /* ZSTD_wildcopy() is used to copy into the literals buffer, + * so we have to oversize the buffer by WILDCOPY_OVERLENGTH bytes. + */ + zc->seqStore.maxNbLit = blockSize; + ptr = zc->seqStore.litStart + blockSize + WILDCOPY_OVERLENGTH; /* ldm bucketOffsets table */ if (params.ldmParams.enableLdm) { diff --git a/lib/compress/zstd_compress_internal.h b/lib/compress/zstd_compress_internal.h index 20b1364b..1c95b7de 100644 --- a/lib/compress/zstd_compress_internal.h +++ b/lib/compress/zstd_compress_internal.h @@ -316,7 +316,8 @@ MEM_STATIC void ZSTD_storeSeq(seqStore_t* seqStorePtr, size_t litLength, const v #endif assert((size_t)(seqStorePtr->sequences - seqStorePtr->sequencesStart) < seqStorePtr->maxNbSeq); /* copy Literals */ - assert(seqStorePtr->lit + litLength <= seqStorePtr->litStart + 128 KB); + assert(seqStorePtr->maxNbLit <= 128 KB); + assert(seqStorePtr->lit + litLength <= seqStorePtr->litStart + seqStorePtr->maxNbLit); ZSTD_wildcopy(seqStorePtr->lit, literals, litLength); seqStorePtr->lit += litLength; From e984d01912896981d70a552dac6983c13a182d13 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Tue, 28 Aug 2018 13:42:01 -0700 Subject: [PATCH 3/4] Small test fixes --- tests/fuzzer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index fd7ccff7..641357e7 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -1381,7 +1381,7 @@ static int basicUnitTests(U32 seed, double compressibility) size_t const bound = ZSTD_compressBound(_3BYTESTESTLENGTH); size_t nbSeq = 1; while (nbSeq <= maxNbSeq) { - CHECK(ZSTD_compressCCtx(cctx, compressedBuffer, bound, CNBuffer, nbSeq * 3, 3)); + CHECK(ZSTD_compressCCtx(cctx, compressedBuffer, bound, CNBuffer, nbSeq * 3, 19)); /* Check every sequence for the first 100, then skip more rapidly. */ if (nbSeq < 100) { ++nbSeq; @@ -1405,7 +1405,7 @@ static int basicUnitTests(U32 seed, double compressibility) DISPLAYLEVEL(3, "OK \n"); - DISPLAYLEVEL(3, "test%3i : incompressible data and ill suited dictionary : ", testNb++); + DISPLAYLEVEL(3, "test%3i : growing literals buffer : ", testNb++); RDG_genBuffer(CNBuffer, CNBuffSize, 0.0, 0.1, seed); { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); size_t const bound = ZSTD_compressBound(CNBuffSize); From e3b5286197f94bf517642efe3acd4badcaf76ec3 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Tue, 28 Aug 2018 13:56:47 -0700 Subject: [PATCH 4/4] Fix decodecorpus --- tests/decodecorpus.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/decodecorpus.c b/tests/decodecorpus.c index 5cc21a5d..2c227600 100644 --- a/tests/decodecorpus.c +++ b/tests/decodecorpus.c @@ -621,6 +621,7 @@ static size_t writeLiteralsBlock(U32* seed, frame_t* frame, size_t contentSize) static inline void initSeqStore(seqStore_t *seqStore) { seqStore->maxNbSeq = MAX_NB_SEQ; + seqStore->maxNbLit = ZSTD_BLOCKSIZE_MAX; seqStore->sequencesStart = SEQUENCE_BUFFER; seqStore->litStart = SEQUENCE_LITERAL_BUFFER; seqStore->llCode = SEQUENCE_LLCODE;