[ELF] Allow invalid sh_size%sh_entsize!=0 for non-SHF_MERGE sections

[MaskRay]

Summary

Fixes https://bugs.llvm.org/show_bug.cgi?id=45370
Fixes https://github.com/Clozure/ccl/issues/273

.stab holds a table of 12-byte entries. GNU as before 2.35 incorrectly
sets sh_entsize(.stab) to 20 on 64-bit architectures:
https://sourceware.org/bugzilla/show_bug.cgi?id=25768

We should not emit the confusing error:
"SHF_MERGE section size (...) must be a multiple of sh_entsize (20)

[psmith]

LGTM, thanks for fixing. A minor stylistic comment that me might want to put the check for SHF_MERGE as the first item as I don't think we'd create a merge section for anything else.

[grimar]

In D77368#1959036, @psmith wrote:

A minor stylistic comment that me might want to put the check for SHF_MERGE as the first item as I don't think we'd create a merge section for anything else.

Yeah, sounds like a bit more natural behavior for a function called shouldMerge.

lld/ELF/InputFiles.cpp

452

nit: Perhaps no need to have flags variable? sec.sh_flags should look good when inlined.

lld/test/ELF/invalid/entsize.yaml

2

Perhaps add a bit of information abut why we even care about this case, e.g. link to a GNU as bug?

[MaskRay]

Address comments

[MaskRay]

Update description

lld/ELF/InputFiles.cpp

 //===- InputFiles.cpp -----------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
StringRef ObjFile<ELFT>::getShtGroupSignature(ArrayRef<Elf_Shdr> sections,
   // a bug-compatibility.
   if (signature.empty() && sym.getType() == STT_SECTION)
     return getSectionName(sec);
   return signature;
 }
 
 template <class ELFT>
 bool ObjFile<ELFT>::shouldMerge(const Elf_Shdr &sec, StringRef name) {
+  if (!(sec.sh_flags & SHF_MERGE))
+    return false;
+
   // On a regular link we don't merge sections if -O0 (default is -O1). This
   // sometimes makes the linker significantly faster, although the output will
   // be bigger.
   //
   // Doing the same for -r would create a problem as it would combine sections
   // with different sh_entsize. One option would be to just copy every SHF_MERGE
   // section as is to the output. While this would produce a valid ELF file with
   // usable SHF_MERGE sections, tools like (llvm-)?dwarfdump get confused when
   // they see two .debug_str. We could have separate logic for combining
   // SHF_MERGE sections based both on their name and sh_entsize, but that seems
   // to be more trouble than it is worth. Instead, we just use the regular (-O1)
   // logic for -r.
   if (config->optimize == 0 && !config->relocatable)
     return false;
 
   // A mergeable section with size 0 is useless because they don't have
   // any data to merge. A mergeable string section with size 0 can be
   // argued as invalid because it doesn't end with a null character.
   // We'll avoid a mess by handling them as if they were non-mergeable.
   if (sec.sh_size == 0)
     return false;
 
   // Check for sh_entsize. The ELF spec is not clear about the zero
   // sh_entsize. It says that "the member [sh_entsize] contains 0 if
   // the section does not hold a table of fixed-size entries". We know
   // that Rust 1.13 produces a string mergeable section with a zero
   // sh_entsize. Here we just accept it rather than being picky about it.
   uint64_t entSize = sec.sh_entsize;
   if (entSize == 0)
     return false;
   if (sec.sh_size % entSize)
     fatal(toString(this) + ":(" + name + "): SHF_MERGE section size (" +
           Twine(sec.sh_size) + ") must be a multiple of sh_entsize (" +
           Twine(entSize) + ")");
 
-  uint64_t flags = sec.sh_flags;
-  if (!(flags & SHF_MERGE))
-    return false;
-  if (flags & SHF_WRITE)
+  if (sec.sh_flags & SHF_WRITE)
     fatal(toString(this) + ":(" + name +
           "): writable SHF_MERGE section is not supported");
 
   return true;
 }
 
 // This is for --just-symbols.
 //

lld/test/ELF/invalid/entsize.yaml

+## Ignore invalid sh_entsize (sh_size%sh_entsize!=0) for non-SHF_MERGE sections.
+## GNU as before 2.35 had such a bug for .stab
+## https://sourceware.org/bugzilla/show_bug.cgi?id=25768
+# RUN: yaml2obj %s -o %t.o
+# RUN: ld.lld -e 0 --fatal-warnings %t.o -o /dev/null
+
+!ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name:    .stab
+    Type:    SHT_PROGBITS
+    EntSize: 20
+    Size:    24