File compressed with wlog=25 gets corrupted #3350

tansy · 2022-12-14T06:13:13Z

Describe the bug
File compressed with wlog=25 gets corrupted

To Reproduce
I tried to compress silesia.tar with wlog=25. I went fine but when tested i reported error and decompression to stdout failed in the middle of the file.

$ zstd -k -19 --zstd=wlog=25 silesia.tar
$  zstd -t silesia.tar.zst 
silesia.tar.zst      : 84.9 MiB...     silesia.tar.zst : Decoding error (36) : Corrupted block detected

I uploaded these files to the cloud. You can check them here:
Original, compressed: silesia.tar.lz
Corrupted zst: silesia.tar.zst

Expected behavior
To be able to decompress compressed file.

Desktop (please complete the following information):

OS: linux
Version: zstd-v1.5.2
Compiler: gcc
Flags: default
Build system: Makefile

Just tested brand new dev branch with the same result.

The text was updated successfully, but these errors were encountered:

Cyan4973 · 2022-12-14T19:47:33Z

This is surprising.

For information, I tested the exact same command, using the dev branch,
zstd -k -19 --zstd=wlog=25 silesia.tar
and there was no such problem, everything round-tripped properly.

Your report implies that you have observed this problem multiple times, that it's reproducible ?

edit : I can confirm that the file provided as silesia.tar.zst is corrupted,
but I can't reproduce the same file on my side.

tansy · 2022-12-15T00:45:40Z

I will drop you binaries later.

Your report implies that you have observed this problem multiple times, that it's reproducible?

Yes, I compiled dev branch with gcc and clang with the same result.
You can reproduce it as well - other binaries (windows) from 1.5.2 release share the same "feature".

I would compile windows version but my wine has trouble with these functions `InitializeConditionVariable', `SleepConditionVariableCS' on that computer.

terrelln · 2022-12-15T01:13:23Z

This is definitely unexpected... I'll take a look at the compressed file and see if I see anything obviously wrong.

In the mean time, can you provide more information about your system? Anything you can think of would help, but at least:

Filesystem the data is stored on
Compiler versions
OS version: uname -a output
CPU: lscpu output

Cyan4973 · 2022-12-15T01:29:31Z

I could test with the exact silesia.tar produced by silesia.tar.lz,
and it does round-trip properly on macos.

Windows is a bit more complex to validate,
but your report suggests that the problem also happens on linux,
and that shouldn't be too much different...

terrelln · 2022-12-15T01:52:56Z

It is failing because an offset is too large, specifically 65305816. So I went through and set the windowSize = 1u << 28, and tried to decompress again. When I did that it just failed slightly later in the same block. Which suggests that it is bitstream corruption, not corruption in the LZ match finder. E.g. we're not finding a match at offset 65305816, and emitting it, it is just the first failure when we receive a corrupt bitstream.

My best guess is that your filesystem is somehow different, and maybe exposing some bug in the zstd CLI's I/O code. Can you try not writing the file to the filesystem, and decompressing it directly, e.g. this command:

zstd -k -19 --zstd=wlog=25 silesia.tar -c | zstd -t

That will help us narrow down the issue by taking the filesystem out of the picture.

terrelln · 2022-12-15T01:58:12Z

Can you also try with --single-thread?

tansy · 2022-12-15T01:58:32Z

Here are binaries. I tried it with gcc 4.9.4, 6.5, 8.1, clang 3.8, 6. There are only two left, I thrown the rest. They all produced bit identical output, verified with md5sum.
The cpu is dual core something x32/x64, don't remember the model (it has SSE3 that's all I remember, lscpu doesn't even say that, just i686, 32-bit, 66-bit, LE) , running 32-bit linux (slackware).

In the archive there is windows binary I cooked which produces correct output. This (windoze) program is single threaded. I suspect that to be a reason, at least for windows. If not then I don't know. It come to me after experiencing those crazy functions I never heard of. IMO there are simpler solutions to that. Even winpthreads, even for msvc.

[ed] Here are results for these windows binaries, which you can check yourself:

$ wc -c * | sort -r
211957760 silesia.tar
 52803703 silesia.tar.zst-correct
 52800542 silesia.tar.zst_w64
 52800542 silesia.tar.zst_w32
 52800542 silesia.tar.zst_lx

$ md5sum *
059b5b012f624870d3a30ba65c3c5982 *silesia.tar
84b4cf54637d12d69a587a6981642a62 *silesia.tar.zst-correct
661c82d9171231b6bfe9b018bd1fc204 *silesia.tar.zst_lx
661c82d9171231b6bfe9b018bd1fc204 *silesia.tar.zst_w32
6b34307d301167b91eb946eb161639c4 *silesia.tar.zst_w64

