From 0c14f35df5045c5c3a72a978c1e591b3d501ed45 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Thu, 3 Sep 2026 10:15:23 -0700 Subject: [PATCH 1/8] iaa: stop offering a stream to IAA after a history-window rejection IAA's decompressor has a fixed 4 kB history buffer. A stream produced by zlib, whose window is up to 32 kB, therefore cannot be decoded at all: QPL returns QPL_STS_BAD_DIST_ERR (217) as soon as it meets a match distance above 4096. Today the shim has no way to tell that failure apart from any other. Within one stream that costs nothing extra, because the zlib fall-through pins the stream to zlib and nothing is resubmitted until it is reset -- but the reset is exactly what the workload does. Lucene reuses one Inflater and resets it once per stored-field document, so the shim pays a device round trip per document, forever, and never learns. This is not a corner case. In an OpenSearch/Lucene stored-fields read workload over a zlib-written index, 99.96% of 7,397,239 IAA inflate submissions came back with status 217, and the wasted work made the IAA path measurably slower than plain software zlib -- the only accelerator configuration in that campaign that lost to it. Measured with this branch against its own base, built from one cmake line and run in one session on one host over one restored index: the search throughput ceiling goes from 11,487.8 to 11,739.4 ops/s, +2.19%. Traffic to the device falls 92.0% per device-second -- 6,424 to 511 work-queue requests, 9.02 MB to 0.72 MB -- with the mean bytes per request unchanged at about 1,400, which is what whole submissions disappearing looks like. At a load both builds absorb completely, so the delivered work is equal and the cost is directly comparable, server CPU falls from 73.3% to 70.1% and median service time from 2.97 to 2.64 ms. This does not turn IAA inflate into a win on zlib-written data and is not meant to. The 4 kB window is a property of the device and no bookkeeping changes it. Unshimmed zlib measured 11,852.9 ops/s in the same session, so the fixed path is still 0.96% below it; what remains is the shim's own interposition plus the one submission per stream the fix has to spend to learn the answer from the device. What the change buys is that the path stops paying for work it cannot use: it closes most of a 3.08% regression against plain zlib, and at equal delivered load it now costs the same CPU as zlib rather than 3.2 points more. The window is a property of whichever compressor produced the bytes, not of the individual block, so the rejection is worth remembering: * UncompressIAA() gains an optional out-parameter that distinguishes QPL_STS_BAD_DIST_ERR from every other failure. Other statuses stay lumped together, since only this one predicts the next call. * Per stream, a flag on InflateSettings suppresses further IAA submissions. It is deliberately NOT cleared by inflateReset(): a reset begins a new stream from the same producer, and Lucene resets its Inflater once per stored-field document, so clearing it would make the flag useless. ResetInflateStreamState() carries a comment saying so, next to the fields that are cleared there. * InflateStreamSettings::SetFromCopy() copies the flag explicitly. It copies field by field rather than by value, so a new member is otherwise silently dropped and a stream from inflateCopy() would go back to submitting. A copy shares the producer, so it inherits the verdict. Two other inflate entry points need nothing. uncompress2() reaches IsIAADecompressible() with zlib-format window bits, where the real window is read out of the two-byte header: that answer is authoritative, nothing is guessed, and there is no waste to remove. gzread() pins a file to zlib on its first accelerator failure of any kind, so nothing more is submitted for the rest of that read pass; gzrewind() clears the pin deliberately, to offer the accelerator another try at a file whose stream a mid-file fallback had taken away. One wasted submission per pass is all that path can spend, and within a pass there is no second one to suppress. gzread() carries a comment saying so; uncompress2() leans on the rationale already recorded in IsIAADecompressible(). The remaining exposure is raw deflate and gzip, the two formats where IsIAADecompressible() has no header to read and is guessing from input length alone. Raw deflate is what Lucene uses. The bet is one-directional. Declining to offload can only cost throughput, never correctness, and the fallback path is the one that was already producing every byte of output. Tests: four new cases in IAAWindowRejectionTest. One calls UncompressIAA directly on the software QPL path, so it runs without a device and pins the out-parameter contract in both directions -- set on a 32 kB-window stream, left alone on a 12 kB one, and safely omitted. Three go through the public API and assert what the fix depends on: the flag survives inflateReset(), it propagates through inflateCopy(), and it stays per-stream, with a second stream still served by IAA afterwards. Those three need a working device to produce a 217 at all and skip with a message without one. Both traps were checked by mutation: clearing the flag in ResetInflateStreamState() or dropping the SetFromCopy() line fails the matching test. A unit-level probe that replays Lucene's call shape -- 480 raw-deflate streams of period 20000, each inflated whole through one reused z_stream with inflateReset() between them -- goes from 480 rejected submissions to 1, with 480/480 byte-correct output in both arms. The one that remains is the fix working as designed: it has to learn the answer from the device once before it can stop asking. The existing suite is unchanged: 12,836 checks with USE_IAA=ON, 0 failures, and the same skip list before and after; the only new results are the four above. Signed-off-by: Olasoji --- iaa.cpp | 11 +- iaa.h | 16 ++- tests/inflate_test.cpp | 285 ++++++++++++++++++++++++++++++++++++++++- zlib_accel.cpp | 46 ++++++- zlib_accel.h | 6 + 5 files changed, 355 insertions(+), 9 deletions(-) diff --git a/iaa.cpp b/iaa.cpp index 3acb2c4..50c10ab 100644 --- a/iaa.cpp +++ b/iaa.cpp @@ -197,7 +197,8 @@ int CompressIAA(uint8_t* input, uint32_t* input_length, uint8_t* output, int UncompressIAA(uint8_t* input, uint32_t* input_length, uint8_t* output, uint32_t* output_length, qpl_path_t execution_path, - int window_bits, bool* end_of_stream, bool detect_gzip_ext) { + int window_bits, bool* end_of_stream, bool detect_gzip_ext, + bool* window_too_large) { Log(LogLevel::LOG_INFO, "UncompressIAA() Line ", __LINE__, " input_length ", *input_length, "\n"); @@ -235,6 +236,14 @@ int UncompressIAA(uint8_t* input, uint32_t* input_length, uint8_t* output, qpl_status status = qpl_execute_job(job); if (status != QPL_STS_OK && status != QPL_STS_MORE_OUTPUT_NEEDED) { + // QPL_STS_BAD_DIST_ERR means the stream referenced a match further back + // than IAA's 4 kB history buffer. Unlike the other failures this one is not + // about this call: it says the producer used a larger window, and every + // remaining block of the same stream will be rejected for the same reason. + // Report it separately so the caller can stop submitting. + if (status == QPL_STS_BAD_DIST_ERR && window_too_large != nullptr) { + *window_too_large = true; + } Log(LogLevel::LOG_ERROR, "UncompressIAA() Line ", __LINE__, " qpl_execute_job status ", status, "\n"); return 1; diff --git a/iaa.h b/iaa.h index 94e1b5b..77d4e73 100644 --- a/iaa.h +++ b/iaa.h @@ -55,10 +55,18 @@ int CompressIAA(uint8_t* input, uint32_t* input_length, uint8_t* output, int window_bits, uint32_t max_compressed_size = 0, bool gzip_ext = false); -int UncompressIAA(uint8_t* input, uint32_t* input_length, uint8_t* output, - uint32_t* output_length, qpl_path_t execution_path, - int window_bits, bool* end_of_stream, - bool detect_gzip_ext = false); +// window_too_large, when non-null, is set to true if the job was rejected +// because the stream references match distances beyond IAA's fixed 4 kB history +// buffer (QPL_STS_BAD_DIST_ERR). That is a property of whichever compressor +// produced the stream, not of the individual block, so a caller that sees it +// can stop offering the rest of that stream to IAA. It is never set to false; +// the caller owns initialisation. +VISIBLE_FOR_TESTING int UncompressIAA(uint8_t* input, uint32_t* input_length, + uint8_t* output, uint32_t* output_length, + qpl_path_t execution_path, + int window_bits, bool* end_of_stream, + bool detect_gzip_ext = false, + bool* window_too_large = nullptr); VISIBLE_FOR_TESTING bool SupportedOptionsIAA(int window_bits, uint32_t input_length, diff --git a/tests/inflate_test.cpp b/tests/inflate_test.cpp index 2819441..c588823 100644 --- a/tests/inflate_test.cpp +++ b/tests/inflate_test.cpp @@ -2,7 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 // inflate() regression suites: the IGZIP inflate path, the accelerator -> -// IGZIP fallbacks, the flush/data_type gate and the dictionary fallback. +// IGZIP fallbacks, the flush/data_type gate, the dictionary fallback and the +// remembered IAA history-window rejection. #include @@ -11,6 +12,7 @@ #include #include "../config/config.h" +#include "../iaa.h" #include "../zlib_accel.h" #include "test_utils.h" @@ -2049,3 +2051,284 @@ TEST_F(DictionaryMidstreamFallbackRegressionTest, } #endif #endif + +#ifdef USE_IAA +// IAA's decompressor has a fixed 4 kB history buffer, so it cannot decode a +// stream whose producer used a larger window -- zlib's default is 32 kB. QPL +// reports that as QPL_STS_BAD_DIST_ERR, and it is the one decompress failure +// that predicts the next call: the window belongs to the compressor, not to the +// block. IsIAADecompressible() cannot see it for raw deflate or gzip, where +// there is no header to read the window out of, so the only way to know is to +// be told once and remember. +class IAAWindowRejectionTest : public ::testing::Test {}; + +// Run one whole stream through strm and check the bytes. Returns the last +// inflate() code so the caller can assert on it, or Z_DATA_ERROR if the output +// came back wrong. +static int InflateWholeStream(z_streamp strm, const std::string& compressed, + const char* expected, size_t expected_length) { + std::vector output(expected_length + 1024); + strm->next_in = + reinterpret_cast(const_cast(compressed.data())); + strm->avail_in = static_cast(compressed.size()); + strm->next_out = output.data(); + strm->avail_out = static_cast(output.size()); + int ret = Z_OK; + for (int guard = 0; guard < 128; guard++) { + ret = inflate(strm, Z_NO_FLUSH); + if (ret != Z_OK && ret != Z_BUF_ERROR) { + break; + } + } + if (ret != Z_STREAM_END) { + return ret; + } + if (strm->total_out != expected_length || + memcmp(output.data(), expected, expected_length) != 0) { + return Z_DATA_ERROR; + } + return Z_STREAM_END; +} + +// Every case below except the first needs QPL to get far enough into a job to +// report the oversized window. Without a usable device it fails at job +// initialization instead, which leaves the flag correctly clear -- so the test +// would be asserting the opposite of what it means to. Probe with a stream IAA +// can definitely decode: a short one, whose matches cannot reach back 4 kB +// because the whole payload is smaller than that. +static bool IAAHardwareDecompressWorks() { + const size_t input_length = 2048; + char* input = GenerateSeededCompressibleBlock(input_length, /*seed=*/0x4144); + if (input == nullptr) { + return false; + } + SetCompressPath(ZLIB, /*zlib_fallback=*/true, false, false); + std::string compressed; + size_t output_upper_bound = 0; + ExecutionPath compress_path = UNDEFINED; + int ret = ZlibCompress(input, input_length, &compressed, -15, Z_FINISH, + &output_upper_bound, &compress_path); + DestroyBlock(input); + if (ret != Z_STREAM_END) { + return false; + } + + std::vector output(input_length + 1024); + uint32_t input_len = static_cast(compressed.size()); + uint32_t output_len = static_cast(output.size()); + bool end_of_stream = false; + ret = UncompressIAA(reinterpret_cast(&compressed[0]), &input_len, + output.data(), &output_len, qpl_path_hardware, + /*window_bits=*/-15, &end_of_stream); + return ret == 0 && end_of_stream && output_len == input_length; +} + +// The contract UncompressIAA() now offers its callers, checked on QPL's +// software path so that it holds on a host with no device: the software path +// rejects an oversized window for the same reason and with the same status. +TEST_F(IAAWindowRejectionTest, UncompressIAAReportsOversizedWindow) { + SetCompressPath(ZLIB, /*zlib_fallback=*/true, false, false); + + const size_t input_length = 64 * 1024; + char* input = GenerateSeededCompressibleBlock(input_length, /*seed=*/0x7e11); + ASSERT_NE(input, nullptr); + + // Two encodings of the same bytes. GenerateSeededCompressibleBlock() repeats + // a string every 8192 bytes, so with zlib's full window the first is + // guaranteed to contain a match distance IAA cannot reach; restricted to 4 + // kB, the second cannot contain one. + std::string wide; + std::string narrow; + size_t output_upper_bound = 0; + ExecutionPath compress_path = UNDEFINED; + ASSERT_EQ(ZlibCompress(input, input_length, &wide, -15, Z_FINISH, + &output_upper_bound, &compress_path), + Z_STREAM_END); + ASSERT_EQ(ZlibCompress(input, input_length, &narrow, -12, Z_FINISH, + &output_upper_bound, &compress_path), + Z_STREAM_END); + + std::vector output(input_length + 1024); + uint32_t input_len = static_cast(wide.size()); + uint32_t output_len = static_cast(output.size()); + bool end_of_stream = false; + bool window_too_large = false; + EXPECT_NE(UncompressIAA(reinterpret_cast(&wide[0]), &input_len, + output.data(), &output_len, qpl_path_software, + /*window_bits=*/-15, &end_of_stream, + /*detect_gzip_ext=*/false, &window_too_large), + 0); + EXPECT_TRUE(window_too_large); + + // A stream IAA can follow decodes, and leaves the flag alone. Never setting + // it to false is what lets a caller pass one bool through a whole stream. + input_len = static_cast(narrow.size()); + output_len = static_cast(output.size()); + end_of_stream = false; + bool narrow_window_too_large = false; + EXPECT_EQ(UncompressIAA(reinterpret_cast(&narrow[0]), &input_len, + output.data(), &output_len, qpl_path_software, + /*window_bits=*/-12, &end_of_stream, + /*detect_gzip_ext=*/false, &narrow_window_too_large), + 0); + EXPECT_FALSE(narrow_window_too_large); + EXPECT_TRUE(end_of_stream); + EXPECT_EQ(output_len, input_length); + EXPECT_EQ(memcmp(output.data(), input, input_length), 0); + + // Omitting the out-parameter has to stay legal: most callers do not care + // which failure they got. + input_len = static_cast(wide.size()); + output_len = static_cast(output.size()); + end_of_stream = false; + EXPECT_NE(UncompressIAA(reinterpret_cast(&wide[0]), &input_len, + output.data(), &output_len, qpl_path_software, + /*window_bits=*/-15, &end_of_stream), + 0); + + DestroyBlock(input); +} + +// The point of the whole change. A rejection has to outlive inflateReset(), +// because a reset is exactly what the callers that matter do between documents: +// Lucene resets its Inflater once per stored field. Clearing the flag on reset +// would forget the lesson before it was ever acted on. +TEST_F(IAAWindowRejectionTest, StreamFlagSurvivesInflateReset) { + if (!IAAHardwareDecompressWorks()) { + GTEST_SKIP() << "no usable IAA device: QPL cannot reach the point where it " + "reports an oversized history window"; + } + SetCompressPath(ZLIB, /*zlib_fallback=*/true, false, false); + SetUncompressPath(IAA, /*zlib_fallback=*/true, false); + + const size_t input_length = 64 * 1024; + char* input = GenerateSeededCompressibleBlock(input_length, /*seed=*/0x7e12); + ASSERT_NE(input, nullptr); + + std::string compressed; + size_t output_upper_bound = 0; + ExecutionPath compress_path = UNDEFINED; + ASSERT_EQ(ZlibCompress(input, input_length, &compressed, -15, Z_FINISH, + &output_upper_bound, &compress_path), + Z_STREAM_END); + + z_stream stream; + memset(&stream, 0, sizeof(z_stream)); + ASSERT_EQ(inflateInit2(&stream, -15), Z_OK); + EXPECT_FALSE(InflateIAAWindowRejected(&stream)); + + // The rejection costs one submission and then falls through to zlib, so the + // bytes are still right. + EXPECT_EQ(InflateWholeStream(&stream, compressed, input, input_length), + Z_STREAM_END); + EXPECT_TRUE(InflateIAAWindowRejected(&stream)); + + ASSERT_EQ(inflateReset(&stream), Z_OK); + EXPECT_TRUE(InflateIAAWindowRejected(&stream)); + // inflateReset() clears the path, so a stream that had forgotten the + // rejection would be dispatched to IAA again here. + EXPECT_EQ(GetInflateExecutionPath(&stream), UNDEFINED); + EXPECT_EQ(InflateWholeStream(&stream, compressed, input, input_length), + Z_STREAM_END); + EXPECT_NE(GetInflateExecutionPath(&stream), IAA); + EXPECT_TRUE(InflateIAAWindowRejected(&stream)); + + // inflateReset2() restarts the stream in a new format, and is the other way + // back to an undefined path. + ASSERT_EQ(inflateReset2(&stream, -15), Z_OK); + EXPECT_TRUE(InflateIAAWindowRejected(&stream)); + + ASSERT_EQ(inflateEnd(&stream), Z_OK); + DestroyBlock(input); +} + +// A copy decodes the rest of the same stream, so it inherits the verdict. The +// settings are rebuilt member by member in SetFromCopy(), not assigned, so this +// is the kind of field that gets silently dropped. +TEST_F(IAAWindowRejectionTest, StreamFlagPropagatesThroughInflateCopy) { + if (!IAAHardwareDecompressWorks()) { + GTEST_SKIP() << "no usable IAA device: QPL cannot reach the point where it " + "reports an oversized history window"; + } + SetCompressPath(ZLIB, /*zlib_fallback=*/true, false, false); + SetUncompressPath(IAA, /*zlib_fallback=*/true, false); + + const size_t input_length = 64 * 1024; + char* input = GenerateSeededCompressibleBlock(input_length, /*seed=*/0x7e13); + ASSERT_NE(input, nullptr); + + std::string compressed; + size_t output_upper_bound = 0; + ExecutionPath compress_path = UNDEFINED; + ASSERT_EQ(ZlibCompress(input, input_length, &compressed, -15, Z_FINISH, + &output_upper_bound, &compress_path), + Z_STREAM_END); + + z_stream source; + memset(&source, 0, sizeof(z_stream)); + ASSERT_EQ(inflateInit2(&source, -15), Z_OK); + EXPECT_EQ(InflateWholeStream(&source, compressed, input, input_length), + Z_STREAM_END); + ASSERT_TRUE(InflateIAAWindowRejected(&source)); + + z_stream dest; + memset(&dest, 0, sizeof(z_stream)); + ASSERT_EQ(inflateCopy(&dest, &source), Z_OK); + EXPECT_TRUE(InflateIAAWindowRejected(&dest)); + // And the copy keeps it across its own reset, like the original. + ASSERT_EQ(inflateReset(&dest), Z_OK); + EXPECT_TRUE(InflateIAAWindowRejected(&dest)); + EXPECT_EQ(InflateWholeStream(&dest, compressed, input, input_length), + Z_STREAM_END); + EXPECT_NE(GetInflateExecutionPath(&dest), IAA); + + ASSERT_EQ(inflateEnd(&dest), Z_OK); + ASSERT_EQ(inflateEnd(&source), Z_OK); + DestroyBlock(input); +} + +// A stream IAA can serve must not be tarred by another stream's rejection: the +// flag is per stream, and there is no process-wide counter behind it. +TEST_F(IAAWindowRejectionTest, RejectionDoesNotAffectOtherStreams) { + if (!IAAHardwareDecompressWorks()) { + GTEST_SKIP() << "no usable IAA device: QPL cannot reach the point where it " + "reports an oversized history window"; + } + SetCompressPath(ZLIB, /*zlib_fallback=*/true, false, false); + SetUncompressPath(IAA, /*zlib_fallback=*/true, false); + + const size_t input_length = 64 * 1024; + char* input = GenerateSeededCompressibleBlock(input_length, /*seed=*/0x7e14); + ASSERT_NE(input, nullptr); + + std::string wide; + std::string narrow; + size_t output_upper_bound = 0; + ExecutionPath compress_path = UNDEFINED; + ASSERT_EQ(ZlibCompress(input, input_length, &wide, -15, Z_FINISH, + &output_upper_bound, &compress_path), + Z_STREAM_END); + ASSERT_EQ(ZlibCompress(input, input_length, &narrow, -12, Z_FINISH, + &output_upper_bound, &compress_path), + Z_STREAM_END); + + z_stream rejected; + memset(&rejected, 0, sizeof(z_stream)); + ASSERT_EQ(inflateInit2(&rejected, -15), Z_OK); + EXPECT_EQ(InflateWholeStream(&rejected, wide, input, input_length), + Z_STREAM_END); + ASSERT_TRUE(InflateIAAWindowRejected(&rejected)); + + z_stream served; + memset(&served, 0, sizeof(z_stream)); + ASSERT_EQ(inflateInit2(&served, -15), Z_OK); + EXPECT_EQ(InflateWholeStream(&served, narrow, input, input_length), + Z_STREAM_END); + EXPECT_FALSE(InflateIAAWindowRejected(&served)); + EXPECT_EQ(GetInflateExecutionPath(&served), IAA); + + ASSERT_EQ(inflateEnd(&served), Z_OK); + ASSERT_EQ(inflateEnd(&rejected), Z_OK); + DestroyBlock(input); +} +#endif // USE_IAA diff --git a/zlib_accel.cpp b/zlib_accel.cpp index d4a9879..a3c3e64 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -428,6 +428,14 @@ struct InflateSettings { // clears the state; ISA-L does not, and keeps parsing whatever follows the // rejected bytes as a new block header, so the latch has to live here. bool data_error = false; + // Set once IAA has rejected a block of this stream for referencing a match + // beyond its 4 kB history buffer. Deliberately NOT cleared by inflateReset: + // the window is a property of the compressor that produced the bytes, and a + // reset starts a new stream from the same producer in every caller that + // matters here (Lucene resets its Inflater once per stored-field document). + // Clearing it would make the flag useless, since almost every rejection + // arrives on a stream that is about to be reset. + bool iaa_window_too_large = false; }; // isal_strm is a raw pointer, so destroying a settings object does not free the @@ -545,6 +553,12 @@ class InflateStreamSettings { settings->stream_end_reached = source.stream_end_reached; settings->bytes_consumed = source.bytes_consumed; settings->data_error = source.data_error; + // A copy decodes the rest of the same stream, so it inherits what IAA + // already said about that stream's history window. This has to be copied + // out by hand like every other field: the settings are rebuilt member by + // member here, not assigned, so a new member is silently dropped + // otherwise. + settings->iaa_window_too_large = source.iaa_window_too_large; map.Set(dest, std::move(settings)); } catch (...) { Log(LogLevel::LOG_ERROR, @@ -625,6 +639,14 @@ static void ResetDeflateStreamState( // Same for the inflate side. A reset stream is ready to decode again; leaving // the terminal state set would wedge every later inflate() at Z_STREAM_END. +// +// iaa_window_too_large deliberately does NOT belong here. It records what IAA +// said about the compressor that produced these bytes, and a reset stream is +// almost always the same caller decoding more output from the same producer -- +// Lucene resets its Inflater once per stored-field document. Clearing it here +// would make the flag useless: it would be forgotten before it was ever +// consulted, and the shim would go back to submitting jobs it knows will be +// rejected. static void ResetInflateStreamState( const std::shared_ptr& settings) { if (settings == nullptr) { @@ -1740,7 +1762,12 @@ int ZEXPORT inflate(z_streamp strm, int flush) { #endif #ifdef USE_IAA + // IsIAADecompressible cannot see match distances, so for raw deflate and + // gzip it has no header to read and is guessing. iaa_window_too_large is + // what a wrong guess, once made, costs being remembered: IAA has already + // told us this stream's producer used a window it cannot follow. iaa_available = configs[USE_IAA_UNCOMPRESS] && + !inflate_settings->iaa_window_too_large && SupportedOptionsIAA(inflate_settings->window_bits, input_len, output_len) && IsIAADecompressible(strm->next_in, input_len, @@ -1778,9 +1805,10 @@ int ZEXPORT inflate(z_streamp strm, int flush) { if (path_selected == IAA) { #ifdef USE_IAA in_call = true; - ret = UncompressIAA(strm->next_in, &input_len, strm->next_out, - &output_len, qpl_path_hardware, - inflate_settings->window_bits, &end_of_stream); + ret = UncompressIAA( + strm->next_in, &input_len, strm->next_out, &output_len, + qpl_path_hardware, inflate_settings->window_bits, &end_of_stream, + /*detect_gzip_ext=*/false, &inflate_settings->iaa_window_too_large); SetInflatePath(inflate_settings, IAA); // IAA inflate is stateless in this wrapper. If stream end was not // reached, use zlib for stateful continuation. @@ -2429,6 +2457,11 @@ bool InflateOwnsIgzipState(z_streamp strm) { return inflate_settings != nullptr && inflate_settings->isal_strm != nullptr; } +bool InflateIAAWindowRejected(z_streamp strm) { + auto inflate_settings = inflate_stream_settings.Get(strm); + return inflate_settings != nullptr && inflate_settings->iaa_window_too_large; +} + enum class FileMode { NONE, READ, WRITE, APPEND }; // Beside the enum rather than beside its first caller. Every gz entry point @@ -3197,6 +3230,13 @@ static int GzreadAcceleratorUncompress(GzipFile* gz, uint8_t* input, bool igzip_available = false; #ifdef USE_IAA + // No remembered window rejection here, unlike inflate(): gzread pins a file + // to zlib on its first accelerator failure of any kind (see + // use_zlib_for_decompression at the call site), so nothing more is submitted + // for the rest of that read pass. gzrewind clears the pin deliberately, to + // offer the accelerator another try at a file whose stream a mid-file + // fallback had taken away. One wasted submission per pass is all this path + // can spend, and within a pass there is no second one to suppress. iaa_available = configs[USE_IAA_UNCOMPRESS] && SupportedOptionsIAA(kWindowBitsGzip, *input_length, *output_length) && diff --git a/zlib_accel.h b/zlib_accel.h index ed21080..e219779 100644 --- a/zlib_accel.h +++ b/zlib_accel.h @@ -23,4 +23,10 @@ ExecutionPath GetGzipFileExecutionPath(gzFile file); bool DeflateOwnsIgzipState(z_streamp strm); bool InflateOwnsIgzipState(z_streamp strm); +// True once IAA has rejected a block of this stream for referencing a match +// beyond its 4 kB history buffer. Tests need it because the record deliberately +// survives inflateReset(), and nothing else about the stream reveals that it is +// being kept. Always false in a build without IAA support. +bool InflateIAAWindowRejected(z_streamp strm); + #pragma GCC visibility pop From e7c80bec83a2dacecf3722d52399573c36fc59d7 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Thu, 10 Sep 2026 16:30:10 -0700 Subject: [PATCH 2/8] iaa: let a declared window retire a remembered rejection The iaa_window_too_large flag records an inference: QPL answered QPL_STS_BAD_DIST_ERR, so whatever compressor produced this stream used a window IAA's 4 kB history buffer cannot follow. inflateReset2() is the caller declaring what the next stream's window is, which is a stronger statement than the inference, and it is safe to believe -- bytes that reached further back than the declared window would be refused by zlib too. A stream rejected once therefore stayed off IAA for good, even after its owner had certified the next stream as 4 kB-safe. Clear the flag in inflateReset2() when the new windowBits declares a window IAA can follow. DeclaresIAACompatibleWindow() answers that question through GetCompressedFormat(), so each format's wrapper offset comes off first: gzip's +16 and raw deflate's negation. Zlib's automatic-detection range declares nothing, because there the stream picks the wrapper. inflateReset() is unchanged and still keeps the verdict, which is the case that matters for callers that reset once per document. The 4 kB window is now IAA_MAX_HISTORY_WINDOW_BITS, retiring the bare 12 in IsIAADecompressible(). Measured on this host's IAA device: before, inflateReset2(-12) on a rejected stream decoded on zlib with the fallback on and returned Z_DATA_ERROR with it off; after, the stream is served by IAA in both configurations. Four mutations of the new rule are each killed by at least one test -- never clearing (2), clearing on every reset2 (2), dropping gzip's offset (1), dropping raw's negation (3). Signed-off-by: Olasoji --- iaa.cpp | 17 ++++++++- iaa.h | 11 ++++++ tests/inflate_test.cpp | 85 ++++++++++++++++++++++++++++++++++++++++++ zlib_accel.cpp | 17 ++++++++- 4 files changed, 128 insertions(+), 2 deletions(-) diff --git a/iaa.cpp b/iaa.cpp index 50c10ab..f45c3f6 100644 --- a/iaa.cpp +++ b/iaa.cpp @@ -285,7 +285,7 @@ bool IsIAADecompressible(uint8_t* input, uint32_t input_length, CompressedFormat format = GetCompressedFormat(window_bits); if (format == CompressedFormat::ZLIB) { int window = GetWindowSizeFromZlibHeader(input, input_length); - return window <= 12; + return window <= IAA_MAX_HISTORY_WINDOW_BITS; } // For raw deflate and gzip formats, QPL always reports total_in == // available_in regardless of where BFINAL=1 falls in the stream. This is @@ -307,4 +307,19 @@ bool IsIAADecompressible(uint8_t* input, uint32_t input_length, return input_length > kZipInputStreamBufferSize; } +bool DeclaresIAACompatibleWindow(int window_bits) { + switch (GetCompressedFormat(window_bits)) { + case CompressedFormat::DEFLATE_RAW: + return -window_bits <= IAA_MAX_HISTORY_WINDOW_BITS; + case CompressedFormat::ZLIB: + return window_bits <= IAA_MAX_HISTORY_WINDOW_BITS; + case CompressedFormat::GZIP: + // 16 is the offset zlib adds to select the gzip wrapper; what is left is + // the window. + return window_bits - 16 <= IAA_MAX_HISTORY_WINDOW_BITS; + default: + return false; + } +} + #endif // USE_IAA diff --git a/iaa.h b/iaa.h index 77d4e73..ff3ac5b 100644 --- a/iaa.h +++ b/iaa.h @@ -16,6 +16,10 @@ inline constexpr unsigned int PREPENDED_BLOCK_LENGTH = 5; inline constexpr unsigned int MAX_BUFFER_SIZE = (2 << 20); +// IAA's decompressor has a fixed 4 kB history buffer, so it can only follow a +// stream whose match distances stay inside a 2^12-byte window. +inline constexpr int IAA_MAX_HISTORY_WINDOW_BITS = 12; + class IAAJob { public: IAAJob() : jobs_(3) {} @@ -76,4 +80,11 @@ VISIBLE_FOR_TESTING bool IsIAADecompressible(uint8_t* input, uint32_t input_length, int window_bits); +// True if window_bits declares a maximum window IAA's history buffer can +// follow, whatever the format's own header says. inflateReset2() takes such a +// declaration from the caller, and it is a stronger statement than a remembered +// rejection: a stream that referenced further back than this would be refused +// by zlib too. +VISIBLE_FOR_TESTING bool DeclaresIAACompatibleWindow(int window_bits); + #endif // USE_IAA diff --git a/tests/inflate_test.cpp b/tests/inflate_test.cpp index c588823..62066d3 100644 --- a/tests/inflate_test.cpp +++ b/tests/inflate_test.cpp @@ -2331,4 +2331,89 @@ TEST_F(IAAWindowRejectionTest, RejectionDoesNotAffectOtherStreams) { ASSERT_EQ(inflateEnd(&rejected), Z_OK); DestroyBlock(input); } + +// The window a caller declares, independent of any hardware: the format's +// wrapper offset has to come off before the window is compared, or a gzip +// stream looks like a 24-bit window and a raw one like a negative window. +TEST_F(IAAWindowRejectionTest, DeclaresIAACompatibleWindowFollowsTheFormat) { + // Raw deflate. + EXPECT_TRUE(DeclaresIAACompatibleWindow(-8)); + EXPECT_TRUE(DeclaresIAACompatibleWindow(-12)); + EXPECT_FALSE(DeclaresIAACompatibleWindow(-13)); + EXPECT_FALSE(DeclaresIAACompatibleWindow(-15)); + // Zlib. + EXPECT_TRUE(DeclaresIAACompatibleWindow(8)); + EXPECT_TRUE(DeclaresIAACompatibleWindow(12)); + EXPECT_FALSE(DeclaresIAACompatibleWindow(13)); + EXPECT_FALSE(DeclaresIAACompatibleWindow(15)); + // Gzip, which zlib selects by adding 16. + EXPECT_TRUE(DeclaresIAACompatibleWindow(16 + 8)); + EXPECT_TRUE(DeclaresIAACompatibleWindow(16 + 12)); + EXPECT_FALSE(DeclaresIAACompatibleWindow(16 + 13)); + EXPECT_FALSE(DeclaresIAACompatibleWindow(16 + 15)); + // Anything this shim does not map to a format cannot be declared compatible, + // including zlib's automatic-detection range, where the stream picks the + // wrapper and the caller has therefore declared nothing. + EXPECT_FALSE(DeclaresIAACompatibleWindow(0)); + EXPECT_FALSE(DeclaresIAACompatibleWindow(32 + 15)); + EXPECT_FALSE(DeclaresIAACompatibleWindow(-16)); +} + +// A narrowing inflateReset2() is the caller declaring the next stream's window, +// so it retires the verdict: the flag is an inference about the previous +// stream's producer, and a declaration outranks an inference. Without this a +// stream that was rejected once never reaches IAA again even after the caller +// has said the data cannot reference beyond 4 kB. +TEST_F(IAAWindowRejectionTest, NarrowingInflateReset2ClearsTheVerdict) { + if (!IAAHardwareDecompressWorks()) { + GTEST_SKIP() << "no usable IAA device: QPL cannot reach the point where it " + "reports an oversized history window"; + } + SetCompressPath(ZLIB, /*zlib_fallback=*/true, false, false); + SetUncompressPath(IAA, /*zlib_fallback=*/true, false); + + const size_t input_length = 64 * 1024; + char* input = GenerateSeededCompressibleBlock(input_length, /*seed=*/0x7e15); + ASSERT_NE(input, nullptr); + + std::string wide; + std::string narrow; + size_t output_upper_bound = 0; + ExecutionPath compress_path = UNDEFINED; + ASSERT_EQ(ZlibCompress(input, input_length, &wide, -15, Z_FINISH, + &output_upper_bound, &compress_path), + Z_STREAM_END); + ASSERT_EQ(ZlibCompress(input, input_length, &narrow, -12, Z_FINISH, + &output_upper_bound, &compress_path), + Z_STREAM_END); + + z_stream stream; + memset(&stream, 0, sizeof(z_stream)); + ASSERT_EQ(inflateInit2(&stream, -15), Z_OK); + EXPECT_EQ(InflateWholeStream(&stream, wide, input, input_length), + Z_STREAM_END); + ASSERT_TRUE(InflateIAAWindowRejected(&stream)); + + // Same window, so nothing has been declared and the verdict stands. This is + // the control: without it the test would pass on a build that cleared the + // flag on every reset. + ASSERT_EQ(inflateReset2(&stream, -15), Z_OK); + EXPECT_TRUE(InflateIAAWindowRejected(&stream)); + EXPECT_EQ(InflateWholeStream(&stream, narrow, input, input_length), + Z_STREAM_END); + EXPECT_NE(GetInflateExecutionPath(&stream), IAA); + + // Narrowing to a window IAA can follow retires it, and the next stream is + // offered to IAA again -- and served, since the bytes really do stay inside + // 4 kB. + ASSERT_EQ(inflateReset2(&stream, -12), Z_OK); + EXPECT_FALSE(InflateIAAWindowRejected(&stream)); + EXPECT_EQ(InflateWholeStream(&stream, narrow, input, input_length), + Z_STREAM_END); + EXPECT_EQ(GetInflateExecutionPath(&stream), IAA); + + ASSERT_EQ(inflateEnd(&stream), Z_OK); + DestroyBlock(input); +} + #endif // USE_IAA diff --git a/zlib_accel.cpp b/zlib_accel.cpp index a3c3e64..6351878 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -646,7 +646,9 @@ static void ResetDeflateStreamState( // Lucene resets its Inflater once per stored-field document. Clearing it here // would make the flag useless: it would be forgotten before it was ever // consulted, and the shim would go back to submitting jobs it knows will be -// rejected. +// rejected. inflateReset2() is the one exception, and clears the field itself: +// a caller that declares an IAA-sized window has said what the next stream is, +// which beats an inference drawn from the last one. static void ResetInflateStreamState( const std::shared_ptr& settings) { if (settings == nullptr) { @@ -2116,6 +2118,19 @@ int ZEXPORT inflateReset2(z_streamp strm, int windowBits) { ResetInflateStreamState(inflate_settings); inflate_settings->window_bits = windowBits; +#ifdef USE_IAA + // The one thing that overrides a remembered rejection. That verdict is an + // inference about the compressor that produced the previous stream, and + // inflateReset2() is the caller stating outright what the next stream's + // window is; a declaration IAA can follow wins over the inference, since + // bytes that reached further back would be refused by zlib as well. An + // inflateReset() carries no such statement, which is why the verdict + // survives it. + if (DeclaresIAACompatibleWindow(windowBits)) { + inflate_settings->iaa_window_too_large = false; + } +#endif + if (inflate_settings->isal_strm != nullptr) { #ifdef USE_IGZIP // isal_inflate_reset() deliberately preserves crc_flag and hist_bits, and From ad6c0c8dc9ce204f5d83b01c1365dda3e864c42b Mon Sep 17 00:00:00 2001 From: Olasoji Date: Thu, 10 Sep 2026 16:31:12 -0700 Subject: [PATCH 3/8] iaa: keep a remembered rejection from deciding a stream's fate iaa_window_too_large was a term of iaa_available, so a stream that had been rejected once was never offered to IAA again -- including when IAA was the only engine the configuration allowed. With use_zlib_uncompress=0 and no IGZIP retry, the shim then answered Z_DATA_ERROR for a stream IAA would have decoded. The refusal shape itself is not new: that configuration already refuses anything no accelerator will take, and an input below IAA's 512-byte floor reaches it with no verdict involved. But a remembered verdict put a stream permanently in that category, and an optimization must not change the answer. Apply the flag as a suppression step after every backend's availability is known, and only while some other engine can take the stream. The three terms are that complete set: the config the zlib fall-through tests, QAT, and the IGZIP retry. With none of them present the stream goes to IAA anyway -- a job that probably fails beats refusing data that may decode. Also give the fixture the SetUp/TearDown config snapshot the other regression fixtures have, since the new case turns off igzip_fallback and pins the IAA/QAT traffic split on top of the configs the path helpers write. Five mutations of the new rule are each killed by at least one test: dropping the suppression (2), dropping the whole engine disjunction (1), and dropping any one of the zlib, QAT or IGZIP terms (1 each). Signed-off-by: Olasoji --- tests/inflate_test.cpp | 188 ++++++++++++++++++++++++++++++++++++++++- zlib_accel.cpp | 23 +++-- 2 files changed, 205 insertions(+), 6 deletions(-) diff --git a/tests/inflate_test.cpp b/tests/inflate_test.cpp index 62066d3..69a0dfe 100644 --- a/tests/inflate_test.cpp +++ b/tests/inflate_test.cpp @@ -2060,7 +2060,55 @@ TEST_F(DictionaryMidstreamFallbackRegressionTest, // block. IsIAADecompressible() cannot see it for raw deflate or gzip, where // there is no header to read the window out of, so the only way to know is to // be told once and remember. -class IAAWindowRejectionTest : public ::testing::Test {}; +class IAAWindowRejectionTest : public ::testing::Test { + protected: + void SetUp() override { + saved_use_zlib_compress_ = GetConfig(USE_ZLIB_COMPRESS); + saved_use_iaa_compress_ = GetConfig(USE_IAA_COMPRESS); + saved_use_qat_compress_ = GetConfig(USE_QAT_COMPRESS); + saved_use_igzip_compress_ = GetConfig(USE_IGZIP_COMPRESS); + saved_use_zlib_uncompress_ = GetConfig(USE_ZLIB_UNCOMPRESS); + saved_use_iaa_uncompress_ = GetConfig(USE_IAA_UNCOMPRESS); + saved_use_qat_uncompress_ = GetConfig(USE_QAT_UNCOMPRESS); + saved_use_igzip_uncompress_ = GetConfig(USE_IGZIP_UNCOMPRESS); + // SetCompressPath/SetUncompressPath write these two unconditionally, and + // the no-fallback case below turns the third off, so all three have to come + // back or this fixture makes the suite order-dependent. + saved_iaa_prepend_empty_block_ = GetConfig(IAA_PREPEND_EMPTY_BLOCK); + saved_qat_allow_chunking_ = GetConfig(QAT_COMPRESSION_ALLOW_CHUNKING); + saved_igzip_fallback_ = GetConfig(IGZIP_FALLBACK); + saved_iaa_uncompress_percentage_ = GetConfig(IAA_UNCOMPRESS_PERCENTAGE); + } + + void TearDown() override { + SetConfig(USE_ZLIB_COMPRESS, saved_use_zlib_compress_); + SetConfig(USE_IAA_COMPRESS, saved_use_iaa_compress_); + SetConfig(USE_QAT_COMPRESS, saved_use_qat_compress_); + SetConfig(USE_IGZIP_COMPRESS, saved_use_igzip_compress_); + SetConfig(USE_ZLIB_UNCOMPRESS, saved_use_zlib_uncompress_); + SetConfig(USE_IAA_UNCOMPRESS, saved_use_iaa_uncompress_); + SetConfig(USE_QAT_UNCOMPRESS, saved_use_qat_uncompress_); + SetConfig(USE_IGZIP_UNCOMPRESS, saved_use_igzip_uncompress_); + SetConfig(IAA_PREPEND_EMPTY_BLOCK, saved_iaa_prepend_empty_block_); + SetConfig(QAT_COMPRESSION_ALLOW_CHUNKING, saved_qat_allow_chunking_); + SetConfig(IGZIP_FALLBACK, saved_igzip_fallback_); + SetConfig(IAA_UNCOMPRESS_PERCENTAGE, saved_iaa_uncompress_percentage_); + } + + private: + uint32_t saved_use_zlib_compress_ = 0; + uint32_t saved_use_iaa_compress_ = 0; + uint32_t saved_use_qat_compress_ = 0; + uint32_t saved_use_igzip_compress_ = 0; + uint32_t saved_use_zlib_uncompress_ = 0; + uint32_t saved_use_iaa_uncompress_ = 0; + uint32_t saved_use_qat_uncompress_ = 0; + uint32_t saved_use_igzip_uncompress_ = 0; + uint32_t saved_iaa_prepend_empty_block_ = 0; + uint32_t saved_qat_allow_chunking_ = 0; + uint32_t saved_igzip_fallback_ = 0; + uint32_t saved_iaa_uncompress_percentage_ = 0; +}; // Run one whole stream through strm and check the bytes. Returns the last // inflate() code so the caller can assert on it, or Z_DATA_ERROR if the output @@ -2416,4 +2464,142 @@ TEST_F(IAAWindowRejectionTest, NarrowingInflateReset2ClearsTheVerdict) { DestroyBlock(input); } +// The remembered verdict is an optimization, so it must not change what a +// caller who has turned every fallback off gets back. That configuration +// answers Z_DATA_ERROR whenever no engine will take the data -- an input below +// IAA's 512-byte floor does it with no verdict involved -- but the verdict must +// not be what puts a stream in that category: with nothing else to fall back +// to, the stream is offered to IAA anyway, because a job that probably fails +// beats refusing data that may decode. +TEST_F(IAAWindowRejectionTest, RememberedRejectionWithNoFallbackDoesNotRefuse) { + if (!IAAHardwareDecompressWorks()) { + GTEST_SKIP() << "no usable IAA device: QPL cannot reach the point where it " + "reports an oversized history window"; + } + SetCompressPath(ZLIB, /*zlib_fallback=*/true, false, false); + + const size_t input_length = 64 * 1024; + char* input = GenerateSeededCompressibleBlock(input_length, /*seed=*/0x7e16); + ASSERT_NE(input, nullptr); + const size_t tiny_length = 256; + char* tiny = GenerateSeededCompressibleBlock(tiny_length, /*seed=*/0x7e17); + ASSERT_NE(tiny, nullptr); + + std::string wide; + std::string narrow; + std::string tiny_compressed; + size_t output_upper_bound = 0; + ExecutionPath compress_path = UNDEFINED; + ASSERT_EQ(ZlibCompress(input, input_length, &wide, -15, Z_FINISH, + &output_upper_bound, &compress_path), + Z_STREAM_END); + ASSERT_EQ(ZlibCompress(input, input_length, &narrow, -12, Z_FINISH, + &output_upper_bound, &compress_path), + Z_STREAM_END); + ASSERT_EQ(ZlibCompress(tiny, tiny_length, &tiny_compressed, -15, Z_FINISH, + &output_upper_bound, &compress_path), + Z_STREAM_END); + + // IAA and nothing else: no zlib, no igzip retry. + SetUncompressPath(IAA, /*zlib_fallback=*/false, false); + SetConfig(IGZIP_FALLBACK, 0); + + // The floor case, which no part of this change touches: a stream too short + // for IsIAADecompressible() reaches no engine and is refused. This is the + // configuration's own contract, and it stays exactly as it was. + z_stream small; + memset(&small, 0, sizeof(z_stream)); + ASSERT_EQ(inflateInit2(&small, -15), Z_OK); + EXPECT_EQ(InflateWholeStream(&small, tiny_compressed, tiny, tiny_length), + Z_DATA_ERROR); + EXPECT_FALSE(InflateIAAWindowRejected(&small)); + ASSERT_EQ(inflateEnd(&small), Z_OK); + + // A stream IAA can follow is still served, so the configuration is not + // simply broken. + z_stream served; + memset(&served, 0, sizeof(z_stream)); + ASSERT_EQ(inflateInit2(&served, -15), Z_OK); + EXPECT_EQ(InflateWholeStream(&served, narrow, input, input_length), + Z_STREAM_END); + EXPECT_EQ(GetInflateExecutionPath(&served), IAA); + ASSERT_EQ(inflateEnd(&served), Z_OK); + + // The rejection itself is refused with no fallback to hand it to, which is + // what this configuration means and is already true without the verdict. + z_stream stream; + memset(&stream, 0, sizeof(z_stream)); + ASSERT_EQ(inflateInit2(&stream, -15), Z_OK); + EXPECT_EQ(InflateWholeStream(&stream, wide, input, input_length), + Z_DATA_ERROR); + ASSERT_TRUE(InflateIAAWindowRejected(&stream)); + + // The next stream on the same z_stream is a payload IAA can follow, and the + // verdict does not get to refuse it: with no other engine available the + // suppression stands down and IAA is submitted, so the caller gets the same + // answer as `served` above. The verdict is still recorded -- it just is not + // deciding anything here. + ASSERT_EQ(inflateReset(&stream), Z_OK); + EXPECT_EQ(InflateWholeStream(&stream, narrow, input, input_length), + Z_STREAM_END); + EXPECT_EQ(GetInflateExecutionPath(&stream), IAA); + EXPECT_TRUE(InflateIAAWindowRejected(&stream)); + + // Narrowing the window retires the verdict outright, with no fallback too. + ASSERT_EQ(inflateReset2(&stream, -12), Z_OK); + EXPECT_FALSE(InflateIAAWindowRejected(&stream)); + EXPECT_EQ(InflateWholeStream(&stream, narrow, input, input_length), + Z_STREAM_END); + EXPECT_EQ(GetInflateExecutionPath(&stream), IAA); + + ASSERT_EQ(inflateEnd(&stream), Z_OK); + +#ifdef USE_IGZIP + // With zlib off but IGZIP on, the verdict does its job again: another engine + // can take the stream, so IAA is suppressed and IGZIP serves it. Without the + // suppression the doomed IAA submission would go first and, with no fallback + // behind it, refuse data IGZIP would have decoded. + SetConfig(USE_IGZIP_UNCOMPRESS, 1); + + z_stream to_igzip; + memset(&to_igzip, 0, sizeof(z_stream)); + ASSERT_EQ(inflateInit2(&to_igzip, -15), Z_OK); + EXPECT_EQ(InflateWholeStream(&to_igzip, wide, input, input_length), + Z_DATA_ERROR); + ASSERT_TRUE(InflateIAAWindowRejected(&to_igzip)); + + ASSERT_EQ(inflateReset(&to_igzip), Z_OK); + EXPECT_EQ(InflateWholeStream(&to_igzip, narrow, input, input_length), + Z_STREAM_END); + EXPECT_EQ(GetInflateExecutionPath(&to_igzip), IGZIP); + ASSERT_EQ(inflateEnd(&to_igzip), Z_OK); + SetConfig(USE_IGZIP_UNCOMPRESS, 0); +#endif + +#ifdef USE_QAT + // Same with QAT as the only other engine. The traffic split is pinned at 100% + // IAA so the setup stream is the one that gets rejected rather than a coin + // toss; once the verdict stands the split no longer applies, because a + // suppressed IAA leaves QAT as the only candidate. + SetConfig(USE_QAT_UNCOMPRESS, 1); + SetConfig(IAA_UNCOMPRESS_PERCENTAGE, 100); + + z_stream to_qat; + memset(&to_qat, 0, sizeof(z_stream)); + ASSERT_EQ(inflateInit2(&to_qat, -15), Z_OK); + EXPECT_EQ(InflateWholeStream(&to_qat, wide, input, input_length), + Z_DATA_ERROR); + ASSERT_TRUE(InflateIAAWindowRejected(&to_qat)); + + ASSERT_EQ(inflateReset(&to_qat), Z_OK); + EXPECT_EQ(InflateWholeStream(&to_qat, narrow, input, input_length), + Z_STREAM_END); + EXPECT_EQ(GetInflateExecutionPath(&to_qat), QAT); + ASSERT_EQ(inflateEnd(&to_qat), Z_OK); +#endif + + DestroyBlock(tiny); + DestroyBlock(input); +} + #endif // USE_IAA diff --git a/zlib_accel.cpp b/zlib_accel.cpp index 6351878..8a7e71a 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -1764,12 +1764,7 @@ int ZEXPORT inflate(z_streamp strm, int flush) { #endif #ifdef USE_IAA - // IsIAADecompressible cannot see match distances, so for raw deflate and - // gzip it has no header to read and is guessing. iaa_window_too_large is - // what a wrong guess, once made, costs being remembered: IAA has already - // told us this stream's producer used a window it cannot follow. iaa_available = configs[USE_IAA_UNCOMPRESS] && - !inflate_settings->iaa_window_too_large && SupportedOptionsIAA(inflate_settings->window_bits, input_len, output_len) && IsIAADecompressible(strm->next_in, input_len, @@ -1785,6 +1780,24 @@ int ZEXPORT inflate(z_streamp strm, int flush) { igzip_available = igzip_supported_options; #endif +#ifdef USE_IAA + // IsIAADecompressible cannot see match distances, so for raw deflate and + // gzip it has no header to read and is guessing. iaa_window_too_large is + // what a wrong guess, once made, costs being remembered: IAA has already + // told us this stream's producer used a window it cannot follow. + // + // That memory is an optimization, and an optimization must not change the + // answer, so it only suppresses IAA while some other engine can take the + // stream. The three terms are that complete set: the config the zlib + // fall-through below tests, QAT, and the IGZIP retry. With none of them a + // suppressed stream would be refused outright, so submit it and let IAA + // decide: a job that probably fails beats refusing data that may decode. + if (iaa_available && inflate_settings->iaa_window_too_large && + (configs[USE_ZLIB_UNCOMPRESS] || qat_available || igzip_available)) { + iaa_available = false; + } +#endif + // If both accelerators are enabled, send configured ratio of requests to // one or the other ExecutionPath path_selected = ZLIB; From 9fc01c95a51fd49c3283f4e9c7f0120c3e306710 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Wed, 23 Sep 2026 15:30:31 -0700 Subject: [PATCH 4/8] tests: skip the QAT block of a no-fallback rejection test without a device RememberedRejectionWithNoFallbackDoesNotRefuse guards its whole body on IAAHardwareDecompressWorks(), but its #ifdef USE_QAT block asserts QAT serves a stream once the remembered rejection suppresses IAA, with USE_ZLIB_UNCOMPRESS and IGZIP_FALLBACK both off. Eligibility (SupportedOptionsQAT) never looks at whether a device is present, so on a host built with USE_IAA and USE_QAT where IAA works and QAT does not, inflate()'s fall-through has nothing to catch the failed UncompressQAT call, and the assertion fails. Add the same kind of probe the IAA guard above already uses, and skip the QAT block on it. UncompressQAT needed VISIBLE_FOR_TESTING to call directly from a test binary, which UncompressIAA already carries -- without it, the call compiles but does not link across the DSO boundary the way the rest of the comment in zlib_accel.h's export block describes for the intercepted zlib symbols, and reaches unrelated code instead of failing to resolve. The probe payload has to be a size QATzip's decompressor does not refuse on its own: a short one made the guard report "no device" on every host, including one with a working accelerator, which would have made the fix silently disable the block it exists to protect. Signed-off-by: Olasoji --- qat.h | 7 +++-- tests/inflate_test.cpp | 67 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 67 insertions(+), 7 deletions(-) diff --git a/qat.h b/qat.h index f81285e..b92ee61 100644 --- a/qat.h +++ b/qat.h @@ -44,9 +44,10 @@ int CompressQAT(uint8_t* input, uint32_t* input_length, uint8_t* output, uint32_t* output_length, int window_bits, bool gzip_ext = false); -int UncompressQAT(uint8_t* input, uint32_t* input_length, uint8_t* output, - uint32_t* output_length, int window_bits, bool* end_of_stream, - bool detect_gzip_ext = false); +VISIBLE_FOR_TESTING int UncompressQAT(uint8_t* input, uint32_t* input_length, + uint8_t* output, uint32_t* output_length, + int window_bits, bool* end_of_stream, + bool detect_gzip_ext = false); VISIBLE_FOR_TESTING bool SupportedOptionsQAT(int window_bits, uint32_t input_length); diff --git a/tests/inflate_test.cpp b/tests/inflate_test.cpp index 69a0dfe..684570c 100644 --- a/tests/inflate_test.cpp +++ b/tests/inflate_test.cpp @@ -22,6 +22,10 @@ using namespace config; #include "../igzip.h" #endif +#ifdef USE_QAT +#include "../qat.h" +#endif + #ifdef USE_IGZIP TEST(IGZIPInflateRegressionTest, EmptyInputContinuationKeepsIGZIPPath) { @@ -2171,6 +2175,48 @@ static bool IAAHardwareDecompressWorks() { return ret == 0 && end_of_stream && output_len == input_length; } +#ifdef USE_QAT +// Same probe, for the USE_QAT block in RememberedRejectionWithNoFallbackDoes +// NotRefuse below: SupportedOptionsQAT() is a software eligibility check with +// no view of whether a device is present, so on a host where QAT is compiled +// in but no accelerator works, that block would still select QAT and then +// have no fallback to catch the failure. +// +// The payload has to be past the size where QATzip's decompressor refuses a +// short zlib-produced stream on its own, independently of whether the device +// works -- the same refusal the QAT refusal-set notes elsewhere in this repo +// document -- or the probe reports "no device" on every host, including this +// one. A short probe is what IAAHardwareDecompressWorks() above uses, but it +// does not carry over: this is a different accelerator's decompressor with a +// different floor, not a smaller version of the same check. +static bool QATHardwareUncompressWorks() { + const size_t input_length = 64 * 1024; + char* input = GenerateSeededCompressibleBlock(input_length, /*seed=*/0x9a7); + if (input == nullptr) { + return false; + } + SetCompressPath(ZLIB, /*zlib_fallback=*/true, false, false); + std::string compressed; + size_t output_upper_bound = 0; + ExecutionPath compress_path = UNDEFINED; + int ret = ZlibCompress(input, input_length, &compressed, -15, Z_FINISH, + &output_upper_bound, &compress_path); + DestroyBlock(input); + if (ret != Z_STREAM_END) { + return false; + } + + std::vector output(input_length + 1024); + uint32_t input_len = static_cast(compressed.size()); + uint32_t output_len = static_cast(output.size()); + bool end_of_stream = false; + ret = UncompressQAT(reinterpret_cast(&compressed[0]), &input_len, + output.data(), &output_len, /*window_bits=*/-15, + &end_of_stream); + return ret == 0 && end_of_stream && output_len == input_length; +} +#endif + // The contract UncompressIAA() now offers its callers, checked on QPL's // software path so that it holds on a host with no device: the software path // rejects an oversized window for the same reason and with the same status. @@ -2577,10 +2623,23 @@ TEST_F(IAAWindowRejectionTest, RememberedRejectionWithNoFallbackDoesNotRefuse) { #endif #ifdef USE_QAT - // Same with QAT as the only other engine. The traffic split is pinned at 100% - // IAA so the setup stream is the one that gets rejected rather than a coin - // toss; once the verdict stands the split no longer applies, because a - // suppressed IAA leaves QAT as the only candidate. + // Same with QAT as the only other engine, skipped on a host where QAT is + // compiled in but not usable: with no fallback configured, a failed + // UncompressQAT call here reaches Z_DATA_ERROR rather than the QAT this + // asserts, and that failure says nothing about the verdict under test. + if (!QATHardwareUncompressWorks()) { + // GTEST_SKIP() returns out of the test body, so the two blocks this + // function otherwise frees at the end have to be freed here instead. + DestroyBlock(tiny); + DestroyBlock(input); + GTEST_SKIP() << "no usable QAT device: nothing left to catch a failed " + "UncompressQAT call in a no-fallback configuration"; + } + + // The traffic split is pinned at 100% IAA so the setup stream is the one + // that gets rejected rather than a coin toss; once the verdict stands the + // split no longer applies, because a suppressed IAA leaves QAT as the only + // candidate. SetConfig(USE_QAT_UNCOMPRESS, 1); SetConfig(IAA_UNCOMPRESS_PERCENTAGE, 100); From beb8395ce5ec56ce83803a5b19e9d502d1f0e3b1 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Wed, 23 Sep 2026 16:08:00 -0700 Subject: [PATCH 5/8] iaa: stop counting QAT eligibility as a safe reason to suppress IAA Copilot's second review pass on the remembered-rejection suppression found the gap the first round's "only suppress while some other engine can take the stream" fix left open: qat_available is SupportedOptionsQAT(), a buffer/window eligibility check with no view of whether a QAT device exists, unlike the other two terms in that condition. configs[USE_ZLIB_UNCOMPRESS] and igzip_available are both software paths, where eligible really does mean it works. Counting qat_available the same way let a host with USE_ZLIB_UNCOMPRESS=0, IGZIP_FALLBACK off (or IGZIP not compiled in), and a QAT that is eligible but not functional turn a stream IAA could still decode into Z_DATA_ERROR -- exactly the outcome the suppression's own stated invariant says an optimization must never cause. Drop qat_available from the condition: with QAT the only other engine enabled, the verdict now stands down and IAA gets the same chance at the stream it would with no other engine at all. RememberedRejectionWithNoFallbackDoesNotRefuse's QAT block asserted the now-superseded behavior (QAT serving the narrow stream after suppression) and is updated to assert the corrected one (IAA serving it, since the verdict no longer suppresses on QAT's account). That drops the previous commit's QATHardwareUncompressWorks() probe along with the VISIBLE_FOR_TESTING it needed on UncompressQAT: the block no longer depends on a real QAT call succeeding, only on IAA getting the chance the fix restores, so neither is exercised by anything in this file anymore. Second finding from the same review pass: the test that checks UncompressIAA() leaves its window_too_large out-parameter alone on a successful decode initialized the sentinel to false, which cannot tell "left untouched" apart from "cleared to false" -- both read as false either way. Add the case that can: a sentinel pre-set true, checked to still be true after a successful decode. Signed-off-by: Olasoji --- qat.h | 7 ++-- tests/inflate_test.cpp | 90 ++++++++++++------------------------------ zlib_accel.cpp | 17 +++++--- 3 files changed, 41 insertions(+), 73 deletions(-) diff --git a/qat.h b/qat.h index b92ee61..f81285e 100644 --- a/qat.h +++ b/qat.h @@ -44,10 +44,9 @@ int CompressQAT(uint8_t* input, uint32_t* input_length, uint8_t* output, uint32_t* output_length, int window_bits, bool gzip_ext = false); -VISIBLE_FOR_TESTING int UncompressQAT(uint8_t* input, uint32_t* input_length, - uint8_t* output, uint32_t* output_length, - int window_bits, bool* end_of_stream, - bool detect_gzip_ext = false); +int UncompressQAT(uint8_t* input, uint32_t* input_length, uint8_t* output, + uint32_t* output_length, int window_bits, bool* end_of_stream, + bool detect_gzip_ext = false); VISIBLE_FOR_TESTING bool SupportedOptionsQAT(int window_bits, uint32_t input_length); diff --git a/tests/inflate_test.cpp b/tests/inflate_test.cpp index 684570c..e1f767e 100644 --- a/tests/inflate_test.cpp +++ b/tests/inflate_test.cpp @@ -22,10 +22,6 @@ using namespace config; #include "../igzip.h" #endif -#ifdef USE_QAT -#include "../qat.h" -#endif - #ifdef USE_IGZIP TEST(IGZIPInflateRegressionTest, EmptyInputContinuationKeepsIGZIPPath) { @@ -2175,48 +2171,6 @@ static bool IAAHardwareDecompressWorks() { return ret == 0 && end_of_stream && output_len == input_length; } -#ifdef USE_QAT -// Same probe, for the USE_QAT block in RememberedRejectionWithNoFallbackDoes -// NotRefuse below: SupportedOptionsQAT() is a software eligibility check with -// no view of whether a device is present, so on a host where QAT is compiled -// in but no accelerator works, that block would still select QAT and then -// have no fallback to catch the failure. -// -// The payload has to be past the size where QATzip's decompressor refuses a -// short zlib-produced stream on its own, independently of whether the device -// works -- the same refusal the QAT refusal-set notes elsewhere in this repo -// document -- or the probe reports "no device" on every host, including this -// one. A short probe is what IAAHardwareDecompressWorks() above uses, but it -// does not carry over: this is a different accelerator's decompressor with a -// different floor, not a smaller version of the same check. -static bool QATHardwareUncompressWorks() { - const size_t input_length = 64 * 1024; - char* input = GenerateSeededCompressibleBlock(input_length, /*seed=*/0x9a7); - if (input == nullptr) { - return false; - } - SetCompressPath(ZLIB, /*zlib_fallback=*/true, false, false); - std::string compressed; - size_t output_upper_bound = 0; - ExecutionPath compress_path = UNDEFINED; - int ret = ZlibCompress(input, input_length, &compressed, -15, Z_FINISH, - &output_upper_bound, &compress_path); - DestroyBlock(input); - if (ret != Z_STREAM_END) { - return false; - } - - std::vector output(input_length + 1024); - uint32_t input_len = static_cast(compressed.size()); - uint32_t output_len = static_cast(output.size()); - bool end_of_stream = false; - ret = UncompressQAT(reinterpret_cast(&compressed[0]), &input_len, - output.data(), &output_len, /*window_bits=*/-15, - &end_of_stream); - return ret == 0 && end_of_stream && output_len == input_length; -} -#endif - // The contract UncompressIAA() now offers its callers, checked on QPL's // software path so that it holds on a host with no device: the software path // rejects an oversized window for the same reason and with the same status. @@ -2270,6 +2224,21 @@ TEST_F(IAAWindowRejectionTest, UncompressIAAReportsOversizedWindow) { EXPECT_EQ(output_len, input_length); EXPECT_EQ(memcmp(output.data(), input, input_length), 0); + // The case the block above cannot tell apart from a no-op: starting from an + // already-true flag, so that an implementation which clears the + // out-parameter on every successful call -- rather than leaving an untouched + // one alone -- would be caught rather than passing vacuously. + input_len = static_cast(narrow.size()); + output_len = static_cast(output.size()); + end_of_stream = false; + bool already_true = true; + EXPECT_EQ(UncompressIAA(reinterpret_cast(&narrow[0]), &input_len, + output.data(), &output_len, qpl_path_software, + /*window_bits=*/-12, &end_of_stream, + /*detect_gzip_ext=*/false, &already_true), + 0); + EXPECT_TRUE(already_true); + // Omitting the out-parameter has to stay legal: most callers do not care // which failure they got. input_len = static_cast(wide.size()); @@ -2623,23 +2592,16 @@ TEST_F(IAAWindowRejectionTest, RememberedRejectionWithNoFallbackDoesNotRefuse) { #endif #ifdef USE_QAT - // Same with QAT as the only other engine, skipped on a host where QAT is - // compiled in but not usable: with no fallback configured, a failed - // UncompressQAT call here reaches Z_DATA_ERROR rather than the QAT this - // asserts, and that failure says nothing about the verdict under test. - if (!QATHardwareUncompressWorks()) { - // GTEST_SKIP() returns out of the test body, so the two blocks this - // function otherwise frees at the end have to be freed here instead. - DestroyBlock(tiny); - DestroyBlock(input); - GTEST_SKIP() << "no usable QAT device: nothing left to catch a failed " - "UncompressQAT call in a no-fallback configuration"; - } - - // The traffic split is pinned at 100% IAA so the setup stream is the one - // that gets rejected rather than a coin toss; once the verdict stands the - // split no longer applies, because a suppressed IAA leaves QAT as the only - // candidate. + // Unlike IGZIP above, enabling QAT here does not stand the suppression + // down: QAT's eligibility is a buffer/window check with no view of whether + // a device is present, so treating it as a safe alternative could turn a + // stream IAA can decode into a refusal on a host where QAT is compiled in + // but not functional -- the reason inflate() does not count qat_available + // among the terms that let it suppress IAA. With QAT the only other engine + // enabled, the verdict therefore stays out of the decision and IAA gets + // the same chance at the narrow stream as it would with no other engine at + // all, which is exactly what the traffic split pinned at 100% IAA is here + // to make unambiguous. SetConfig(USE_QAT_UNCOMPRESS, 1); SetConfig(IAA_UNCOMPRESS_PERCENTAGE, 100); @@ -2653,7 +2615,7 @@ TEST_F(IAAWindowRejectionTest, RememberedRejectionWithNoFallbackDoesNotRefuse) { ASSERT_EQ(inflateReset(&to_qat), Z_OK); EXPECT_EQ(InflateWholeStream(&to_qat, narrow, input, input_length), Z_STREAM_END); - EXPECT_EQ(GetInflateExecutionPath(&to_qat), QAT); + EXPECT_EQ(GetInflateExecutionPath(&to_qat), IAA); ASSERT_EQ(inflateEnd(&to_qat), Z_OK); #endif diff --git a/zlib_accel.cpp b/zlib_accel.cpp index 8a7e71a..0568ddc 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -1788,12 +1788,19 @@ int ZEXPORT inflate(z_streamp strm, int flush) { // // That memory is an optimization, and an optimization must not change the // answer, so it only suppresses IAA while some other engine can take the - // stream. The three terms are that complete set: the config the zlib - // fall-through below tests, QAT, and the IGZIP retry. With none of them a - // suppressed stream would be refused outright, so submit it and let IAA - // decide: a job that probably fails beats refusing data that may decode. + // stream -- and "can take" has to mean will actually run it, not merely + // pass the software eligibility check the zlib fall-through below and the + // IGZIP retry both are: zlib and IGZIP are software, so eligible there + // means it works. QAT's eligibility (qat_available) is a buffer/window + // check with no view of whether a device exists, so it is deliberately + // left out of this condition; counting it would let a host with an + // eligible-but-nonfunctional QAT and no other fallback turn a stream IAA + // could have decoded into Z_DATA_ERROR, exactly what this optimization + // must not do. With neither term true a suppressed stream would be + // refused outright, so submit it and let IAA decide: a job that probably + // fails beats refusing data that may decode. if (iaa_available && inflate_settings->iaa_window_too_large && - (configs[USE_ZLIB_UNCOMPRESS] || qat_available || igzip_available)) { + (configs[USE_ZLIB_UNCOMPRESS] || igzip_available)) { iaa_available = false; } #endif From 37587d254b0a1a18a9586bdfe7a8fae5ee108ea8 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Wed, 23 Sep 2026 18:40:42 -0700 Subject: [PATCH 6/8] tests: report a stuck decoder in InflateWholeStream by name Third finding from Copilot's latest pass. On guard exhaustion the loop fell through with ret still Z_OK or Z_BUF_ERROR, and the caller's EXPECT_EQ against the expected code reported a return-code mismatch -- correct, but indistinguishable from every other way this helper fails. Name the guard count and add an explicit failure when it is reached without inflate() reporting completion or an error, so a stuck decoder reads as one. The other two findings from the same pass are not changed. &compressed[0] is not UB for an empty string since C++11 -- reading (not writing) the null terminator at pos == size() is defined for both overloads -- and the case is unreachable here regardless, since the function returns before this line unless deflate already reported Z_STREAM_END. And InflateIAAWindowRejected's visibility follows the same push(default)-block convention its two immediate neighbors, DeflateOwnsIgzipState and InflateOwnsIgzipState, already use; narrowing just the new declaration would leave the header's exported surface unchanged while making it inconsistent with what it sits next to. Signed-off-by: Olasoji --- tests/inflate_test.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/inflate_test.cpp b/tests/inflate_test.cpp index e1f767e..bd0293b 100644 --- a/tests/inflate_test.cpp +++ b/tests/inflate_test.cpp @@ -2115,6 +2115,11 @@ class IAAWindowRejectionTest : public ::testing::Test { // came back wrong. static int InflateWholeStream(z_streamp strm, const std::string& compressed, const char* expected, size_t expected_length) { + // A whole-stream decode finishes or errors out in one or two calls; this is + // headroom, not an expected count. Exhausting it below is reported rather + // than left to surface as a plain return-code mismatch, since "stuck" and + // "wrong answer" want different diagnostics. + constexpr int kMaxInflateCalls = 128; std::vector output(expected_length + 1024); strm->next_in = reinterpret_cast(const_cast(compressed.data())); @@ -2122,12 +2127,17 @@ static int InflateWholeStream(z_streamp strm, const std::string& compressed, strm->next_out = output.data(); strm->avail_out = static_cast(output.size()); int ret = Z_OK; - for (int guard = 0; guard < 128; guard++) { + int guard = 0; + for (; guard < kMaxInflateCalls; guard++) { ret = inflate(strm, Z_NO_FLUSH); if (ret != Z_OK && ret != Z_BUF_ERROR) { break; } } + if (guard == kMaxInflateCalls && (ret == Z_OK || ret == Z_BUF_ERROR)) { + ADD_FAILURE() << "inflate() neither finished nor errored after " + << kMaxInflateCalls << " calls; decoder appears stuck"; + } if (ret != Z_STREAM_END) { return ret; } From 044461713f40ceed330d79ecbd52c00ee52b16b3 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Wed, 23 Sep 2026 19:28:54 -0700 Subject: [PATCH 7/8] iaa: give InflateIAAWindowRejected its own visibility attribute Second finding from Copilot's latest pass, and I was wrong to dismiss it: zlib_accel.h is not a test-only header. Its push(default)/pop wraps the #include specifically to give the whole zlib API default visibility before zlib_accel.cpp's ZEXPORT definitions pick it up -- that is what makes deflate()/inflate()/gzopen()/etc. interposable at all. A declaration placed in that block is exported the same way, into the same public .so symbol table as the real zlib API, with nothing of its own marking it as different. InflateIAAWindowRejected rode along in that block instead of getting a visibility of its own. Move it out, after the pop, marked with a new VISIBLE_FOR_TESTING macro -- the same convention iaa.h/qat.h/igzip.h already use for exactly this case. The two pre-existing accessors that still share the pragma block with the real API (DeflateOwnsIgzipState, InflateOwnsIgzipState) are unchanged; they predate this PR and are a pre-existing instance of the same gap, not this branch's to fix. Signed-off-by: Olasoji --- zlib_accel.h | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/zlib_accel.h b/zlib_accel.h index e219779..3df92ee 100644 --- a/zlib_accel.h +++ b/zlib_accel.h @@ -2,6 +2,17 @@ // SPDX-License-Identifier: Apache-2.0 #pragma once + +// The block below pushes default visibility across the #include, which is +// what gives every ZEXPORT definition in zlib_accel.cpp the visibility +// deflate()/inflate()/gzopen()/etc. need to be interposed at all -- it is not +// a place to add a test accessor. A declaration added there is exported the +// same way the real zlib API is, indistinguishable from it in the built +// .so's symbol table and with no attribute of its own to say otherwise. Use +// this macro after the pop instead, the same convention iaa.h/qat.h/igzip.h +// use for symbols a test binary needs from an otherwise hidden library. +#define VISIBLE_FOR_TESTING __attribute__((visibility("default"))) + #pragma GCC visibility push(default) #include @@ -23,10 +34,10 @@ ExecutionPath GetGzipFileExecutionPath(gzFile file); bool DeflateOwnsIgzipState(z_streamp strm); bool InflateOwnsIgzipState(z_streamp strm); +#pragma GCC visibility pop + // True once IAA has rejected a block of this stream for referencing a match // beyond its 4 kB history buffer. Tests need it because the record deliberately // survives inflateReset(), and nothing else about the stream reveals that it is // being kept. Always false in a build without IAA support. -bool InflateIAAWindowRejected(z_streamp strm); - -#pragma GCC visibility pop +VISIBLE_FOR_TESTING bool InflateIAAWindowRejected(z_streamp strm); From 5f5ce4b61ae558406cec5f9d3ff90a8ae3d3dd00 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Thu, 24 Sep 2026 01:07:17 -0700 Subject: [PATCH 8/8] iaa: igzip_available alone does not mean IGZIP serves the stream Third finding from Copilot's review of this suppression, and it applies the same lesson as the previous commit's qat_available fix to the term that fix left in place. igzip_available being true does not guarantee IGZIP runs: the selection ladder checks qat_available ahead of igzip_available, so whenever QAT is also eligible, QAT gets the call instead of IGZIP -- regardless of which term justified suppressing IAA. The accelerator->IGZIP retry only covers a failed QAT call when IGZIP_FALLBACK is set. With zlib and IGZIP_FALLBACK both off, IGZIP enabled, and a QAT that is eligible but not functional, a reset stream IAA could have decoded was silently handed to QAT instead and lost. Only count igzip_available as safe when it will actually be reached: either QAT is not also eligible, so the ladder falls through to IGZIP directly, or IGZIP_FALLBACK is set, so a QAT failure retries into IGZIP before anything is lost. Added the mixed-backend case Copilot asked for to RememberedRejectionWithNoFallbackDoesNotRefuse: both IGZIP and QAT enabled, IGZIP_FALLBACK off, matching the exact configuration the bug needed. IAA stays eligible there now, same as the QAT-alone case above it, and the traffic split already pinned at 100% IAA is what serves the narrow stream. Signed-off-by: Olasoji --- tests/inflate_test.cpp | 27 +++++++++++++++++++++++++++ zlib_accel.cpp | 26 +++++++++++++++----------- 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/tests/inflate_test.cpp b/tests/inflate_test.cpp index bd0293b..831044c 100644 --- a/tests/inflate_test.cpp +++ b/tests/inflate_test.cpp @@ -2629,6 +2629,33 @@ TEST_F(IAAWindowRejectionTest, RememberedRejectionWithNoFallbackDoesNotRefuse) { ASSERT_EQ(inflateEnd(&to_qat), Z_OK); #endif +#if defined(USE_IGZIP) && defined(USE_QAT) + // The mixed configuration the suppression's igzip_available term still got + // wrong even after the qat_available fix above: with QAT also eligible, + // the selection ladder picks QAT ahead of IGZIP regardless of which one + // justified suppressing IAA, and the accelerator->IGZIP retry only covers + // that QAT choice when IGZIP_FALLBACK is set -- left off here, as in every + // other block in this test. So igzip_available alone must not stand the + // suppression down while qat_available is also true; the traffic split, + // still pinned at 100% IAA from the block above, is what actually serves + // the narrow stream. + SetConfig(USE_IGZIP_UNCOMPRESS, 1); + + z_stream to_mixed; + memset(&to_mixed, 0, sizeof(z_stream)); + ASSERT_EQ(inflateInit2(&to_mixed, -15), Z_OK); + EXPECT_EQ(InflateWholeStream(&to_mixed, wide, input, input_length), + Z_DATA_ERROR); + ASSERT_TRUE(InflateIAAWindowRejected(&to_mixed)); + + ASSERT_EQ(inflateReset(&to_mixed), Z_OK); + EXPECT_EQ(InflateWholeStream(&to_mixed, narrow, input, input_length), + Z_STREAM_END); + EXPECT_EQ(GetInflateExecutionPath(&to_mixed), IAA); + ASSERT_EQ(inflateEnd(&to_mixed), Z_OK); + SetConfig(USE_IGZIP_UNCOMPRESS, 0); +#endif + DestroyBlock(tiny); DestroyBlock(input); } diff --git a/zlib_accel.cpp b/zlib_accel.cpp index 0568ddc..e299365 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -1789,18 +1789,22 @@ int ZEXPORT inflate(z_streamp strm, int flush) { // That memory is an optimization, and an optimization must not change the // answer, so it only suppresses IAA while some other engine can take the // stream -- and "can take" has to mean will actually run it, not merely - // pass the software eligibility check the zlib fall-through below and the - // IGZIP retry both are: zlib and IGZIP are software, so eligible there - // means it works. QAT's eligibility (qat_available) is a buffer/window - // check with no view of whether a device exists, so it is deliberately - // left out of this condition; counting it would let a host with an - // eligible-but-nonfunctional QAT and no other fallback turn a stream IAA - // could have decoded into Z_DATA_ERROR, exactly what this optimization - // must not do. With neither term true a suppressed stream would be - // refused outright, so submit it and let IAA decide: a job that probably - // fails beats refusing data that may decode. + // pass an eligibility check. zlib is always that safe: the fall-through + // below is software with no device to be missing. IGZIP is software too, + // but eligible is not enough on its own: the selection ladder below picks + // QAT ahead of IGZIP whenever qat_available is also true, and QAT's + // eligibility (qat_available) is a buffer/window check with no view of + // whether a device exists -- the reason it is left out of this condition + // entirely. So a stream this suppression hands to "IGZIP" can really be + // handed to a QAT that then fails, and the accelerator retry below only + // reaches IGZIP when IGZIP_FALLBACK is also set; igzip_available alone + // promises nothing about which engine the ladder actually picks. With no + // term true a suppressed stream would be refused outright, so submit it + // and let IAA decide: a job that probably fails beats refusing data that + // may decode. if (iaa_available && inflate_settings->iaa_window_too_large && - (configs[USE_ZLIB_UNCOMPRESS] || igzip_available)) { + (configs[USE_ZLIB_UNCOMPRESS] || + (igzip_available && (!qat_available || configs[IGZIP_FALLBACK])))) { iaa_available = false; } #endif