[Bug libelf/23916] New: [bisected] elifutils-0.175 broke kernel's objtool (elifutils-0.173 works)

[slyfox at inbox dot ru]

https://sourceware.org/bugzilla/show_bug.cgi?id=23916

Bug ID: 23916
Summary: [bisected] elifutils-0.175 broke kernel's objtool
(elifutils-0.173 works)
Product: elfutils
Version: unspecified
Status: UNCONFIRMED
Severity: normal
Priority: P2
Component: libelf
Assignee: unassigned at sourceware dot org
Reporter: slyfox at inbox dot ru
CC: elfutils-devel at sourceware dot org
Target Milestone: ---

Created attachment 11411
--> https://sourceware.org/bugzilla/attachment.cgi?id=11411&action=edit
b.tar.gz

Looks like libelf started adding larger alignment to .debug sections but
something went wrong. Original report: https://bugs.gentoo.org/671760

The symptom: kernel can't be compiled on amd64 against elifutils-0.175:
linux.git $ LANG=C make arch/x86/events/intel/intel-rapl-perf.ko
LD [M] arch/x86/events/intel/intel-rapl-perf.o
ld: arch/x86/events/intel/rapl.o: unable to initialize decompress status
for section .debug_info
....
arch/x86/events/intel/rapl.o: file not recognized: file format not
recognized

