From 1d75697d9b2ca4f431ebbe764400a4b97ab034df Mon Sep 17 00:00:00 2001 From: maxice8 Date: Fri, 5 Oct 2018 21:23:45 -0300 Subject: [PATCH 01/18] don't assume __linux__ means __GLIBC__ on BACKTRACES_ENABLE check for __GLIBC__ instead of __linux__ as musl libc doesn't provide execinfo.h fixes compilation on Alpine Linux and Void Linux musl arches. --- programs/fileio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/fileio.c b/programs/fileio.c index ed3a29cda..1a73a75e4 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -21,7 +21,7 @@ #endif #if !defined(BACKTRACES_ENABLE) && \ - (defined(__linux__) || (defined(__APPLE__) && defined(__MACH__)) ) + (defined(__GLIBC__) || (defined(__APPLE__) && defined(__MACH__)) ) # define BACKTRACES_ENABLE 1 #endif From 8fc79fac07fc87c81b60e07a33884a7e999a1669 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 8 Oct 2018 15:53:29 -0700 Subject: [PATCH 02/18] clarify streaming api doc as suggested by @indygreg in #1360 --- lib/zstd.h | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/lib/zstd.h b/lib/zstd.h index 7b6964be3..9b1410448 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -361,15 +361,21 @@ ZSTDLIB_API size_t ZSTD_CStreamOutSize(void); /**< recommended size for output * The function will update both `pos` fields. * If `input.pos < input.size`, some input has not been consumed. * It's up to the caller to present again remaining data. +* The function tries to flush all data decoded immediately, repecting buffer sizes. * If `output.pos < output.size`, decoder has flushed everything it could. -* @return : 0 when a frame is completely decoded and fully flushed, -* an error code, which can be tested using ZSTD_isError(), -* any other value > 0, which means there is still some decoding to do to complete current frame. -* The return value is a suggested next input size (a hint to improve latency) that will never load more than the current frame. +* But if `output.pos == output.size`, there is no such guarantee, +* it's likely that some decoded data was not flushed and still remains within internal buffers. +* In which case, call ZSTD_decompressStream() again to flush whatever remains in the buffer. +* When no additional input is provided, amount of data flushed is necessarily <= ZSTD_BLOCKSIZE_MAX. +* @return : 0 when a frame is completely decoded and fully flushed, +* or an error code, which can be tested using ZSTD_isError(), +* or any other value > 0, which means there is still some decoding or flushing to do to complete current frame : +* the return value is a suggested next input size (a hint for better latency) +* that will never load more than the current frame. * *******************************************************************************/ typedef ZSTD_DCtx ZSTD_DStream; /**< DCtx and DStream are now effectively same object (>= v1.3.0) */ - /* For compatibility with versions <= v1.2.0, continue to consider them separated. */ + /* For compatibility with versions <= v1.2.0, prefer differentiating them. */ /*===== ZSTD_DStream management functions =====*/ ZSTDLIB_API ZSTD_DStream* ZSTD_createDStream(void); ZSTDLIB_API size_t ZSTD_freeDStream(ZSTD_DStream* zds); @@ -1235,9 +1241,13 @@ ZSTDLIB_API size_t ZSTD_CCtx_resetParameters(ZSTD_CCtx* cctx); typedef enum { - ZSTD_e_continue=0, /* collect more data, encoder decides when to output compressed result, for optimal conditions */ - ZSTD_e_flush, /* flush any data provided so far - frame will continue, future data can still reference previous data for better compression */ - ZSTD_e_end /* flush any remaining data and close current frame. Any additional data starts a new frame. */ + ZSTD_e_continue=0, /* collect more data, encoder decides when to output compressed result, for optimal compression ratio */ + ZSTD_e_flush, /* flush any data provided so far, + * it creates (at least) one new block, that can be decoded immediately on reception; + * frame will continue: any future data can still reference previously compressed data, improving compression. */ + ZSTD_e_end /* flush any remaining data and close current frame. + * any additional data starts a new frame. + * each frame is independent (does not reference any content from previous frame). */ } ZSTD_EndDirective; /*! ZSTD_compress_generic() : From 08da9ad316710713f319978f5083ee7334c56823 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Mon, 8 Oct 2018 15:38:16 -0700 Subject: [PATCH 03/18] Remove Unused Variable --- lib/compress/zstd_lazy.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index 4ca69e3ec..29322f6e1 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -173,7 +173,6 @@ ZSTD_DUBT_findBetterDictMatch ( U32 const btLow = (btMask >= dictHighLimit - dictLowLimit) ? dictLowLimit : dictHighLimit - btMask; size_t commonLengthSmaller=0, commonLengthLarger=0, bestLength=0; - U32 matchEndIdx = current+8+1; (void)dictMode; assert(dictMode == ZSTD_dictMatchState); @@ -188,8 +187,6 @@ ZSTD_DUBT_findBetterDictMatch ( if (matchLength > bestLength) { U32 matchIndex = dictMatchIndex + dictIndexDelta; - if (matchLength > matchEndIdx - matchIndex) - matchEndIdx = matchIndex + (U32)matchLength; if ( (4*(int)(matchLength-bestLength)) > (int)(ZSTD_highbit32(current-matchIndex+1) - ZSTD_highbit32((U32)offsetPtr[0]+1)) ) { DEBUGLOG(2, "ZSTD_DUBT_findBestDictMatch(%u) : found better match length %u -> %u and offsetCode %u -> %u (dictMatchIndex %u, matchIndex %u)", current, (U32)bestLength, (U32)matchLength, (U32)*offsetPtr, ZSTD_REP_MOVE + current - matchIndex, dictMatchIndex, matchIndex); From d121b3451ce32fe293316f908bb3a2431b47000e Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Mon, 8 Oct 2018 15:50:02 -0700 Subject: [PATCH 04/18] Clean Up Debug Log Statements --- lib/compress/zstd_lazy.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index 29322f6e1..93b5dced2 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -188,7 +188,7 @@ ZSTD_DUBT_findBetterDictMatch ( if (matchLength > bestLength) { U32 matchIndex = dictMatchIndex + dictIndexDelta; if ( (4*(int)(matchLength-bestLength)) > (int)(ZSTD_highbit32(current-matchIndex+1) - ZSTD_highbit32((U32)offsetPtr[0]+1)) ) { - DEBUGLOG(2, "ZSTD_DUBT_findBestDictMatch(%u) : found better match length %u -> %u and offsetCode %u -> %u (dictMatchIndex %u, matchIndex %u)", + DEBUGLOG(9, "ZSTD_DUBT_findBetterDictMatch(%u) : found better match length %u -> %u and offsetCode %u -> %u (dictMatchIndex %u, matchIndex %u)", current, (U32)bestLength, (U32)matchLength, (U32)*offsetPtr, ZSTD_REP_MOVE + current - matchIndex, dictMatchIndex, matchIndex); bestLength = matchLength, *offsetPtr = ZSTD_REP_MOVE + current - matchIndex; } @@ -197,7 +197,6 @@ ZSTD_DUBT_findBetterDictMatch ( } } - DEBUGLOG(2, "matchLength:%6zu, match:%p, prefixStart:%p, ip:%p", matchLength, match, prefixStart, ip); if (match[matchLength] < ip[matchLength]) { if (dictMatchIndex <= btLow) { break; } /* beyond tree size, stop the search */ commonLengthSmaller = matchLength; /* all smaller will now have at least this guaranteed common length */ @@ -212,7 +211,7 @@ ZSTD_DUBT_findBetterDictMatch ( if (bestLength >= MINMATCH) { U32 const mIndex = current - ((U32)*offsetPtr - ZSTD_REP_MOVE); (void)mIndex; - DEBUGLOG(2, "ZSTD_DUBT_findBestDictMatch(%u) : found match of length %u and offsetCode %u (pos %u)", + DEBUGLOG(8, "ZSTD_DUBT_findBetterDictMatch(%u) : found match of length %u and offsetCode %u (pos %u)", current, (U32)bestLength, (U32)*offsetPtr, mIndex); } return bestLength; From b8235be865e136d0baf6fa75588281dcb6d8ca8f Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Mon, 8 Oct 2018 15:56:09 -0700 Subject: [PATCH 05/18] Avoid Searching Dictionary in ZSTD_btlazy2 When an Optimal Match is Found Bailing here is important to avoid reading past the end of the input buffer. --- lib/compress/zstd_lazy.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index 93b5dced2..af615e077 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -147,6 +147,7 @@ ZSTD_DUBT_findBetterDictMatch ( ZSTD_matchState_t* ms, const BYTE* const ip, const BYTE* const iend, size_t* offsetPtr, + size_t bestLength, U32 nbCompares, U32 const mls, const ZSTD_dictMode_e dictMode) @@ -172,7 +173,7 @@ ZSTD_DUBT_findBetterDictMatch ( U32 const btMask = (1 << btLog) - 1; U32 const btLow = (btMask >= dictHighLimit - dictLowLimit) ? dictLowLimit : dictHighLimit - btMask; - size_t commonLengthSmaller=0, commonLengthLarger=0, bestLength=0; + size_t commonLengthSmaller=0, commonLengthLarger=0; (void)dictMode; assert(dictMode == ZSTD_dictMatchState); @@ -319,6 +320,11 @@ ZSTD_DUBT_findBestMatch(ZSTD_matchState_t* ms, if ( (4*(int)(matchLength-bestLength)) > (int)(ZSTD_highbit32(current-matchIndex+1) - ZSTD_highbit32((U32)offsetPtr[0]+1)) ) bestLength = matchLength, *offsetPtr = ZSTD_REP_MOVE + current - matchIndex; if (ip+matchLength == iend) { /* equal : no way to know if inf or sup */ + if (dictMode == ZSTD_dictMatchState) { + nbCompares = 0; /* in addition to avoiding checking any + * further in this loop, make sure we + * skip checking in the dictionary. */ + } break; /* drop, to guarantee consistency (miss a little bit of compression) */ } } @@ -342,7 +348,10 @@ ZSTD_DUBT_findBestMatch(ZSTD_matchState_t* ms, *smallerPtr = *largerPtr = 0; if (dictMode == ZSTD_dictMatchState && nbCompares) { - bestLength = ZSTD_DUBT_findBetterDictMatch(ms, ip, iend, offsetPtr, nbCompares, mls, dictMode); + bestLength = ZSTD_DUBT_findBetterDictMatch( + ms, ip, iend, + offsetPtr, bestLength, nbCompares, + mls, dictMode); } assert(matchEndIdx > current+8); /* ensure nextToUpdate is increased */ From f17c1df1ac86475f16d5d577b27a74271960e837 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 8 Oct 2018 17:03:06 -0700 Subject: [PATCH 06/18] backtrace support compiled with more conditions following #1356, only enable backtrace compilation on linux+glibc. Also, disable backtrace by default from "release" compilation, so that less platforms get impacted by the new requirements. Can be manually enabled/disabled using BACKTRACE=1/0. --- programs/Makefile | 20 +++++++++++--------- programs/README.md | 6 +++--- programs/fileio.c | 13 +++++++------ 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/programs/Makefile b/programs/Makefile index 8faa5111b..53f2f280a 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -134,11 +134,13 @@ else LZ4_MSG := $(NO_LZ4_MSG) endif -# enable backtrace symbol names for Linux/Darwin -ALL_SYMBOLS := 0 +# enable backtrace symbol names for Linux & Darwin +BACKTRACE ?= 0 +DEBUGFLAGS = -DBACKTRACE_ENABLE=$(BACKTRACE) ifeq (,$(filter Windows%, $(OS))) -ifeq ($(ALL_SYMBOLS), 1) -DEBUGFLAGS_LD+=-rdynamic +ifeq ($(BACKTRACE), 1) +DEBUGFLAGS += -DBACKTRACE_ENABLE=1 +DEBUGFLAGS_LD += -rdynamic endif endif @@ -168,12 +170,12 @@ endif $(CC) $(FLAGS) $^ $(RES_FILE) -o $@$(EXT) $(LDFLAGS) .PHONY: zstd-release -zstd-release: DEBUGFLAGS := +zstd-release: DEBUGFLAGS := -DBACKTRACE_ENABLE=0 zstd-release: DEBUGFLAGS_LD := zstd-release: zstd zstd32 : CPPFLAGS += $(THREAD_CPP) -zstd32 : LDFLAGS += $(THREAD_LD) +zstd32 : LDFLAGS += $(THREAD_LD) zstd32 : CPPFLAGS += -DZSTD_LEGACY_SUPPORT=$(ZSTD_LEGACY_SUPPORT) zstd32 : $(ZSTDLIB_FILES) zstdcli.c fileio.c bench.c datagen.c dibio.c ifneq (,$(filter Windows%,$(OS))) @@ -185,17 +187,17 @@ zstd-nolegacy : $(ZSTD_FILES) $(ZDICT_FILES) zstdcli.o fileio.c bench.o datagen. $(CC) $(FLAGS) $^ -o $@$(EXT) $(LDFLAGS) zstd-nomt : THREAD_CPP := -zstd-nomt : THREAD_LD := +zstd-nomt : THREAD_LD := zstd-nomt : THREAD_MSG := - multi-threading disabled zstd-nomt : zstd zstd-nogz : ZLIBCPP := -zstd-nogz : ZLIBLD := +zstd-nogz : ZLIBLD := zstd-nogz : ZLIB_MSG := - gzip support is disabled zstd-nogz : zstd zstd-noxz : LZMACPP := -zstd-noxz : LZMALD := +zstd-noxz : LZMALD := zstd-noxz : LZMA_MSG := - xz/lzma support is disabled zstd-noxz : zstd diff --git a/programs/README.md b/programs/README.md index 804cb8b0a..ca9056eaa 100644 --- a/programs/README.md +++ b/programs/README.md @@ -61,12 +61,12 @@ There are however other Makefile targets that create different variations of CLI In which case, linking stage will fail if `lz4` library cannot be found. This is useful to prevent silent feature disabling. -- __ALL_SYMBOLS__ : `zstd` can display a stack backtrace if the execution +- __BACKTRACE__ : `zstd` can display a stack backtrace when execution generates a runtime exception. By default, this feature may be degraded/disabled on some platforms unless additional compiler directives are - applied. When triaging a runtime issue, enabling this feature can provided + applied. When triaging a runtime issue, enabling this feature can provide more context to determine the location of the fault. - Example : `make zstd ALL_SYMBOLS=1` + Example : `make zstd BACKTRACE=1` #### Aggregation of parameters diff --git a/programs/fileio.c b/programs/fileio.c index 1a73a75e4..aff1b512c 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -20,9 +20,10 @@ # define _POSIX_SOURCE 1 /* disable %llu warnings with MinGW on Windows */ #endif -#if !defined(BACKTRACES_ENABLE) && \ - (defined(__GLIBC__) || (defined(__APPLE__) && defined(__MACH__)) ) -# define BACKTRACES_ENABLE 1 +#if !defined(BACKTRACE_ENABLE) \ + && ((defined(__linux__) && defined(__GLIBC__)) \ + || (defined(__APPLE__) && defined(__MACH__)) ) +# define BACKTRACE_ENABLE 1 #endif @@ -37,7 +38,7 @@ #include #include /* errno */ #include -#ifdef BACKTRACES_ENABLE +#ifdef BACKTRACE_ENABLE # include /* backtrace, backtrace_symbols */ #endif @@ -167,7 +168,7 @@ static void clearHandler(void) /*-********************************************************* * Termination signal trapping (Print debug stack trace) ***********************************************************/ -#ifdef BACKTRACES_ENABLE +#ifdef BACKTRACE_ENABLE #define MAX_STACK_FRAMES 50 @@ -208,7 +209,7 @@ static void ABRThandler(int sig) { void FIO_addAbortHandler() { -#ifdef BACKTRACES_ENABLE +#ifdef BACKTRACE_ENABLE signal(SIGABRT, ABRThandler); signal(SIGFPE, ABRThandler); signal(SIGILL, ABRThandler); From 1e0c5466c5a2cd52210270f56f61ac5ca2b1ec4a Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 9 Oct 2018 12:57:42 -0700 Subject: [PATCH 07/18] fixed BACKTRACE_ENABLE macro test --- programs/Makefile | 1 - programs/fileio.c | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/programs/Makefile b/programs/Makefile index 53f2f280a..9e241357b 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -136,7 +136,6 @@ endif # enable backtrace symbol names for Linux & Darwin BACKTRACE ?= 0 -DEBUGFLAGS = -DBACKTRACE_ENABLE=$(BACKTRACE) ifeq (,$(filter Windows%, $(OS))) ifeq ($(BACKTRACE), 1) DEBUGFLAGS += -DBACKTRACE_ENABLE=1 diff --git a/programs/fileio.c b/programs/fileio.c index aff1b512c..8b1e9f2f2 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -38,7 +38,7 @@ #include #include /* errno */ #include -#ifdef BACKTRACE_ENABLE +#if defined(BACKTRACE_ENABLE) && (BACKTRACE_ENABLE >= 1) # include /* backtrace, backtrace_symbols */ #endif @@ -168,7 +168,7 @@ static void clearHandler(void) /*-********************************************************* * Termination signal trapping (Print debug stack trace) ***********************************************************/ -#ifdef BACKTRACE_ENABLE +#if defined(BACKTRACE_ENABLE) && (BACKTRACE_ENABLE>=1) #define MAX_STACK_FRAMES 50 @@ -209,7 +209,7 @@ static void ABRThandler(int sig) { void FIO_addAbortHandler() { -#ifdef BACKTRACE_ENABLE +#if defined(BACKTRACE_ENABLE) && (BACKTRACE_ENABLE>=1) signal(SIGABRT, ABRThandler); signal(SIGFPE, ABRThandler); signal(SIGILL, ABRThandler); From e0ab6b61b7df61172bb9f2ce7eb9d76106d5e556 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 9 Oct 2018 17:12:21 -0700 Subject: [PATCH 08/18] fixed explicit BACKTRACE order and automatic linux backtrace detection : __GLIBC__ must be tested after #include --- programs/Makefile | 6 ++++-- programs/fileio.c | 18 +++++++++--------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/programs/Makefile b/programs/Makefile index 9e241357b..ac17caee8 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -134,8 +134,10 @@ else LZ4_MSG := $(NO_LZ4_MSG) endif -# enable backtrace symbol names for Linux & Darwin -BACKTRACE ?= 0 +# explicit backtrace enable/disable for Linux & Darwin +ifeq ($(BACKTRACE), 0) +DEBUGFLAGS += -DBACKTRACE_ENABLE=0 +endif ifeq (,$(filter Windows%, $(OS))) ifeq ($(BACKTRACE), 1) DEBUGFLAGS += -DBACKTRACE_ENABLE=1 diff --git a/programs/fileio.c b/programs/fileio.c index 8b1e9f2f2..16bfd530d 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -20,12 +20,6 @@ # define _POSIX_SOURCE 1 /* disable %llu warnings with MinGW on Windows */ #endif -#if !defined(BACKTRACE_ENABLE) \ - && ((defined(__linux__) && defined(__GLIBC__)) \ - || (defined(__APPLE__) && defined(__MACH__)) ) -# define BACKTRACE_ENABLE 1 -#endif - /*-************************************* * Includes @@ -38,9 +32,6 @@ #include #include /* errno */ #include -#if defined(BACKTRACE_ENABLE) && (BACKTRACE_ENABLE >= 1) -# include /* backtrace, backtrace_symbols */ -#endif #if defined (_MSC_VER) # include @@ -168,8 +159,17 @@ static void clearHandler(void) /*-********************************************************* * Termination signal trapping (Print debug stack trace) ***********************************************************/ +#if !defined(BACKTRACE_ENABLE) \ + && ((defined(__linux__) && defined(__GLIBC__)) \ + || (defined(__APPLE__) && defined(__MACH__)) ) +# define BACKTRACE_ENABLE 1 +#endif + + #if defined(BACKTRACE_ENABLE) && (BACKTRACE_ENABLE>=1) +#include /* backtrace, backtrace_symbols */ + #define MAX_STACK_FRAMES 50 static void ABRThandler(int sig) { From 70d8c2a0313deb2a64b549a80fad4eb63596dec2 Mon Sep 17 00:00:00 2001 From: Julian Fessard Date: Tue, 9 Oct 2018 17:14:57 -0700 Subject: [PATCH 09/18] fileio.c: Disable backtrace when built with address sanitizer Covers clang and gcc's sanitizer flags. Can still be overridden through CFLAGS on commandline. --- programs/fileio.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/programs/fileio.c b/programs/fileio.c index 8b1e9f2f2..a2d5af2c4 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -20,6 +20,14 @@ # define _POSIX_SOURCE 1 /* disable %llu warnings with MinGW on Windows */ #endif +#if defined(__has_feature) && !defined(BACKTRACE_ENABLE) /* Clang compiler */ +# if (__has_feature(address_sanitizer)) +# define BACKTRACE_ENABLE 0 +# endif /* __has_feature(address_sanitizer) */ +#elif defined(__SANITIZE_ADDRESS__) && !defined(BACKTRACE_ENABLE) /* GCC compiler */ +# define BACKTRACE_ENABLE 0 +#endif + #if !defined(BACKTRACE_ENABLE) \ && ((defined(__linux__) && defined(__GLIBC__)) \ || (defined(__APPLE__) && defined(__MACH__)) ) From b304b679e50be863f4f532ff176617f06056ef62 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 9 Oct 2018 17:56:59 -0700 Subject: [PATCH 10/18] use #if BACKTRACE_ENABLE directly as suggested by @terrelln --- programs/fileio.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 1eb9a1e0a..7cbaab7da 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -166,14 +166,20 @@ static void clearHandler(void) # define BACKTRACE_ENABLE 0 #endif -#if !defined(BACKTRACE_ENABLE) \ - && ((defined(__linux__) && defined(__GLIBC__)) \ - || (defined(__APPLE__) && defined(__MACH__)) ) -# define BACKTRACE_ENABLE 1 +#if !defined(BACKTRACE_ENABLE) +/* automatic detector : backtrace enabled by default on linux+glibc and osx */ +# if (defined(__linux__) && defined(__GLIBC__)) \ + || (defined(__APPLE__) && defined(__MACH__)) +# define BACKTRACE_ENABLE 1 +# else +# define BACKTRACE_ENABLE 0 +# endif #endif +/* note : after this point, BACKTRACE_ENABLE is necessarily defined */ -#if defined(BACKTRACE_ENABLE) && (BACKTRACE_ENABLE>=1) + +#if BACKTRACE_ENABLE #include /* backtrace, backtrace_symbols */ @@ -216,7 +222,7 @@ static void ABRThandler(int sig) { void FIO_addAbortHandler() { -#if defined(BACKTRACE_ENABLE) && (BACKTRACE_ENABLE>=1) +#if BACKTRACE_ENABLE signal(SIGABRT, ABRThandler); signal(SIGFPE, ABRThandler); signal(SIGILL, ABRThandler); From d3ec23313d4b9f9856608c18d7d6d729615904f9 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 10 Oct 2018 15:48:43 -0700 Subject: [PATCH 11/18] improved decompression speed while reviewing #1364, I found a decompression speed improvement. On my laptop, the new code decompresses +5-6% faster on clang and +2-3% faster on gcc. not bad for an accidental optimization... --- lib/common/bitstream.h | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/lib/common/bitstream.h b/lib/common/bitstream.h index 2f91460c5..00b22049a 100644 --- a/lib/common/bitstream.h +++ b/lib/common/bitstream.h @@ -339,17 +339,8 @@ MEM_STATIC size_t BIT_getUpperBits(size_t bitContainer, U32 const start) MEM_STATIC size_t BIT_getMiddleBits(size_t bitContainer, U32 const start, U32 const nbBits) { -#if defined(__BMI__) && defined(__GNUC__) && __GNUC__*1000+__GNUC_MINOR__ >= 4008 /* experimental */ -# if defined(__x86_64__) - if (sizeof(bitContainer)==8) - return _bextr_u64(bitContainer, start, nbBits); - else -# endif - return _bextr_u32(bitContainer, start, nbBits); -#else assert(nbBits < BIT_MASK_SIZE); return (bitContainer >> start) & BIT_mask[nbBits]; -#endif } MEM_STATIC size_t BIT_getLowerBits(size_t bitContainer, U32 const nbBits) @@ -366,9 +357,11 @@ MEM_STATIC size_t BIT_getLowerBits(size_t bitContainer, U32 const nbBits) * @return : value extracted */ MEM_STATIC size_t BIT_lookBits(const BIT_DStream_t* bitD, U32 nbBits) { -#if defined(__BMI__) && defined(__GNUC__) /* experimental; fails if bitD->bitsConsumed + nbBits > sizeof(bitD->bitContainer)*8 */ +#if 1 + assert(bitD->bitsConsumed + nbBits <= sizeof(bitD->bitContainer)*8); return BIT_getMiddleBits(bitD->bitContainer, (sizeof(bitD->bitContainer)*8) - bitD->bitsConsumed - nbBits, nbBits); #else + /* previous code path, seems slower */ U32 const regMask = sizeof(bitD->bitContainer)*8 - 1; return ((bitD->bitContainer << (bitD->bitsConsumed & regMask)) >> 1) >> ((regMask-nbBits) & regMask); #endif From 7791f192ee8ee6e80352fd9d906d015aaa7eb2e9 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 10 Oct 2018 16:36:11 -0700 Subject: [PATCH 12/18] removed one assert() which can be triggered when input is corrupted. --- lib/common/bitstream.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/common/bitstream.h b/lib/common/bitstream.h index 00b22049a..faa0c2163 100644 --- a/lib/common/bitstream.h +++ b/lib/common/bitstream.h @@ -340,6 +340,7 @@ MEM_STATIC size_t BIT_getUpperBits(size_t bitContainer, U32 const start) MEM_STATIC size_t BIT_getMiddleBits(size_t bitContainer, U32 const start, U32 const nbBits) { assert(nbBits < BIT_MASK_SIZE); + assert(start < sizeof(bitContainer)*8); return (bitContainer >> start) & BIT_mask[nbBits]; } @@ -357,11 +358,13 @@ MEM_STATIC size_t BIT_getLowerBits(size_t bitContainer, U32 const nbBits) * @return : value extracted */ MEM_STATIC size_t BIT_lookBits(const BIT_DStream_t* bitD, U32 nbBits) { + /* arbitrate between double-shift and shift+mask */ #if 1 - assert(bitD->bitsConsumed + nbBits <= sizeof(bitD->bitContainer)*8); + /* if bitD->bitsConsumed + nbBits > sizeof(bitD->bitContainer)*8, + * bitstream is likely corrupted, and result is undefined */ return BIT_getMiddleBits(bitD->bitContainer, (sizeof(bitD->bitContainer)*8) - bitD->bitsConsumed - nbBits, nbBits); #else - /* previous code path, seems slower */ + /* this code path is slower on my os-x laptop */ U32 const regMask = sizeof(bitD->bitContainer)*8 - 1; return ((bitD->bitContainer << (bitD->bitsConsumed & regMask)) >> 1) >> ((regMask-nbBits) & regMask); #endif From 433059bbb2d861e434b321fbe0a89933971f821b Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 10 Oct 2018 17:06:25 -0700 Subject: [PATCH 13/18] fixed minor gcc warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gcc-8 on Linux doesn't like usage of strncat : `warning: ‘strncat’ output truncated before terminating nul copying as many bytes from a string as its length`. Not sure what was wrong, it might be a false positive, but the logic is simple enough to replaced by a simple `memcpy()`, thus avoiding the shenanigans of null-terminated strings. --- programs/fileio.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 7cbaab7da..c24f4defb 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -1219,7 +1219,8 @@ FIO_determineCompressedName(const char* srcFileName, const char* suffix) size_t const sfnSize = strlen(srcFileName); size_t const suffixSize = strlen(suffix); - if (dfnbCapacity <= sfnSize+suffixSize+1) { /* resize name buffer */ + if (dfnbCapacity <= sfnSize+suffixSize+1) { + /* resize buffer for dstName */ free(dstFileNameBuffer); dfnbCapacity = sfnSize + suffixSize + 30; dstFileNameBuffer = (char*)malloc(dfnbCapacity); @@ -1227,8 +1228,8 @@ FIO_determineCompressedName(const char* srcFileName, const char* suffix) EXM_THROW(30, "zstd: %s", strerror(errno)); } } assert(dstFileNameBuffer != NULL); - strncpy(dstFileNameBuffer, srcFileName, sfnSize+1 /* Include null */); - strncat(dstFileNameBuffer, suffix, suffixSize); + memcpy(dstFileNameBuffer, srcFileName, sfnSize); + memcpy(dstFileNameBuffer+sfnSize, suffix, suffixSize+1 /* Include terminating null */); return dstFileNameBuffer; } From c012e9540ae4d9d040928f418d2eef11ecab56a1 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 10 Oct 2018 17:33:04 -0700 Subject: [PATCH 14/18] removed one assert() that can be triggered by a corrupted bitstream. --- lib/common/bitstream.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/common/bitstream.h b/lib/common/bitstream.h index faa0c2163..f8ec7efc9 100644 --- a/lib/common/bitstream.h +++ b/lib/common/bitstream.h @@ -340,7 +340,7 @@ MEM_STATIC size_t BIT_getUpperBits(size_t bitContainer, U32 const start) MEM_STATIC size_t BIT_getMiddleBits(size_t bitContainer, U32 const start, U32 const nbBits) { assert(nbBits < BIT_MASK_SIZE); - assert(start < sizeof(bitContainer)*8); + /* if start > bitMask, bitstream is corrupted, and result is undefined */ return (bitContainer >> start) & BIT_mask[nbBits]; } From 6ed3b526e4937900b2c8fc85800137784ae963a9 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 10 Oct 2018 18:26:44 -0700 Subject: [PATCH 15/18] restored bitMask for shift values since corrupted bitstreams can generate too large values. This slightly reduces the benefits from clang on my laptop. gcc results and code generation are not affected. --- lib/common/bitstream.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/common/bitstream.h b/lib/common/bitstream.h index f8ec7efc9..ef89b9878 100644 --- a/lib/common/bitstream.h +++ b/lib/common/bitstream.h @@ -339,9 +339,10 @@ MEM_STATIC size_t BIT_getUpperBits(size_t bitContainer, U32 const start) MEM_STATIC size_t BIT_getMiddleBits(size_t bitContainer, U32 const start, U32 const nbBits) { + U32 const regMask = sizeof(bitContainer)*8 - 1; + /* if start > regMask, bitstream is corrupted, and result is undefined */ assert(nbBits < BIT_MASK_SIZE); - /* if start > bitMask, bitstream is corrupted, and result is undefined */ - return (bitContainer >> start) & BIT_mask[nbBits]; + return (bitContainer >> (start & regMask)) & BIT_mask[nbBits]; } MEM_STATIC size_t BIT_getLowerBits(size_t bitContainer, U32 const nbBits) From f31715f5e0a94d74e2361c34d85a9bdd13f50acd Mon Sep 17 00:00:00 2001 From: Ori Livneh Date: Wed, 10 Oct 2018 22:32:03 -0400 Subject: [PATCH 16/18] Enable use of bswap intrinsics in clang Necessary because clang disguises itself as an older (__GNUC_MINOR__ = 2) GCC. --- lib/common/mem.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/common/mem.h b/lib/common/mem.h index 2051bcad1..5da248756 100644 --- a/lib/common/mem.h +++ b/lib/common/mem.h @@ -39,6 +39,10 @@ extern "C" { # define MEM_STATIC static /* this version may generate warnings for unused static functions; disable the relevant warning */ #endif +#ifndef __has_builtin +# define __has_builtin(x) 0 /* compat. with non-clang compilers */ +#endif + /* code only tested on 32 and 64 bits systems */ #define MEM_STATIC_ASSERT(c) { enum { MEM_static_assert = 1/(int)(!!(c)) }; } MEM_STATIC void MEM_check(void) { MEM_STATIC_ASSERT((sizeof(size_t)==4) || (sizeof(size_t)==8)); } @@ -198,7 +202,8 @@ MEM_STATIC U32 MEM_swap32(U32 in) { #if defined(_MSC_VER) /* Visual Studio */ return _byteswap_ulong(in); -#elif defined (__GNUC__) && (__GNUC__ * 100 + __GNUC_MINOR__ >= 403) +#elif (defined (__GNUC__) && (__GNUC__ * 100 + __GNUC_MINOR__ >= 403)) \ + || (defined(__clang__) && __has_builtin(__builtin_bswap32)) return __builtin_bswap32(in); #else return ((in << 24) & 0xff000000 ) | @@ -212,7 +217,8 @@ MEM_STATIC U64 MEM_swap64(U64 in) { #if defined(_MSC_VER) /* Visual Studio */ return _byteswap_uint64(in); -#elif defined (__GNUC__) && (__GNUC__ * 100 + __GNUC_MINOR__ >= 403) +#elif (defined (__GNUC__) && (__GNUC__ * 100 + __GNUC_MINOR__ >= 403)) \ + || (defined(__clang__) && __has_builtin(__builtin_bswap64)) return __builtin_bswap64(in); #else return ((in << 56) & 0xff00000000000000ULL) | From 1e6208e75ef0b80b50341da1df5f60a41b137e59 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 11 Oct 2018 14:40:12 -0700 Subject: [PATCH 17/18] bumped version number to v1.3.7 updated documentation --- Makefile | 11 +++++++++ doc/zstd_manual.html | 58 ++++++++++++++++++++++++++++++++------------ lib/zstd.h | 2 +- programs/zstd.1 | 6 ++--- tests/.gitignore | 1 + 5 files changed, 59 insertions(+), 19 deletions(-) diff --git a/Makefile b/Makefile index 9ee0b0ba6..c63db80e9 100644 --- a/Makefile +++ b/Makefile @@ -62,27 +62,38 @@ zstdmt: zlibwrapper: lib $(MAKE) -C $(ZWRAPDIR) all +## test: run long-duration tests .PHONY: test test: MOREFLAGS += -g -DDEBUGLEVEL=1 -Werror test: MOREFLAGS="$(MOREFLAGS)" $(MAKE) -j -C $(PRGDIR) allVariants $(MAKE) -C $(TESTDIR) $@ +## shortest: same as `make check` .PHONY: shortest shortest: $(MAKE) -C $(TESTDIR) $@ +## check: run basic tests for `zstd` cli .PHONY: check check: shortest +## examples: build all examples in `/examples` directory .PHONY: examples examples: lib CPPFLAGS=-I../lib LDFLAGS=-L../lib $(MAKE) -C examples/ all +## manual: generate API documentation in html format .PHONY: manual manual: $(MAKE) -C contrib/gen_html $@ +## man: generate man page +.PHONY: man +man: + $(MAKE) -C programs $@ + +## contrib: build all supported projects in `/contrib` directory .PHONY: contrib contrib: lib $(MAKE) -C contrib/pzstd all diff --git a/doc/zstd_manual.html b/doc/zstd_manual.html index 4a8985f26..f9b1daa8a 100644 --- a/doc/zstd_manual.html +++ b/doc/zstd_manual.html @@ -1,10 +1,10 @@ -zstd 1.3.6 Manual +zstd 1.3.7 Manual -

