[ELF] SHF_MERGE section with 0 entsize is not fatal

[emaste]

Summary

For now just treat such sections as non-mergeable.

Originally in D18222, now with test case update.

[emaste]

test/ELF/merge-zero-size.s

1 ↗

(On Diff #50870)

It looks like llvm-mc will not allow me to generate a section with sh_entsize 0 so using the existing ELF object seems reasonable. Perhaps I should rename it to merge-shentsize-zero.elf though, and this test file needs to be renamed with a .test extension instead of .s.

[grimar]

That is LGTM. Lets see what Rui think.

[ruiu]

LGTM

[grimar]

test/ELF/merge-zero-size.s

1 ↗

(On Diff #50870)

Agree with rename to merge-shentsize-zero.elf. Don't see reasons to rename to .test instead of .s though.

[emaste]

test/ELF/merge-zero-size.s

1 ↗

(On Diff #50870)

because the existing .s tests are assembly source and invoke llvm-mc (with two exceptions), and the .test tests act on existing objects etc.

[grimar]

test/ELF/merge-zero-size.s

1 ↗

(On Diff #50870)

Not sure such rule exist. For example llvm33-rela-outside-group.s uses llvm33-rela-outside-group.o

[emaste]

test/ELF/merge-zero-size.s

1 ↗

(On Diff #50870)

Ok, but it still contains asm source code. The test file I moved it out of is named invalid-elf.test. Anyway, I don't think it matters and is now committed.

lld/trunk/ELF/InputFiles.cpp

 template <class ELFT> static bool shouldMerge(const typename ELFT::Shdr &Sec) {
   typedef typename ELFT::uint uintX_t;
   uintX_t Flags = Sec.sh_flags;
   if (!(Flags & SHF_MERGE))
     return false;
   if (Flags & SHF_WRITE)
     fatal("writable SHF_MERGE sections are not supported");
   uintX_t EntSize = Sec.sh_entsize;
-  if (!EntSize || Sec.sh_size % EntSize)
+  if (!EntSize)
+    return false;
+  if (Sec.sh_size % EntSize)
     fatal("SHF_MERGE section size must be a multiple of sh_entsize");
 
   // Don't try to merge if the aligment is larger than the sh_entsize and this
   // is not SHF_STRINGS.
   //
   // Since this is not a SHF_STRINGS, we would need to pad after every entity.
   // It would be equivalent for the producer of the .o to just set a larger
   // sh_entsize.

lld/trunk/test/ELF/invalid-elf.test

 # RUN: not ld.lld %p/Inputs/invalid-section-index.elf -o %t2 2>&1 | \
 # RUN:   FileCheck --check-prefix=INVALID-SECTION-INDEX-LLD %s
 # INVALID-SECTION-INDEX-LLD: invalid section index
 
 # RUN: not ld.lld %p/Inputs/invalid-shstrndx.so -o %t2 2>&1 | \
 # RUN:   FileCheck --check-prefix=INVALID-SECTION-INDEX %s
 # INVALID-SECTION-INDEX: Invalid section index
 
-# RUN: not ld.lld %p/Inputs/invalid-shentsize-zero.elf -o %t2 2>&1 | \
-# RUN:   FileCheck --check-prefix=INVALID-SHENTSIZE-ZERO %s
-# INVALID-SHENTSIZE-ZERO: SHF_MERGE section size must be a multiple of sh_entsize
-
 # RUN: not ld.lld %p/Inputs/invalid-multiple-eh-relocs.elf -o %t2 2>&1 | \
 # RUN:   FileCheck --check-prefix=INVALID-EH-RELOCS %s
 # INVALID-EH-RELOCS: multiple relocation sections to .eh_frame are not supported
 
 .long foo

lld/trunk/test/ELF/merge-zero-size.test

+# RUN: ld.lld %p/Inputs/merge-shentsize-zero.elf -o %t2 2>&1 | \
+# RUN:   FileCheck -allow-empty %s
+# CHECK-NOT: SHF_MERGE section size must be a multiple of sh_entsize
+