ensure conformance with custom LZ4_DISTANCE_MAX
authorYann Collet <cyan@fb.com>
Mon, 15 Jul 2019 19:11:34 +0000 (12:11 -0700)
committerYann Collet <cyan@fb.com>
Mon, 15 Jul 2019 19:11:34 +0000 (12:11 -0700)
It's now possible to select a custom LZ4_DISTANCE_MAX at compile time,
provided it's <= 65535.

However, in some cases (when compressing in byU16 mode),
the new distance wasn't respected,
as it used to implied that it was necessarily within range.

Added a distance check for this case.
Also : added a new TravisCI test which ensures that
custom LZ4_DISTANCE_MAX compiles correctly
and compresses correctly (relying on `assert()` to find outsized offsets).

.travis.yml
lib/README.md
lib/lz4.c
tests/Makefile
tests/frametest.c

index af5cf0f..bd29630 100644 (file)
@@ -33,6 +33,10 @@ matrix:
       script:
         - CC=clang MOREFLAGS=-fsanitize=address make -C tests test-frametest test-fuzzer
 
+    - name: Custom LZ4_DISTANCE_MAX
+      script:
+        - MOREFLAGS=-DLZ4_DISTANCE_MAX=8000 make check
+
     - name: (Precise) g++ and clang CMake test
       dist: precise
       script:
index cf1505f..cba2c34 100644 (file)
@@ -56,8 +56,8 @@ The following build macro can be selected at compilation time :
 - `LZ4_DISTANCE_MAX` : control the maximum offset that the compressor will allow.
   Set to 65535 by default, which is the maximum value supported by lz4 format.
   Reducing maximum distance will reduce opportunities for LZ4 to find matches,
-  hence will produce worse the compression ratio.
-  However, a smaller max distance may allow compatibility with specific decoders using limited memory budget.
+  hence will produce a worse compression ratio.
+  However, a smaller max distance can allow compatibility with specific decoders using limited memory budget.
   This build macro only influences the compressed output of the compressor.
 
 - `LZ4_DISABLE_DEPRECATE_WARNINGS` : invoking a deprecated function will make the compiler generate a warning.
index 38ad6ab..abbdd97 100644 (file)
--- a/lib/lz4.c
+++ b/lib/lz4.c
@@ -413,7 +413,8 @@ static const int LZ4_minLength = (MFLIMIT+1);
 #define MB *(1 <<20)
 #define GB *(1U<<30)
 
-#if (LZ4_DISTANCE_MAX > 65535)   /* max supported by LZ4 format */
+#define LZ4_DISTANCE_ABSOLUTE_MAX 65535
+#if (LZ4_DISTANCE_MAX > LZ4_DISTANCE_ABSOLUTE_MAX)   /* max supported by LZ4 format */
 #  error "LZ4_DISTANCE_MAX is too big : must be <= 65535"
 #endif
 
@@ -915,10 +916,14 @@ LZ4_FORCE_INLINE int LZ4_compress_generic(
                 forwardH = LZ4_hashPosition(forwardIp, tableType);
                 LZ4_putIndexOnHash(current, h, cctx->hashTable, tableType);
 
+                DEBUGLOG(7, "candidate at pos=%u  (offset=%u \n", matchIndex, current - matchIndex);
                 if ((dictIssue == dictSmall) && (matchIndex < prefixIdxLimit)) { continue; }    /* match outside of valid area */
                 assert(matchIndex < current);
-                if ((tableType != byU16) && (matchIndex+LZ4_DISTANCE_MAX < current)) { continue; } /* too far */
-                if (tableType == byU16) { assert((current - matchIndex) <= LZ4_DISTANCE_MAX); } /* too_far presumed impossible with byU16 */
+                if ( ((tableType != byU16) || (LZ4_DISTANCE_MAX < LZ4_DISTANCE_ABSOLUTE_MAX))
+                  && (matchIndex+LZ4_DISTANCE_MAX < current)) {
+                    continue;
+                } /* too far */
+                assert((current - matchIndex) <= LZ4_DISTANCE_MAX);  /* match now expected within distance */
 
                 if (LZ4_read32(match) == LZ4_read32(ip)) {
                     if (maybe_extMem) offset = current - matchIndex;
@@ -1082,7 +1087,7 @@ _next_match:
             LZ4_putIndexOnHash(current, h, cctx->hashTable, tableType);
             assert(matchIndex < current);
             if ( ((dictIssue==dictSmall) ? (matchIndex >= prefixIdxLimit) : 1)
-              && ((tableType==byU16) ? 1 : (matchIndex+LZ4_DISTANCE_MAX >= current))
+              && (((tableType==byU16) && (LZ4_DISTANCE_MAX == LZ4_DISTANCE_ABSOLUTE_MAX)) ? 1 : (matchIndex+LZ4_DISTANCE_MAX >= current))
               && (LZ4_read32(match) == LZ4_read32(ip)) ) {
                 token=op++;
                 *token=0;
index 65713ef..422baba 100644 (file)
@@ -448,7 +448,7 @@ test-fuzzer32: CFLAGS += -m32
 test-fuzzer32: test-fuzzer
 
 test-frametest: frametest
-       ./frametest $(FUZZER_TIME)
+       ./frametest -v $(FUZZER_TIME)
 
 test-frametest32: CFLAGS += -m32
 test-frametest32: test-frametest
index 69dd5aa..91813bb 100644 (file)
@@ -46,6 +46,7 @@
 #define LZ4F_STATIC_LINKING_ONLY
 #include "lz4frame.h"
 #include "lz4frame.h"
+#define LZ4_STATIC_LINKING_ONLY  /* LZ4_DISTANCE_MAX */
 #include "lz4.h"        /* LZ4_VERSION_STRING */
 #define XXH_STATIC_LINKING_ONLY
 #include "xxhash.h"     /* XXH64 */
@@ -540,7 +541,7 @@ int basicTests(U32 seed, double compressibility)
                                               cdict, NULL) );
         DISPLAYLEVEL(3, "compressed %u bytes into %u bytes \n",
                         (unsigned)dictSize, (unsigned)cSizeWithDict);
-        if (cSizeWithDict >= cSizeNoDict) goto _output_error;  /* must be more efficient */
+        if ((LZ4_DISTANCE_MAX > dictSize) && (cSizeWithDict >= cSizeNoDict)) goto _output_error;  /* must be more efficient */
         crcOrig = XXH64(CNBuffer, dictSize, 0);
 
         DISPLAYLEVEL(3, "LZ4F_decompress_usingDict : ");