[bug] Fix entropy repeat mode bug

[senhuang42]

Fixes a bug from OSS-Fuzz that occurs in the block splitter: the change to the blockState entropy repeat mode was erroneously happening before ZSTD_blockState_confirmRepcodesAndEntropyTables() and would get overwritten.

Testing - this was never caught by the OSS fuzzer when it was running on the advanced API. But it was caught within a few days of adding block splitter to the old API fuzzers.

[terrelln]

Testing - this was never caught by the OSS fuzzer when it was running on the advanced API. But it was caught within a few days of adding block splitter to the old API fuzzers.

I haven't looked at the PR yet, but why is this the case? Is it because the fuzzer that caught it doesn't use the advanced API, and we need to extend its coverage?

[senhuang42]

I haven't looked at the PR yet, but why is this the case? Is it because the fuzzer that caught it doesn't use the advanced API, and we need to extend its coverage?

The fuzzer that caught it is dictionary_round_trip, which uses compress_usingDict. The dictionary_stream_round_trip fuzzer does use block splitter, but with compressStream2. So maybe it's the case that this repeat mode issue only surfaces on single-shot dict compression? But then again I'm not entirely sure why that would be the case for this particular issue.

[terrelln]

The fuzzer that caught it is dictionary_round_trip, which uses compress_usingDict

Could we extend it to use the advanced API 1/2 the time? E.g. the same way simple_round_trip does.

[senhuang42]

Could we extend it to use the advanced API 1/2 the time? E.g. the same way simple_round_trip does.

I was wrong - dictionary_round_trip only uses the old API 1/16 times, compress2 otherwise.

zstd/tests/fuzz/dictionary_round_trip.c

Line 37 in 8a3bdfa

if (FUZZ_dataProducer_uint32Range(producer, 0, 15) == 0) {

[terrelln]

Do you think there is a reason why the fuzzer didn't catch it until we added the old API? Or do you think it is just random chance?

[senhuang42]

Do you think there is a reason why the fuzzer didn't catch it until we added the old API? Or do you think it is just random chance?

I had investigated a bit, and it's still not really clear to me how this ends up happening, though I'll keep looking. Random chance being the reason seems somewhat unlikely since the old API is only hit with 1/16th of the frequency, and the old API only uses block splitter with high compression levels.