| Message ID | CAOyqgcUVTs3MQKg7a-JPBXVrHh7LxkiwjuPo_9gcqFV1x9aROg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | libbacktrace patch committed: Don't assume compressed section aligned | expand |
On ELF64, it looks like BFD uses 8-byte alignment for compressed `.debug_*` sections while gold/lld/mold use 1-byte alignment. I do not know how the Solaris linker sets the alignment. The specification's wording makes me confused whether it really requires 8-byte alignment, even if a non-packed `Elf64_Chdr` surely requires 8. > The sh_size and sh_addralign fields of the section header for a compressed section reflect the requirements of the compressed section. There are many `.debug_*` sections. So avoiding some alignment padding seems a very natural extension (a DWARF v5 -gsplit-dwarf relocatable file has ~10 `.debug_*` sections), even if the specification doesn't allow it with a very strict interpretation... (Off-topic: I wonder whether ELF control structures should use unaligned LEB128 more. REL/RELA can naturally be replaced with a LEB128 one similar to wasm.) On Fri, Mar 8, 2024 at 1:57 PM Ian Lance Taylor <iant@golang.org> wrote: > > Reportedly when lld compresses debug sections, it fails to set the > alignment of the compressed section such that the compressed header > can be read directly. To me this seems like a bug in lld. However, > libbacktrace needs to work around it. This patch, originally by the > GitHub user ubyte, does that. Bootstrapped and tested on > x86_64-pc-linux-gnu. Committed to mainline. > > Ian > > * elf.c (elf_uncompress_chdr): Don't assume compressed section is > aligned.
On Fri, Mar 8, 2024 at 2:48 PM Fangrui Song <maskray@google.com> wrote: > > On ELF64, it looks like BFD uses 8-byte alignment for compressed > `.debug_*` sections while gold/lld/mold use 1-byte alignment. I do not > know how the Solaris linker sets the alignment. > > The specification's wording makes me confused whether it really > requires 8-byte alignment, even if a non-packed `Elf64_Chdr` surely > requires 8. Since compressed sections begin with a compression header structure that identifies the compression algorithm, compressed sections must be aligned to the alignment of the compression header. I don't think there is any ambiguity here. > > The sh_size and sh_addralign fields of the section header for a compressed section reflect the requirements of the compressed section. > > There are many `.debug_*` sections. So avoiding some alignment padding > seems a very natural extension (a DWARF v5 -gsplit-dwarf relocatable > file has ~10 `.debug_*` sections), even if the specification doesn't > allow it with a very strict interpretation... > > (Off-topic: I wonder whether ELF control structures should use > unaligned LEB128 more. REL/RELA can naturally be replaced with a > LEB128 one similar to wasm.) > > On Fri, Mar 8, 2024 at 1:57 PM Ian Lance Taylor <iant@golang.org> wrote: > > > > Reportedly when lld compresses debug sections, it fails to set the > > alignment of the compressed section such that the compressed header > > can be read directly. To me this seems like a bug in lld. However, > > libbacktrace needs to work around it. This patch, originally by the > > GitHub user ubyte, does that. Bootstrapped and tested on > > x86_64-pc-linux-gnu. Committed to mainline. > > > > Ian > > > > * elf.c (elf_uncompress_chdr): Don't assume compressed section is > > aligned. > > > > -- > 宋方睿
diff --git a/libbacktrace/elf.c b/libbacktrace/elf.c index 7841c86cd9c..3cd87020b03 100644 --- a/libbacktrace/elf.c +++ b/libbacktrace/elf.c @@ -5076,7 +5076,7 @@ elf_uncompress_chdr (struct backtrace_state *state, backtrace_error_callback error_callback, void *data, unsigned char **uncompressed, size_t *uncompressed_size) { - const b_elf_chdr *chdr; + b_elf_chdr chdr; char *alc; size_t alc_len; unsigned char *po; @@ -5088,27 +5088,30 @@ elf_uncompress_chdr (struct backtrace_state *state, if (compressed_size < sizeof (b_elf_chdr)) return 1; - chdr = (const b_elf_chdr *) compressed; + /* The lld linker can misalign a compressed section, so we can't safely read + the fields directly as we can for other ELF sections. See + https://github.com/ianlancetaylor/libbacktrace/pull/120. */ + memcpy (&chdr, compressed, sizeof (b_elf_chdr)); alc = NULL; alc_len = 0; - if (*uncompressed != NULL && *uncompressed_size >= chdr->ch_size) + if (*uncompressed != NULL && *uncompressed_size >= chdr.ch_size) po = *uncompressed; else { - alc_len = chdr->ch_size; + alc_len = chdr.ch_size; alc = backtrace_alloc (state, alc_len, error_callback, data); if (alc == NULL) return 0; po = (unsigned char *) alc; } - switch (chdr->ch_type) + switch (chdr.ch_type) { case ELFCOMPRESS_ZLIB: if (!elf_zlib_inflate_and_verify (compressed + sizeof (b_elf_chdr), compressed_size - sizeof (b_elf_chdr), - zdebug_table, po, chdr->ch_size)) + zdebug_table, po, chdr.ch_size)) goto skip; break; @@ -5116,7 +5119,7 @@ elf_uncompress_chdr (struct backtrace_state *state, if (!elf_zstd_decompress (compressed + sizeof (b_elf_chdr), compressed_size - sizeof (b_elf_chdr), (unsigned char *)zdebug_table, po, - chdr->ch_size)) + chdr.ch_size)) goto skip; break; @@ -5126,7 +5129,7 @@ elf_uncompress_chdr (struct backtrace_state *state, } *uncompressed = po; - *uncompressed_size = chdr->ch_size; + *uncompressed_size = chdr.ch_size; return 1; @@ -6876,8 +6879,8 @@ elf_add (struct backtrace_state *state, const char *filename, int descriptor, } } - // A debuginfo file may not have a useful .opd section, but we can use the - // one from the original executable. + /* A debuginfo file may not have a useful .opd section, but we can use the + one from the original executable. */ if (opd == NULL) opd = caller_opd;