Compressing specific file at q=4 broken on x86_64-w64-mingw32 since specific commit #67

Closed
opened 2026-01-29 20:31:11 +00:00 by claunia · 4 comments
Owner

Originally created by @fasterthanlime on GitHub (Nov 12, 2015).

Running this:

./bro.exe -q 4 -i butler.exe > /dev/null
  • Where bro.exe is a 64-bit (PE32+) binary compiled using mingw-gcc (tested with v4.6 & v5.2)
  • Where butler.exe is the file contained in that 7z archive: https://misc.amos.me/butler/windows-amd64/v0.4.5/butler.7z (no need to trust/run the .exe, it's just a sample file to be compressed by brotli)

...fails starting with ea48ce5a6f, with the following message:

$ brotli/tools/bro.exe -q 4 -i butler.exe | tail

This application has requested the Runtime to terminate it in an unusual way.
Please contact the application's support team for more information.
Assertion failed!

Program: C:\msys64\home\amos\tmp\brotli\tools\bro.exe
File: ./write_bits.h, Line 52

Expression: bits < 1UL << n_bits

Compiling+running with #define BIT_WRITER_DEBUG uncommented yields the following last few lines:

WriteBits   6  0x000000000000001d    19554445
WriteBits   0  0x0000000000000000    19554451
WriteBits   6  0x0000000000000011    19554451
WriteBits   2  0x0000000000000002    19554457
WriteBits   9  0x00000000000001c3    19554459
WriteBits   8  0x0000000000000095    19554468
WriteBits   6  0x0000000000000027    19554476
WriteBits  16  0x0000000000000028    19554482
WriteBits  12  0x0000000000000fff    19554498
WriteBits  36  0x0000000003437182    19554510

I think 1UL << 36 might be overflowing on Windows, where unsigned longs seem to.. remain 32-bit ? (cf. http://stackoverflow.com/questions/384502/what-is-the-bit-size-of-long-on-64-bit-windows)

It might be worth using explicitly-sized types throughout the brotli codebase, I've never seen a large-scale "replace int with size_t" refactor go well :)

Originally created by @fasterthanlime on GitHub (Nov 12, 2015). Running this: ``` ./bro.exe -q 4 -i butler.exe > /dev/null ``` - Where `bro.exe` is a 64-bit (PE32+) binary compiled using mingw-gcc (tested with v4.6 & v5.2) - Where `butler.exe` is the file contained in that 7z archive: https://misc.amos.me/butler/windows-amd64/v0.4.5/butler.7z (no need to trust/run the .exe, it's just a sample file to be compressed by brotli) ...fails starting with https://github.com/google/brotli/commit/ea48ce5a6fa8a3224dd270a7f60428084e7b590d, with the following message: ``` $ brotli/tools/bro.exe -q 4 -i butler.exe | tail This application has requested the Runtime to terminate it in an unusual way. Please contact the application's support team for more information. Assertion failed! Program: C:\msys64\home\amos\tmp\brotli\tools\bro.exe File: ./write_bits.h, Line 52 Expression: bits < 1UL << n_bits ``` Compiling+running with `#define BIT_WRITER_DEBUG` uncommented yields the following last few lines: ``` WriteBits 6 0x000000000000001d 19554445 WriteBits 0 0x0000000000000000 19554451 WriteBits 6 0x0000000000000011 19554451 WriteBits 2 0x0000000000000002 19554457 WriteBits 9 0x00000000000001c3 19554459 WriteBits 8 0x0000000000000095 19554468 WriteBits 6 0x0000000000000027 19554476 WriteBits 16 0x0000000000000028 19554482 WriteBits 12 0x0000000000000fff 19554498 WriteBits 36 0x0000000003437182 19554510 ``` I think `1UL << 36` might be overflowing on Windows, where unsigned longs seem to.. remain 32-bit ? (cf. http://stackoverflow.com/questions/384502/what-is-the-bit-size-of-long-on-64-bit-windows) It might be worth using explicitly-sized types throughout the brotli codebase, I've never seen a large-scale "replace int with size_t" refactor go well :)
Author
Owner

@fasterthanlime commented on GitHub (Nov 12, 2015):

Replacing 1UL with 1ULL in the assert in enc/write_bits.h seems to fix it.

@fasterthanlime commented on GitHub (Nov 12, 2015): Replacing `1UL` with `1ULL` in the assert in `enc/write_bits.h` seems to fix it.
Author
Owner

@eustas commented on GitHub (Nov 12, 2015):

Hello.

Thank you for the report. Most probably this affects not only mingw, but some other compilers.
And, again, we are very grateful for the proposed fix.

I believe, the fix will be landed tomorrow.

Best regards,
Eugene.

@eustas commented on GitHub (Nov 12, 2015): Hello. Thank you for the report. Most probably this affects not only mingw, but some other compilers. And, again, we are very grateful for the proposed fix. I believe, the fix will be landed tomorrow. Best regards, Eugene.
Author
Owner

@szabadka commented on GitHub (Nov 12, 2015):

This was fixed in PR #266

@szabadka commented on GitHub (Nov 12, 2015): This was fixed in PR #266
Author
Owner

@fasterthanlime commented on GitHub (Nov 12, 2015):

Thanks a lot for the quick fix! 🌟

@fasterthanlime commented on GitHub (Nov 12, 2015): Thanks a lot for the quick fix! :star2:
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/brotli#67