From de0499f7fa315f36725c93b4d97bd2ceaf6a0dbb Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Mon, 15 Apr 2019 22:49:44 -0700 Subject: [PATCH 1/3] [libzstd] Require ZSTD_MULTITHREAD to create a ZSTDMT_CCtx ZSTDMT was broken when compiled without ZSTD_MULTITHREAD defined, because `ZSTD_CCtx_setParameter(cctx, ZSTD_c_nbWorkers, nbWorkerss)` failed. It was detected by the MSVC test which runs the fuzzer with multithreading disabled. This is a very niche use case of a deprecated API, because the API is inefficient and synchronous, since `threading.h` will be synchronous. Users almost certainly don't want this, and anyone who tested their code should realize that it is broken. Therefore, I think it is safe to require `ZSTD_MULTITHREAD` to be defined to use ZSTDMT. --- CHANGELOG | 1 + lib/compress/zstdmt_compress.c | 4 ++++ lib/compress/zstdmt_compress.h | 8 +++++++- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 61d0b3f56..bf04a8a13 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,7 @@ api: Make ZSTD_e_flush and ZSTD_e_end block for maximum forward progress api: Rename ZSTD_CCtxParam_getParameter to ZSTD_CCtxParams_getParameter api: Rename ZSTD_CCtxParam_setParameter to ZSTD_CCtxParams_setParameter api: Don't export ZSTDMT functions from the shared library by default +api: Require ZSTD_MULTITHREAD to be defined to use ZSTDMT api: Add ZSTD_decompressBound() to provide an upper bound on decompressed size by @shakeelrao api: Fix ZSTD_decompressDCtx() corner cases with a dictionary api: Move ZSTD_getDictID_*() functions to the stable section diff --git a/lib/compress/zstdmt_compress.c b/lib/compress/zstdmt_compress.c index 4b29fb3e4..d0b4ee84a 100644 --- a/lib/compress/zstdmt_compress.c +++ b/lib/compress/zstdmt_compress.c @@ -874,6 +874,10 @@ ZSTDMT_CCtx* ZSTDMT_createCCtx_advanced(unsigned nbWorkers, ZSTD_customMem cMem) int initError; DEBUGLOG(3, "ZSTDMT_createCCtx_advanced (nbWorkers = %u)", nbWorkers); +#ifndef ZSTD_MULTITHREAD + return NULL; +#endif + if (nbWorkers < 1) return NULL; nbWorkers = MIN(nbWorkers , ZSTDMT_NBWORKERS_MAX); if ((cMem.customAlloc!=NULL) ^ (cMem.customFree!=NULL)) diff --git a/lib/compress/zstdmt_compress.h b/lib/compress/zstdmt_compress.h index 8d274ce8d..12e6bcb3a 100644 --- a/lib/compress/zstdmt_compress.h +++ b/lib/compress/zstdmt_compress.h @@ -19,12 +19,16 @@ /* Note : This is an internal API. * These APIs used to be exposed with ZSTDLIB_API, * because it used to be the only way to invoke MT compression. - * Now, it's recommended to use ZSTD_compress_generic() instead. + * Now, it's recommended to use ZSTD_compress2 and ZSTD_compressStream2() + * instead. * * If you depend on these APIs and can't switch, then define * ZSTD_LEGACY_MULTITHREADED_API when making the dynamic library. * However, we may completely remove these functions in a future * release, so please switch soon. + * + * This API requires ZSTD_MULTITHREAD to be defined during compilation, + * otherwise ZSTDMT_createCCtx*() will fail. */ #ifdef ZSTD_LEGACY_MULTITHREADED_API @@ -51,7 +55,9 @@ /* === Memory management === */ typedef struct ZSTDMT_CCtx_s ZSTDMT_CCtx; +/* Requires ZSTD_MULTITHREAD to be defined during compilation, otherwise it will return NULL. */ ZSTDMT_API ZSTDMT_CCtx* ZSTDMT_createCCtx(unsigned nbWorkers); +/* Requires ZSTD_MULTITHREAD to be defined during compilation, otherwise it will return NULL. */ ZSTDMT_API ZSTDMT_CCtx* ZSTDMT_createCCtx_advanced(unsigned nbWorkers, ZSTD_customMem cMem); ZSTDMT_API size_t ZSTDMT_freeCCtx(ZSTDMT_CCtx* mtctx); From 82d5a9870e1809e64faee64bb14acacd523e7166 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Mon, 15 Apr 2019 22:54:07 -0700 Subject: [PATCH 2/3] [visual] Compile with ZSTD_MULTITHREAD=1 This branch `appveyorTests` runs the tests that `master` runs, so passing Appveyor means that the tests are fixed. --- build/VS2008/fuzzer/fuzzer.vcproj | 8 ++++---- build/VS2010/fuzzer/fuzzer.vcxproj | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/build/VS2008/fuzzer/fuzzer.vcproj b/build/VS2008/fuzzer/fuzzer.vcproj index 46cc4d4f2..c81791f4c 100644 --- a/build/VS2008/fuzzer/fuzzer.vcproj +++ b/build/VS2008/fuzzer/fuzzer.vcproj @@ -45,7 +45,7 @@ Name="VCCLCompilerTool" Optimization="0" AdditionalIncludeDirectories="$(SolutionDir)..\..\lib;$(SolutionDir)..\..\lib\common;$(SolutionDir)..\..\lib\compress;$(SolutionDir)..\..\lib\dictBuilder;$(SolutionDir)..\..\lib\legacy;$(SolutionDir)..\..\programs" - PreprocessorDefinitions="WIN32;_DEBUG;_CONSOLE" + PreprocessorDefinitions="ZSTD_MULTITHREAD=1;WIN32;_DEBUG;_CONSOLE" MinimalRebuild="true" BasicRuntimeChecks="3" RuntimeLibrary="3" @@ -121,7 +121,7 @@ EnableIntrinsicFunctions="true" OmitFramePointers="true" AdditionalIncludeDirectories="$(SolutionDir)..\..\lib;$(SolutionDir)..\..\lib\common;$(SolutionDir)..\..\lib\compress;$(SolutionDir)..\..\lib\dictBuilder;$(SolutionDir)..\..\lib\legacy;$(SolutionDir)..\..\programs" - PreprocessorDefinitions="WIN32;NDEBUG;_CONSOLE" + PreprocessorDefinitions="ZSTD_MULTITHREAD=1;WIN32;NDEBUG;_CONSOLE" RuntimeLibrary="0" EnableFunctionLevelLinking="true" UsePrecompiledHeader="0" @@ -195,7 +195,7 @@ Name="VCCLCompilerTool" Optimization="0" AdditionalIncludeDirectories="$(SolutionDir)..\..\lib;$(SolutionDir)..\..\lib\common;$(SolutionDir)..\..\lib\compress;$(SolutionDir)..\..\lib\dictBuilder;$(SolutionDir)..\..\lib\legacy;$(SolutionDir)..\..\programs" - PreprocessorDefinitions="WIN32;_DEBUG;_CONSOLE" + PreprocessorDefinitions="ZSTD_MULTITHREAD=1;WIN32;_DEBUG;_CONSOLE" MinimalRebuild="true" BasicRuntimeChecks="3" RuntimeLibrary="3" @@ -272,7 +272,7 @@ EnableIntrinsicFunctions="true" OmitFramePointers="true" AdditionalIncludeDirectories="$(SolutionDir)..\..\lib;$(SolutionDir)..\..\lib\common;$(SolutionDir)..\..\lib\dictBuilder;$(SolutionDir)..\..\lib\legacy;$(SolutionDir)..\..\lib\compress;$(SolutionDir)..\..\programs" - PreprocessorDefinitions="WIN32;NDEBUG;_CONSOLE" + PreprocessorDefinitions="ZSTD_MULTITHREAD=1;WIN32;NDEBUG;_CONSOLE" RuntimeLibrary="0" EnableFunctionLevelLinking="true" UsePrecompiledHeader="0" diff --git a/build/VS2010/fuzzer/fuzzer.vcxproj b/build/VS2010/fuzzer/fuzzer.vcxproj index 53881c194..de649de1c 100644 --- a/build/VS2010/fuzzer/fuzzer.vcxproj +++ b/build/VS2010/fuzzer/fuzzer.vcxproj @@ -90,7 +90,7 @@ Level4 Disabled - WIN32;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) + ZSTD_MULTITHREAD=1;WIN32;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) true false @@ -105,7 +105,7 @@ Level4 Disabled - WIN32;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) + ZSTD_MULTITHREAD=1;WIN32;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) true false @@ -122,7 +122,7 @@ MaxSpeed true true - WIN32;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) + ZSTD_MULTITHREAD=1;WIN32;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) false false MultiThreaded @@ -142,7 +142,7 @@ MaxSpeed true true - WIN32;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) + ZSTD_MULTITHREAD=1;WIN32;NDEBUG;_CONSOLE;%(PreprocessorDefinitions) false false MultiThreaded From a17fe4c9e51cefe28c1097e157cab26d7989ae0e Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Tue, 16 Apr 2019 10:22:47 -0700 Subject: [PATCH 3/3] [visual] Fix unreachable code warning --- lib/compress/zstdmt_compress.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/compress/zstdmt_compress.c b/lib/compress/zstdmt_compress.c index d0b4ee84a..38fbb9076 100644 --- a/lib/compress/zstdmt_compress.c +++ b/lib/compress/zstdmt_compress.c @@ -22,6 +22,7 @@ /* ====== Dependencies ====== */ #include /* memcpy, memset */ #include /* INT_MAX, UINT_MAX */ +#include "mem.h" /* MEM_STATIC */ #include "pool.h" /* threadpool */ #include "threading.h" /* mutex */ #include "zstd_compress_internal.h" /* MIN, ERROR, ZSTD_*, ZSTD_highbit32 */ @@ -867,17 +868,13 @@ size_t ZSTDMT_CCtxParam_setNbWorkers(ZSTD_CCtx_params* params, unsigned nbWorker return ZSTD_CCtxParams_setParameter(params, ZSTD_c_nbWorkers, (int)nbWorkers); } -ZSTDMT_CCtx* ZSTDMT_createCCtx_advanced(unsigned nbWorkers, ZSTD_customMem cMem) +MEM_STATIC ZSTDMT_CCtx* ZSTDMT_createCCtx_advanced_internal(unsigned nbWorkers, ZSTD_customMem cMem) { ZSTDMT_CCtx* mtctx; U32 nbJobs = nbWorkers + 2; int initError; DEBUGLOG(3, "ZSTDMT_createCCtx_advanced (nbWorkers = %u)", nbWorkers); -#ifndef ZSTD_MULTITHREAD - return NULL; -#endif - if (nbWorkers < 1) return NULL; nbWorkers = MIN(nbWorkers , ZSTDMT_NBWORKERS_MAX); if ((cMem.customAlloc!=NULL) ^ (cMem.customFree!=NULL)) @@ -906,6 +903,17 @@ ZSTDMT_CCtx* ZSTDMT_createCCtx_advanced(unsigned nbWorkers, ZSTD_customMem cMem) return mtctx; } +ZSTDMT_CCtx* ZSTDMT_createCCtx_advanced(unsigned nbWorkers, ZSTD_customMem cMem) +{ +#ifdef ZSTD_MULTITHREAD + return ZSTDMT_createCCtx_advanced_internal(nbWorkers, cMem); +#else + (void)nbWorkers; + (void)cMem; + return NULL; +#endif +} + ZSTDMT_CCtx* ZSTDMT_createCCtx(unsigned nbWorkers) { return ZSTDMT_createCCtx_advanced(nbWorkers, ZSTD_defaultCMem);