zstd/lib/common
Yann Collet f299fa39ac fix a subtle issue in continue mode
The deep fuzzer tests caught a subtle bug that was probably there for a long time.
The impact of the bug is not a crash, or any other clear error signal,
rather, it reduces performance, by cutting data into smaller blocks.
Eventually, the following test would fail because it produces too many 1-byte blocks,
requiring more space than buffer can provide :
`./zstreamtest_asan --mt -s3514 -t1678312 -i1678314`

The root scenario is as follows :
- Create context, initialize it using explicit parameters or a `cdict` to pin them down, set `pledgedSrcSize=1`
- The compression parameters will not be adapted, but `windowSize` and `blockSize` will be automatically set to `1`.
  `windowSize` and `blockSize` are dynamic values, set within `ZSTD_resetCCtx_internal()`.
  The automatic adaptation makes it possible to generate smaller contexts for smaller input sizes.
- Complete compression
- New compression with same context, using same parameters, but `pledgedSrcSize=ZSTD_CONTENTSIZE_UNKNOWN`
  trigger "continue mode"
- Continue mode doesn't modify blockSize, because it used to depend on `windowLog` only,
  but in fact, it also depends on `pledgedSrcSize`.
- The "old" blocksize (1) is still there,
  next compression will use this value to cut input into blocks,
  resulting in more blocks and worse performance than necessary performance.

Given the scenario, and its possible variants, I'm surprised it did not show up before.
But I suspect it did show up, it's just that it never triggered an error, because "worse performance" is not a trigger.
The above test is a special corner case, where performance is so impacted that it reaches an error case.

The fix works, but I'm not completely pleased.
I think the current code relies too much on implied relations between variables.
This will likely break again in the future when some related part of the code change.
Unfortunately, no time to make larger changes if we want to keep the release target for zstd v1.3.3.
So a longer term fix will have to be considered after the release.

To do : create a reliable test case which triggers this scenario for CI tests.
2017-12-19 09:43:03 +01:00
..
bitstream.h no longer supported starting C++17 2017-12-04 18:00:53 -08:00
compiler.h updated license header 2017-09-08 00:09:23 -07:00
entropy_common.c minor code refactor in HUF module 2017-03-05 21:07:20 -08:00
error_private.c changed error code when pos<=size condition is not respected 2017-09-27 10:35:56 -07:00
error_private.h updated license header 2017-09-08 00:09:23 -07:00
fse_decompress.c Merge pull request #796 from terrelln/is-error 2017-08-15 12:37:28 -07:00
fse.h fix proper naming on FSE_createCTable() arguments in fse.h 2017-09-30 11:08:50 -07:00
huf.h Ensure dictionary Huff table can encode any symbol 2017-10-03 13:22:13 -07:00
mem.h fixed one UB pointer arithmetic 2017-11-17 11:40:08 -08:00
pool.c removed direct call to malloc() from pool.c 2017-10-31 17:43:24 -07:00
pool.h updated license header 2017-09-08 00:09:23 -07:00
threading.c [libzstd] pthread function prefixed with ZSTD_ 2017-09-27 11:48:48 -07:00
threading.h [libzstd] pthread function prefixed with ZSTD_ 2017-09-27 11:48:48 -07:00
xxhash.c [libzstd] Fix FORCE_INLINE macro 2017-08-14 21:12:05 -07:00
xxhash.h xxhash can be included twice in any order 2017-03-01 13:29:29 -08:00
zstd_common.c merged repcode search into BT match search 2017-11-13 02:23:48 -08:00
zstd_errors.h changed error code when pos<=size condition is not respected 2017-09-27 10:35:56 -07:00
zstd_internal.h fix a subtle issue in continue mode 2017-12-19 09:43:03 +01:00