Fix remaining_length accounting after Huffman decode in RegressionPredictor / ComposedPredictor - #137
Open
alexey-milovidov wants to merge 1 commit into
Conversation
…mposed predictors
RegressionPredictor::load decremented remaining_length by the uncompressed
index count (coeff_size * sizeof(int)) after Huffman decode(), but decode()
advances the read pointer only by the compressed stream size. The uncompressed
count overshoots the compressed stream, understating remaining_length for the
subsequent quantizer encoder.load(), whose bound check then spuriously rejects
valid data with "SZ3 Huffman: tree exceeds compressed buffer".
RegressionPredictor is only used by ALGO_LORENZO_REG, so a column compressed
with CODEC(SZ3('ALGO_LORENZO_REG', ...)) could be written on insert but fail
every subsequent read with CORRUPTED_DATA (effective data loss). Small element
counts are affected; large counts happen to leave enough slack to survive the
understated bound.
Account for exactly the bytes decode() consumed via pointer difference.
ComposedPredictor::load had the mirror defect (it never decremented
remaining_length for its selection stream); tightened the same way.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ayzk
added a commit
that referenced
this pull request
Sep 1, 2026
Bug fixes only: no compressed-format change and no interface signature change. Covers the reviewed content of #131, #133, #134, #135, #137, #138 and #139, plus the findings ported from the fz branch. #132 is only partly covered -- see the PR body for the four exclusions and the measurement behind each. 31 dataset x algorithm x error-bound combinations are byte-identical to master, and every master-produced file still decompresses to the same bytes.
ayzk
added a commit
that referenced
this pull request
Sep 15, 2026
Consolidates the eight open bug-fix pull requests (#131 #132 #133 #134 #135 #137 #138 #139) with the findings from reviewing them, as one change. The compressed stream is untrusted, and several modules read lengths and counts out of it and then used them to index, allocate or loop without checking them against anything. The format is unchanged. A stream written by this build is byte-identical to one written by master, and either build reads the other's output; verified on interp/lorenzo_reg/nopred in 1D, 2D and 3D. Encoder::decode now takes the remaining byte count ------------------------------------------------ Its signature carried the symbol count and nothing else, so an encoder could not tell how many bytes it was allowed to touch. HuffmanEncoder read its payload length out of the stream and walked that far; BypassEncoder memcpy'd sizeof(T) * targetLength; RunlengthEncoder read a value and a count per run. None of them had anything to compare against. decode() now takes `size_t &remaining_length` alongside targetLength and charges what it consumed, the same shape load() already had. The two numbers are independent -- an entropy coder's bitstream has no terminator, so the symbol count is what says stop, while the byte count is what says how far it may read -- and both are available at every call site. The symbol count stays where it is in the stream, so no bytes moved. Other bounds ------------ - HuffmanEncoderV2's tree loading, and XtcBasedEncoder, against corrupted input - ComposedPredictor's predictor selection index, and its value - RegressionPredictor's coefficient stream, which each block consumes N + 1 entries of - InterpolationDecomposition's stored dimensions, which drive a grid walk over buffers that conf sizes - the declared bin count, against the element count conf carries - the bins InterpolationDecomposition and TimeSeriesDecomposition walk, checked once before the walk rather than on each access - Config::load reading one byte past the config, and validating contents only where the config is a compressed-stream trailer - the HDF5 filter's compressed buffer, sized from SZ_compress_size_bound Also ---- - Huffman's shift for single-symbol input, and the non-finite float cast in LinearQuantizer, were undefined behaviour - scratch buffers are held as unique_ptr so an exception from the encoder or the lossless layer does not leak them - PR #132's bound on the internal decompression buffer is dropped: that buffer is sized from the bin count and type, which no bound derivable from conf alone covers Not taken from the PRs as written: #132's internal-buffer bound (above), and #135's XtcBasedEncoder bounds are reinstated in a form that does not reject valid streams. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ayzk
added a commit
that referenced
this pull request
Sep 15, 2026
Consolidates the eight open bug-fix pull requests (#131 #132 #133 #134 #135 #137 #138 #139) with the findings from reviewing them, as one change. The compressed stream is untrusted, and several modules read lengths and counts out of it and then used them to index, allocate or loop without checking them against anything. The format is unchanged. A stream written by this build is byte-identical to one written by master, and either build reads the other's output; verified on interp/lorenzo_reg/nopred in 1D, 2D and 3D. Encoder::decode now takes the remaining byte count ------------------------------------------------ Its signature carried the symbol count and nothing else, so an encoder could not tell how many bytes it was allowed to touch. HuffmanEncoder read its payload length out of the stream and walked that far; BypassEncoder memcpy'd sizeof(T) * targetLength; RunlengthEncoder read a value and a count per run. None of them had anything to compare against. decode() now takes `size_t &remaining_length` alongside targetLength and charges what it consumed, the same shape load() already had. The two numbers are independent -- an entropy coder's bitstream has no terminator, so the symbol count is what says stop, while the byte count is what says how far it may read -- and both are available at every call site. The symbol count stays where it is in the stream, so no bytes moved. Other bounds ------------ - HuffmanEncoderV2's tree loading, and XtcBasedEncoder, against corrupted input - ComposedPredictor's predictor selection index, and its value - RegressionPredictor's coefficient stream, which each block consumes N + 1 entries of - InterpolationDecomposition's stored dimensions, which drive a grid walk over buffers that conf sizes - the declared bin count, against the element count conf carries - the bins InterpolationDecomposition and TimeSeriesDecomposition walk, checked once before the walk rather than on each access - Config::load reading one byte past the config, and validating contents only where the config is a compressed-stream trailer - the HDF5 filter's compressed buffer, sized from SZ_compress_size_bound Also ---- - Huffman's shift for single-symbol input, and the non-finite float cast in LinearQuantizer, were undefined behaviour - scratch buffers are held as unique_ptr so an exception from the encoder or the lossless layer does not leak them - PR #132's bound on the internal decompression buffer is dropped: that buffer is sized from the bin count and type, which no bound derivable from conf alone covers Not taken from the PRs as written: #132's internal-buffer bound (above), and #135's XtcBasedEncoder bounds are reinstated in a form that does not reject valid streams. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ayzk
added a commit
that referenced
this pull request
Sep 15, 2026
Consolidates the eight open bug-fix pull requests (#131 #132 #133 #134 #135 #137 #138 #139) with the findings from reviewing them, as one change. The compressed stream is untrusted, and several modules read lengths and counts out of it and then used them to index, allocate or loop without checking them against anything. The format is unchanged. A stream written by this build is byte-identical to one written by master, and either build reads the other's output; verified on interp/lorenzo_reg/nopred in 1D, 2D and 3D. Encoder::decode now takes the remaining byte count ------------------------------------------------ Its signature carried the symbol count and nothing else, so an encoder could not tell how many bytes it was allowed to touch. HuffmanEncoder read its payload length out of the stream and walked that far; BypassEncoder memcpy'd sizeof(T) * targetLength; RunlengthEncoder read a value and a count per run. None of them had anything to compare against. decode() now takes `size_t &remaining_length` alongside targetLength and charges what it consumed, the same shape load() already had. The two numbers are independent -- an entropy coder's bitstream has no terminator, so the symbol count is what says stop, while the byte count is what says how far it may read -- and both are available at every call site. The symbol count stays where it is in the stream, so no bytes moved. Other bounds ------------ - HuffmanEncoderV2's tree loading, and XtcBasedEncoder, against corrupted input - ComposedPredictor's predictor selection index, and its value - RegressionPredictor's coefficient stream, which each block consumes N + 1 entries of - InterpolationDecomposition's stored dimensions, which drive a grid walk over buffers that conf sizes - the declared bin count, against the element count conf carries - the bins InterpolationDecomposition and TimeSeriesDecomposition walk, checked once before the walk rather than on each access - Config::load reading one byte past the config, and validating contents only where the config is a compressed-stream trailer - the HDF5 filter's compressed buffer, sized from SZ_compress_size_bound Also ---- - Huffman's shift for single-symbol input, and the non-finite float cast in LinearQuantizer, were undefined behaviour - scratch buffers are held as unique_ptr so an exception from the encoder or the lossless layer does not leak them - PR #132's bound on the internal decompression buffer is dropped: that buffer is sized from the bin count and type, which no bound derivable from conf alone covers Not taken from the PRs as written: #132's internal-buffer bound (above), and #135's XtcBasedEncoder bounds are reinstated in a form that does not reject valid streams. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ayzk
added a commit
that referenced
this pull request
Sep 15, 2026
Consolidates the eight open bug-fix pull requests (#131 #132 #133 #134 #135 #137 #138 #139) with the findings from reviewing them, as one change. The compressed stream is untrusted, and several modules read lengths and counts out of it and then used them to index, allocate or loop without checking them against anything. The format is unchanged. A stream written by this build is byte-identical to one written by master, and either build reads the other's output; verified on interp/lorenzo_reg/nopred in 1D, 2D and 3D. Encoder::decode now takes the remaining byte count ------------------------------------------------ Its signature carried the symbol count and nothing else, so an encoder could not tell how many bytes it was allowed to touch. HuffmanEncoder read its payload length out of the stream and walked that far; BypassEncoder memcpy'd sizeof(T) * targetLength; RunlengthEncoder read a value and a count per run. None of them had anything to compare against. decode() now takes `size_t &remaining_length` alongside targetLength and charges what it consumed, the same shape load() already had. The two numbers are independent -- an entropy coder's bitstream has no terminator, so the symbol count is what says stop, while the byte count is what says how far it may read -- and both are available at every call site. The symbol count stays where it is in the stream, so no bytes moved. Other bounds ------------ - HuffmanEncoderV2's tree loading, and XtcBasedEncoder, against corrupted input - ComposedPredictor's predictor selection index, and its value - RegressionPredictor's coefficient stream, which each block consumes N + 1 entries of - InterpolationDecomposition's stored dimensions, which drive a grid walk over buffers that conf sizes - the declared bin count, against the element count conf carries - the bins InterpolationDecomposition and TimeSeriesDecomposition walk, checked once before the walk rather than on each access - Config::load reading one byte past the config, and validating contents only where the config is a compressed-stream trailer - the HDF5 filter's compressed buffer, sized from SZ_compress_size_bound Also ---- - Huffman's shift for single-symbol input, and the non-finite float cast in LinearQuantizer, were undefined behaviour - scratch buffers are held as unique_ptr so an exception from the encoder or the lossless layer does not leak them - PR #132's bound on the internal decompression buffer is dropped: that buffer is sized from the bin count and type, which no bound derivable from conf alone covers Not taken from the PRs as written: #132's internal-buffer bound (above), and #135's XtcBasedEncoder bounds are reinstated in a form that does not reject valid streams. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ayzk
added a commit
that referenced
this pull request
Sep 15, 2026
Consolidates the eight open bug-fix pull requests (#131 #132 #133 #134 #135 #137 #138 #139) with the findings from reviewing them, as one change. The compressed stream is untrusted, and several modules read lengths and counts out of it and then used them to index, allocate or loop without checking them against anything. The format is unchanged. A stream written by this build is byte-identical to one written by master, and either build reads the other's output; verified on interp/lorenzo_reg/nopred in 1D, 2D and 3D. Encoder::decode now takes the remaining byte count ------------------------------------------------ Its signature carried the symbol count and nothing else, so an encoder could not tell how many bytes it was allowed to touch. HuffmanEncoder read its payload length out of the stream and walked that far; BypassEncoder memcpy'd sizeof(T) * targetLength; RunlengthEncoder read a value and a count per run. None of them had anything to compare against. decode() now takes `size_t &remaining_length` alongside targetLength and charges what it consumed, the same shape load() already had. The two numbers are independent -- an entropy coder's bitstream has no terminator, so the symbol count is what says stop, while the byte count is what says how far it may read -- and both are available at every call site. The symbol count stays where it is in the stream, so no bytes moved. Other bounds ------------ - HuffmanEncoderV2's tree loading, and XtcBasedEncoder, against corrupted input - ComposedPredictor's predictor selection index, and its value - RegressionPredictor's coefficient stream, which each block consumes N + 1 entries of - InterpolationDecomposition's stored dimensions, which drive a grid walk over buffers that conf sizes - the declared bin count, against the element count conf carries - the bins InterpolationDecomposition and TimeSeriesDecomposition walk, checked once before the walk rather than on each access - Config::load reading one byte past the config, and validating contents only where the config is a compressed-stream trailer - the HDF5 filter's compressed buffer, sized from SZ_compress_size_bound Also ---- - Huffman's shift for single-symbol input, and the non-finite float cast in LinearQuantizer, were undefined behaviour - scratch buffers are held as unique_ptr so an exception from the encoder or the lossless layer does not leak them - PR #132's bound on the internal decompression buffer is dropped: that buffer is sized from the bin count and type, which no bound derivable from conf alone covers Not taken from the PRs as written: #132's internal-buffer bound (above), and #135's XtcBasedEncoder bounds are reinstated in a form that does not reject valid streams. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ayzk
added a commit
that referenced
this pull request
Sep 15, 2026
Consolidates the eight open bug-fix pull requests (#131 #132 #133 #134 #135 #137 #138 #139) with the findings from reviewing them, as one change. The compressed stream is untrusted, and several modules read lengths and counts out of it and then used them to index, allocate or loop without checking them against anything. The format is unchanged. A stream written by this build is byte-identical to one written by master, and either build reads the other's output; verified on interp/lorenzo_reg/nopred in 1D, 2D and 3D. Encoder::decode now takes the remaining byte count ------------------------------------------------ Its signature carried the symbol count and nothing else, so an encoder could not tell how many bytes it was allowed to touch. HuffmanEncoder read its payload length out of the stream and walked that far; BypassEncoder memcpy'd sizeof(T) * targetLength; RunlengthEncoder read a value and a count per run. None of them had anything to compare against. decode() now takes `size_t &remaining_length` alongside targetLength and charges what it consumed, the same shape load() already had. The two numbers are independent -- an entropy coder's bitstream has no terminator, so the symbol count is what says stop, while the byte count is what says how far it may read -- and both are available at every call site. The symbol count stays where it is in the stream, so no bytes moved. Other bounds ------------ - HuffmanEncoderV2's tree loading, and XtcBasedEncoder, against corrupted input - ComposedPredictor's predictor selection index, and its value - RegressionPredictor's coefficient stream, which each block consumes N + 1 entries of - InterpolationDecomposition's stored dimensions, which drive a grid walk over buffers that conf sizes - the declared bin count, against the element count conf carries - the bins InterpolationDecomposition and TimeSeriesDecomposition walk, checked once before the walk rather than on each access - Config::load reading one byte past the config, and validating contents only where the config is a compressed-stream trailer - the HDF5 filter's compressed buffer, sized from SZ_compress_size_bound Also ---- - Huffman's shift for single-symbol input, and the non-finite float cast in LinearQuantizer, were undefined behaviour - scratch buffers are held as unique_ptr so an exception from the encoder or the lossless layer does not leak them - PR #132's bound on the internal decompression buffer is dropped: that buffer is sized from the bin count and type, which no bound derivable from conf alone covers Not taken from the PRs as written: #132's internal-buffer bound (above), and #135's XtcBasedEncoder bounds are reinstated in a form that does not reject valid streams. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ayzk
added a commit
that referenced
this pull request
Sep 15, 2026
Consolidates the eight open bug-fix pull requests (#131 #132 #133 #134 #135 #137 #138 #139) with the findings from reviewing them, as one change. The compressed stream is untrusted, and several modules read lengths and counts out of it and then used them to index, allocate or loop without checking them against anything. The format is unchanged. A stream written by this build is byte-identical to one written by master, and either build reads the other's output; verified on interp/lorenzo_reg/nopred in 1D, 2D and 3D. Encoder::decode now takes the remaining byte count ------------------------------------------------ Its signature carried the symbol count and nothing else, so an encoder could not tell how many bytes it was allowed to touch. HuffmanEncoder read its payload length out of the stream and walked that far; BypassEncoder memcpy'd sizeof(T) * targetLength; RunlengthEncoder read a value and a count per run. None of them had anything to compare against. decode() now takes `size_t &remaining_length` alongside targetLength and charges what it consumed, the same shape load() already had. The two numbers are independent -- an entropy coder's bitstream has no terminator, so the symbol count is what says stop, while the byte count is what says how far it may read -- and both are available at every call site. The symbol count stays where it is in the stream, so no bytes moved. Other bounds ------------ - HuffmanEncoderV2's tree loading, and XtcBasedEncoder, against corrupted input - ComposedPredictor's predictor selection index, and its value - RegressionPredictor's coefficient stream, which each block consumes N + 1 entries of - InterpolationDecomposition's stored dimensions, which drive a grid walk over buffers that conf sizes - the declared bin count, against the element count conf carries - the bins InterpolationDecomposition and TimeSeriesDecomposition walk, checked once before the walk rather than on each access - Config::load reading one byte past the config, and validating contents only where the config is a compressed-stream trailer - the HDF5 filter's compressed buffer, sized from SZ_compress_size_bound Also ---- - Huffman's shift for single-symbol input, and the non-finite float cast in LinearQuantizer, were undefined behaviour - scratch buffers are held as unique_ptr so an exception from the encoder or the lossless layer does not leak them - PR #132's bound on the internal decompression buffer is dropped: that buffer is sized from the bin count and type, which no bound derivable from conf alone covers Not taken from the PRs as written: #132's internal-buffer bound (above), and #135's XtcBasedEncoder bounds are reinstated in a form that does not reject valid streams. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ayzk
added a commit
that referenced
this pull request
Sep 15, 2026
Consolidates the eight open bug-fix pull requests (#131 #132 #133 #134 #135 #137 #138 #139) with the findings from reviewing them, as one change. The compressed stream is untrusted, and several modules read lengths and counts out of it and then used them to index, allocate or loop without checking them against anything. The format is unchanged. A stream written by this build is byte-identical to one written by master, and either build reads the other's output; verified on interp/lorenzo_reg/nopred in 1D, 2D and 3D. Encoder::decode now takes the remaining byte count ------------------------------------------------ Its signature carried the symbol count and nothing else, so an encoder could not tell how many bytes it was allowed to touch. HuffmanEncoder read its payload length out of the stream and walked that far; BypassEncoder memcpy'd sizeof(T) * targetLength; RunlengthEncoder read a value and a count per run. None of them had anything to compare against. decode() now takes `size_t &remaining_length` alongside targetLength and charges what it consumed, the same shape load() already had. The two numbers are independent -- an entropy coder's bitstream has no terminator, so the symbol count is what says stop, while the byte count is what says how far it may read -- and both are available at every call site. The symbol count stays where it is in the stream, so no bytes moved. Other bounds ------------ - HuffmanEncoderV2's tree loading, and XtcBasedEncoder, against corrupted input - ComposedPredictor's predictor selection index, and its value - RegressionPredictor's coefficient stream, which each block consumes N + 1 entries of - InterpolationDecomposition's stored dimensions, which drive a grid walk over buffers that conf sizes - the declared bin count, against the element count conf carries - the bins InterpolationDecomposition and TimeSeriesDecomposition walk, checked once before the walk rather than on each access - Config::load reading one byte past the config, and validating contents only where the config is a compressed-stream trailer - the HDF5 filter's compressed buffer, sized from SZ_compress_size_bound Also ---- - Huffman's shift for single-symbol input, and the non-finite float cast in LinearQuantizer, were undefined behaviour; so were two signed overflows on values taken from the stream, the doubled state count in HuffmanEncoder::load and the doubled index in LinearQuantizer::recover_pred - scratch buffers are held as unique_ptr so an exception from the encoder or the lossless layer does not leak them - PR #132's bound on the internal decompression buffer is dropped: that buffer is sized from the bin count and type, which no bound derivable from conf alone covers Not taken from the PRs as written: #132's internal-buffer bound (above), and #135's XtcBasedEncoder bounds are reinstated in a form that does not reject valid streams. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ayzk
added a commit
that referenced
this pull request
Sep 15, 2026
Consolidates the eight open bug-fix pull requests (#131 #132 #133 #134 #135 #137 #138 #139) with the findings from reviewing them, as one change. The compressed stream is untrusted, and several modules read lengths and counts out of it and then used them to index, allocate or loop without checking them against anything. The format is unchanged. A stream written by this build is byte-identical to one written by master, and either build reads the other's output; verified on interp/lorenzo_reg/nopred in 1D, 2D and 3D. Encoder::decode now takes the remaining byte count ------------------------------------------------ Its signature carried the symbol count and nothing else, so an encoder could not tell how many bytes it was allowed to touch. HuffmanEncoder read its payload length out of the stream and walked that far; BypassEncoder memcpy'd sizeof(T) * targetLength; RunlengthEncoder read a value and a count per run. None of them had anything to compare against. decode() now takes `size_t &remaining_length` alongside targetLength and charges what it consumed, the same shape load() already had. The two numbers are independent -- an entropy coder's bitstream has no terminator, so the symbol count is what says stop, while the byte count is what says how far it may read -- and both are available at every call site. The symbol count stays where it is in the stream, so no bytes moved. Other bounds ------------ - HuffmanEncoderV2's tree loading, and XtcBasedEncoder, against corrupted input - ComposedPredictor's predictor selection index, and its value - RegressionPredictor's coefficient stream, which each block consumes N + 1 entries of - InterpolationDecomposition's stored dimensions, which drive a grid walk over buffers that conf sizes - the declared bin count, against the element count conf carries - the bins InterpolationDecomposition and TimeSeriesDecomposition walk, checked once before the walk rather than on each access - Config::load reading one byte past the config, and validating contents only where the config is a compressed-stream trailer - the HDF5 filter's compressed buffer, sized from SZ_compress_size_bound - ALGO_LOSSLESS's output buffer: the declared size went to ZSTD_decompress as the capacity of a buffer the caller owns, so a stream declaring more than conf.num elements wrote past it. The size check that followed ran after the write. Lossless_zstd now honours a caller's capacity Also ---- - Huffman's shift for single-symbol input, and the non-finite float cast in LinearQuantizer, were undefined behaviour; so were two signed overflows on values taken from the stream, the doubled state count in HuffmanEncoder::load and the doubled index in LinearQuantizer::recover_pred - scratch buffers are held as unique_ptr so an exception from the encoder or the lossless layer does not leak them - PR #132's bound on the internal decompression buffer is dropped: that buffer is sized from the bin count and type, which no bound derivable from conf alone covers XtcBasedEncoder's magicInts lookups are clamped on both sides rather than rejected on one. LASTIDX is the table's length, and the encoder walks to it whenever no entry reaches minDiff -- which is every input with fewer than two triplets, since minDiff is then still INT_MAX. Both sides read one past the table there; rejecting it on decode alone broke ALGO_BIOMDXTC for inputs under six elements. Its bit-packing buffer is also zeroed: it went into the compressed output uninitialised, which is why the same input did not compress to the same bytes twice. MDZ passed its buffer capacity to decompress() as the stream length, having discarded what compress() returned. zstd rejected every frame, and the result was decoded from uninitialised memory without anything noticing. Not taken from the PRs as written: #132's internal-buffer bound (above). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
RegressionPredictor::loadthreads aremaining_lengthbyte counter through its sub-reads so later reads know how many bytes are left. After Huffman-decoding the regression coefficients it does:decodeadvances the read pointercby the compressed (Huffman) stream size, butremaining_lengthis decremented bycoeff_size * sizeof(int)— the uncompressed index count. Those two are unrelated, so after this pointremaining_lengthno longer reflects the bytes actually consumed (it typically overshoots the compressed stream and understates the remainder, and can even wrap past zero).ComposedPredictor::loadhas the mirror defect: it Huffman-decodes a per-block predictor-selection stream but never decrementsremaining_lengthfor it at all.Today the
readhelpers inMemoryUtil.hpponly guard the counter withassert, so a release build tolerates the wrong value and the bug is latent. But any reader that actually enforces the bound will then reject valid data. This surfaced while integrating SZ3 into ClickHouse, whose hardened build makes those reads throw instead ofassert: a column compressed withALGO_LORENZO_REGcould be written but failed every subsequent read.Fix
Decrement
remaining_lengthby exactly the bytesdecodeconsumed, measured as the pointer advance:and apply the same accounting to the selection stream in
ComposedPredictor::load. Behavior for valid data is unchanged; the counter now tracks the real byte cursor.Context
Found while integrating SZ3 into ClickHouse. Companion fix on the ClickHouse fork: ClickHouse#2