What is funny, is that windows binaries released produce different output.

yoniko · 2022-12-15T02:06:34Z

One interesting point is that this operating system is 32 bit.
We should test this on a 32-bit system to see if the issue reproduces.

tansy · 2022-12-15T02:28:43Z

We should test this on a 32-bit system to see if the issue reproduces.

As I said, although not precised that, these windows binaries (x32/x64) fail on 64-bit windows (10).

All single threaded, on linux and windows were correct.
You have a culprit. You owe me a 🥩🍷.

PS. (As a side note) Is it normal that clang is faster with compression and gcc faster with decompression?
PPS. Speaking of threads - what's the difference between zstd and zstd-mt?

terrelln · 2022-12-15T04:18:07Z

I've reproduced the issue! Thanks @tansy for providing clear instructions for how to reproduce it!

I was only able to reproduce it with the file provided by @tansy with SHA1 63bb9c2deeec0a6e1b346271331567a2cc5903a9, not my own version of silesia.tar which was the same size, but a different SHA1. I was also only able to reproduce it in 32-bit mode.

make -j zstd MOREFLAGS=-m32
./zstd -c -19 --zstd=wlog=25 silesia.tar | ./zstd -t

We'll get this debugged & fixed shortly!

terrelln · 2022-12-15T04:19:39Z

PS. (As a side note) Is it normal that clang is faster with compression and gcc faster with decompression?

Yeah, they vary a bit. Every release there is a bit of noise, but that is what we currently observe. We're working on transitioning to compiling zstd with clang internally, and as part of that process one of the engineers working on clang is helping us get clang's decompression performance up to par with gcc.

PPS. Speaking of threads - what's the difference between zstd and zstd-mt?

zstd-mt is the same as zstd -T0.

terrelln · 2022-12-15T04:35:27Z

The first bad commit is c90e81a.

To work around this issue, please use release v1.4.9. I will also provide a 1-line patch to build a version of zstd without this issue tomorrow. And we will put up a fix shortly.

tansy · 2022-12-15T06:39:20Z

My best guess is that your filesystem is somehow different, and maybe exposing some bug in the zstd CLI's I/O code.

