+++ This bug was initially created as a clone of Bug #23916 +++ The ELF compression header has a field (ch_addralign) that is set to the alignment of the uncompressed section. This way the section itself can have a different alignment than the decompressed section. bfd (and readelf) however explicitly make the section sh_addralign and ch_addralign equal. It also always sets the alignment to 1 which is wrong when using a Elf32_Chdr (which has alignment of 4) or Elf64_Chdr (which has alignment of 8). This shows up with tools that use elfutils libelf which sets up the alignment correctly. First gas creates a compressed section with on alignment of 1. Second libelf accepts this, but corrects the alignment when it writes out the section. Third bfd_check_compression_header sanity checks the section alignment, but it checks that the compressed and decompressed alignment is equal?!? I think it wanted to check that the alignment is a power of 2 instead.
Created attachment 11413 [details] Proposed patch to handle compressed section alignment correctly The attached git format-patch resolved this issue by handling the (de)compressed section alignment correctly.
The master branch has been updated by Nick Clifton <nickc@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=4207142d6a5d2359170c5f9a140fc1a2351fbda9 commit 4207142d6a5d2359170c5f9a140fc1a2351fbda9 Author: Mark Wielaard <mark@klomp.org> Date: Tue Nov 27 11:59:10 2018 +0000 Handle ELF compressed header alignment correctly by setting up the section alignment correctly for the Elf32_Chdr or Elf64_Chdr type and respect the ch_addralign field when decompressing the section data. PR binutils/23919 binutils* readelf.c (dump_sections_as_strings): Remove bogus addralign check. (dump_sections_as_bytes): Likewise. (load_specific_debug_sections): Likewise. * testsuite/binutils-all/dw2-3.rS: Adjust alignment. * testsuite/binutils-all/dw2-3.rt: Likewise. bfd * bfd.c (bfd_update_compression_header): Explicitly set alignment. (bfd_check_compression_header): Add uncompressed_alignment_power argument. Check ch_addralign is a power of 2. * bfd-in2.h: Regenerated. * compress.c (bfd_compress_section_contents): Get and set orig_uncompressed_alignment_pow if section is decompressed. (bfd_is_section_compressed_with_header): Add and get uncompressed_align_pow_p argument. (bfd_is_section_compressed): Add uncompressed_align_power argument to bfd_is_section_compressed_with_header call. (bfd_init_section_decompress_status): Get and set uncompressed_alignment_power. * elf.c (_bfd_elf_make_section_from_shdr): Add uncompressed_align_power argument to bfd_is_section_compressed_with_header call.
Hi Mark, Thanks very much for the bug report, and especially for a patch to fix it! I have applied the patch, so I hope that this problem is now resolved. One very minor point - in the future, would you mind providing the ChangeLog entries as plain text, rather than a context diff ? It did not matter this time, but often the diff will not apply because the changelog has changed by the time that the patch is applied. Cheers Nick
(In reply to Nick Clifton from comment #3) > Thanks very much for the bug report, and especially for a patch to fix it! > I have applied the patch, so I hope that this problem is now resolved. Thanks! > One very minor point - in the future, would you mind providing the > ChangeLog entries as plain text, rather than a context diff ? It > did not matter this time, but often the diff will not apply because > the changelog has changed by the time that the patch is applied. OK, I will in the future. Do you have some script that helps you handle separate ChangeLog entries? Note that gnulib contains some helpers for merging ChangeLog entries that I have used somewhat successfully with mercurial and git. They work the other way around though, you explicitly do add the ChangeLog entries in the commit/ChangeLog file, but when a merge action takes place they handle the entry specially. https://gnu.wildebeest.org/blog/mjw/2012/03/16/automagically-merging-changelog-files-with-mercurial-or-git/
On Linux/x86-64, I got /export/build/gnu/binutils/build-x86_64-linux/binutils/objcopy -O elf32-x86-64 --compress-debug-sections=zlib-gabi tmpdir/dw2-3.o tmpdir/debug_str.copy.o Executing on host: /export/build/gnu/binutils/build-x86_64-linux/binutils/objcopy -O elf32-x86-64 --compress-debug-sections=zlib-gabi tmpdir/dw2-3.o tmpdir/debug_str.copy.o (timeout = 300) spawn -ignore SIGHUP /export/build/gnu/binutils/build-x86_64-linux/binutils/objcopy -O elf32-x86-64 --compress-debug-sections=zlib-gabi tmpdir/dw2-3.o tmpdir/debug_str.copy.o^M /export/build/gnu/binutils/build-x86_64-linux/binutils/objcopy: tmpdir/debug_str.copy.o: error: alignment power 32767 of section `.zdebug_line' is too big^M /export/build/gnu/binutils/build-x86_64-linux/binutils/objcopy:tmpdir/debug_str.copy.o[.text]: file in wrong format^M /export/build/gnu/binutils/build-x86_64-linux/binutils/objcopy: tmpdir/debug_str.copy.o: error: alignment power 32767 of section `.zdebug_line' is too big /export/build/gnu/binutils/build-x86_64-linux/binutils/objcopy:tmpdir/debug_str.copy.o[.text]: file in wrong format FAIL: objcopy (Convert x86-64 object with zlib-gnu to x32 (2))
The master branch has been updated by H.J. Lu <hjl@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=131a5a648d314cd15811158150573cb40eb3abd0 commit 131a5a648d314cd15811158150573cb40eb3abd0 Author: H.J. Lu <hjl.tools@gmail.com> Date: Tue Nov 27 06:02:36 2018 -0800 Initialize *uncompressed_align_pow_p to 0 Initialize *uncompressed_align_pow_p to 0 since *uncompressed_align_pow_p is passed to bfd_is_section_compressed_with_header as uninitialized, PR binutils/23919 * compress.c (bfd_is_section_compressed_with_header): Initialize *uncompressed_align_pow_p to 0.
Fixed for 2.32.
When we backported these fixes to our 2_31 based branch, we got a surprising regression in gold/testsuite: $ cat gold/testsuite/debug_msg.sh.log Did not find expected error in debug_msg_so.err: debug_msg.so: error: undefined reference to 'undef_fn1()' Actual error output below: gcctestdir/ld: internal error in read_header_prolog, at dwarf_reader.cc:1678 collect2: error: ld returned 1 exit status FAIL debug_msg.sh (exit status: 1)
This change triggered gcctestdir/collect-ld: warning: gdb_index_test_cdebug_gabi.o: section .debug_str contains incorrectly aligned strings; the alignment of those strings won't be preserved gcctestdir/collect-ld: warning: gdb_index_test_3.o: section .debug_str contains incorrectly aligned strings; the alignment of those strings won't be preserved gcctestdir/collect-ld: error: treating warnings as errors collect2: error: ld returned 1 exit status make[3]: *** [gdb_index_test_3] Error 1 in gold tests.
(In reply to cvs-commit@gcc.gnu.org from comment #6) > The master branch has been updated by H.J. Lu <hjl@sourceware.org>: > > https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git; > h=131a5a648d314cd15811158150573cb40eb3abd0 > > commit 131a5a648d314cd15811158150573cb40eb3abd0 > Author: H.J. Lu <hjl.tools@gmail.com> > Date: Tue Nov 27 06:02:36 2018 -0800 > > Initialize *uncompressed_align_pow_p to 0 > > Initialize *uncompressed_align_pow_p to 0 since *uncompressed_align_pow_p > is passed to bfd_is_section_compressed_with_header as uninitialized, > > PR binutils/23919 > * compress.c (bfd_is_section_compressed_with_header): Initialize > *uncompressed_align_pow_p to 0. I think this is correct, the uncompressed_align_pow_p wasn't set explicitly when the compressed section was gnu-zlib, in which case the alignment has to be assumed to be always 1.
(In reply to H.J. Lu from comment #9) > This change triggered > > gcctestdir/collect-ld: warning: gdb_index_test_cdebug_gabi.o: section > .debug_str contains incorrectly aligned strings; the alignment of those > strings won't be preserved > gcctestdir/collect-ld: warning: gdb_index_test_3.o: section .debug_str > contains incorrectly aligned strings; the alignment of those strings won't > be preserved > gcctestdir/collect-ld: error: treating warnings as errors > collect2: error: ld returned 1 exit status > make[3]: *** [gdb_index_test_3] Error 1 > > in gold tests. I think this comes from do_add_input_section in gold/merge.cc. It decompresses the section, but doesn't use ch_addralign, but this->addralign(), when calculating the alignment constraint.
The master branch has been updated by H.J. Lu <hjl@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=5f6c22aee74f17393b82934a5682d985672e011a commit 5f6c22aee74f17393b82934a5682d985672e011a Author: H.J. Lu <hjl.tools@gmail.com> Date: Sun Dec 2 05:42:36 2018 -0800 gold: Get alignment of uncompressed section from ch_addralign The ELF compression header has a field (ch_addralign) that is set to the alignment of the uncompressed section. This way the section itself can have a different alignment than the decompressed section. Update decompress_input_section to get alignment of the decompressed section and use it when merging decompressed strings. PR binutils/23919 * merge.cc (Output_merge_string<Char_type>::do_add_input_section): Get addralign from decompressed_section_contents. * object.cc (build_compressed_section_map): Set info.addralign. (Object::decompressed_section_contents): Add a palign argument and store p->second.addralign in *palign if it isn't NULL. * object.h (Compressed_section_info): Add addralign. (section_is_compressed): Add a palign argument, default it to NULL, store p->second.addralign in *palign if it isn't NULL. (Object::decompressed_section_contents): Likewise. * output.cc (Output_section::add_input_section): Get addralign from section_is_compressed.
The binutils-2_31-branch branch has been updated by Nick Clifton <nickc@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=88739f776b733b0b84600b283417f862a010bb5d commit 88739f776b733b0b84600b283417f862a010bb5d Author: Nick Clifton <nickc@redhat.com> Date: Mon Feb 18 15:08:57 2019 +0000 Import patch to fix PR23919 from the mainline. PR binutils/23919 bfd * bfd.c (bfd_update_compression_header): Explicitly set alignment. (bfd_check_compression_header): Add uncompressed_alignment_power argument. Check ch_addralign is a power of 2. * bfd-in2.h: Regenerated. * compress.c (bfd_compress_section_contents): Get and set orig_uncompressed_alignment_pow if section is decompressed. (bfd_is_section_compressed_with_header): Add and get uncompressed_align_pow_p argument. (bfd_is_section_compressed): Add uncompressed_align_power argument to bfd_is_section_compressed_with_header call. (bfd_init_section_decompress_status): Get and set uncompressed_alignment_power. * elf.c (_bfd_elf_make_section_from_shdr): Add uncompressed_align_power argument to bfd_is_section_compressed_with_header call. * compress.c (bfd_is_section_compressed_with_header): Initialize * uncompressed_align_pow_p to 0. binutils* readelf.c (dump_sections_as_strings): Remove bogus addralign check. (dump_sections_as_bytes): Likewise. (load_specific_debug_sections): Likewise. * testsuite/binutils-all/dw2-3.rS: Adjust alignment. * testsuite/binutils-all/dw2-3.rt: Likewise. gold * merge.cc (Output_merge_string<Char_type>::do_add_input_section): Get addralign from decompressed_section_contents. * object.cc (build_compressed_section_map): Set info.addralign. (Object::decompressed_section_contents): Add a palign argument and store p->second.addralign in *palign if it isn't NULL. * object.h (Compressed_section_info): Add addralign. (section_is_compressed): Add a palign argument, default it to NULL, store p->second.addralign in *palign if it isn't NULL. (Object::decompressed_section_contents): Likewise. * output.cc (Output_section::add_input_section): Get addralign from section_is_compressed.
The upstream/gdb-8.2-branch branch has been updated by Pedro Alves <palves@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=6d33d996167a33552b68c036f1b1571a208ace42 commit 6d33d996167a33552b68c036f1b1571a208ace42 Author: Nick Clifton <nickc@redhat.com> Date: Tue Feb 26 19:57:46 2019 +0000 Import patch to fix PR23919 from the mainline. PR binutils/23919 bfd * bfd.c (bfd_update_compression_header): Explicitly set alignment. (bfd_check_compression_header): Add uncompressed_alignment_power argument. Check ch_addralign is a power of 2. * bfd-in2.h: Regenerated. * compress.c (bfd_compress_section_contents): Get and set orig_uncompressed_alignment_pow if section is decompressed. (bfd_is_section_compressed_with_header): Add and get uncompressed_align_pow_p argument. (bfd_is_section_compressed): Add uncompressed_align_power argument to bfd_is_section_compressed_with_header call. (bfd_init_section_decompress_status): Get and set uncompressed_alignment_power. * elf.c (_bfd_elf_make_section_from_shdr): Add uncompressed_align_power argument to bfd_is_section_compressed_with_header call. * compress.c (bfd_is_section_compressed_with_header): Initialize * uncompressed_align_pow_p to 0. binutils* readelf.c (dump_sections_as_strings): Remove bogus addralign check. (dump_sections_as_bytes): Likewise. (load_specific_debug_sections): Likewise. * testsuite/binutils-all/dw2-3.rS: Adjust alignment. * testsuite/binutils-all/dw2-3.rt: Likewise. gold * merge.cc (Output_merge_string<Char_type>::do_add_input_section): Get addralign from decompressed_section_contents. * object.cc (build_compressed_section_map): Set info.addralign. (Object::decompressed_section_contents): Add a palign argument and store p->second.addralign in *palign if it isn't NULL. * object.h (Compressed_section_info): Add addralign. (section_is_compressed): Add a palign argument, default it to NULL, store p->second.addralign in *palign if it isn't NULL. (Object::decompressed_section_contents): Likewise. * output.cc (Output_section::add_input_section): Get addralign from section_is_compressed.
The gdb-8.2-branch branch has been updated by Pedro Alves <palves@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=6d33d996167a33552b68c036f1b1571a208ace42 commit 6d33d996167a33552b68c036f1b1571a208ace42 Author: Nick Clifton <nickc@redhat.com> Date: Tue Feb 26 19:57:46 2019 +0000 Import patch to fix PR23919 from the mainline. PR binutils/23919 bfd * bfd.c (bfd_update_compression_header): Explicitly set alignment. (bfd_check_compression_header): Add uncompressed_alignment_power argument. Check ch_addralign is a power of 2. * bfd-in2.h: Regenerated. * compress.c (bfd_compress_section_contents): Get and set orig_uncompressed_alignment_pow if section is decompressed. (bfd_is_section_compressed_with_header): Add and get uncompressed_align_pow_p argument. (bfd_is_section_compressed): Add uncompressed_align_power argument to bfd_is_section_compressed_with_header call. (bfd_init_section_decompress_status): Get and set uncompressed_alignment_power. * elf.c (_bfd_elf_make_section_from_shdr): Add uncompressed_align_power argument to bfd_is_section_compressed_with_header call. * compress.c (bfd_is_section_compressed_with_header): Initialize * uncompressed_align_pow_p to 0. binutils* readelf.c (dump_sections_as_strings): Remove bogus addralign check. (dump_sections_as_bytes): Likewise. (load_specific_debug_sections): Likewise. * testsuite/binutils-all/dw2-3.rS: Adjust alignment. * testsuite/binutils-all/dw2-3.rt: Likewise. gold * merge.cc (Output_merge_string<Char_type>::do_add_input_section): Get addralign from decompressed_section_contents. * object.cc (build_compressed_section_map): Set info.addralign. (Object::decompressed_section_contents): Add a palign argument and store p->second.addralign in *palign if it isn't NULL. * object.h (Compressed_section_info): Add addralign. (section_is_compressed): Add a palign argument, default it to NULL, store p->second.addralign in *palign if it isn't NULL. (Object::decompressed_section_contents): Likewise. * output.cc (Output_section::add_input_section): Get addralign from section_is_compressed.