From db1668a43b14df7159400d552e34d20627ba3218 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 29 Sep 2017 18:05:18 -0700 Subject: [PATCH 1/5] fix : srcSize written in frame header when multiple files compressed This information used to be disabled when nbFiles>1. It was badly initialized later in the code, resulting in an error. --- lib/compress/zstd_compress.c | 10 ++++++---- programs/fileio.c | 15 +++++++++------ 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 4c2254abe..9553ea9cd 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -822,11 +822,13 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, ZSTD_compResetPolicy_e const crp, ZSTD_buffered_policy_e const zbuff) { + DEBUGLOG(4, "ZSTD_resetCCtx_internal"); assert(!ZSTD_isError(ZSTD_checkCParams(params.cParams))); + DEBUGLOG(4, "pledgedSrcSize: %u", (U32)pledgedSrcSize); if (crp == ZSTDcrp_continue) { if (ZSTD_equivalentParams(params, zc->appliedParams)) { - DEBUGLOG(5, "ZSTD_equivalentParams()==1"); + DEBUGLOG(4, "ZSTD_equivalentParams()==1"); assert(!(params.ldmParams.enableLdm && params.ldmParams.hashEveryLog == ZSTD_LDM_HASHEVERYLOG_NOTSET)); zc->entropy->hufCTable_repeatMode = HUF_repeat_none; @@ -2011,8 +2013,6 @@ static size_t ZSTD_compressBegin_internal(ZSTD_CCtx* cctx, ZSTD_buffered_policy_e zbuff) { DEBUGLOG(4, "ZSTD_compressBegin_internal"); - DEBUGLOG(4, "dict ? %s", dict ? "dict" : (cdict ? "cdict" : "none")); - DEBUGLOG(4, "dictMode : %u", (U32)dictMode); /* params are supposed to be fully validated at this point */ assert(!ZSTD_isError(ZSTD_checkCParams(params.cParams))); assert(!((dict) && (cdict))); /* either dict or cdict, not both */ @@ -2485,7 +2485,7 @@ size_t ZSTD_resetCStream(ZSTD_CStream* zcs, unsigned long long pledgedSrcSize) ZSTD_CCtx_params params = zcs->requestedParams; params.fParams.contentSizeFlag = (pledgedSrcSize > 0); params.cParams = ZSTD_getCParamsFromCCtxParams(params, pledgedSrcSize, 0); - DEBUGLOG(5, "ZSTD_resetCStream"); + DEBUGLOG(4, "ZSTD_resetCStream"); return ZSTD_resetCStream_internal(zcs, NULL, 0, ZSTD_dm_auto, zcs->cdict, params, pledgedSrcSize); } @@ -2497,6 +2497,7 @@ size_t ZSTD_initCStream_internal(ZSTD_CStream* zcs, const void* dict, size_t dictSize, const ZSTD_CDict* cdict, ZSTD_CCtx_params params, unsigned long long pledgedSrcSize) { + DEBUGLOG(4, "ZSTD_initCStream_internal"); assert(!ZSTD_isError(ZSTD_checkCParams(params.cParams))); assert(!((dict) && (cdict))); /* either dict or cdict, not both */ @@ -2768,6 +2769,7 @@ size_t ZSTD_compress_generic (ZSTD_CCtx* cctx, cctx->requestedParams, cctx->pledgedSrcSizePlusOne-1, 0 /*dictSize*/); memset(&cctx->prefixDict, 0, sizeof(cctx->prefixDict)); /* single usage */ assert(prefixDict.dict==NULL || cctx->cdict==NULL); /* only one can be set */ + DEBUGLOG(4, "ZSTD_compress_generic : transparent init stage"); #ifdef ZSTD_MULTITHREAD if (params.nbThreads > 1) { diff --git a/programs/fileio.c b/programs/fileio.c index 9c5e2c40b..76726b475 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -757,6 +757,7 @@ static int FIO_compressFilename_internal(cRess_t ress, U64 readsize = 0; U64 compressedfilesize = 0; U64 const fileSize = UTIL_getFileSize(srcFileName); + DISPLAYLEVEL(5, "%s: %u bytes \n", srcFileName, (U32)fileSize); switch (g_compressionType) { case FIO_zstdCompression: @@ -796,7 +797,7 @@ static int FIO_compressFilename_internal(cRess_t ress, /* init */ #ifdef ZSTD_NEWAPI - /* nothing, reset is implied */ + CHECK( ZSTD_resetCStream(ress.cctx, fileSize) ); /* to pass fileSize */ #elif defined(ZSTD_MULTITHREAD) CHECK( ZSTDMT_resetCStream(ress.cctx, fileSize) ); #else @@ -849,10 +850,10 @@ static int FIO_compressFilename_internal(cRess_t ress, /* End of Frame */ { size_t result = 1; - while (result!=0) { /* note : is there any possibility of endless loop ? */ + while (result != 0) { ZSTD_outBuffer outBuff = { ress.dstBuffer, ress.dstBufferSize, 0 }; #ifdef ZSTD_NEWAPI - ZSTD_inBuffer inBuff = { NULL, 0, 0}; + ZSTD_inBuffer inBuff = { NULL, 0, 0 }; result = ZSTD_compress_generic(ress.cctx, &outBuff, &inBuff, ZSTD_e_end); #elif defined(ZSTD_MULTITHREAD) @@ -863,8 +864,10 @@ static int FIO_compressFilename_internal(cRess_t ress, if (ZSTD_isError(result)) EXM_THROW(26, "Compression error during frame end : %s", ZSTD_getErrorName(result)); - { size_t const sizeCheck = fwrite(ress.dstBuffer, 1, outBuff.pos, dstFile); - if (sizeCheck!=outBuff.pos) EXM_THROW(27, "Write error : cannot write frame end into %s", dstFileName); } + { size_t const sizeCheck = fwrite(ress.dstBuffer, 1, outBuff.pos, dstFile); + if (sizeCheck!=outBuff.pos) + EXM_THROW(27, "Write error : cannot write frame end into %s", dstFileName); + } compressedfilesize += outBuff.pos; } } @@ -974,7 +977,7 @@ int FIO_compressMultipleFilenames(const char** inFileNamesTable, unsigned nbFile char* dstFileName = (char*)malloc(FNSPACE); size_t const suffixSize = suffix ? strlen(suffix) : 0; U64 const srcSize = (nbFiles != 1) ? 0 : UTIL_getFileSize(inFileNamesTable[0]) ; - int const isRegularFile = (nbFiles > 1) ? 0 : UTIL_isRegularFile(inFileNamesTable[0]); /* won't write frame content size when nbFiles > 1 */ + int const isRegularFile = (nbFiles > 1) ? 1 : UTIL_isRegularFile(inFileNamesTable[0]); /* if nbFiles > 1, it's not stdin */ cRess_t ress = FIO_createCResources(dictFileName, compressionLevel, srcSize, isRegularFile, comprParams); /* init */ From fbd5ab70272095255948255368ac4f2f64aca780 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 29 Sep 2017 19:40:27 -0700 Subject: [PATCH 2/5] minor fix : no longer use fake srcSize during resource creation srcSize is read and provided at each file, not at resource creation. This used to be useful with older API, because it could not re-adapt parameters between sessions. At some point, it will be better to remove the old code, and only keep the new_api. It works fine by now. --- lib/compress/zstd_compress.c | 12 ++++++------ programs/fileio.c | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 9553ea9cd..6f369eba3 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -512,7 +512,7 @@ size_t ZSTD_CCtx_setParametersUsingCCtxParams( ZSTDLIB_API size_t ZSTD_CCtx_setPledgedSrcSize(ZSTD_CCtx* cctx, unsigned long long pledgedSrcSize) { - DEBUGLOG(5, " setting pledgedSrcSize to %u", (U32)pledgedSrcSize); + DEBUGLOG(4, " setting pledgedSrcSize to %u", (U32)pledgedSrcSize); if (cctx->streamStage != zcss_init) return ERROR(stage_wrong); cctx->pledgedSrcSizePlusOne = pledgedSrcSize+1; return 0; @@ -524,7 +524,7 @@ size_t ZSTD_CCtx_loadDictionary_advanced( { if (cctx->streamStage != zcss_init) return ERROR(stage_wrong); if (cctx->staticSize) return ERROR(memory_allocation); /* no malloc for static CCtx */ - DEBUGLOG(5, "load dictionary of size %u", (U32)dictSize); + DEBUGLOG(4, "load dictionary of size %u", (U32)dictSize); ZSTD_freeCDict(cctx->cdictLocal); /* in case one already exists */ if (dict==NULL || dictSize==0) { /* no dictionary mode */ cctx->cdictLocal = NULL; @@ -792,13 +792,13 @@ static U32 ZSTD_equivalentParams(ZSTD_CCtx_params params1, static size_t ZSTD_continueCCtx(ZSTD_CCtx* cctx, ZSTD_CCtx_params params, U64 pledgedSrcSize) { U32 const end = (U32)(cctx->nextSrc - cctx->base); - DEBUGLOG(5, "continue mode"); + DEBUGLOG(4, "continue mode"); cctx->appliedParams = params; cctx->pledgedSrcSizePlusOne = pledgedSrcSize+1; cctx->consumedSrcSize = 0; if (pledgedSrcSize == ZSTD_CONTENTSIZE_UNKNOWN) cctx->appliedParams.fParams.contentSizeFlag = 0; - DEBUGLOG(5, "pledged content size : %u ; flag : %u", + DEBUGLOG(4, "pledged content size : %u ; flag : %u", (U32)pledgedSrcSize, cctx->appliedParams.fParams.contentSizeFlag); cctx->lowLimit = end; cctx->dictLimit = end; @@ -2124,9 +2124,9 @@ size_t ZSTD_compressEnd (ZSTD_CCtx* cctx, endResult = ZSTD_writeEpilogue(cctx, (char*)dst + cSize, dstCapacity-cSize); if (ZSTD_isError(endResult)) return endResult; if (cctx->appliedParams.fParams.contentSizeFlag) { /* control src size */ - DEBUGLOG(5, "end of frame : controlling src size"); + DEBUGLOG(4, "end of frame : controlling src size"); if (cctx->pledgedSrcSizePlusOne != cctx->consumedSrcSize+1) { - DEBUGLOG(5, "error : pledgedSrcSize = %u, while realSrcSize = %u", + DEBUGLOG(4, "error : pledgedSrcSize = %u, while realSrcSize = %u", (U32)cctx->pledgedSrcSizePlusOne-1, (U32)cctx->consumedSrcSize); return ERROR(srcSize_wrong); } } diff --git a/programs/fileio.c b/programs/fileio.c index 76726b475..6dc43de81 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -423,7 +423,7 @@ static cRess_t FIO_createCResources(const char* dictFileName, int cLevel, CHECK( ZSTD_CCtx_setParameter(ress.cctx, ZSTD_p_contentSizeFlag, srcIsRegularFile) ); CHECK( ZSTD_CCtx_setParameter(ress.cctx, ZSTD_p_dictIDFlag, g_dictIDFlag) ); CHECK( ZSTD_CCtx_setParameter(ress.cctx, ZSTD_p_checksumFlag, g_checksumFlag) ); - CHECK( ZSTD_CCtx_setPledgedSrcSize(ress.cctx, srcSize) ); + (void)srcSize; /* compression level */ CHECK( ZSTD_CCtx_setParameter(ress.cctx, ZSTD_p_compressionLevel, cLevel) ); /* long distance matching */ From 8afb151c9b0ddb102cee7637b22552fec1d6acb0 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 29 Sep 2017 22:14:37 -0700 Subject: [PATCH 3/5] cli: fixed wrong initialization in MT mode It's not good to mix old and new API ZSTD_resetCStream() doesn't just set pledgedSrcSize : it also sets the CCtx for a single thread compression. Problem is, when 2+ threads are defined in cctx->requestedParams, ZSTD_compress_generic() will want to start MT compression, since initialization is supposed to have already happened (thanks to ZSTD_resetCStream()) except that the underlying ZSTDMT_CCtx* object is not created, resulting in a segfault. This is an invalid construction (correct one is to use ZSTD_CCtx_setPledgedSrcSize()). I haven't found a nice way to mitigate this impact if someone makes the same mistake. At some point, removing the old API to keep only the new API within fileio.c will limit these risks. --- lib/compress/zstd_compress.c | 3 ++- programs/fileio.c | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 6f369eba3..7fccc180f 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2756,6 +2756,7 @@ size_t ZSTD_compress_generic (ZSTD_CCtx* cctx, ZSTD_inBuffer* input, ZSTD_EndDirective endOp) { + DEBUGLOG(5, "ZSTD_compress_generic"); /* check conditions */ if (output->pos > output->size) return ERROR(GENERIC); if (input->pos > input->size) return ERROR(GENERIC); @@ -2798,7 +2799,7 @@ size_t ZSTD_compress_generic (ZSTD_CCtx* cctx, #ifdef ZSTD_MULTITHREAD if (cctx->appliedParams.nbThreads > 1) { size_t const flushMin = ZSTDMT_compressStream_generic(cctx->mtctx, output, input, endOp); - DEBUGLOG(5, "ZSTDMT_compressStream_generic : %u", (U32)flushMin); + DEBUGLOG(5, "ZSTDMT_compressStream_generic result : %u", (U32)flushMin); if ( ZSTD_isError(flushMin) || (endOp == ZSTD_e_end && flushMin == 0) ) { /* compression completed */ ZSTD_startNewCompression(cctx); diff --git a/programs/fileio.c b/programs/fileio.c index 6dc43de81..1319a5753 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -446,6 +446,7 @@ static cRess_t FIO_createCResources(const char* dictFileName, int cLevel, CHECK( ZSTD_CCtx_setParameter(ress.cctx, ZSTD_p_targetLength, comprParams->targetLength) ); CHECK( ZSTD_CCtx_setParameter(ress.cctx, ZSTD_p_compressionStrategy, (U32)comprParams->strategy) ); /* multi-threading */ + DISPLAYLEVEL(5,"set nb threads = %u \n", g_nbThreads); CHECK( ZSTD_CCtx_setParameter(ress.cctx, ZSTD_p_nbThreads, g_nbThreads) ); /* dictionary */ CHECK( ZSTD_CCtx_loadDictionary(ress.cctx, dictBuffer, dictBuffSize) ); @@ -797,7 +798,8 @@ static int FIO_compressFilename_internal(cRess_t ress, /* init */ #ifdef ZSTD_NEWAPI - CHECK( ZSTD_resetCStream(ress.cctx, fileSize) ); /* to pass fileSize */ + if (fileSize!=0) /* if stdin, fileSize==0, but is effectively unknown */ + ZSTD_CCtx_setPledgedSrcSize(ress.cctx, fileSize); #elif defined(ZSTD_MULTITHREAD) CHECK( ZSTDMT_resetCStream(ress.cctx, fileSize) ); #else From 4946993f870357b9091c96804de4bd84df133230 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 2 Oct 2017 12:29:25 -0700 Subject: [PATCH 4/5] removed isRegularFile parameter no longer useful : size of src is determined for each file. --- programs/fileio.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 1319a5753..c30349ae5 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -382,7 +382,7 @@ typedef struct { } cRess_t; static cRess_t FIO_createCResources(const char* dictFileName, int cLevel, - U64 srcSize, int srcIsRegularFile, + U64 srcSize, ZSTD_compressionParameters* comprParams) { cRess_t ress; memset(&ress, 0, sizeof(ress)); @@ -420,7 +420,6 @@ static cRess_t FIO_createCResources(const char* dictFileName, int cLevel, #ifdef ZSTD_NEWAPI { /* frame parameters */ - CHECK( ZSTD_CCtx_setParameter(ress.cctx, ZSTD_p_contentSizeFlag, srcIsRegularFile) ); CHECK( ZSTD_CCtx_setParameter(ress.cctx, ZSTD_p_dictIDFlag, g_dictIDFlag) ); CHECK( ZSTD_CCtx_setParameter(ress.cctx, ZSTD_p_checksumFlag, g_checksumFlag) ); (void)srcSize; @@ -453,7 +452,6 @@ static cRess_t FIO_createCResources(const char* dictFileName, int cLevel, } #elif defined(ZSTD_MULTITHREAD) { ZSTD_parameters params = ZSTD_getParams(cLevel, srcSize, dictBuffSize); - params.fParams.contentSizeFlag = srcIsRegularFile; params.fParams.checksumFlag = g_checksumFlag; params.fParams.noDictIDFlag = !g_dictIDFlag; if (comprParams->windowLog) params.cParams.windowLog = comprParams->windowLog; @@ -468,7 +466,6 @@ static cRess_t FIO_createCResources(const char* dictFileName, int cLevel, } #else { ZSTD_parameters params = ZSTD_getParams(cLevel, srcSize, dictBuffSize); - params.fParams.contentSizeFlag = srcIsRegularFile; params.fParams.checksumFlag = g_checksumFlag; params.fParams.noDictIDFlag = !g_dictIDFlag; if (comprParams->windowLog) params.cParams.windowLog = comprParams->windowLog; @@ -798,12 +795,12 @@ static int FIO_compressFilename_internal(cRess_t ress, /* init */ #ifdef ZSTD_NEWAPI - if (fileSize!=0) /* if stdin, fileSize==0, but is effectively unknown */ - ZSTD_CCtx_setPledgedSrcSize(ress.cctx, fileSize); + if (fileSize!=0) /* when src is stdin, fileSize==0, but is effectively unknown */ + ZSTD_CCtx_setPledgedSrcSize(ress.cctx, fileSize); /* note : fileSize==0 means "empty" */ #elif defined(ZSTD_MULTITHREAD) - CHECK( ZSTDMT_resetCStream(ress.cctx, fileSize) ); + CHECK( ZSTDMT_resetCStream(ress.cctx, fileSize) ); /* note : fileSize==0 means "unknown" */ #else - CHECK( ZSTD_resetCStream(ress.cctx, fileSize) ); + CHECK( ZSTD_resetCStream(ress.cctx, fileSize) ); /* note : fileSize==0 means "unknown" */ #endif /* Main compression loop */ @@ -863,9 +860,10 @@ static int FIO_compressFilename_internal(cRess_t ress, #else result = ZSTD_endStream(ress.cctx, &outBuff); #endif - if (ZSTD_isError(result)) + if (ZSTD_isError(result)) { EXM_THROW(26, "Compression error during frame end : %s", ZSTD_getErrorName(result)); + } { size_t const sizeCheck = fwrite(ress.dstBuffer, 1, outBuff.pos, dstFile); if (sizeCheck!=outBuff.pos) EXM_THROW(27, "Write error : cannot write frame end into %s", dstFileName); @@ -956,9 +954,8 @@ int FIO_compressFilename(const char* dstFileName, const char* srcFileName, { clock_t const start = clock(); U64 const srcSize = UTIL_getFileSize(srcFileName); - int const isRegularFile = UTIL_isRegularFile(srcFileName); - cRess_t const ress = FIO_createCResources(dictFileName, compressionLevel, srcSize, isRegularFile, comprParams); + cRess_t const ress = FIO_createCResources(dictFileName, compressionLevel, srcSize, comprParams); int const result = FIO_compressFilename_dstFile(ress, dstFileName, srcFileName, compressionLevel); double const seconds = (double)(clock() - start) / CLOCKS_PER_SEC; @@ -979,8 +976,7 @@ int FIO_compressMultipleFilenames(const char** inFileNamesTable, unsigned nbFile char* dstFileName = (char*)malloc(FNSPACE); size_t const suffixSize = suffix ? strlen(suffix) : 0; U64 const srcSize = (nbFiles != 1) ? 0 : UTIL_getFileSize(inFileNamesTable[0]) ; - int const isRegularFile = (nbFiles > 1) ? 1 : UTIL_isRegularFile(inFileNamesTable[0]); /* if nbFiles > 1, it's not stdin */ - cRess_t ress = FIO_createCResources(dictFileName, compressionLevel, srcSize, isRegularFile, comprParams); + cRess_t ress = FIO_createCResources(dictFileName, compressionLevel, srcSize, comprParams); /* init */ if (dstFileName==NULL) From 7e00df4a496985880baee79d677a53573e9abaf7 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 2 Oct 2017 16:27:25 -0700 Subject: [PATCH 5/5] bumped version number and updated NEWS in anticipation for release --- NEWS | 14 ++++++++++++-- lib/zstd.h | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index cddc8fc13..215c0a329 100644 --- a/NEWS +++ b/NEWS @@ -1,15 +1,25 @@ v1.3.2 new : long range mode, using --long command, by Stella Lau (@stellamplau) -license : changed /examples license to BSD + GPLv2 -license : fix a few header files to reflect new license (#825) +new : ability to generate and decode magicless frames (#591) +changed : maximum nb of threads reduced to 200, to avoid address space exhaustion in 32-bits mode fix : multi-threading compression works with custom allocators fix : ZSTD_sizeof_CStream() was over-evaluating memory usage fix : a rare compression bug when compression generates very large distances and bunch of other conditions (only possible at --ultra -22) fix : 32-bits build can now decode large offsets (levels 21+) +cli : added LZ4 frame support by default, by Felix Handte (@felixhandte) +cli : improved --list output cli : new : can split input file for dictionary training, using command -B# +cli : new : clean operation artefact on Ctrl-C interruption cli : fix : do not change /dev/null permissions when using command -t with root access, reported by @mike155 (#851) +cli : fix : write file size in header in multiple-files mode +api : added macro ZSTD_COMPRESSBOUND() for static allocation +api : experimental : new advanced decompression API +api : fix : sizeof_CCtx() used to over-estimate build: fix : no-multithread variant compiles without pool.c dependency, reported by Mitchell Blank Jr (@mitchblank) (#819) build: better compatibility with reproducible builds, by Bernhard M. Wiedemann (@bmwiedemann) (#818) +example : added streaming_memory_usage +license : changed /examples license to BSD + GPLv2 +license : fix a few header files to reflect new license (#825) v1.3.1 New license : BSD + GPLv2 diff --git a/lib/zstd.h b/lib/zstd.h index 02241fd3f..12f41e631 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -59,7 +59,7 @@ extern "C" { /*------ Version ------*/ #define ZSTD_VERSION_MAJOR 1 #define ZSTD_VERSION_MINOR 3 -#define ZSTD_VERSION_RELEASE 1 +#define ZSTD_VERSION_RELEASE 2 #define ZSTD_VERSION_NUMBER (ZSTD_VERSION_MAJOR *100*100 + ZSTD_VERSION_MINOR *100 + ZSTD_VERSION_RELEASE) ZSTDLIB_API unsigned ZSTD_versionNumber(void); /**< useful to check dll version */