Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a bug in the zlib decompressor #87

Closed
wants to merge 1 commit into from
Closed

Fix a bug in the zlib decompressor #87

wants to merge 1 commit into from

Conversation

rui314
Copy link

@rui314 rui314 commented Apr 5, 2022

libbacktrace occasionally fails to decompress compressed debug info
even though the sections contain valid zlib streams. The cause of the
issue is an off-by-one error.

If a zlib data block is a plain data (uncompressed data), the next two
bytes contain the size of the block. These two bytes value is byte-
aligned, so if we read-ahead more than 8 bits, we need to unread it.

So, the correct condition to determine whether or not we need to
unread a byte is bits >= 8 and not bits > 8. Due to this error,
if the last read bits happened to end at a byte boundary, the next
byte would be skipped. That caused the decompression failure.

This bug was originally reported against the mold linker.
rui314/mold#402

libbacktrace occasionally fails to decompress compressed debug info
even though the sections contain valid zlib streams. The cause of the
issue is the off-by-one error.

If a zlib data block is a plain data (uncompressed data), the next two
bytes contain the size of the block. These two bytes value is byte-
aligned, so if we read-ahead more than 8 bits, we need to unread it.

So, the correct condition to determine whether or not we need to
unread a byte is `bits >= 8` and not `bits > 8`. Due to this error,
if the last read bits happened to end at a byte boundary, the next
byte would be skipped. That caused the decompression failure.

This bug was originally reported against the mold linker.
rui314/mold#402
@ianlancetaylor
Copy link
Owner

Thanks very much for analyzing and fixing this bug.

The master sources for libbacktrace are in the GCC tree, so I took your patch and committed it there. I've then mirrored it back here.

@rui314
Copy link
Author

rui314 commented Apr 7, 2022

FYI, I implemented a workaround to mold so that it doesn't trigger this bug.
rui314/mold@ba63479

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Apr 15, 2022
mold 1.2.0 is a new release of the high-speed linker. The highlight of this
release is the 32-bit ARM support. We also added other features, and as
always, we fixed many bugs and compatibility issues in this release.

New features:

- The ARM32 target is now supported.

- --gdb-index is implemented. If this option is given, mold creates an
  .gdb_index section in an output file to speed up GNU debugger. Users have
  to compile their object files with -ggnu-pubnames to use this flag. mold
  used to ignore --gdb-index. (a7475dd)

- mold now supports the following flags: --start-address, -Tbss, -Tdata,
  -Ttext, --oformat=binary, --disable-new-dtags

Deprecated features:

- An experimental, mold-specific --preload flag has been marked as
  deprecated. It's still usable, but a warning message will be displayed
  if that flag is given.

Bug fixes and compatibility improvements:

- -dy and -dn are now accepted as aliases for -Bdynamic and -Bstatic,
  respectively. (82e8072)

- -static-pie now works with older versions of glibc thanks to a few bug
  fixes. (3d68824, 0884f27)

- Issues found by UndefinedBehaviorSanizer, AddressSanitizer and
  ThreadSanitizer are fixed. (bf26753, f4753b3, e1e4e9f)

- mold used to place sections with very large section alignment
  requirements to wrong places in an output file. That caused a mysterious
  crash of a produced binary (#405). That bug was most noticeable when
  Nvidia-provided object files are given because they tend to contain such
  sections. This bug has been fixed. (100922b)

- .ctors and .dtors sections are now recognized by mold, and their contents
  are sorted with a special rule. This shouldn't affect most build
  environments because these sections have been superseded by .init_array
  and .fini_array sections a long time ago. But it looks like some old
  i386 compilers are still using .ctors and .dtors. (392781a)

- For a non-position-independent executable, we have to make address-taken
  PLT entries as "canonical". Marking all PLT entries canonical should be
  harmless in theory, so we did so. However, some programs, notably Qt
  library, assume that non-address-taken PLTs can never be canonical
  (#352). For the sake of compatibility with such programs, we now make
  PLTs canonical only when their addresses are taken. (e0bc74a)

- mold now defines _TLS_MODULE_BASE_ symbol. A reference to this symbol
  can occur if -mtls-dialect=gnu2 is given to a compiler. The flag tells
  the compiler to use TLSDESC mechanism instead of the regular TLS access
  mechanism to access thread-local variables. (5feab82)

- libbacktrace sometimes fail to read compressed debug sections in
  mold-generated files due to a bug. We not only fix that libbacktrace's
  bug (ianlancetaylor/libbacktrace#87) but also implemented a workaround
  to mold (ba63479) so that mold works with older versions of libbacktrace.

- [ARM64] mold now recognizes R_AARCH64_LD_PREL_LO19 relocation. (146ddd7)

- [RISCV64] The correct semantics of R_RISCV_ALIGN is implemented.
  (0daf623)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants