From dba767c0bba79f7ba7caac937799f4a6e5d36dc3 Mon Sep 17 00:00:00 2001 From: Bimba Shrestha Date: Fri, 15 Nov 2019 18:44:51 -0800 Subject: [PATCH 01/13] Leaving room for checksum --- lib/compress/zstd_compress.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index d4e7590b..4ff825d9 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2480,8 +2480,11 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, * enough for SuperBlock compression. * In such case, fall back to normal compression. This is possible because * targetCBlockSize is best effort not a guarantee. */ - if (cSize != ERROR(dstSize_tooSmall)) return cSize; - else { + if (cSize == ERROR(dstSize_tooSmall) || (dstCapacity - cSize) < 4) { + /* We check (dstCapacity - cSize) < 4 above because we have to make sure + * to leave enouch room for the checksum that will eventually get added in + * the epilogue. Otherwise, we're just going to throw the dstSize_tooSmall + * error there instead of here */ BYTE* const ostart = (BYTE*)dst; /* If ZSTD_noCompressSuperBlock fails with dstSize_tooSmall, * compress normally. @@ -2498,7 +2501,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, MEM_writeLE24(ostart, cBlockHeader24); cSize += ZSTD_blockHeaderSize; } - } + } else return cSize; } if (!ZSTD_isError(cSize) && cSize != 0) { @@ -2853,7 +2856,7 @@ static size_t ZSTD_checkDictNCount(short* normalizedCounter, unsigned dictMaxSym size_t ZSTD_loadCEntropy(ZSTD_compressedBlockState_t* bs, void* workspace, short* offcodeNCount, unsigned* offcodeMaxValue, - const void* const dict, size_t dictSize) + const void* const dict, size_t dictSize) { const BYTE* dictPtr = (const BYTE*)dict; /* skip magic num and dict ID */ const BYTE* const dictEnd = dictPtr + dictSize; From 2d5d961a60b72c5a7cffaf25d0d16b599f45343d Mon Sep 17 00:00:00 2001 From: Bimba Shrestha Date: Fri, 15 Nov 2019 19:00:53 -0800 Subject: [PATCH 02/13] Typo in comment --- lib/compress/zstd_compress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 4ff825d9..0639d327 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2482,7 +2482,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, * targetCBlockSize is best effort not a guarantee. */ if (cSize == ERROR(dstSize_tooSmall) || (dstCapacity - cSize) < 4) { /* We check (dstCapacity - cSize) < 4 above because we have to make sure - * to leave enouch room for the checksum that will eventually get added in + * to leave enough room for the checksum that will eventually get added in * the epilogue. Otherwise, we're just going to throw the dstSize_tooSmall * error there instead of here */ BYTE* const ostart = (BYTE*)dst; From dade64428f093b111195cc1ac2e474633735a0cd Mon Sep 17 00:00:00 2001 From: Bimba Shrestha Date: Mon, 18 Nov 2019 08:43:14 -0800 Subject: [PATCH 03/13] Output regular uncompressed block when compressSequences fails --- lib/compress/zstd_compress.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 0639d327..a883d21c 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2496,7 +2496,13 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, srcSize, zc->entropyWorkspace, HUF_WORKSPACE_SIZE /* statically allocated in resetCCtx */, zc->bmi2); - if (!ZSTD_isError(cSize) && cSize != 0) { + + if (cSize == 0) { + /* If compressSequences didn't work, we just output a regular + * uncompressed block */ + cSize = ZSTD_noCompressBlock(dst, dstCapacity, src, srcSize, lastBlock); + FORWARD_IF_ERROR(cSize); + } else { U32 const cBlockHeader24 = lastBlock + (((U32)bt_compressed)<<1) + (U32)(cSize << 3); MEM_writeLE24(ostart, cBlockHeader24); cSize += ZSTD_blockHeaderSize; From 80586f5e8003fe235f1ef5bccaee88966baaaddf Mon Sep 17 00:00:00 2001 From: Bimba Shrestha Date: Mon, 18 Nov 2019 13:53:55 -0800 Subject: [PATCH 04/13] Reversing condition order and forwarding error --- lib/compress/zstd_compress.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index a883d21c..6330b882 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2480,11 +2480,13 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, * enough for SuperBlock compression. * In such case, fall back to normal compression. This is possible because * targetCBlockSize is best effort not a guarantee. */ - if (cSize == ERROR(dstSize_tooSmall) || (dstCapacity - cSize) < 4) { - /* We check (dstCapacity - cSize) < 4 above because we have to make sure + if (cSize != ERROR(dstSize_tooSmall) && (dstCapacity - cSize) >= 4) + /* We check (dstCapacity - cSize) >= 4 above because we have to make sure * to leave enough room for the checksum that will eventually get added in * the epilogue. Otherwise, we're just going to throw the dstSize_tooSmall * error there instead of here */ + return cSize; + else { BYTE* const ostart = (BYTE*)dst; /* If ZSTD_noCompressSuperBlock fails with dstSize_tooSmall, * compress normally. @@ -2497,6 +2499,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, zc->entropyWorkspace, HUF_WORKSPACE_SIZE /* statically allocated in resetCCtx */, zc->bmi2); + FORWARD_IF_ERROR(cSize); if (cSize == 0) { /* If compressSequences didn't work, we just output a regular * uncompressed block */ @@ -2507,7 +2510,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, MEM_writeLE24(ostart, cBlockHeader24); cSize += ZSTD_blockHeaderSize; } - } else return cSize; + } } if (!ZSTD_isError(cSize) && cSize != 0) { From 8f0c2d04c8a5ff3e94169ac22c56c9efca7a7154 Mon Sep 17 00:00:00 2001 From: Bimba Shrestha Date: Tue, 19 Nov 2019 10:03:07 -0800 Subject: [PATCH 05/13] Going back to original flow but removing else return --- lib/compress/zstd_compress.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 6330b882..064a38ff 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2480,13 +2480,11 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, * enough for SuperBlock compression. * In such case, fall back to normal compression. This is possible because * targetCBlockSize is best effort not a guarantee. */ - if (cSize != ERROR(dstSize_tooSmall) && (dstCapacity - cSize) >= 4) + if (cSize == ERROR(dstSize_tooSmall) || (dstCapacity - cSize) < 4) { /* We check (dstCapacity - cSize) >= 4 above because we have to make sure * to leave enough room for the checksum that will eventually get added in * the epilogue. Otherwise, we're just going to throw the dstSize_tooSmall * error there instead of here */ - return cSize; - else { BYTE* const ostart = (BYTE*)dst; /* If ZSTD_noCompressSuperBlock fails with dstSize_tooSmall, * compress normally. From 0451accab1363778cd1a2fa10660cafdf5143a20 Mon Sep 17 00:00:00 2001 From: Bimba Shrestha Date: Thu, 21 Nov 2019 13:06:26 -0800 Subject: [PATCH 06/13] Checking noCompressBlock explicitly for rep code confirmation --- lib/compress/zstd_compress.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 064a38ff..f8c64994 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2470,8 +2470,10 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, * encode using ZSTD_noCompressSuperBlock writing sub blocks * in uncompressed mode. */ + int usingNoCompressSuperBlock = 0; if (cSize == 0) { cSize = ZSTD_noCompressSuperBlock(dst, dstCapacity, src, srcSize, zc->appliedParams.targetCBlockSize, lastBlock); + usingNoCompressSuperBlock = 1; /* In compression, there is an assumption that a compressed block is always * within the size of ZSTD_compressBound(). However, SuperBlock compression * can exceed the limit due to overhead of headers from SubBlocks. @@ -2511,7 +2513,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, } } - if (!ZSTD_isError(cSize) && cSize != 0) { + if (!ZSTD_isError(cSize) && !usingNoCompressSuperBlock) { /* confirm repcodes and entropy tables when emitting a compressed block */ ZSTD_compressedBlockState_t* const tmp = zc->blockState.prevCBlock; zc->blockState.prevCBlock = zc->blockState.nextCBlock; From 10bce1919e2a3fb7d64b5ce7b56ecb7a98fd2655 Mon Sep 17 00:00:00 2001 From: Bimba Shrestha Date: Thu, 21 Nov 2019 13:08:27 -0800 Subject: [PATCH 07/13] Mixed declration fix --- lib/compress/zstd_compress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index f8c64994..9211190e 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2457,6 +2457,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, const void* src, size_t srcSize, U32 lastBlock) { size_t cSize = 0; + int usingNoCompressSuperBlock = 0; DEBUGLOG(5, "ZSTD_compressBlock_targetCBlockSize (dstCapacity=%u, dictLimit=%u, nextToUpdate=%u, srcSize=%zu)", (unsigned)dstCapacity, (unsigned)zc->blockState.matchState.window.dictLimit, (unsigned)zc->blockState.matchState.nextToUpdate, srcSize); @@ -2470,7 +2471,6 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, * encode using ZSTD_noCompressSuperBlock writing sub blocks * in uncompressed mode. */ - int usingNoCompressSuperBlock = 0; if (cSize == 0) { cSize = ZSTD_noCompressSuperBlock(dst, dstCapacity, src, srcSize, zc->appliedParams.targetCBlockSize, lastBlock); usingNoCompressSuperBlock = 1; From 707a12c419ecdb5d1f91d9bc18703b8bbead2585 Mon Sep 17 00:00:00 2001 From: Bimba Shrestha Date: Fri, 22 Nov 2019 17:25:36 -0800 Subject: [PATCH 08/13] Test enough room for checksum in superblock --- tests/fuzzer.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index a61667ed..8dbaa338 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -427,7 +427,6 @@ static int basicUnitTests(U32 const seed, double compressibility) } } DISPLAYLEVEL(3, "OK \n"); - DISPLAYLEVEL(3, "test%3i : decompress with null dict : ", testNb++); { ZSTD_DCtx* const dctx = ZSTD_createDCtx(); assert(dctx != NULL); { size_t const r = ZSTD_decompress_usingDict(dctx, @@ -490,6 +489,19 @@ static int basicUnitTests(U32 const seed, double compressibility) } DISPLAYLEVEL(3, "OK \n"); + DISPLAYLEVEL(3, "test%3d: superblock enough room for checksum : ", testNb++) + { + /* This tests whether or not we leave enough room for the checksum at the end + * of the dst buffer. The bug that motivated this test was found by the + * stream_round_trip fuzzer but this crashes for the same reason and is + * far more compact than re-creating the stream_round_trip fuzzer's code path */ + ZSTD_CCtx *cctx = ZSTD_createCCtx(); + ZSTD_CCtx_setParameter(cctx, ZSTD_c_targetCBlockSize, 64); + assert(!ZSTD_isError(ZSTD_compress2(cctx, compressedBuffer, 1339, CNBuffer, 1278))); + ZSTD_freeCCtx(cctx); + } + DISPLAYLEVEL(3, "OK \n"); + DISPLAYLEVEL(3, "test%3d : check CCtx size after compressing empty input : ", testNb++); { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); size_t const r = ZSTD_compressCCtx(cctx, compressedBuffer, compressedBufferSize, NULL, 0, 19); From d4e17d0776f4cb05e9b8d044186dd6f0a541c077 Mon Sep 17 00:00:00 2001 From: Bimba Shrestha Date: Tue, 26 Nov 2019 12:17:43 -0800 Subject: [PATCH 09/13] Negating bool, updating bool on inner branches --- lib/compress/zstd_compress.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 9211190e..502b6b33 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2457,7 +2457,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, const void* src, size_t srcSize, U32 lastBlock) { size_t cSize = 0; - int usingNoCompressSuperBlock = 0; + int compressSuperBlock = 1; DEBUGLOG(5, "ZSTD_compressBlock_targetCBlockSize (dstCapacity=%u, dictLimit=%u, nextToUpdate=%u, srcSize=%zu)", (unsigned)dstCapacity, (unsigned)zc->blockState.matchState.window.dictLimit, (unsigned)zc->blockState.matchState.nextToUpdate, srcSize); @@ -2473,7 +2473,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, */ if (cSize == 0) { cSize = ZSTD_noCompressSuperBlock(dst, dstCapacity, src, srcSize, zc->appliedParams.targetCBlockSize, lastBlock); - usingNoCompressSuperBlock = 1; + compressSuperBlock = 0; /* In compression, there is an assumption that a compressed block is always * within the size of ZSTD_compressBound(). However, SuperBlock compression * can exceed the limit due to overhead of headers from SubBlocks. @@ -2491,6 +2491,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, /* If ZSTD_noCompressSuperBlock fails with dstSize_tooSmall, * compress normally. */ + compressSuperBlock = 1; cSize = ZSTD_compressSequences(&zc->seqStore, &zc->blockState.prevCBlock->entropy, &zc->blockState.nextCBlock->entropy, &zc->appliedParams, @@ -2501,6 +2502,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, FORWARD_IF_ERROR(cSize); if (cSize == 0) { + compressSuperBlock = 0; /* If compressSequences didn't work, we just output a regular * uncompressed block */ cSize = ZSTD_noCompressBlock(dst, dstCapacity, src, srcSize, lastBlock); @@ -2513,7 +2515,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, } } - if (!ZSTD_isError(cSize) && !usingNoCompressSuperBlock) { + if (!ZSTD_isError(cSize) && compressSuperBlock) { /* confirm repcodes and entropy tables when emitting a compressed block */ ZSTD_compressedBlockState_t* const tmp = zc->blockState.prevCBlock; zc->blockState.prevCBlock = zc->blockState.nextCBlock; From 1fc9352f815802037b5fe461c5a31437a3dcb3ef Mon Sep 17 00:00:00 2001 From: Bimba Shrestha Date: Mon, 2 Dec 2019 21:39:06 -0800 Subject: [PATCH 10/13] Using bss var instead of creating new bool --- lib/compress/zstd_compress.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 34cfbe5e..2348b1ab 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2455,11 +2455,11 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, const void* src, size_t srcSize, U32 lastBlock) { size_t cSize = 0; - int compressSuperBlock = 1; + size_t bss; DEBUGLOG(5, "ZSTD_compressBlock_targetCBlockSize (dstCapacity=%u, dictLimit=%u, nextToUpdate=%u, srcSize=%zu)", (unsigned)dstCapacity, (unsigned)zc->blockState.matchState.window.dictLimit, (unsigned)zc->blockState.matchState.nextToUpdate, srcSize); - { const size_t bss = ZSTD_buildSeqStore(zc, src, srcSize); + { bss = ZSTD_buildSeqStore(zc, src, srcSize); FORWARD_IF_ERROR(bss); if (bss == ZSTDbss_compress) { cSize = ZSTD_compressSuperBlock(zc, dst, dstCapacity, lastBlock); @@ -2471,7 +2471,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, */ if (cSize == 0) { cSize = ZSTD_noCompressSuperBlock(dst, dstCapacity, src, srcSize, zc->appliedParams.targetCBlockSize, lastBlock); - compressSuperBlock = 0; + bss = ZSTDbss_noCompress; /* In compression, there is an assumption that a compressed block is always * within the size of ZSTD_compressBound(). However, SuperBlock compression * can exceed the limit due to overhead of headers from SubBlocks. @@ -2489,7 +2489,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, /* If ZSTD_noCompressSuperBlock fails with dstSize_tooSmall, * compress normally. */ - compressSuperBlock = 1; + bss = ZSTDbss_compress; cSize = ZSTD_compressSequences(&zc->seqStore, &zc->blockState.prevCBlock->entropy, &zc->blockState.nextCBlock->entropy, &zc->appliedParams, @@ -2500,7 +2500,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, FORWARD_IF_ERROR(cSize); if (cSize == 0) { - compressSuperBlock = 0; + bss = ZSTDbss_noCompress; /* If compressSequences didn't work, we just output a regular * uncompressed block */ cSize = ZSTD_noCompressBlock(dst, dstCapacity, src, srcSize, lastBlock); @@ -2513,7 +2513,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, } } - if (!ZSTD_isError(cSize) && compressSuperBlock) { + if (!ZSTD_isError(cSize) && bss == ZSTDbss_compress) { /* confirm repcodes and entropy tables when emitting a compressed block */ ZSTD_compressedBlockState_t* const tmp = zc->blockState.prevCBlock; zc->blockState.prevCBlock = zc->blockState.nextCBlock; From ffb0463041e7fa0e2d2cea55e908b7e00a968d4d Mon Sep 17 00:00:00 2001 From: Bimba Shrestha Date: Wed, 4 Dec 2019 14:52:27 -0800 Subject: [PATCH 11/13] Refactor --- lib/compress/zstd_compress.c | 138 +++++++++++++++++------------------ 1 file changed, 68 insertions(+), 70 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 2348b1ab..2f28b836 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2450,82 +2450,80 @@ out: return cSize; } +static void ZSTD_confirmRepcodesAndEntropyTables(ZSTD_CCtx* zc) +{ + ZSTD_compressedBlockState_t* const tmp = zc->blockState.prevCBlock; + zc->blockState.prevCBlock = zc->blockState.nextCBlock; + zc->blockState.nextCBlock = tmp; +} + +static size_t ZSTD_compressBlock_targetCBlockSize_init(ZSTD_CCtx* zc, + void* dst, size_t dstCapacity, + const void* src, size_t srcSize, + U32 lastBlock) +{ + const size_t bss = ZSTD_buildSeqStore(zc, src, srcSize); + FORWARD_IF_ERROR(bss); + if (bss == ZSTDbss_compress) + return ZSTD_compressSuperBlock(zc, dst, dstCapacity, lastBlock); + return 0; +} + +static void ZSTD_compressBlock_targetCBlockSize_end(ZSTD_CCtx* zc) +{ + if (zc->blockState.prevCBlock->entropy.fse.offcode_repeatMode == FSE_repeat_valid) + zc->blockState.prevCBlock->entropy.fse.offcode_repeatMode = FSE_repeat_check; +} + +static size_t ZSTD_compressBlock_targetCBlockSize_body(ZSTD_CCtx* zc, + size_t cSize, void* dst, size_t dstCapacity, + const void* src, size_t srcSize, + U32 lastBlock) +{ + /* Superblock compression was successful */ + if (cSize != 0) { + ZSTD_confirmRepcodesAndEntropyTables(zc); + return cSize; + } + + /* Superblock compression failed, attempt to emit noCompress superblocks + * and return early if that is successful and we have enough room for checksum */ + cSize = ZSTD_noCompressSuperBlock(dst, dstCapacity, src, srcSize, zc->appliedParams.targetCBlockSize, lastBlock); + if (cSize != ERROR(dstSize_tooSmall) && (dstCapacity - cSize) >= 4) + return cSize; + + /* noCompress superblock emission failed. Attempt to compress normally + * and return early if that is successful */ + cSize = ZSTD_compressSequences(&zc->seqStore, + &zc->blockState.prevCBlock->entropy, &zc->blockState.nextCBlock->entropy, + &zc->appliedParams, (BYTE*)dst+ZSTD_blockHeaderSize, dstCapacity-ZSTD_blockHeaderSize, + srcSize, zc->entropyWorkspace, HUF_WORKSPACE_SIZE, zc->bmi2); + FORWARD_IF_ERROR(cSize); + if (cSize != 0) { + U32 const cBlockHeader24 = lastBlock + (((U32)bt_compressed)<<1) + (U32)(cSize << 3); + MEM_writeLE24((BYTE*)dst, cBlockHeader24); + cSize += ZSTD_blockHeaderSize; + ZSTD_confirmRepcodesAndEntropyTables(zc); + return cSize; + } + + /* Everything failed. Just emit a regular noCompress block */ + return ZSTD_noCompressBlock(dst, dstCapacity, src, srcSize, lastBlock); +} + static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, void* dst, size_t dstCapacity, const void* src, size_t srcSize, - U32 lastBlock) { + U32 lastBlock) +{ size_t cSize = 0; - size_t bss; DEBUGLOG(5, "ZSTD_compressBlock_targetCBlockSize (dstCapacity=%u, dictLimit=%u, nextToUpdate=%u, srcSize=%zu)", (unsigned)dstCapacity, (unsigned)zc->blockState.matchState.window.dictLimit, (unsigned)zc->blockState.matchState.nextToUpdate, srcSize); - - { bss = ZSTD_buildSeqStore(zc, src, srcSize); - FORWARD_IF_ERROR(bss); - if (bss == ZSTDbss_compress) { - cSize = ZSTD_compressSuperBlock(zc, dst, dstCapacity, lastBlock); - } } - - /* Superblock compression may fail, in which case - * encode using ZSTD_noCompressSuperBlock writing sub blocks - * in uncompressed mode. - */ - if (cSize == 0) { - cSize = ZSTD_noCompressSuperBlock(dst, dstCapacity, src, srcSize, zc->appliedParams.targetCBlockSize, lastBlock); - bss = ZSTDbss_noCompress; - /* In compression, there is an assumption that a compressed block is always - * within the size of ZSTD_compressBound(). However, SuperBlock compression - * can exceed the limit due to overhead of headers from SubBlocks. - * This breaks in streaming mode where output buffer in compress context is - * allocated ZSTD_compressBound() amount of memory, which may not be big - * enough for SuperBlock compression. - * In such case, fall back to normal compression. This is possible because - * targetCBlockSize is best effort not a guarantee. */ - if (cSize == ERROR(dstSize_tooSmall) || (dstCapacity - cSize) < 4) { - /* We check (dstCapacity - cSize) >= 4 above because we have to make sure - * to leave enough room for the checksum that will eventually get added in - * the epilogue. Otherwise, we're just going to throw the dstSize_tooSmall - * error there instead of here */ - BYTE* const ostart = (BYTE*)dst; - /* If ZSTD_noCompressSuperBlock fails with dstSize_tooSmall, - * compress normally. - */ - bss = ZSTDbss_compress; - cSize = ZSTD_compressSequences(&zc->seqStore, - &zc->blockState.prevCBlock->entropy, &zc->blockState.nextCBlock->entropy, - &zc->appliedParams, - ostart+ZSTD_blockHeaderSize, dstCapacity-ZSTD_blockHeaderSize, - srcSize, - zc->entropyWorkspace, HUF_WORKSPACE_SIZE /* statically allocated in resetCCtx */, - zc->bmi2); - - FORWARD_IF_ERROR(cSize); - if (cSize == 0) { - bss = ZSTDbss_noCompress; - /* If compressSequences didn't work, we just output a regular - * uncompressed block */ - cSize = ZSTD_noCompressBlock(dst, dstCapacity, src, srcSize, lastBlock); - FORWARD_IF_ERROR(cSize); - } else { - U32 const cBlockHeader24 = lastBlock + (((U32)bt_compressed)<<1) + (U32)(cSize << 3); - MEM_writeLE24(ostart, cBlockHeader24); - cSize += ZSTD_blockHeaderSize; - } - } - } - - if (!ZSTD_isError(cSize) && bss == ZSTDbss_compress) { - /* confirm repcodes and entropy tables when emitting a compressed block */ - ZSTD_compressedBlockState_t* const tmp = zc->blockState.prevCBlock; - zc->blockState.prevCBlock = zc->blockState.nextCBlock; - zc->blockState.nextCBlock = tmp; - } - /* We check that dictionaries have offset codes available for the first - * block. After the first block, the offcode table might not have large - * enough codes to represent the offsets in the data. - */ - if (zc->blockState.prevCBlock->entropy.fse.offcode_repeatMode == FSE_repeat_valid) - zc->blockState.prevCBlock->entropy.fse.offcode_repeatMode = FSE_repeat_check; - + cSize = ZSTD_compressBlock_targetCBlockSize_init(zc, dst, dstCapacity, src, srcSize, lastBlock); + FORWARD_IF_ERROR(cSize); + cSize = ZSTD_compressBlock_targetCBlockSize_body(zc, cSize, dst, dstCapacity, src, srcSize, lastBlock); + FORWARD_IF_ERROR(cSize); + ZSTD_compressBlock_targetCBlockSize_end(zc); return cSize; } From 2ec556fec2fcde585f307f6eae4dfa08cd4d46f4 Mon Sep 17 00:00:00 2001 From: Bimba Shrestha Date: Wed, 4 Dec 2019 15:23:13 -0800 Subject: [PATCH 12/13] Moving init/end functions, moving compressSuperBlock inside body() --- lib/compress/zstd_compress.c | 78 +++++++++++++++++------------------- 1 file changed, 37 insertions(+), 41 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 2f28b836..52badd5b 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2457,54 +2457,46 @@ static void ZSTD_confirmRepcodesAndEntropyTables(ZSTD_CCtx* zc) zc->blockState.nextCBlock = tmp; } -static size_t ZSTD_compressBlock_targetCBlockSize_init(ZSTD_CCtx* zc, - void* dst, size_t dstCapacity, - const void* src, size_t srcSize, - U32 lastBlock) -{ - const size_t bss = ZSTD_buildSeqStore(zc, src, srcSize); - FORWARD_IF_ERROR(bss); - if (bss == ZSTDbss_compress) - return ZSTD_compressSuperBlock(zc, dst, dstCapacity, lastBlock); - return 0; -} - -static void ZSTD_compressBlock_targetCBlockSize_end(ZSTD_CCtx* zc) -{ - if (zc->blockState.prevCBlock->entropy.fse.offcode_repeatMode == FSE_repeat_valid) - zc->blockState.prevCBlock->entropy.fse.offcode_repeatMode = FSE_repeat_check; -} - static size_t ZSTD_compressBlock_targetCBlockSize_body(ZSTD_CCtx* zc, - size_t cSize, void* dst, size_t dstCapacity, + const size_t bss, void* dst, size_t dstCapacity, const void* src, size_t srcSize, U32 lastBlock) { - /* Superblock compression was successful */ - if (cSize != 0) { - ZSTD_confirmRepcodesAndEntropyTables(zc); - return cSize; + /* Attempt superblock compression and return early if successful */ + { + if (bss == ZSTDbss_compress) { + size_t cSize = ZSTD_compressSuperBlock(zc, dst, dstCapacity, lastBlock); + FORWARD_IF_ERROR(cSize); + if (cSize != 0) { + ZSTD_confirmRepcodesAndEntropyTables(zc); + return cSize; + } + } } /* Superblock compression failed, attempt to emit noCompress superblocks * and return early if that is successful and we have enough room for checksum */ - cSize = ZSTD_noCompressSuperBlock(dst, dstCapacity, src, srcSize, zc->appliedParams.targetCBlockSize, lastBlock); - if (cSize != ERROR(dstSize_tooSmall) && (dstCapacity - cSize) >= 4) - return cSize; + { + size_t cSize = ZSTD_noCompressSuperBlock(dst, dstCapacity, src, srcSize, zc->appliedParams.targetCBlockSize, lastBlock); + if (cSize != ERROR(dstSize_tooSmall) && (dstCapacity - cSize) >= 4) + return cSize; + } /* noCompress superblock emission failed. Attempt to compress normally * and return early if that is successful */ - cSize = ZSTD_compressSequences(&zc->seqStore, - &zc->blockState.prevCBlock->entropy, &zc->blockState.nextCBlock->entropy, - &zc->appliedParams, (BYTE*)dst+ZSTD_blockHeaderSize, dstCapacity-ZSTD_blockHeaderSize, - srcSize, zc->entropyWorkspace, HUF_WORKSPACE_SIZE, zc->bmi2); - FORWARD_IF_ERROR(cSize); - if (cSize != 0) { - U32 const cBlockHeader24 = lastBlock + (((U32)bt_compressed)<<1) + (U32)(cSize << 3); - MEM_writeLE24((BYTE*)dst, cBlockHeader24); - cSize += ZSTD_blockHeaderSize; - ZSTD_confirmRepcodesAndEntropyTables(zc); - return cSize; + { + size_t cSize = ZSTD_compressSequences(&zc->seqStore, + &zc->blockState.prevCBlock->entropy, &zc->blockState.nextCBlock->entropy, + &zc->appliedParams, (BYTE*)dst+ZSTD_blockHeaderSize, dstCapacity-ZSTD_blockHeaderSize, + srcSize, zc->entropyWorkspace, HUF_WORKSPACE_SIZE, zc->bmi2); + FORWARD_IF_ERROR(cSize); + if (cSize != 0) { + U32 const cBlockHeader24 = lastBlock + (((U32)bt_compressed)<<1) + (U32)(cSize << 3); + MEM_writeLE24((BYTE*)dst, cBlockHeader24); + cSize += ZSTD_blockHeaderSize; + ZSTD_confirmRepcodesAndEntropyTables(zc); + return cSize; + } } /* Everything failed. Just emit a regular noCompress block */ @@ -2517,13 +2509,17 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, U32 lastBlock) { size_t cSize = 0; + const size_t bss = ZSTD_buildSeqStore(zc, src, srcSize); DEBUGLOG(5, "ZSTD_compressBlock_targetCBlockSize (dstCapacity=%u, dictLimit=%u, nextToUpdate=%u, srcSize=%zu)", (unsigned)dstCapacity, (unsigned)zc->blockState.matchState.window.dictLimit, (unsigned)zc->blockState.matchState.nextToUpdate, srcSize); - cSize = ZSTD_compressBlock_targetCBlockSize_init(zc, dst, dstCapacity, src, srcSize, lastBlock); + FORWARD_IF_ERROR(bss); + + cSize = ZSTD_compressBlock_targetCBlockSize_body(zc, bss, dst, dstCapacity, src, srcSize, lastBlock); FORWARD_IF_ERROR(cSize); - cSize = ZSTD_compressBlock_targetCBlockSize_body(zc, cSize, dst, dstCapacity, src, srcSize, lastBlock); - FORWARD_IF_ERROR(cSize); - ZSTD_compressBlock_targetCBlockSize_end(zc); + + if (zc->blockState.prevCBlock->entropy.fse.offcode_repeatMode == FSE_repeat_valid) + zc->blockState.prevCBlock->entropy.fse.offcode_repeatMode = FSE_repeat_check; + return cSize; } From e1913dc87fe8cff6e623a5ee44c69e44e5e098c2 Mon Sep 17 00:00:00 2001 From: Bimba Shrestha Date: Wed, 4 Dec 2019 15:51:17 -0800 Subject: [PATCH 13/13] Making const, removing unnecessary indent, changing parameter order --- lib/compress/zstd_compress.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 52badd5b..20fbdb10 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2458,26 +2458,24 @@ static void ZSTD_confirmRepcodesAndEntropyTables(ZSTD_CCtx* zc) } static size_t ZSTD_compressBlock_targetCBlockSize_body(ZSTD_CCtx* zc, - const size_t bss, void* dst, size_t dstCapacity, + void* dst, size_t dstCapacity, const void* src, size_t srcSize, - U32 lastBlock) + const size_t bss, U32 lastBlock) { /* Attempt superblock compression and return early if successful */ - { - if (bss == ZSTDbss_compress) { - size_t cSize = ZSTD_compressSuperBlock(zc, dst, dstCapacity, lastBlock); - FORWARD_IF_ERROR(cSize); - if (cSize != 0) { - ZSTD_confirmRepcodesAndEntropyTables(zc); - return cSize; - } + if (bss == ZSTDbss_compress) { + size_t const cSize = ZSTD_compressSuperBlock(zc, dst, dstCapacity, lastBlock); + FORWARD_IF_ERROR(cSize); + if (cSize != 0) { + ZSTD_confirmRepcodesAndEntropyTables(zc); + return cSize; } } /* Superblock compression failed, attempt to emit noCompress superblocks * and return early if that is successful and we have enough room for checksum */ { - size_t cSize = ZSTD_noCompressSuperBlock(dst, dstCapacity, src, srcSize, zc->appliedParams.targetCBlockSize, lastBlock); + size_t const cSize = ZSTD_noCompressSuperBlock(dst, dstCapacity, src, srcSize, zc->appliedParams.targetCBlockSize, lastBlock); if (cSize != ERROR(dstSize_tooSmall) && (dstCapacity - cSize) >= 4) return cSize; } @@ -2485,7 +2483,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize_body(ZSTD_CCtx* zc, /* noCompress superblock emission failed. Attempt to compress normally * and return early if that is successful */ { - size_t cSize = ZSTD_compressSequences(&zc->seqStore, + size_t const cSize = ZSTD_compressSequences(&zc->seqStore, &zc->blockState.prevCBlock->entropy, &zc->blockState.nextCBlock->entropy, &zc->appliedParams, (BYTE*)dst+ZSTD_blockHeaderSize, dstCapacity-ZSTD_blockHeaderSize, srcSize, zc->entropyWorkspace, HUF_WORKSPACE_SIZE, zc->bmi2); @@ -2493,9 +2491,8 @@ static size_t ZSTD_compressBlock_targetCBlockSize_body(ZSTD_CCtx* zc, if (cSize != 0) { U32 const cBlockHeader24 = lastBlock + (((U32)bt_compressed)<<1) + (U32)(cSize << 3); MEM_writeLE24((BYTE*)dst, cBlockHeader24); - cSize += ZSTD_blockHeaderSize; ZSTD_confirmRepcodesAndEntropyTables(zc); - return cSize; + return cSize + ZSTD_blockHeaderSize; } } @@ -2514,7 +2511,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize(ZSTD_CCtx* zc, (unsigned)dstCapacity, (unsigned)zc->blockState.matchState.window.dictLimit, (unsigned)zc->blockState.matchState.nextToUpdate, srcSize); FORWARD_IF_ERROR(bss); - cSize = ZSTD_compressBlock_targetCBlockSize_body(zc, bss, dst, dstCapacity, src, srcSize, lastBlock); + cSize = ZSTD_compressBlock_targetCBlockSize_body(zc, dst, dstCapacity, src, srcSize, bss, lastBlock); FORWARD_IF_ERROR(cSize); if (zc->blockState.prevCBlock->entropy.fse.offcode_repeatMode == FSE_repeat_valid)