Sources Bugzilla – Bug 12921
sh_offset for SHT_NOBITS sections
Last modified: 2011-06-24 14:03:19 UTC
A recent change in ld broke prelink. E>g. on the attached testcase: struct A { char a; struct A *b; int *c; }; static struct A local = { 77, &local, 0 }; int vbss[16384] __attribute__((aligned (4096))); int vdata __attribute__((aligned (4096))) = 5; asm (".text; .balign 4096; vtext:; .previous"); int main() { asm volatile ("" : : "m" (local), "m" (vbss[0]), "m" (vdata)); return 0; } we used to emit (e.g. in 2.21.0.51.0.6, but like since forever): [24] .data PROGBITS 0000000000603000 003000 001028 00 WA 0 0 4096 [25] .bss NOBITS 0000000000605000 004028 011000 00 WA 0 0 4096 [26] .comment PROGBITS 0000000000000000 004028 000058 01 MS 0 0 1 [27] .shstrtab STRTAB 0000000000000000 004080 0000ee 00 0 0 1 but now (2.21.52.0.1) we emit: [24] .data PROGBITS 0000000000603000 003000 002004 00 WA 0 0 4096 [25] .bss NOBITS 0000000000606000 006000 011000 00 WA 0 0 4096 [26] .comment PROGBITS 0000000000000000 005004 000059 01 MS 0 0 1 [27] .shstrtab STRTAB 0000000000000000 00505d 0000ee 00 0 0 1 The problem is that the sh_offset for .bss is no longer smaller or equal to the next section's sh_offset, but, what's more important, is that it is even way after the end of the file on disk. When elfutils libelf tries to write this during prelink --undo, it will write 0x6000 bytes of file instead of just 0x505d+0xee the original file had. Can this change be please reverted?
This is most likely http://sourceware.org/ml/binutils/2011-05/msg00299.html Why on earth does prelink write to a SHT_NOBITS section? Incidentally, we've put bss sh_offset potentially past end of file for a very long time. The recent change just made it visible in cases where the bss section got its own PT_LOAD header (due to alignment).
No, that last comment was wrong. It wasn't sh_offset but header p_offset that could be past end of file.
The intention of prelink --undo is that it reverts the binary/shared library to the original, bitwise, content. During prelinking the binary/shared library usually grows, and on undo prelink feeds the saved ElfXX_Shdr/ElfXX_Phdr etc. content to libelf to restore the file. And, apparently sh_offset beyond end of file mean at least for elfutils libelf that it pads with zeros up to that spot. Of course one could probably tweak libelf (haven't tried the other implementation how it behaves nor looked how hard would it be), but the question is why is such an sh_offset desirable. When the section has no content in the file (SHT_NOBITS or zero sized section), why can't sh_offset simply be what it used to (either equal to sh_offset of the next section, or of last section + its in the file size)?
(In reply to comment #3) > The intention of prelink --undo is that it reverts the binary/shared library to > the original, bitwise, content. During prelinking the binary/shared library > usually grows, and on undo prelink feeds the saved ElfXX_Shdr/ElfXX_Phdr etc. > content to libelf to restore the file. And, apparently sh_offset beyond end of > file mean at least for elfutils libelf that it pads with zeros up to that spot. > It is too bad that we didn't have a testcase binutils so that we would have prevented it from happening. When this bug is fixed, we should also add a testcase.
Sure, at least it got caught by prelink testsuite.
Actually older binutils do give cases of sh_offset past end of file. ld/testsuite/ld-elf/tbss3.s assembled and linked with older binutils gives .tbss with sh_offset past end of file. The same file s/tbss/bss/ and correct .bss section flags and you'll see the same thing with .bss. Now, if I revert the elf.c part of http://sourceware.org/ml/binutils/2011-05/msg00299.html you'll have what you want for your testcase, but then in the tbss3.s case .tbss will have sh_offset of zero and PT_TLS segment will have p_offset of zero. Not that there is anything really wrong with a zero for sh_offset of SHT_NOBITS sections or zero p_offset for zero p_filesz headers. In fact that would be the logical value to choose for a section/header that doesn't have contents in the file. I wonder what would break if we did that for all such sections and headers? Probably quite a lot, which is why I was trying to set sh_offset to as unsurprising a value as I thought possible. I chose such that sh_addr % sh_align == sh_offset % sh_align to make the tbss3 testcase give the same sh_offset and p_offset as older linkers.
Created attachment 5813 [details] possible fix I'll run the testsuite overnight with my big list of targets to see how this goes.
(In reply to comment #7) > Created attachment 5813 [details] > possible fix > > I'll run the testsuite overnight with my big list of targets to see how this > goes. We also need a testcase for this bug.
CVSROOT: /cvs/src Module name: src Changes by: amodra@sourceware.org 2011-06-24 03:36:40 Modified files: bfd : ChangeLog elf.c Log message: PR ld/12921 * elf.c (assign_file_positions_for_load_sections): Don't align sh_offset for all SHT_NOBITS sections here, just .tbss sections that don't get a PT_LOAD. Patches: http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/ChangeLog.diff?cvsroot=src&r1=1.5399&r2=1.5400 http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf.c.diff?cvsroot=src&r1=1.541&r2=1.542
CVSROOT: /cvs/src Module name: src Branch: binutils-2_21-branch Changes by: amodra@sourceware.org 2011-06-24 03:39:32 Modified files: bfd : ChangeLog elf.c Log message: PR ld/12921 * elf.c (assign_file_positions_for_load_sections): Don't align sh_offset for all SHT_NOBITS sections here, just .tbss sections that don't get a PT_LOAD. Patches: http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/ChangeLog.diff?cvsroot=src&only_with_tag=binutils-2_21-branch&r1=1.5180.2.36&r2=1.5180.2.37 http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf.c.diff?cvsroot=src&only_with_tag=binutils-2_21-branch&r1=1.524.2.3&r2=1.524.2.4
Fixed
CVSROOT: /cvs/src Module name: src Changes by: hjl@sourceware.org 2011-06-24 14:03:16 Modified files: ld/testsuite : ChangeLog ld/testsuite/ld-i386: i386.exp ld/testsuite/ld-x86-64: x86-64.exp Added files: ld/testsuite/ld-i386: pr12921.d pr12921.s ld/testsuite/ld-x86-64: pr12921.d pr12921.s Log message: Add testcases for PR ld/12921. 2011-06-24 H.J. Lu <hongjiu.lu@intel.com> PR ld/12921 * ld-i386/i386.exp: Run pr12921. * ld-x86-64/x86-64.exp: Likewise. * ld-i386/pr12921.d: New. * ld-i386/pr12921.s: Likewise. * ld-x86-64/pr12921.d: Likewise. * ld-x86-64/pr12921.s: Likewise. Patches: http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ChangeLog.diff?cvsroot=src&r1=1.1433&r2=1.1434 pr12921.d.diff?cvsroot=src&r1=NONE&r2=1.1">http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-i386/pr12921.d.diff?cvsroot=src&r1=NONE&r2=1.1 pr12921.s.diff?cvsroot=src&r1=NONE&r2=1.1">http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-i386/pr12921.s.diff?cvsroot=src&r1=NONE&r2=1.1 http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-i386/i386.exp.diff?cvsroot=src&r1=1.39&r2=1.40 pr12921.d.diff?cvsroot=src&r1=NONE&r2=1.1">http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-x86-64/pr12921.d.diff?cvsroot=src&r1=NONE&r2=1.1 pr12921.s.diff?cvsroot=src&r1=NONE&r2=1.1">http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-x86-64/pr12921.s.diff?cvsroot=src&r1=NONE&r2=1.1 http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-x86-64/x86-64.exp.diff?cvsroot=src&r1=1.34&r2=1.35