$ zstd-dev-22k14-gcc-6.5.0_sl_lx32 -19 --zstd=wlog=25 -c silesia.tar | zstd -t
/*stdin*\            : 99.8 MiB...     /*stdin*\ : Decoding error (36) : Corrupted block detected

$ zstd-dev-22k14-clang-3.8_sl_lx32 -19 --zstd=wlog=25 -c silesia.tar | zstd -t
/*stdin*\            : 80.8 MiB...     /*stdin*\ : Decoding error (36) : Corrupted block detected

@tansy

Fix an off-by-one error in the compressor that emits corrupt blocks if: * Zstd is compiled in 32-bit mode * The windowLog == 25 exactly * An offset of 2^25-3, 2^25-2, 2^25-1, or 2^25 is emitted * The bitstream had 7 bits leftover before writing the offset This bug has been present since before v1.0, but wasn't able to easily be triggered, since until somewhat recently zstd wasn't able to find matches that were within 128KB of the window size. Add a test case, and fix 2 bugs in `ZSTD_compressSequences()`: * The `ZSTD_isRLE()` check was incorrect. It wouldn't produce corruption, but it could waste CPU and not emit RLE even if the block was RLE * One windowSize was `1 << windowLog`, not `1u << windowLog` Thanks to @tansy for finding the issue, and giving us a reproducer! Fixes Issue facebook#3350.

terrelln · 2022-12-15T21:48:16Z

PR #3361 should fix the issue.

Specifically the patch to this line:

zstd/lib/compress/zstd_compress.c

Line 2627 in 6be3181

const int longOffsets = cctxParams->cParams.windowLog > STREAM_ACCUMULATOR_MIN;

- const int longOffsets = cctxParams->cParams.windowLog > STREAM_ACCUMULATOR_MIN;
+ const int longOffsets = cctxParams->cParams.windowLog >= STREAM_ACCUMULATOR_MIN;

@tansy

Fix an off-by-one error in the compressor that emits corrupt blocks if: * Zstd is compiled in 32-bit mode * The windowLog == 25 exactly * An offset of 2^25-3, 2^25-2, 2^25-1, or 2^25 is emitted * The bitstream had 7 bits leftover before writing the offset This bug has been present since before v1.0, but wasn't able to easily be triggered, since until somewhat recently zstd wasn't able to find matches that were within 128KB of the window size. Add a test case, and fix 2 bugs in `ZSTD_compressSequences()`: * The `ZSTD_isRLE()` check was incorrect. It wouldn't produce corruption, but it could waste CPU and not emit RLE even if the block was RLE * One windowSize was `1 << windowLog`, not `1u << windowLog` Thanks to @tansy for finding the issue, and giving us a reproducer! Fixes Issue facebook#3350.

@tansy

Fix an off-by-one error in the compressor that emits corrupt blocks if: * Zstd is compiled in 32-bit mode * The windowLog == 25 exactly * An offset of 2^25-3, 2^25-2, 2^25-1, or 2^25 is emitted * The bitstream had 7 bits leftover before writing the offset This bug has been present since before v1.0, but wasn't able to easily be triggered, since until somewhat recently zstd wasn't able to find matches that were within 128KB of the window size. Add a test case, and fix 2 bugs in `ZSTD_compressSequences()`: * The `ZSTD_isRLE()` check was incorrect. It wouldn't produce corruption, but it could waste CPU and not emit RLE even if the block was RLE * One windowSize was `1 << windowLog`, not `1u << windowLog` Thanks to @tansy for finding the issue, and giving us a reproducer! Fixes Issue #3350.

terrelln · 2022-12-15T22:47:29Z

Please reopen if you find that the issue persists on the latest dev branch.

tansy · 2022-12-16T04:45:45Z

Two questions:

Is -19 the only option to trigger this error? I tried some smaller level and it didn't.

New test fails with old source at

zstreamtest 
Starting zstream tester (32-bits, 1.5.3)
Seed = 5970
Trying 5 different sets of parameters
zstreamtest: ../lib/compress/../common/bitstream.h:183: BIT_addBits: Assertion `nbBits + bitC->bitPos < sizeof(bitC->bitContainer) * 8' failed.
Aborted

Is it the error?

Cyan4973 self-assigned this Dec 14, 2022

yoniko mentioned this issue Dec 15, 2022

FATAL ERROR: zstd uncompress failed with error code 10 #3260

Closed

terrelln mentioned this issue Dec 15, 2022

Fix corruption that rarely occurs in 32-bit mode with wlog=25 #3361

Merged

terrelln closed this as completed Dec 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

File compressed with wlog=25 gets corrupted #3350

File compressed with wlog=25 gets corrupted #3350

tansy commented Dec 14, 2022 •

edited

Loading

Cyan4973 commented Dec 14, 2022 •

edited

Loading

tansy commented Dec 15, 2022 •

edited

Loading

terrelln commented Dec 15, 2022

Cyan4973 commented Dec 15, 2022

terrelln commented Dec 15, 2022

terrelln commented Dec 15, 2022

tansy commented Dec 15, 2022 •

edited

Loading

yoniko commented Dec 15, 2022

tansy commented Dec 15, 2022 •

edited

Loading

terrelln commented Dec 15, 2022

terrelln commented Dec 15, 2022

terrelln commented Dec 15, 2022

tansy commented Dec 15, 2022

terrelln commented Dec 15, 2022

terrelln commented Dec 15, 2022

tansy commented Dec 16, 2022

File compressed with wlog=25 gets corrupted #3350

File compressed with wlog=25 gets corrupted #3350

Comments

tansy commented Dec 14, 2022 • edited Loading

Cyan4973 commented Dec 14, 2022 • edited Loading

tansy commented Dec 15, 2022 • edited Loading

terrelln commented Dec 15, 2022

Cyan4973 commented Dec 15, 2022

terrelln commented Dec 15, 2022

terrelln commented Dec 15, 2022

tansy commented Dec 15, 2022 • edited Loading

yoniko commented Dec 15, 2022

tansy commented Dec 15, 2022 • edited Loading

terrelln commented Dec 15, 2022

terrelln commented Dec 15, 2022

terrelln commented Dec 15, 2022

tansy commented Dec 15, 2022

terrelln commented Dec 15, 2022

terrelln commented Dec 15, 2022

tansy commented Dec 16, 2022

tansy commented Dec 14, 2022 •

edited

Loading

Cyan4973 commented Dec 14, 2022 •

edited

Loading

tansy commented Dec 15, 2022 •

edited

Loading

tansy commented Dec 15, 2022 •

edited

Loading

tansy commented Dec 15, 2022 •

edited

Loading