From e33dfd984fa71d44b690921abd88130574926e67 Mon Sep 17 00:00:00 2001 From: fgenesis Date: Tue, 18 Jan 2022 00:36:34 +0100 Subject: [PATCH 1/6] Rename ZSTD_d_stableOutBuffer to ZSTD_d_outBufferMode and add exposed window decompression mode --- lib/common/zstd_internal.h | 3 +- lib/decompress/zstd_decompress.c | 46 +++++++++---- lib/decompress/zstd_decompress_internal.h | 2 +- lib/zstd.h | 84 ++++++++++++++--------- tests/zstreamtest.c | 64 +++++++++++++++-- 5 files changed, 146 insertions(+), 53 deletions(-) diff --git a/lib/common/zstd_internal.h b/lib/common/zstd_internal.h index e4d36ce0905..a4fc6493d07 100644 --- a/lib/common/zstd_internal.h +++ b/lib/common/zstd_internal.h @@ -276,7 +276,8 @@ MEM_STATIC size_t ZSTD_limitCopy(void* dst, size_t dstCapacity, const void* src, /* Controls whether the input/output buffer is buffered or stable. */ typedef enum { ZSTD_bm_buffered = 0, /* Buffer the input/output */ - ZSTD_bm_stable = 1 /* ZSTD_inBuffer/ZSTD_outBuffer is stable */ + ZSTD_bm_stable = 1, /* ZSTD_inBuffer/ZSTD_outBuffer is stable */ + ZSTD_bm_expose = 2 /* Set ZSTD_outBuffer to internal window */ } ZSTD_bufferMode_e; diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 0031e98cfb1..41530b6a55a 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -1708,9 +1708,9 @@ ZSTD_bounds ZSTD_dParam_getBounds(ZSTD_dParameter dParam) bounds.upperBound = (int)ZSTD_f_zstd1_magicless; ZSTD_STATIC_ASSERT(ZSTD_f_zstd1 < ZSTD_f_zstd1_magicless); return bounds; - case ZSTD_d_stableOutBuffer: + case ZSTD_d_outBufferMode: bounds.lowerBound = (int)ZSTD_bm_buffered; - bounds.upperBound = (int)ZSTD_bm_stable; + bounds.upperBound = (int)ZSTD_bm_expose; return bounds; case ZSTD_d_forceIgnoreChecksum: bounds.lowerBound = (int)ZSTD_d_validateChecksum; @@ -1751,7 +1751,7 @@ size_t ZSTD_DCtx_getParameter(ZSTD_DCtx* dctx, ZSTD_dParameter param, int* value case ZSTD_d_format: *value = (int)dctx->format; return 0; - case ZSTD_d_stableOutBuffer: + case ZSTD_d_outBufferMode: *value = (int)dctx->outBufferMode; return 0; case ZSTD_d_forceIgnoreChecksum: @@ -1778,8 +1778,8 @@ size_t ZSTD_DCtx_setParameter(ZSTD_DCtx* dctx, ZSTD_dParameter dParam, int value CHECK_DBOUNDS(ZSTD_d_format, value); dctx->format = (ZSTD_format_e)value; return 0; - case ZSTD_d_stableOutBuffer: - CHECK_DBOUNDS(ZSTD_d_stableOutBuffer, value); + case ZSTD_d_outBufferMode: + CHECK_DBOUNDS(ZSTD_d_outBufferMode, value); dctx->outBufferMode = (ZSTD_bufferMode_e)value; return 0; case ZSTD_d_forceIgnoreChecksum: @@ -1873,11 +1873,11 @@ static int ZSTD_DCtx_isOversizedTooLong(ZSTD_DStream* zds) return zds->oversizedDuration >= ZSTD_WORKSPACETOOLARGE_MAXDURATION; } -/* Checks that the output buffer hasn't changed if ZSTD_obm_stable is used. */ +/* Checks that the output buffer hasn't changed if ZSTD_bm_stable is used. */ static size_t ZSTD_checkOutBuffer(ZSTD_DStream const* zds, ZSTD_outBuffer const* output) { ZSTD_outBuffer const expect = zds->expectedOutBuffer; - /* No requirement when ZSTD_obm_stable is not enabled. */ + /* No requirement when ZSTD_bm_stable is not enabled. */ if (zds->outBufferMode != ZSTD_bm_stable) return 0; /* Any buffer is allowed in zdss_init, this must be the same for every other call until @@ -1888,7 +1888,7 @@ static size_t ZSTD_checkOutBuffer(ZSTD_DStream const* zds, ZSTD_outBuffer const* /* The buffer must match our expectation exactly. */ if (expect.dst == output->dst && expect.pos == output->pos && expect.size == output->size) return 0; - RETURN_ERROR(dstBuffer_wrong, "ZSTD_d_stableOutBuffer enabled but output differs!"); + RETURN_ERROR(dstBuffer_wrong, "ZSTD_d_outBufferMode == ZSTD_bufmode_stable but output differs!"); } /* Calls ZSTD_decompressContinue() with the right parameters for ZSTD_decompressStream() @@ -1900,7 +1900,7 @@ static size_t ZSTD_decompressContinueStream( ZSTD_DStream* zds, char** op, char* oend, void const* src, size_t srcSize) { int const isSkipFrame = ZSTD_isSkipFrame(zds); - if (zds->outBufferMode == ZSTD_bm_buffered) { + if (zds->outBufferMode != ZSTD_bm_stable) { size_t const dstSize = isSkipFrame ? 0 : zds->outBuffSize - zds->outStart; size_t const decodedSize = ZSTD_decompressContinue(zds, zds->outBuff + zds->outStart, dstSize, src, srcSize); @@ -1944,7 +1944,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB "forbidden. in: pos: %u vs size: %u", (U32)input->pos, (U32)input->size); RETURN_ERROR_IF( - output->pos > output->size, + output->pos > output->size && zds->outBufferMode != ZSTD_bm_expose, dstSize_tooSmall, "forbidden. out: pos: %u vs size: %u", (U32)output->pos, (U32)output->size); @@ -1991,6 +1991,8 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB DEBUGLOG(5, "ZSTD_decompressStream: detected legacy version v0.%u", legacyVersion); RETURN_ERROR_IF(zds->staticSize, memory_allocation, "legacy support is incompatible with static dctx"); + RETURN_ERROR_IF(zds->outBufferMode == ZSTD_bm_expose, parameter_unsupported, + "legacy support is incompatible with ZSTD_bufmode_expose"); FORWARD_IF_ERROR(ZSTD_initLegacyStream(&zds->legacyContext, zds->previousLegacyVersion, legacyVersion, dict, dictSize), ""); @@ -2022,6 +2024,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB /* check for single-pass mode opportunity */ if (zds->fParams.frameContentSize != ZSTD_CONTENTSIZE_UNKNOWN && zds->fParams.frameType != ZSTD_skippableFrame + && zds->outBufferMode != ZSTD_bm_expose && (U64)(size_t)(oend-op) >= zds->fParams.frameContentSize) { size_t const cSize = ZSTD_findFrameCompressedSize(istart, (size_t)(iend-istart)); if (cSize <= (size_t)(iend-istart)) { @@ -2037,12 +2040,12 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB break; } } - /* Check output buffer is large enough for ZSTD_odm_stable. */ + /* Check output buffer is large enough for ZSTD_bm_stable. */ if (zds->outBufferMode == ZSTD_bm_stable && zds->fParams.frameType != ZSTD_skippableFrame && zds->fParams.frameContentSize != ZSTD_CONTENTSIZE_UNKNOWN && (U64)(size_t)(oend-op) < zds->fParams.frameContentSize) { - RETURN_ERROR(dstSize_tooSmall, "ZSTD_obm_stable passed but ZSTD_outBuffer is too small"); + RETURN_ERROR(dstSize_tooSmall, "ZSTD_bm_stable passed but ZSTD_outBuffer is too small"); } /* Consume header (see ZSTDds_decodeFrameHeader) */ @@ -2068,7 +2071,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB /* Adapt buffer sizes to frame header instructions */ { size_t const neededInBuffSize = MAX(zds->fParams.blockSizeMax, 4 /* frame checksum */); - size_t const neededOutBuffSize = zds->outBufferMode == ZSTD_bm_buffered + size_t const neededOutBuffSize = zds->outBufferMode != ZSTD_bm_stable ? ZSTD_decodingBufferSize_min(zds->fParams.windowSize, zds->fParams.frameContentSize) : 0; @@ -2149,10 +2152,25 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB } case zdss_flush: { size_t const toFlushSize = zds->outEnd - zds->outStart; + if (zds->outBufferMode == ZSTD_bm_expose) + { + output->dst = op = zds->outBuff + zds->outStart; + output->size = toFlushSize; + zds->streamStage = zdss_flushdone; + zds->outStart += toFlushSize; + someMoreWork = 0; + break; + } size_t const flushedSize = ZSTD_limitCopy(op, (size_t)(oend-op), zds->outBuff + zds->outStart, toFlushSize); op += flushedSize; zds->outStart += flushedSize; if (flushedSize == toFlushSize) { /* flush completed */ + case zdss_flushdone: + if (zds->outBufferMode == ZSTD_bm_expose) /* Caller should have consumed & acknowledged the output */ + { + assert(output->dst == NULL); + assert(output->size == 0); + } zds->streamStage = zdss_read; if ( (zds->outBuffSize < zds->fParams.frameContentSize) && (zds->outStart + zds->fParams.blockSizeMax > zds->outBuffSize) ) { @@ -2176,7 +2194,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB input->pos = (size_t)(ip - (const char*)(input->src)); output->pos = (size_t)(op - (char*)(output->dst)); - /* Update the expected output buffer for ZSTD_obm_stable. */ + /* Update the expected output buffer for ZSTD_bm_stable. */ zds->expectedOutBuffer = *output; if ((ip==istart) && (op==ostart)) { /* no forward progress */ diff --git a/lib/decompress/zstd_decompress_internal.h b/lib/decompress/zstd_decompress_internal.h index 2b5a53850ac..76625c23a6d 100644 --- a/lib/decompress/zstd_decompress_internal.h +++ b/lib/decompress/zstd_decompress_internal.h @@ -91,7 +91,7 @@ typedef enum { ZSTDds_getFrameHeaderSize, ZSTDds_decodeFrameHeader, ZSTDds_decodeSkippableHeader, ZSTDds_skipFrame } ZSTD_dStage; typedef enum { zdss_init=0, zdss_loadHeader, - zdss_read, zdss_load, zdss_flush } ZSTD_dStreamStage; + zdss_read, zdss_load, zdss_flush, zdss_flushdone } ZSTD_dStreamStage; typedef enum { ZSTD_use_indefinitely = -1, /* Use the dictionary indefinitely */ diff --git a/lib/zstd.h b/lib/zstd.h index a88ae7bf8ed..018b87585fe 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -549,7 +549,7 @@ typedef enum { * within the experimental section of the API. * At the time of this writing, they include : * ZSTD_d_format - * ZSTD_d_stableOutBuffer + * ZSTD_d_outBufferMode * ZSTD_d_forceIgnoreChecksum * ZSTD_d_refMultipleDDicts * Because they are not stable, it's necessary to define ZSTD_STATIC_LINKING_ONLY to access them. @@ -1321,6 +1321,52 @@ typedef enum { ZSTD_ps_disable = 2 /* Do not use the feature */ } ZSTD_paramSwitch_e; +typedef enum { + /* Default. Let the (de)compressor allocate internal input/output buffers as needed. */ + ZSTD_bufmode_buffered = 0, + + /* Tells the (de)compressor that the ZSTD_outBuffer will ALWAYS be the same + * between calls, except for the modifications that zstd makes to pos (the + * caller must not modify pos). This is checked by the decompressor, and + * decompression will fail if it ever changes. Therefore the ZSTD_outBuffer + * MUST be large enough to fit the entire decompressed frame. This will be + * checked when the frame content size is known.The data in the ZSTD_outBuffer + * in the range[dst, dst + pos) MUST not be modified during decompression + * or you will get data corruption. + * + * When this flags is enabled zstd won't allocate an output buffer, because + * it can write directly to the ZSTD_outBuffer, but it will still allocate + * an input buffer large enough to fit any compressed block.This will also + * avoid the memcpy() from the internal output buffer to the ZSTD_outBuffer. + * If you need to avoid the input buffer allocation use the buffer-less + * streaming API. + * + * NOTE: So long as the ZSTD_outBuffer always points to valid memory, using + * this flag is ALWAYS memory safe, and will never access out -of-bounds + * memory. However, decompression WILL fail if you violate the preconditions. + * + * WARNING: The data in the ZSTD_outBuffer in the range[dst, dst + pos) MUST + * not be modified during decompression or you will get data corruption. This + * is because zstd needs to reference data in the ZSTD_outBuffer to regenerate + * matches. Normally zstd maintains its own buffer for this purpose, but passing + * this flag tells zstd to use the user provided buffer. */ + ZSTD_bufmode_stable = 1, + + /* Tells the decompressor to not copy its internal buffer. + * Instead, once output data are available, the ZSTD_outBuffer's size and dst + * fields are updated so that dst[0..size) is a view into the internal + * decompressor window; no copy operation takes place. + * The caller must zero the ZSTD_outBuffer size and dst fields to acknowledge + * that the data were processed before continuing decompression. + * Not doing so will assert. + * The data stay valid until the next call into the decompressor. + * The window data must NOT be modified or you will get data corruption. + * NOTE: The ZSTD_outBuffer's pos field is not used and should be 0. + * This mode is not compatible with legacy streams and is currently + * only supported for decompression. */ + ZSTD_bufmode_expose = 2, +} ZSTD_bufmode_e; + /*************************************** * Frame size functions ***************************************/ @@ -1862,7 +1908,7 @@ ZSTDLIB_STATIC_API size_t ZSTD_CCtx_refPrefix_advanced(ZSTD_CCtx* cctx, const vo * Experimental parameter. * Default is 0 == disabled. Set to 1 to enable. * - * Tells he compressor that the ZSTD_outBuffer will not be resized between + * Tells the compressor that the ZSTD_outBuffer will not be resized between * calls. Specifically: (out.size - out.pos) will never grow. This gives the * compressor the freedom to say: If the compressed data doesn't fit in the * output buffer then return ZSTD_error_dstSizeTooSmall. This allows us to @@ -2092,37 +2138,13 @@ ZSTDLIB_STATIC_API size_t ZSTD_DCtx_getParameter(ZSTD_DCtx* dctx, ZSTD_dParamete * allowing selection between ZSTD_format_e input compression formats */ #define ZSTD_d_format ZSTD_d_experimentalParam1 -/* ZSTD_d_stableOutBuffer + +/* ZSTD_d_outBufferMode * Experimental parameter. - * Default is 0 == disabled. Set to 1 to enable. - * - * Tells the decompressor that the ZSTD_outBuffer will ALWAYS be the same - * between calls, except for the modifications that zstd makes to pos (the - * caller must not modify pos). This is checked by the decompressor, and - * decompression will fail if it ever changes. Therefore the ZSTD_outBuffer - * MUST be large enough to fit the entire decompressed frame. This will be - * checked when the frame content size is known. The data in the ZSTD_outBuffer - * in the range [dst, dst + pos) MUST not be modified during decompression - * or you will get data corruption. - * - * When this flags is enabled zstd won't allocate an output buffer, because - * it can write directly to the ZSTD_outBuffer, but it will still allocate - * an input buffer large enough to fit any compressed block. This will also - * avoid the memcpy() from the internal output buffer to the ZSTD_outBuffer. - * If you need to avoid the input buffer allocation use the buffer-less - * streaming API. - * - * NOTE: So long as the ZSTD_outBuffer always points to valid memory, using - * this flag is ALWAYS memory safe, and will never access out-of-bounds - * memory. However, decompression WILL fail if you violate the preconditions. - * - * WARNING: The data in the ZSTD_outBuffer in the range [dst, dst + pos) MUST - * not be modified during decompression or you will get data corruption. This - * is because zstd needs to reference data in the ZSTD_outBuffer to regenerate - * matches. Normally zstd maintains its own buffer for this purpose, but passing - * this flag tells zstd to use the user provided buffer. + * Default is ZSTD_bufmode_buffered. + * Param has values of type ZSTD_bufmode_e */ -#define ZSTD_d_stableOutBuffer ZSTD_d_experimentalParam2 +#define ZSTD_d_outBufferMode ZSTD_d_experimentalParam2 /* ZSTD_d_forceIgnoreChecksum * Experimental parameter. diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index 72fd72ea368..06a1836426a 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -664,7 +664,7 @@ static int basicUnitTests(U32 seed, double compressibility) DISPLAYLEVEL(3, "test%3i : ZSTD_decompressStream() single pass on empty frame : ", testNb++); { ZSTD_DCtx* dctx = ZSTD_createDCtx(); size_t const dctxSize = ZSTD_sizeof_DCtx(dctx); - CHECK_Z(ZSTD_DCtx_setParameter(dctx, ZSTD_d_stableOutBuffer, 1)); + CHECK_Z(ZSTD_DCtx_setParameter(dctx, ZSTD_d_outBufferMode, ZSTD_bufmode_stable)); outBuff.dst = decodedBuffer; outBuff.pos = 0; @@ -684,13 +684,13 @@ static int basicUnitTests(U32 seed, double compressibility) } DISPLAYLEVEL(3, "OK \n"); - /* Decompression with ZSTD_d_stableOutBuffer */ + /* Decompression with ZSTD_d_outBufferMode == ZSTD_bufmode_stable */ cSize = ZSTD_compress(compressedBuffer, compressedBufferSize, CNBuffer, CNBufferSize, 1); CHECK_Z(cSize); { ZSTD_DCtx* dctx = ZSTD_createDCtx(); size_t const dctxSize0 = ZSTD_sizeof_DCtx(dctx); size_t dctxSize1; - CHECK_Z(ZSTD_DCtx_setParameter(dctx, ZSTD_d_stableOutBuffer, 1)); + CHECK_Z(ZSTD_DCtx_setParameter(dctx, ZSTD_d_outBufferMode, ZSTD_bufmode_stable)); outBuff.dst = decodedBuffer; outBuff.pos = 0; @@ -725,7 +725,7 @@ static int basicUnitTests(U32 seed, double compressibility) DISPLAYLEVEL(3, "test%3i : ZSTD_decompressStream() stable out buffer too small : ", testNb++); ZSTD_DCtx_reset(dctx, ZSTD_reset_session_only); - CHECK_Z(ZSTD_DCtx_setParameter(dctx, ZSTD_d_stableOutBuffer, 1)); + CHECK_Z(ZSTD_DCtx_setParameter(dctx, ZSTD_d_outBufferMode, ZSTD_bufmode_stable)); inBuff.src = compressedBuffer; inBuff.size = cSize; inBuff.pos = 0; @@ -738,7 +738,7 @@ static int basicUnitTests(U32 seed, double compressibility) DISPLAYLEVEL(3, "test%3i : ZSTD_decompressStream() stable out buffer modified : ", testNb++); ZSTD_DCtx_reset(dctx, ZSTD_reset_session_only); - CHECK_Z(ZSTD_DCtx_setParameter(dctx, ZSTD_d_stableOutBuffer, 1)); + CHECK_Z(ZSTD_DCtx_setParameter(dctx, ZSTD_d_outBufferMode, ZSTD_bufmode_stable)); inBuff.src = compressedBuffer; inBuff.size = cSize - 1; inBuff.pos = 0; @@ -754,7 +754,7 @@ static int basicUnitTests(U32 seed, double compressibility) DISPLAYLEVEL(3, "test%3i : ZSTD_decompressStream() buffered output : ", testNb++); ZSTD_DCtx_reset(dctx, ZSTD_reset_session_only); - CHECK_Z(ZSTD_DCtx_setParameter(dctx, ZSTD_d_stableOutBuffer, 0)); + CHECK_Z(ZSTD_DCtx_setParameter(dctx, ZSTD_d_outBufferMode, ZSTD_bufmode_buffered)); outBuff.pos = 0; inBuff.pos = 0; inBuff.size = 0; @@ -770,6 +770,58 @@ static int basicUnitTests(U32 seed, double compressibility) ZSTD_freeDCtx(dctx); } + /* Decompression with ZSTD_d_outBufferMode == ZSTD_bufmode_expose */ + { ZSTD_DCtx* dctx = ZSTD_createDCtx(); + CHECK_Z(ZSTD_DCtx_setParameter(dctx, ZSTD_d_outBufferMode, ZSTD_bufmode_expose)); + + DISPLAYLEVEL(3, "test%3i : ZSTD_decompressStream() exposed window all input: ", testNb++); + inBuff.src = compressedBuffer; + inBuff.pos = 0; + inBuff.size = cSize; + outBuff.dst = NULL; /* Set by decomp */ + outBuff.size = 0; /* Set by decomp */ + outBuff.pos = 0; /* Not used */ + size_t total = 0; + while (inBuff.pos < cSize) { + CHECK_Z(ZSTD_decompressStream(dctx, &outBuff, &inBuff)); + CHECK(!outBuff.dst != !outBuff.size, "Should have both dst & size set or neither"); + if (outBuff.size) { + CHECK(memcmp((char*)CNBuffer + total, outBuff.dst, outBuff.size) != 0, "Corruption!"); + total += outBuff.size; + outBuff.size = 0; /* Acknowledge */ + outBuff.dst = NULL; + } + } + CHECK(total != CNBufferSize, "Wrong size!"); + DISPLAYLEVEL(3, "OK \n"); + + DISPLAYLEVEL(3, "test%3i : ZSTD_decompressStream() exposed window piecemeal : ", testNb++); + ZSTD_DCtx_reset(dctx, ZSTD_reset_session_only); + CHECK_Z(ZSTD_DCtx_setParameter(dctx, ZSTD_d_outBufferMode, ZSTD_bufmode_expose)); + inBuff.src = compressedBuffer; + inBuff.pos = 0; + inBuff.size = 0; + outBuff.dst = NULL; /* Set by decomp */ + outBuff.size = 0; /* Set by decomp */ + outBuff.pos = 0; /* Not used */ + total = 0; + while (inBuff.pos < cSize) { + inBuff.size += MIN(cSize - inBuff.pos, 1 + (FUZ_rand(&coreSeed) & 15)); + CHECK_Z(ZSTD_decompressStream(dctx, &outBuff, &inBuff)); + CHECK(!outBuff.dst != !outBuff.size, "Should have both dst & size set or neither"); + if (outBuff.size) { + CHECK(memcmp((char*)CNBuffer + total, outBuff.dst, outBuff.size) != 0, "Corruption!"); + total += outBuff.size; + outBuff.size = 0; /* Acknowledge */ + outBuff.dst = NULL; + } + } + CHECK(total != CNBufferSize, "Wrong size!"); + DISPLAYLEVEL(3, "OK \n"); + + ZSTD_freeDCtx(dctx); + } + /* Compression with ZSTD_c_stable{In,Out}Buffer */ { ZSTD_CCtx* cctx = ZSTD_createCCtx(); ZSTD_inBuffer in; From 951453a6678e91b849092f5dfdfba9ba2876c63f Mon Sep 17 00:00:00 2001 From: fgenesis Date: Thu, 20 Jan 2022 19:34:40 +0100 Subject: [PATCH 2/6] small cleanup of prev. commit --- lib/decompress/zstd_decompress.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 41530b6a55a..0bcf4bc0670 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -1877,10 +1877,18 @@ static int ZSTD_DCtx_isOversizedTooLong(ZSTD_DStream* zds) static size_t ZSTD_checkOutBuffer(ZSTD_DStream const* zds, ZSTD_outBuffer const* output) { ZSTD_outBuffer const expect = zds->expectedOutBuffer; - /* No requirement when ZSTD_bm_stable is not enabled. */ - if (zds->outBufferMode != ZSTD_bm_stable) + /* No requirement when ZSTD_bm_buffered is enabled (default). */ + if (zds->outBufferMode == ZSTD_bm_buffered) return 0; - /* Any buffer is allowed in zdss_init, this must be the same for every other call until + /* With ZSTD_bm_expose, output must be cleared because it will be set only on buffer flush. + Clearing output is not strictly necessary but adds extrra safety against user error. */ + if (zds->outBufferMode == ZSTD_bm_expose) { + if(output->dst || output->size) + RETURN_ERROR(dstBuffer_wrong, "must acknowledge output by clearing dst and size"); + return 0; + } + /* ZSTD_bm_stable: + * Any buffer is allowed in zdss_init, this must be the same for every other call until * the context is reset. */ if (zds->streamStage == zdss_init) @@ -2166,11 +2174,6 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB zds->outStart += flushedSize; if (flushedSize == toFlushSize) { /* flush completed */ case zdss_flushdone: - if (zds->outBufferMode == ZSTD_bm_expose) /* Caller should have consumed & acknowledged the output */ - { - assert(output->dst == NULL); - assert(output->size == 0); - } zds->streamStage = zdss_read; if ( (zds->outBuffSize < zds->fParams.frameContentSize) && (zds->outStart + zds->fParams.blockSizeMax > zds->outBuffSize) ) { From 347f3ab410e1ca982ae7c6637368b177b732fbe2 Mon Sep 17 00:00:00 2001 From: fgenesis Date: Thu, 20 Jan 2022 19:52:38 +0100 Subject: [PATCH 3/6] replace dirty case with a cleaner zdss_flushdone block --- lib/decompress/zstd_decompress.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 0bcf4bc0670..c7088c5f424 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -2172,21 +2172,23 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB size_t const flushedSize = ZSTD_limitCopy(op, (size_t)(oend-op), zds->outBuff + zds->outStart, toFlushSize); op += flushedSize; zds->outStart += flushedSize; - if (flushedSize == toFlushSize) { /* flush completed */ - case zdss_flushdone: - zds->streamStage = zdss_read; - if ( (zds->outBuffSize < zds->fParams.frameContentSize) - && (zds->outStart + zds->fParams.blockSizeMax > zds->outBuffSize) ) { - DEBUGLOG(5, "restart filling outBuff from beginning (left:%i, needed:%u)", - (int)(zds->outBuffSize - zds->outStart), - (U32)zds->fParams.blockSizeMax); - zds->outStart = zds->outEnd = 0; - } + if (flushedSize != toFlushSize) { + /* cannot complete flush */ + someMoreWork = 0; break; - } } - /* cannot complete flush */ - someMoreWork = 0; - break; + } + } + ZSTD_FALLTHROUGH; + case zdss_flushdone: /* flush completed */ + zds->streamStage = zdss_read; + if ( (zds->outBuffSize < zds->fParams.frameContentSize) + && (zds->outStart + zds->fParams.blockSizeMax > zds->outBuffSize) ) { + DEBUGLOG(5, "restart filling outBuff from beginning (left:%i, needed:%u)", + (int)(zds->outBuffSize - zds->outStart), + (U32)zds->fParams.blockSizeMax); + zds->outStart = zds->outEnd = 0; + } + break; default: assert(0); /* impossible */ From 67487687cc3d6518074c46ada2a95daf15f6bcaa Mon Sep 17 00:00:00 2001 From: fgenesis Date: Thu, 20 Jan 2022 20:10:55 +0100 Subject: [PATCH 4/6] add one more test that output != 0 fails properly in exposed buffer mode --- tests/zstreamtest.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index 06a1836426a..5fcd559c65e 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -819,6 +819,30 @@ static int basicUnitTests(U32 seed, double compressibility) CHECK(total != CNBufferSize, "Wrong size!"); DISPLAYLEVEL(3, "OK \n"); + DISPLAYLEVEL(3, "test%3i : ZSTD_decompressStream() exposed window output error check : ", testNb++); + ZSTD_DCtx_reset(dctx, ZSTD_reset_session_only); + CHECK_Z(ZSTD_DCtx_setParameter(dctx, ZSTD_d_outBufferMode, ZSTD_bufmode_expose)); + inBuff.src = compressedBuffer; + inBuff.pos = 0; + inBuff.size = 0; + outBuff.dst = NULL; /* Set by decomp */ + outBuff.size = 0; /* Set by decomp */ + outBuff.pos = 0; /* Not used */ + int mustfail = 0; + while (inBuff.pos < cSize) { + inBuff.size += MIN(cSize - inBuff.pos, 1 + (FUZ_rand(&coreSeed) & 15)); + size_t dec = ZSTD_decompressStream(dctx, &outBuff, &inBuff); + if (mustfail) { + CHECK(ZSTD_getErrorCode(dec) != ZSTD_error_dstBuffer_wrong, "should fail when output is not cleared"); + break; /* failed as it should -> done here */ + } + else + CHECK_Z(dec); + CHECK(!outBuff.dst != !outBuff.size, "Should have both dst & size set or neither"); + if (outBuff.size) + mustfail = 1; + } + ZSTD_freeDCtx(dctx); } From 681dfef42d4886ae3875f50083ea113baaaf7507 Mon Sep 17 00:00:00 2001 From: fgenesis Date: Thu, 20 Jan 2022 21:06:08 +0100 Subject: [PATCH 5/6] Make C90 happy --- lib/decompress/zstd_decompress.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index c7088c5f424..01d51a74ce1 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -2160,6 +2160,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB } case zdss_flush: { size_t const toFlushSize = zds->outEnd - zds->outStart; + size_t flushedSize; if (zds->outBufferMode == ZSTD_bm_expose) { output->dst = op = zds->outBuff + zds->outStart; @@ -2169,7 +2170,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB someMoreWork = 0; break; } - size_t const flushedSize = ZSTD_limitCopy(op, (size_t)(oend-op), zds->outBuff + zds->outStart, toFlushSize); + flushedSize = ZSTD_limitCopy(op, (size_t)(oend-op), zds->outBuff + zds->outStart, toFlushSize); op += flushedSize; zds->outStart += flushedSize; if (flushedSize != toFlushSize) { From 38ce46112d8f383fb5cbf0187ce38d5eb6214c66 Mon Sep 17 00:00:00 2001 From: fgenesis Date: Thu, 20 Jan 2022 21:30:47 +0100 Subject: [PATCH 6/6] Fix build with VS2008. This should really be ok C89 now. --- tests/zstreamtest.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index 5fcd559c65e..b6fd68973a3 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -772,16 +772,17 @@ static int basicUnitTests(U32 seed, double compressibility) /* Decompression with ZSTD_d_outBufferMode == ZSTD_bufmode_expose */ { ZSTD_DCtx* dctx = ZSTD_createDCtx(); - CHECK_Z(ZSTD_DCtx_setParameter(dctx, ZSTD_d_outBufferMode, ZSTD_bufmode_expose)); - + int mustfail = 0; + size_t total = 0; + size_t dec = 0; DISPLAYLEVEL(3, "test%3i : ZSTD_decompressStream() exposed window all input: ", testNb++); + CHECK_Z(ZSTD_DCtx_setParameter(dctx, ZSTD_d_outBufferMode, ZSTD_bufmode_expose)); inBuff.src = compressedBuffer; inBuff.pos = 0; inBuff.size = cSize; outBuff.dst = NULL; /* Set by decomp */ outBuff.size = 0; /* Set by decomp */ outBuff.pos = 0; /* Not used */ - size_t total = 0; while (inBuff.pos < cSize) { CHECK_Z(ZSTD_decompressStream(dctx, &outBuff, &inBuff)); CHECK(!outBuff.dst != !outBuff.size, "Should have both dst & size set or neither"); @@ -828,10 +829,9 @@ static int basicUnitTests(U32 seed, double compressibility) outBuff.dst = NULL; /* Set by decomp */ outBuff.size = 0; /* Set by decomp */ outBuff.pos = 0; /* Not used */ - int mustfail = 0; while (inBuff.pos < cSize) { inBuff.size += MIN(cSize - inBuff.pos, 1 + (FUZ_rand(&coreSeed) & 15)); - size_t dec = ZSTD_decompressStream(dctx, &outBuff, &inBuff); + dec = ZSTD_decompressStream(dctx, &outBuff, &inBuff); if (mustfail) { CHECK(ZSTD_getErrorCode(dec) != ZSTD_error_dstBuffer_wrong, "should fail when output is not cleared"); break; /* failed as it should -> done here */