elfcopy/strip: Ensure sections have required alignment on output

[emaste]

Summary

Found during exp-run for PR 198611 [1], reported in ELF tool chain ticket 485 [2]. It looks like the root cause is a NASM bug.

I'm not sure about the correct way to handle this case, but an assertion failure from libelf is probably not it. We can try to correct the alignment (as in this change), or refuse to accept the input file.

section_type_alignment is derived from _libelf_falign and _libelf_xlate_shtype. We could consider adding these as extensions to the libelf interface, but it doesn't seem worthwhile when the duplication is relatively small.

[1] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=198611
[2] https://sourceforge.net/p/elftoolchain/tickets/485/

[emaste]

elfcopy/sections.c

871 ↗

(On Diff #4827)

We'd also need a osh.sh_addralign = s->align; here

[kib]

I did not completely deciphered the original problem, there is no concise explanation.

But I do not understand the motivation for the patch. Alignment of the section is not determined by its type, only by the sh_addralign member of the section record. The resulting alignment of the section combined from several sections in object files must be the greatest alignment of pieces.

So I have no idea what check fires in the libelf, but I also do not understand why this check is needed at all.

[emaste]

In D2292#5, @kostikbel wrote:

I did not completely deciphered the original problem, there is no concise explanation.

libelf has checks or assertions on alignment for data types / sections - i.e., that Elf32_Addr is 4-byte aligned, that Elf64_Addr is 8-byte aligned, etc. Running strip on object files from (only) one port triggers either the check (returning an error to strip, which is then reported to the user) or the assertion (and it then aborts).

But I do not understand the motivation for the patch. Alignment of the section is not determined by its type, only by the sh_addralign member of the section record. The resulting alignment of the section combined from several sections in object files must be the greatest alignment of pieces.

Right, alignment is not determined by the type, but the section type implies a certain minimum required alignment, due to the data types it contains - in this case, .rela.text must have at least 8-byte alignment. The file produced in the port has 4 in the sh_addralign member. So there's a bug that needs to be fixed there.

However, such files do exist, leaving us with a choice of one of three options in strip/elfcopy:

1. reject the input file, and force the user to fix it
2. process the file, leaving the misaligned sh_addralign intact
3. increase the alignment to the minimum required

jkoshy@ has a libelf change to fix #1 so the error is reported in all cases, instead of hitting an assertion in 1/2 of them. The patch here implements option #3

For reference, the the offending sections in the input .o are shown below. Note that the sections in the input .o are actually at least 8-byte aligned, even though sh_addralign is 4. I do not know if this is coincidental, or if they are being produced with correct alignment and an incorrect sh_addralign value.

[Nr] Name              Type             Address           Offset
     Size              EntSize          Flags  Link  Info  Align
[ 5] .symtab           SYMTAB           0000000000000000  00002220
     0000000000000870  0000000000000018           6    82     4
[ 7] .rela.text        RELA             0000000000000000  00003600
     0000000000000e70  0000000000000018           5     3     4

here they are in GNU strip 2.17.50's output:

[Nr] Name              Type             Address           Offset
     Size              EntSize          Flags  Link  Info  Align
[ 4] .rela.text        RELA             0000000000000000  000035c8
     0000000000000e70  0000000000000018           6     3     8
[ 6] .symtab           SYMTAB           0000000000000000  00002210
     0000000000000858  0000000000000018           7    81     8

in GNU strip 2.25's output:

[Nr] Name              Type             Address           Offset
     Size              EntSize          Flags  Link  Info  Align
[ 4] .rela.text        RELA             0000000000000000  000033c8
     0000000000000e70  0000000000000018   I       6     3     8
[ 6] .symtab           SYMTAB           0000000000000000  00002010
     0000000000000858  0000000000000018           7    81     8

and in elfcopy's output, with this patch:

[Nr] Name              Type             Address           Offset
     Size              EntSize          Flags  Link  Info  Align
[ 5] .rela.text        RELA             0000000000000000  000035c8
     0000000000000e70  0000000000000018           6     3     8
[ 6] .symtab           SYMTAB           0000000000000000  00002210
     0000000000000858  0000000000000018           7    81     8

[kib]

In D2292#6, @emaste wrote:

In D2292#5, @kostikbel wrote:

I did not completely deciphered the original problem, there is no concise explanation.

libelf has checks or assertions on alignment for data types / sections - i.e., that Elf32_Addr is 4-byte aligned, that Elf64_Addr is 8-byte aligned, etc. Running strip on object files from (only) one port triggers either the check (returning an error to strip, which is then reported to the user) or the assertion (and it then aborts).

But I do not understand the motivation for the patch. Alignment of the section is not determined by its type, only by the sh_addralign member of the section record. The resulting alignment of the section combined from several sections in object files must be the greatest alignment of pieces.

Right, alignment is not determined by the type, but the section type implies a certain minimum required alignment, due to the data types it contains - in this case, .rela.text must have at least 8-byte alignment. The file produced in the port has 4 in the sh_addralign member. So there's a bug that needs to be fixed there.

Hmm, I think I do not agree with this. The specified alignment is only relevant for the final link result. There is nothing wrong for the *object* file to have unaligned relocation section, it is a job of the static linker to properly align it in the final object. The standard talks about applicability of sh_addralign to sh_addr, and sh_addr specifies memory address of the section in the final image (if not zero).

However, such files do exist, leaving us with a choice of one of three options in strip/elfcopy:

1. reject the input file, and force the user to fix it
2. process the file, leaving the misaligned sh_addralign intact

I think #2 is the right approach for the *object* file.

3. increase the alignment to the minimum required

[emaste]

In D2292#7, @kostikbel wrote:

There is nothing wrong for the *object* file to have unaligned relocation section, it is a job of the static linker to properly align it in the final object.

Ok, but it's the opposite case here: the relocation section is actually 16-byte aligned, but sh_addralign is only 4.

NASM bug with patch filed here: http://bugzilla.nasm.us/show_bug.cgi?id=3392307

I still think it makes sense for us to follow GNU strip's (likely BFD's) behaviour and increase the sh_addralign in the output file.

[emaste]

In discussion with jkoshy it sounds like he'll add an extension API to libelf to return appropriate alignment for a given section/type, and I'll update this change to use that, once it's done.

[emaste]

Actually set alignment in output

[emaste]

It may be that the right approach is to just have elfcopy/strip fail on such broken input and we should just drop this patch.