[PATCH] D27770: [ELF][MIPS] Allow .MIPS.abiflags larger than one Elf_Mips_ABIFlags struct

[Alexander Richardson via Phabricator via llvm-commits]

arichardson added a comment.

In https://reviews.llvm.org/D27770#625122, @ruiu wrote:

> Because this patch is to workaround for a old tool's bug, I want to know how important/severe that is first. We should generally avoid bug compatibility, so it needs a stronger reason than a ABI-compliant feature.


The problem is that this bug happens to be in the default linker for FreeBSD on MIPS (and therefore in many static libraries built with that linker) so for many programs it is not possible to link them using LLD.



================
Comment at: test/ELF/mips-merge-abiflags.s:6
+# RUN: llvm-readobj -file-headers -sections -mips-abi-flags \
+# RUN:   %p/Inputs/mips-concatenated-abiflags.so | FileCheck \
+# RUN:   --check-prefix=CONCAT %s
----------------
atanasyan wrote:
> arichardson wrote:
> > atanasyan wrote:
> > >   # I guess you want to check %t.exe (file produced by LLD) not mips-concatenated-abiflags.so here.
> > >   # It is enough to use the `-mips-abi-flags` flag only and do not check ELF header and sections list.
> > >   # Does LLD really show the "invalid size of .MIPS.abiflags section" error message if concatenated .MIPS.abiflags sections are in a DSO (not a regular object file)?
> > 1. I wanted to check that the .so  file generates  "The .MIPS.abiflags section has a wrong size." and that .MIPS.abiflags size is 48. I guess I can remove the other checks like testing the ELF header
> > 2. I also wanted to test the size of the .MIPS.abiflags, but I guess I can remove testing the header
> > 3. Seems you are right it only happens with .o files that have multiple abiflags structs. Will update the test case. Should I remove the .so file? or also test that linking against such a file works?
> I think we do not need to check .so file at all. Linker never reads an .MIPS.abiflags section from .so file. We need to check that LLD reads .o file with "invalid" .MIPS.abiflags section and generates a valid output.
> 
> I take a look at BFD linker's code. As far as I can see this linker accepts any .MIPS.abiflags sections with size equal or greater than `sizeof(Elf_Mips_ABIFlags)`, but reads and uses only the first `sizeof(Elf_Mips_ABIFlags)` bytes. I agree that LLD should be tolerant to the .MIPS.abiflags section with a size greater than `sizeof(Elf_Mips_ABIFlags)` if the `version` field is valid. Maybe the section just padding by zero bytes for some reasons. But we do not need to create a workaround for "invalid" object files (created by another linker) more complicated than provided by BFD.
> 
> As to me I would check that a section size is equal or greater than `sizeof(Elf_Mips_ABIFlags)`, then check the `version` field, and then read the first `sizeof(Elf_Mips_ABIFlags)` bytes from the section.
Thanks for looking. I'll update the patch to do that.
Should I emit a warning if the size is not equal to sizeof(Elf_Mips_ABIFlags)?


https://reviews.llvm.org/D27770