Fix calculation for using the "fast path" with Deflate

CMakeLists.txt

@@ -1,6 +1,6 @@
 cmake_minimum_required(VERSION 3.20)
 project(inflatelib
-    VERSION 0.1
+    VERSION 0.2
     DESCRIPTION "A Deflate and Deflate64 decompression library"
     HOMEPAGE_URL "https://github.com/microsoft/inflatelib"
     LANGUAGES C CXX

src/include/inflatelib.h

@@ -48,16 +48,34 @@ extern "C"
 {
 #endif
 
-#define INFLATELIB_VERSION_STRING "0.0.1"
+#define INFLATELIB_VERSION_STRING "0.2.0"
 #define INFLATELIB_VERSION_MAJOR 0
-#define INFLATELIB_VERSION_MINOR 0
-#define INFLATELIB_VERSION_PATCH 1
+#define INFLATELIB_VERSION_MINOR 2
+#define INFLATELIB_VERSION_PATCH 0
 
     typedef void* (*inflatelib_alloc)(void* userData, size_t bytes, size_t alignment);
     typedef void (*inflatelib_free)(void* userData, void* allocatedPtr, size_t bytes, size_t alignment);
 
     struct inflatelib_state; /* Opaque to client applications */
 
+    /*
+     * This struct stores all data and state for decompressing Deflate/Deflate64 encoded data. With the exception of
+     * success/error codes, all input and output data is passed through this struct. Specifically, input and output
+     * buffers are set by the caller via 'next_in'/'avail_in' and 'next_out'/'avail_out' respectively. The "inflate" and
+     * "inflate64" functions communicate the amount of data consumed/written by updating these buffers and lengths. For
+     * example, you can determine the amount of data consumed/written by a call to 'inflatelib_inflate' or
+     * 'inflatelib_inflate64' by either subtracting 'next_in'/'next_out' after the call by their original pointers
+     * (interpreted as uintptr_t values) or by subtracting 'avail_in'/'avail_out' after the call from the sizes passed
+     * in. Alternatively, you can subtract 'total_in'/'total_out' after the call by their values before the call (or
+     * equivalently by setting them to zero before the call and reading their values after the call).
+     *
+     * Internally, these objects behave as a state machine. After being initialized, they transition to either "Deflate"
+     * or "Deflate64" decoding states via calls to 'inflatelib_inflate' and 'inflatelib_inflate64' respectively. When
+     * in these states, the only valid operations are to destroy the object, put the object back into the initialized
+     * state via 'inflatelib_reset', or to decompress more data using the function that corresponds to the current
+     * state: either 'inflatelib_inflate' for the "Deflate" state or 'inflatelib_inflate64' for the "Deflate64" state.
+     * If you attempt to call 'inflatelib_inflate64' when in the "Deflate" state or vice-versa, the call will fail.
+     */
     typedef struct inflatelib_stream
     {
         /*
@@ -150,12 +168,18 @@ extern "C"
     INFLATELIB_EXPORT int INFLATELIB_CALLCONV inflatelib_destroy(inflatelib_stream* stream);
 
     /*
-     *
+     * "Inflates" the Deflate encoded data from next_in/avail_in, writing the decoded data to next_out/avail_out. The
+     * 'stream' MUST be in either the "initialized" state or the "Deflate" state. This function returns one of the
+     * status values defined above, notably it returns 'INFLATELIB_EOF' when the last block of data has been fully
+     * processed.
      */
     INFLATELIB_EXPORT int INFLATELIB_CALLCONV inflatelib_inflate(inflatelib_stream* stream);
 
     /*
-     *
+     * "Inflates" the Deflate64 encoded data from next_in/avail_in, writing the decoded data to next_out/avail_out. The
+     * 'stream' MUST be in either the "initialized" state or the "Deflate64" state. This function returns one of the
+     * status values defined above, notably it returns 'INFLATELIB_EOF' when the last block of data has been fully
+     * processed.
      */
     INFLATELIB_EXPORT int INFLATELIB_CALLCONV inflatelib_inflate64(inflatelib_stream* stream);
 

src/lib/huffman_tree.c

@@ -322,7 +322,7 @@ int huffman_tree_lookup_unchecked(huffman_tree* tree, inflatelib_stream* stream,
     if (tableEntry->code_length == 0)
     {
         /* Zero means unassigned; this is an error */
-        if (format_error_message(stream, "Input bit sequence 0x%2X is not a valid Huffman code for the encoded table", input) < 0)
+        if (format_error_message(stream, "Input bit sequence 0x%02zX is not a valid Huffman code for the encoded table", input) < 0)
         {
             stream->error_msg = "Input bit sequence is not a valid Huffman code for the encoded table";
         }

src/lib/inflate.c

@@ -786,9 +786,19 @@ static const inflater_tables* const inflate_tables[] = {&deflate_tables, &deflat
 /* The maximum number of bytes that a single compressed block operation can consume. These values are used to optimize
  * the likely path where we have enough data for a single operation so we don't have to continuously check to see if we
  * have enough data. These values are calculated as follows:
- * Deflate: 15 bit length + 5 extra bits + 15 bit distance + 13 extra bits = 48 bits = 6 bytes
- * Deflate64: 15 bit length + 16 extra bits + 15 bit distance + 14 extra bits = 60 bits = 8 bytes (rounded up) */
-static const size_t max_compressed_op_size[] = {6, 8};
+ * Deflate:
+ *      15 bit length:      0 bits in stream  -> read 2 bytes = 16 bits in stream -> 1 bit leftover
+ *      5 extra bits:       1 bit in stream   -> read 2 bytes = 17 bits in stream -> 12 bits leftover
+ *      15 bit distance:    12 bits in stream -> read 2 bytes = 28 bits in stream -> 13 bits leftover
+ *      13 extra bits:      13 bits in stream -> read 2 bytes = 29 bits in stream -> 16 bits leftover
+ *          Total Bytes Needed:                       8 bytes
+ * Deflate64:
+ *      15 bit length:      0 bits in stream  -> read 2 bytes = 16 bits in stream -> 1 bit leftover
+ *      16 extra bits:      1 bit in stream   -> read 2 bytes = 17 bits in stream -> 1 bit leftover
+ *      15 bit distance:    1 bit in stream   -> read 2 bytes = 17 bits in stream -> 2 bits leftover
+ *      14 extra bits:      2 bits in stream  -> read 2 bytes = 18 bits in stream -> 4 bits leftover
+ *          Total Bytes Needed:                       8 bytes */
+static const size_t max_compressed_op_size = 8;
 
 /* static int inflater_read_compressed_fast(inflatelib_stream* stream); */
 static int inflater_read_compressed_fast(inflatelib_stream* stream);
@@ -802,7 +812,6 @@ static int inflater_read_compressed(inflatelib_stream* stream)
     uint16_t symbol;
     int opResult, keepGoing = 1;
     const inflater_tables* tables = inflate_tables[state->mode];
-    const size_t maxOpSize = max_compressed_op_size[state->mode];
 
     /* On entry, try and write any data we previously wrote to the window, but did not consume */
     bytesCopied = window_copy_output(&state->window, out, outSize);
@@ -815,7 +824,7 @@ static int inflater_read_compressed(inflatelib_stream* stream)
         {
         case ifstate_reading_literal_length_code:
             /* The fast path requires that we start in 'ifstate_reading_literal_length_code' */
-            if ((state->bitstream.length >= maxOpSize) && outSize)
+            if ((state->bitstream.length >= max_compressed_op_size) && outSize)
             {
                 stream->next_out = out;
                 stream->avail_out = outSize;
@@ -1063,10 +1072,9 @@ static int inflater_read_compressed_fast(inflatelib_stream* stream)
     uint32_t blockLength, blockDistance;
     int opResult;
     const inflater_tables* tables = inflate_tables[state->mode];
-    const size_t maxOpSize = max_compressed_op_size[state->mode];
 
     assert(state->ifstate == ifstate_reading_literal_length_code);
-    while ((state->bitstream.length >= maxOpSize) && outSize)
+    while ((state->bitstream.length >= max_compressed_op_size) && outSize)
     {
         opResult = huffman_tree_lookup_unchecked(&state->literal_length_tree, stream, &symbol);
         if (opResult < 0)

test/cpp/InflateTests.cpp

@@ -23,6 +23,9 @@ static int fopen_s(FILE** streamptr, const char* filename, const char* mode)
 #include <inflatelib.hpp>
 #include <filesystem>
 
+// Some tests need to see internal state for proper execution
+#include <internal.h>
+
 // These tests have backing test files compiled from 'test/data' and placed into '${buildRoot}/test/data'. When running
 // this test, that path is '../data' relative to the test executable.
 #ifdef _WIN32
@@ -697,3 +700,67 @@ TEST_CASE("InflateReset", "[inflate][inflate64]")
     doInflate64();
     stream.reset();
 }
+
+TEST_CASE("InflateCodeLensStress", "[inflate]")
+{
+    auto doCodeLensStressTest =
+        []<inflate_t inflateFunc>(
+            const char* inputFileName, const char* outputFileName, std::size_t initialReadSize, std::size_t bytesToConsume) {
+            auto input = read_file(data_directory / inputFileName);
+            auto output = read_file(data_directory / outputFileName);
+            auto outputBuffer = std::make_unique<std::byte[]>(output.size);
+
+            std::span<const std::byte> inputSpan = {input.buffer.get(), input.size};
+            std::span<std::byte> outputSpan = {outputBuffer.get(), output.size};
+
+            inflatelib::stream stream;
+
+            // Do the initial read, which should set up the tables
+            REQUIRE(initialReadSize <= inputSpan.size());
+            auto initialReadSpan = inputSpan.first(initialReadSize);
+            inputSpan = inputSpan.subspan(initialReadSize);
+            REQUIRE((stream.*inflateFunc)(initialReadSpan, outputSpan)); // Should not be EOF yet
+            REQUIRE(initialReadSpan.empty()); // All data should be consumed, even if there are leftover bits
+
+            // ASan works much better if it knows the bounds of the buffers, so we copy input to a new buffer
+            auto inputBuffer = std::make_unique<std::byte[]>(bytesToConsume);
+
+            for (bool keepGoing = true; keepGoing;)
+            {
+                // We start with an output buffer that should be large enough to hold all the output. If we run out of
+                // input, but still aren't done, that means that we're stuck waiting for more space to write output to
+                // and are therefore stuck in an infinite loop, so fail out.
+                REQUIRE(!inputSpan.empty());
+
+                // Because 'bitsream' wants to fill its buffer to at least 16 bits, we could have more than a byte already in
+                // the stream, which we need to account for to match 'bytesToConsume'
+                auto nextReadSize = (stream.get()->internal->bitstream.bits_in_buffer >= 8) ? bytesToConsume - 1 : bytesToConsume;
+                nextReadSize = std::min(nextReadSize, inputSpan.size());
+
+                auto bufferStart = inputBuffer.get() + (bytesToConsume - nextReadSize);
+                std::memcpy(bufferStart, inputSpan.data(), nextReadSize);
+
+                std::span<const std::byte> nextReadSpan = {bufferStart, nextReadSize};
+                inputSpan = inputSpan.subspan(nextReadSize);
+                keepGoing = (stream.*inflateFunc)(nextReadSpan, outputSpan);
+                REQUIRE(nextReadSpan.empty()); // Otherwise some other failure occurred
+            }
+
+            REQUIRE(inputSpan.empty());  // Should have consumed all bytes
+            REQUIRE(outputSpan.empty()); // Should have written all bytes
+            REQUIRE(std::memcmp(outputBuffer.get(), output.buffer.get(), output.size) == 0);
+        };
+
+    // NOTE: We loop using the range 7-9 for 'bytesToConsume' because 7 guarantees that we always take the slow path,
+    // 8 uses both the slow and fast path, and 9 guarantees the fast path
+    for (std::size_t bytesToConsume = 7; bytesToConsume <= 9; ++bytesToConsume)
+    {
+        // NOTE: 'dynamic.code-len-stress.deflate.in' has 1833 bytes plus a few bits before the encoded data we care about
+        doCodeLensStressTest.operator()<&inflatelib::stream::inflate>(
+            "dynamic.code-len-stress.deflate.in.bin", "dynamic.code-len-stress.deflate.out.bin", 1834, bytesToConsume);
+
+        // NOTE: 'dynamic.code-len-stress.deflate64.in' has 2184 bytes before the encoded data we care about
+        doCodeLensStressTest.operator()<&inflatelib::stream::inflate64>(
+            "dynamic.code-len-stress.deflate64.in.bin", "dynamic.code-len-stress.deflate64.out.bin", 2184, bytesToConsume);
+    }
+}

test/data/CMakeLists.txt

@@ -22,6 +22,10 @@ set(INPUT
     "dynamic.overlap.deflate.out"
     "dynamic.overlap.deflate64.in"
     "dynamic.overlap.deflate64.out"
+    "dynamic.code-len-stress.deflate.in"
+    "dynamic.code-len-stress.deflate.out"
+    "dynamic.code-len-stress.deflate64.in"
+    "dynamic.code-len-stress.deflate64.out"
     "dynamic.length-distance-stress.deflate.in"
     "dynamic.length-distance-stress.deflate.out"
     "dynamic.length-distance-stress.deflate64.in"

test/data/dynamic.code-len-stress.deflate.in

@@ -0,0 +1,112 @@
+
+# This uses maximum code lengths (15 bits) for length/distances to test code paths that may be sensitive to reading a
+# lot of data in a single "pass"
+>1
+1   # bfinal = true
+10  # Compressed with dynamic codes
+
+# NOTE: 0 bytes + 3 bits total written
+
+11100   # HLIT = 285 (28 + 257)
+11101   # HDIST = 30 (29 + 1)
+1111    # HCLEN = 19 (15 + 4)
+
+# NOTE: 2 bytes + 1 bit total written
+
+# Code Length Alphabet Code Lengths:
+# NOTE: Alphabet becomes 0=15, 1=18
+#16  17  18   0   8   7   9   6  10   5  11   4  12   3  13   2  14   1  15
+000 000 001 000 000 000 000 000 000 000 000 000 000 000 000 000 000 000 001
+
+# NOTE: 9 bytes + 2 bits total written
+
+# Literal/Length Code Lengths:
+>>1
+# Literal values 0-255 all get codes (length 15)
+0000000000000000000000000000000000000000000000000000000000000000 # 0-63
+0000000000000000000000000000000000000000000000000000000000000000 # 64-127
+0000000000000000000000000000000000000000000000000000000000000000 # 128-191
+0000000000000000000000000000000000000000000000000000000000000000 # 192-255
+0 # 256 (end of block)
+1 >1 0010000 >>1 # Write zero for 257-283
+0 # 284
+
+# NOTE: 42 bytes + 4 bits total written
+
+# Distance Alphabet Code Lengths:
+000000000000000000000000000000 # 0-29 - all have code length of 15
+
+# NOTE: 46 bytes + 2 bits total written
+
+# Literal/Length Tree:
+#   Symbol      Bit Count   Code
+#   0-255       15          000000000000000-000000011111111
+#   END         15          000000100000000
+#   284         15          000000100000001
+#
+# Distance Tree:
+#   Symbol      Bit Count   Code
+#   0-29        15          000000000000000-000000000011101
+
+# Encoded Data:
+>>1
+
+# Write the values 0-255 (we'll copy them a bunch of times)
+000000000000000 000000000000001 000000000000010 000000000000011 000000000000100 000000000000101 000000000000110 000000000000111
+000000000001000 000000000001001 000000000001010 000000000001011 000000000001100 000000000001101 000000000001110 000000000001111
+000000000010000 000000000010001 000000000010010 000000000010011 000000000010100 000000000010101 000000000010110 000000000010111
+000000000011000 000000000011001 000000000011010 000000000011011 000000000011100 000000000011101 000000000011110 000000000011111
+000000000100000 000000000100001 000000000100010 000000000100011 000000000100100 000000000100101 000000000100110 000000000100111
+000000000101000 000000000101001 000000000101010 000000000101011 000000000101100 000000000101101 000000000101110 000000000101111
+000000000110000 000000000110001 000000000110010 000000000110011 000000000110100 000000000110101 000000000110110 000000000110111
+000000000111000 000000000111001 000000000111010 000000000111011 000000000111100 000000000111101 000000000111110 000000000111111
+000000001000000 000000001000001 000000001000010 000000001000011 000000001000100 000000001000101 000000001000110 000000001000111
+000000001001000 000000001001001 000000001001010 000000001001011 000000001001100 000000001001101 000000001001110 000000001001111
+000000001010000 000000001010001 000000001010010 000000001010011 000000001010100 000000001010101 000000001010110 000000001010111
+000000001011000 000000001011001 000000001011010 000000001011011 000000001011100 000000001011101 000000001011110 000000001011111
+000000001100000 000000001100001 000000001100010 000000001100011 000000001100100 000000001100101 000000001100110 000000001100111
+000000001101000 000000001101001 000000001101010 000000001101011 000000001101100 000000001101101 000000001101110 000000001101111
+000000001110000 000000001110001 000000001110010 000000001110011 000000001110100 000000001110101 000000001110110 000000001110111
+000000001111000 000000001111001 000000001111010 000000001111011 000000001111100 000000001111101 000000001111110 000000001111111
+000000010000000 000000010000001 000000010000010 000000010000011 000000010000100 000000010000101 000000010000110 000000010000111
+000000010001000 000000010001001 000000010001010 000000010001011 000000010001100 000000010001101 000000010001110 000000010001111
+000000010010000 000000010010001 000000010010010 000000010010011 000000010010100 000000010010101 000000010010110 000000010010111
+000000010011000 000000010011001 000000010011010 000000010011011 000000010011100 000000010011101 000000010011110 000000010011111
+000000010100000 000000010100001 000000010100010 000000010100011 000000010100100 000000010100101 000000010100110 000000010100111
+000000010101000 000000010101001 000000010101010 000000010101011 000000010101100 000000010101101 000000010101110 000000010101111
+000000010110000 000000010110001 000000010110010 000000010110011 000000010110100 000000010110101 000000010110110 000000010110111
+000000010111000 000000010111001 000000010111010 000000010111011 000000010111100 000000010111101 000000010111110 000000010111111
+000000011000000 000000011000001 000000011000010 000000011000011 000000011000100 000000011000101 000000011000110 000000011000111
+000000011001000 000000011001001 000000011001010 000000011001011 000000011001100 000000011001101 000000011001110 000000011001111
+000000011010000 000000011010001 000000011010010 000000011010011 000000011010100 000000011010101 000000011010110 000000011010111
+000000011011000 000000011011001 000000011011010 000000011011011 000000011011100 000000011011101 000000011011110 000000011011111
+000000011100000 000000011100001 000000011100010 000000011100011 000000011100100 000000011100101 000000011100110 000000011100111
+000000011101000 000000011101001 000000011101010 000000011101011 000000011101100 000000011101101 000000011101110 000000011101111
+000000011110000 000000011110001 000000011110010 000000011110011 000000011110100 000000011110101 000000011110110 000000011110111
+000000011111000 000000011111001 000000011111010 000000011111011 000000011111100 000000011111101 000000011111110 000000011111111
+
+# NOTE: 526 bytes + 2 bits total written
+
+# Now copy it a bunch of times to fill up the 64k buffer (repeat 255 times)
+repeat (255) {
+    000000100000001 >1 11101 >>1 000000000001111 >1 111111 >>1 # Length = 256, Distance = 256
+}
+
+# NOTE: 1833 bytes + 1 bits total written
+
+# At this point, we want to stress the max code lengths, but we want to do so with every possible "remainder" in the
+# stream from prior operations (0-15).
+# NOTE: A single length/distance pair has a max of 15 + 5 + 15 + 13 = 48 bits, which gives an even 6 bytes, so we can't
+#       just repeat the longest possible "command" several times and exercise the above scenario, so we also need to
+#       inject a shorter command that will only write 47 bits
+
+# Each "iteration" of this loop causes 0-255 to get repeated twice
+repeat (16) {
+    000000100000001 >1 11101 >>1 000000000011101 >1 1111111111111 >>1 # Length = 256, Distance = 32768 (48 bits total = 6 bytes)
+    000000100000001 >1 11101 >>1 000000000011011 >1 111111111111  >>1 # Length = 256, Distance = 16384 (47 bits total = 5 bytes + 7 bits)
+}
+
+# At this point, we've repeated the values 0-255: 1 + 255 + 16 * 2 = 288 times
+
+# End
+000000100000000

test/data/dynamic.code-len-stress.deflate.out

@@ -0,0 +1,4 @@
+
+# The input for this test repeats the values 0-255 288 times
+<dec
+repeat (288) { 0...255 }