zstd 1.3.6 Manual

+

zstd 1.3.7 Manual

Contents

    @@ -313,11 +313,17 @@ size_t ZSTD_endStream(ZSTD_CStream* zcs, ZSTD_outBuffer* output); The function will update both `pos` fields. If `input.pos < input.size`, some input has not been consumed. It's up to the caller to present again remaining data. + The function tries to flush all data decoded immediately, repecting buffer sizes. If `output.pos < output.size`, decoder has flushed everything it could. - @return : 0 when a frame is completely decoded and fully flushed, - an error code, which can be tested using ZSTD_isError(), - any other value > 0, which means there is still some decoding to do to complete current frame. - The return value is a suggested next input size (a hint to improve latency) that will never load more than the current frame. + But if `output.pos == output.size`, there is no such guarantee, + it's likely that some decoded data was not flushed and still remains within internal buffers. + In which case, call ZSTD_decompressStream() again to flush whatever remains in the buffer. + When no additional input is provided, amount of data flushed is necessarily <= ZSTD_BLOCKSIZE_MAX. + @return : 0 when a frame is completely decoded and fully flushed, + or an error code, which can be tested using ZSTD_isError(), + or any other value > 0, which means there is still some decoding or flushing to do to complete current frame : + the return value is a suggested next input size (a hint for better latency) + that will never load more than the current frame.
    @@ -600,22 +606,40 @@ size_t ZSTD_initCStream_usingCDict_advanced(ZSTD_CStream* zcs, const ZSTD_CDict* 
    size_t ZSTD_resetCStream(ZSTD_CStream* zcs, unsigned long long pledgedSrcSize);
 
      start a new compression job, using same parameters from previous job.
-  This is typically useful to skip dictionary loading stage, since it will re-use it in-place..
+  This is typically useful to skip dictionary loading stage, since it will re-use it in-place.
   Note that zcs must be init at least once before using ZSTD_resetCStream().
   If pledgedSrcSize is not known at reset time, use macro ZSTD_CONTENTSIZE_UNKNOWN.
   If pledgedSrcSize > 0, its value must be correct, as it will be written in header, and controlled at the end.
   For the time being, pledgedSrcSize==0 is interpreted as "srcSize unknown" for compatibility with older programs,
   but it will change to mean "empty" in future version, so use macro ZSTD_CONTENTSIZE_UNKNOWN instead.
- @return : 0, or an error code (which can be tested using ZSTD_isError()) 
+ @return : 0, or an error code (which can be tested using ZSTD_isError())
+ 
 

    typedef struct {
-    unsigned long long ingested;
-    unsigned long long consumed;
-    unsigned long long produced;
-    unsigned currentJobID;
+    unsigned long long ingested;   /* nb input bytes read and buffered */
+    unsigned long long consumed;   /* nb input bytes actually compressed */
+    unsigned long long produced;   /* nb of compressed bytes generated and buffered */
+    unsigned long long flushed;    /* nb of compressed bytes flushed : not provided; can be tracked from caller side */
+    unsigned currentJobID;         /* MT only : latest started job nb */
+    unsigned nbActiveWorkers;      /* MT only : nb of workers actively compressing at probe time */
 } ZSTD_frameProgression;

    +
    size_t ZSTD_toFlushNow(ZSTD_CCtx* cctx);
+
      Tell how many bytes are ready to be flushed immediately.
+  Useful for multithreading scenarios (nbWorkers >= 1).
+  Probe the oldest active job, defined as oldest job not yet entirely flushed,
+  and check its output buffer.
+ @return : amount of data stored in oldest job and ready to be flushed immediately.
+  if @return == 0, it means either :
+  + there is no active job (could be checked with ZSTD_frameProgression()), or
+  + oldest job is still actively compressing data,
+    but everything it has produced has also been flushed so far,
+    therefore flushing speed is currently limited by production speed of oldest job
+    irrespective of the speed of concurrent newer jobs.
+ 
+

    +

    Advanced Streaming decompression functions

    typedef enum { DStream_p_maxWindowSize } ZSTD_DStreamParameter_e;
 size_t ZSTD_setDStreamParameter(ZSTD_DStream* zds, ZSTD_DStreamParameter_e paramType, unsigned paramValue);   /* obsolete : this API will be removed in a future version */
 size_t ZSTD_initDStream_usingDict(ZSTD_DStream* zds, const void* dict, size_t dictSize); /**< note: no dictionary will be used if dict == NULL or dictSize < 8 */
@@ -1015,9 +1039,13 @@ size_t ZSTD_CCtx_refPrefix_advanced(ZSTD_CCtx* cctx,
 

    
 
 
    typedef enum {
-    ZSTD_e_continue=0, /* collect more data, encoder decides when to output compressed result, for optimal conditions */
-    ZSTD_e_flush,      /* flush any data provided so far - frame will continue, future data can still reference previous data for better compression */
-    ZSTD_e_end         /* flush any remaining data and close current frame. Any additional data starts a new frame. */
+    ZSTD_e_continue=0, /* collect more data, encoder decides when to output compressed result, for optimal compression ratio */
+    ZSTD_e_flush,      /* flush any data provided so far,
+                        * it creates (at least) one new block, that can be decoded immediately on reception;
+                        * frame will continue: any future data can still reference previously compressed data, improving compression. */
+    ZSTD_e_end         /* flush any remaining data and close current frame.
+                        * any additional data starts a new frame.
+                        * each frame is independent (does not reference any content from previous frame). */
 } ZSTD_EndDirective;
 

    
 
    size_t ZSTD_compress_generic (ZSTD_CCtx* cctx,
diff --git a/lib/zstd.h b/lib/zstd.h
index 9b1410448..f2af4ac8c 100644
--- a/lib/zstd.h
+++ b/lib/zstd.h
@@ -71,7 +71,7 @@ extern "C" {
 /*------   Version   ------*/
 #define ZSTD_VERSION_MAJOR    1
 #define ZSTD_VERSION_MINOR    3
-#define ZSTD_VERSION_RELEASE  6
+#define ZSTD_VERSION_RELEASE  7
 
 #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 */
diff --git a/programs/zstd.1 b/programs/zstd.1
index 27ee6684c..674f89841 100644
--- a/programs/zstd.1
+++ b/programs/zstd.1
@@ -1,5 +1,5 @@
 .
-.TH "ZSTD" "1" "September 2018" "zstd 1.3.5" "User Commands"
+.TH "ZSTD" "1" "October 2018" "zstd 1.3.7" "User Commands"
 .
 .SH "NAME"
 \fBzstd\fR \- zstd, zstdmt, unzstd, zstdcat \- Compress or decompress \.zst files
@@ -123,8 +123,8 @@ Compress using \fB#\fR working threads (default: 1)\. If \fB#\fR is 0, attempt t
 Does not spawn a thread for compression, use a single thread for both I/O and compression\. In this mode, compression is serialized with I/O, which is slightly slower\. (This is different from \fB\-T1\fR, which spawns 1 compression thread in parallel of I/O)\. This mode is the only one available when multithread support is disabled\. Single\-thread mode features lower memory usage\. Final compressed result is slightly different from \fB\-T1\fR\.
 .
 .TP
-\fB\-\-adapt\fR
-\fBzstd\fR will dynamically adapt compression level to perceived I/O conditions\. Compression level adaptation can be observed live by using command \fB\-v\fR\. The feature works when combined with multi\-threading and \fB\-\-long\fR mode\. It does not work with \fB\-\-single\-thread\fR\. It sets window size to 8 MB by default (can be changed manually, see \fBwlog\fR)\. Due to the chaotic nature of dynamic adaptation, compressed result is not reproducible\. \fInote\fR : at the time of this writing, \fB\-\-adapt\fR can remain stuck at low speed when combined with multiple worker threads (>=2)\.
+\fB\-\-adapt[=min=#,max=#]\fR
+\fBzstd\fR will dynamically adapt compression level to perceived I/O conditions\. Compression level adaptation can be observed live by using command \fB\-v\fR\. Adaptation can be constrained between supplied \fBmin\fR and \fBmax\fR levels\. The feature works when combined with multi\-threading and \fB\-\-long\fR mode\. It does not work with \fB\-\-single\-thread\fR\. It sets window size to 8 MB by default (can be changed manually, see \fBwlog\fR)\. Due to the chaotic nature of dynamic adaptation, compressed result is not reproducible\. \fInote\fR : at the time of this writing, \fB\-\-adapt\fR can remain stuck at low speed when combined with multiple worker threads (>=2)\.
 .
 .TP
 \fB\-D file\fR
diff --git a/tests/.gitignore b/tests/.gitignore
index da536251d..1f08c3995 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -1,6 +1,7 @@
 # local binary (Makefile)
 fullbench
 fullbench32
+fullbench-lib
 fuzzer
 fuzzer32
 fuzzer-dll

From a2708de83452476f2bd3919fce1e5b27202f46e4 Mon Sep 17 00:00:00 2001
From: Yann Collet 
Date: Thu, 11 Oct 2018 16:49:55 -0700
Subject: [PATCH 18/18] update NEWS for v1.3.7

---
 NEWS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/NEWS b/NEWS
index 40805c187..fa1a22325 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,9 @@
+v1.3.7
+perf: slightly better decompression speed on clang (depending on hardware target)
+fix : performance of dictionary compression for small input < 4 KB at levels 9 and 10
+build: no longer build backtrace by default in release mode; restrict further automatic mode
+build: control backtrace support through build macro BACKTRACE
+
 v1.3.6
 perf: much faster dictionary builder, by @jenniferliu
 perf: faster dictionary compression on small data when using multiple contexts, by @felixhandte