Minimal reproducer (against kernel's objtool):

$ cat rapl.c.c
void a(void) {}
$ gcc -nostdinc -g -o rapl.o -c rapl.c.c
$ ./objtool orc generate --module --no-fp --retpoline rapl.o
$ ld -m elf_x86_64 -z max-page-size=0x200000 -r -o intel-rapl-perf.o rapl.o
ld: rapl.o: unable to initialize decompress status for section .debug_info
...
rapl.o: file not recognized: file format not recognized

Bisected down to something plausible:

$ git bisect bad
cf10453f8252df81225796d98548ba6eac113df3 is the first bad commit
commit cf10453f8252df81225796d98548ba6eac113df3
Author: Mark Wielaard <***@klomp.org>
Date: Tue Nov 13 21:18:09 2018 +0100

libelf: Correctly setup alignment of SHF_COMPRESSED section data.

We didn't set the alignment of SHF_COMPRESSED sections correctly.
Those sections start with an Elf(32|64)_Chdr. Make sure sh_addralign
is setup to be able to read such a struct directly. Likewise don't
trust the alignment set on any SHF_COMPRESSED section, but always
make the (raw) compressed data aligned correctly for the reading the
Elf(32|64)_Chdr directly.

Signed-off-by: Mark Wielaard <***@klomp.org>

:040000 040000 99b0262c62d71ac5aaa178519df9a4db917ab1bd
df8299442f6fae9677d31699b88638a325eb9527 M libelf

$ git bisect log
# bad: [628b4a93c6863b9982d817db6acaacbc4e116453] tests: Call test_cleanup in
backtrace-subr.sh check_unsupported.
# good: [aa36de0335e3ce12898954985a208f6336731289] Prepare for 0.173
git bisect start 'master' 'elfutils-0.173'
# good: [2876b3b648f665736ac9c879d34de5e3866ba8f9] Handle ADD/SUB relocations
git bisect good 2876b3b648f665736ac9c879d34de5e3866ba8f9
# good: [f2d59180b90b56b32240f0ba106add050a1b7d09] strip: Extract code to
update shdrstrndx into new common function.
git bisect good f2d59180b90b56b32240f0ba106add050a1b7d09
# good: [d3e6266754b95244063aa1e40c531fdd57259332] strip: Also handle gnu
compressed debug sections with --reloc-debug-sections
git bisect good d3e6266754b95244063aa1e40c531fdd57259332
# good: [d3e6266754b95244063aa1e40c531fdd57259332] strip: Also handle gnu
compressed debug sections with --reloc-debug-sections
git bisect good d3e6266754b95244063aa1e40c531fdd57259332
# bad: [a01938d584b91e747167bb4b3f30ec300c4d6e43] libelf: Mark both fsize and
msize with const attribute.
git bisect bad a01938d584b91e747167bb4b3f30ec300c4d6e43
# bad: [a01938d584b91e747167bb4b3f30ec300c4d6e43] libelf: Mark both fsize and
msize with const attribute.
git bisect bad a01938d584b91e747167bb4b3f30ec300c4d6e43
# bad: [22ec8efc1dd87cdc7892523457eb55990b967224] elflint: Allow
PT_GNU_EH_FRAME segment to match SHT_X86_64_UNWIND section.
git bisect bad 22ec8efc1dd87cdc7892523457eb55990b967224
# bad: [cf10453f8252df81225796d98548ba6eac113df3] libelf: Correctly setup
alignment of SHF_COMPRESSED section data.
git bisect bad cf10453f8252df81225796d98548ba6eac113df3
# first bad commit: [cf10453f8252df81225796d98548ba6eac113df3] libelf:
Correctly setup alignment of SHF_COMPRESSED section data.

[slyfox at inbox dot ru]

https://sourceware.org/bugzilla/show_bug.cgi?id=23916

--- Comment #1 from Sergei Trofimovich <slyfox at inbox dot ru> ---
objtool is a part of linux kernel:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/objtool

Attached b.tar.gz contains:
- objtool binary
- object files before and after mangling with objtool

Note the .debug_info offset and alignment change:

$ readelf -a rapl.o.back > pre
$ readelf -a rapl.o > post
$ diff -U 0 pre post

- [ 4] .debug_info PROGBITS 0000000000000000 00000047
- 000000000000003c 0000000000000000 C 0 0 1
+ [ 4] .debug_info PROGBITS 0000000000000000 00000048
+ 000000000000003c 0000000000000000 C 0 0 8

My wild guess is that section's start is not adjusted correctly.

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=23916

--- Comment #2 from Mark Wielaard <mark at klomp dot org> ---
Thanks. I am fairly sure this is a bug in binutils/bfd.

First gas creates a compressed section with the wrong alignment.
Because a compressed section has a Chdr it needs to be aligned
to either 4 or 8 depending on ELF class (32 or 64 bit). The actual
alignment of the uncompressed section data is contained in the Chdr
as ch_addralign. gas/bfd seems to always just use the same (1) alignment
for both the uncompressed section data and compressed data.

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.

The following (obviously somewhat incorrect, because it just ignores the
alignment completely) patch seems to fix/workaround things:

diff --git a/bfd/bfd.c b/bfd/bfd.c
index 15becd7ae8..9cc05a0174 100644
--- a/bfd/bfd.c
+++ b/bfd/bfd.c
@@ -2404,7 +2404,7 @@ bfd_check_compression_header (bfd *abfd, bfd_byte
*contents,
chdr.ch_addralign = bfd_get_64 (abfd, &echdr->ch_addralign);
}
if (chdr.ch_type == ELFCOMPRESS_ZLIB
- && chdr.ch_addralign == 1U << sec->alignment_power)
+ /* && chdr.ch_addralign == 1U << sec->alignment_power */)
{
*uncompressed_size = chdr.ch_size;
return TRUE;

I'll file a proper bug and patch tomorrow against binutils.

[slyfox at inbox dot ru]

https://sourceware.org/bugzilla/show_bug.cgi?id=23916

--- Comment #3 from Sergei Trofimovich <slyfox at inbox dot ru> ---
Oh, interesting!

On a related note: should eu-elflint complain on originally compiled
rapl.o.back (from tarball)? That would ease detecting non-conformant binutils
versions.

Today no warnings are issued:

$ eu-elflint rapl.o rapl.o.back

rapl.o:
No errors

rapl.o.back:
No errors

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=23916

Mark Wielaard <mark at klomp dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Blocks| |23919


Referenced Bugs:

https://sourceware.org/bugzilla/show_bug.cgi?id=23919
[Bug 23919] bfd doesn't handle ELF compressed data alignment

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=23916

--- Comment #4 from Mark Wielaard <mark at klomp dot org> ---
(In reply to Sergei Trofimovich from comment #3)

Post by slyfox at inbox dot ru
On a related note: should eu-elflint complain on originally compiled
rapl.o.back (from tarball)? That would ease detecting non-conformant
binutils versions.

Yes, that is probably a good idea, I filed two new bugs:

Bug 23920 - elflint should check sanity check Chdr fields and uncompressed data
of compressed sections

Bug 23921 - elflint should check (minimum) section alignment sanity

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=23916

--- Comment #5 from Mark Wielaard <mark at klomp dot org> ---
I filed a binutils bug with patch.
https://sourceware.org/bugzilla/show_bug.cgi?id=23919

If you are able to test patch that on your setup to see if it resolves the
issue with objtool that would be really helpful.