Nick Terrell [Thu, 2 Feb 2023 18:53:08 +0000 (10:53 -0800)]
Fix ZSTD_getOffsetInfo() when nbSeq == 0
In 32-bit mode, ZSTD_getOffsetInfo() can be called when nbSeq == 0, and
in this case the offset table is uninitialized. The function should just
return 0 for both values, because there are no sequences.
Nick Terrell [Thu, 2 Feb 2023 00:38:36 +0000 (16:38 -0800)]
Fix 32-bit decoding with large dictionary
The 32-bit decoder could corrupt the regenerated data by using regular
offset mode when there were actually long offsets. This is because we
were only considering the window size in the calculation, not the
dictionary size. So a large dictionary could allow longer offsets.
Fix this in two ways:
1. Instead of looking at the window size, look at the total referencable
bytes in the history buffer. Use this in the comparison instead of
the window size. Additionally, we were comparing against the wrong
value, it was too low. Fix that by computing exactly the maximum
offset for regular sequence decoding.
2. If it is possible that we have long offsets due to (1), then check
the offset code decoding table, and if the decoding table's maximum
number of additional bits is no more than STREAM_ACCUMULATOR_MIN,
then we can't have long offsets.
This gates us to be using the long offsets decoder only when we are very
likely to actually have long offsets.
Note that this bug only affects the decoding of the data, and the
original compressed data, if re-read with a patched decoder, will
correctly regenerate the orginal data. Except that the encoder also had
the same issue previously.
Nick Terrell [Mon, 30 Jan 2023 19:15:15 +0000 (11:15 -0800)]
Simplify 32-bit long offsets decoding logic
The previous code had an issue when `bitsConsumed == 32` it would read 0
bits for the `ofBits` read, which violates the precondition of
`BIT_readBitsFast()`. This can happen when the stream is corrupted.
Fix thie issue by always reading the maximum possible number of extra
bits. I've measured neutral decoding performance, likely because this
branch is unlikely, but this should be faster anyways. And if not, it is
only 32-bit decoding, so performance isn't as critical.
Nick Terrell [Thu, 26 Jan 2023 20:11:25 +0000 (12:11 -0800)]
[huf] Fix bug in fast C decoders
The input bounds checks were buggy because they were only breaking from
the inner loop, not the outer loop. The fuzzers found this immediately.
The fix is to use `goto _out` instead of `break`.
This condition can happen on corrupted inputs.
I've benchmarked before and after on x86-64 and there were small changes
in performance, some positive, and some negative, and they end up about
balacing out.
Yann Collet [Thu, 26 Jan 2023 00:29:34 +0000 (16:29 -0800)]
modify cli-test logic : ignore stderr message by default
Previously, cli-test would, by default, check that a stderr output is strictly identical to a saved outcome.
When there was no instructions on how to interpret stderr, it would default to requiring it to be empty.
There are many tests cases though where stderr content doesn't matter, and we are mainly interested in the return code of the cli.
For these cases, it was possible to set a .ignore document, which would instruct to ignore stderr content.
This PR update the logic, to make .ignore the default.
When willing to check that stderr content is empty, one must now add an empty .strict file.
This will allow status message to evolve without triggering many cli-tests errors.
This is especially important when some of these status include compression results, which may change as a result of compression optimizations.
It also makes it easier to add new tests which only care about the CLI's return code.
Yonatan Komornik [Thu, 26 Jan 2023 06:40:25 +0000 (22:40 -0800)]
Merge pull request #3457 from yoniko/fix-rowhash-cli
[Bugfix] CLI row hash flags set the wrong values
`--[no-]row-match-finder` do the opposite of what they are supposed to.
In effect the no option would activate row hash while the other option will disable it.
This commit fixes the issue and changes the code to use the more readable enum values.
Nick Terrell [Tue, 24 Jan 2023 04:23:43 +0000 (20:23 -0800)]
[version-test] Work around bugs in v0.7.3 dict builder
Before calling a dictionary good, make sure that it can compress an
input. If v0.7.3 rejects v0.7.3's dictionary, fall back to the v1.0
dictionary. This is not the job of the verison test to test it, because
we cannot fix this code.
Nick Terrell [Sat, 14 Jan 2023 00:34:52 +0000 (16:34 -0800)]
[huf] Add generic C versions of the fast decoding loops
Add generic C versions of the fast decoding loops to serve architectures
that don't have an assembly implementation. Also allow selecting the C
decoding loop over the assembly decoding loop through a zstd
decompression parameter `ZSTD_d_disableHuffmanAssembly`.
I benchmarked on my Intel i9-9900K and my Macbook Air with an M1 processor.
The benchmark command forces zstd to compress without any matches, using
only literals compression, and measures only Huffman decompression speed:
The new fast decoding loops outperform the previous implementation uniformly,
but don't beat the x86-64 assembly. Additionally, the fast C decoding loops suffer
from the same stability problems that we've seen in the past, where the assembly
version doesn't. So even though clang gets close to assembly on x86-64, it still
has stability issues.
Fix the following documentation bugs:
* Note that `ZSTD_estimate*` functions are not compatible with the external matchfinder API
* Note that `ZSTD_estimateCStreamSize_usingCCtxParams()` is not compatible with `nbWorkers >= 1`
* Remove incorrect warning that the legacy streaming API is incompatible with advanced parameters and/or dictionary compression
* Note that `ZSTD_initCStream()` is incompatible with dictionary compression
* Warn that
Yann Collet [Sat, 21 Jan 2023 01:53:05 +0000 (17:53 -0800)]
refactor : --rm is ignored with stdout
`zstd` CLI has progressively moved to the policy of
ignoring `--rm` command when the output is `stdout`.
The primary drive is to feature a behavior more consistent with `gzip`,
when `--rm` is the default, but is also ignored when output is `stdout`.
Other policies are certainly possible, but would break from this `gzip` convention.
The new policy was inconsistenly enforced, depending on the exact list of commands.
For example, it was possible to circumvent it by using `-c --rm` in this order,
which would re-establish source removal.
- Update the CLI so that it necessarily catch these situations and ensure that `--rm` is always disabled when output is `stdout`.
- Added a warning message in this case (for verbosity 3 `-v`).
- Added an `assert()`, which controls that `--rm` is no longer active with `stdout`
- Added tests, which control the behavior, even when `--rm` is added after `-c`
- Removed some legacy code which where trying to apply a specific policy for the `stdout` + `--rm` case, which is no longer possible
W. Felix Handte [Fri, 20 Jan 2023 22:32:49 +0000 (17:32 -0500)]
Add Additional Flags to PGO Build
In GCC, we can add a couple more flags to give us confidence that the profile
data is actually being found and used.
Also, my system for example doesn't have a binary installed under the name
`llvm-profdata`, but it does have, e.g., `llvm-profdata-13`, etc. So this
commit adds a variable that can be overridden.
Nick Terrell [Thu, 19 Jan 2023 20:21:31 +0000 (12:21 -0800)]
[versions-test] Work around bug in dictionary builder for older versions
Older versions of zstandard have a bug in the dictionary builder, that
can cause dictionary building to fail. The process still exits 0, but
the dictionary is not created.
For reference, the bug is that it creates a dictionary that starts with
the zstd dictionary magic, in the process of writing the dictionary header,
but the header isn't fully written yet, and zstd fails compressions in
this case, because the dictionary is malformated. We fixed this later on
by trying to load the dictionary as a zstd dictionary, but if that fails
we fallback to content only (by default).
The fix is to:
1. Make the dictionary determinsitic by sorting the input files.
Previously the bug would only sometimes occur, when the input files
were in a particular order.
2. If dictionary creation fails, fallback to the `head` dictionary.
Nick Terrell [Fri, 20 Jan 2023 00:27:24 +0000 (16:27 -0800)]
Cap hashLog & chainLog to ensure that we only use 32 bits of hash
* Cap shortCache chainLog to 24
* Cap row match finder hashLog so that rowLog <= 24
* Add unit tests to expose all cases. The row match finder unit tests
are only run in 64-bit mode, because they allocate ~1GB.
Nick Terrell [Wed, 21 Dec 2022 00:25:24 +0000 (16:25 -0800)]
[tests] Fix version test determinism
The dictionary source files were taken from the `dev` branch before this
commit, which could introduce non-determinism on PR jobs. Instead take
the sources from the PR checkout.
This PR also adds stderr logging, and verbose output for the jobs that
are failing, to help catch the failure if it occurs again.