[ICF] Ignore SHF_GROUP flag when comparing two sections.

[ruiu]

Summary

SHF_GROUP bit doesn't make sense in executables or DSOs, so linkers are
expected to remove that bit from section flags. We did that when we create
output sections.

This patch is to do that earlier than before. Now the flag is dropped when
we instantiate input section objects.

This change improves ICF. Previously, two sections that differ only in
SHF_GROUP flag were not merged, because when the control reached ICF,
the flag was still there. Now the flag is dropped before reaching to ICF,
so the difference is ignored naturally.

This issue was found by pcc.

[pcc]

LGTM

lld/ELF/InputSection.cpp

InputSectionBase::InputSectionBase(InputFile *File, uint64_t Flags,
   // The ELF spec states that a value of 0 means the section has
   // no alignment constraits.
   uint32_t V = std::max<uint64_t>(Alignment, 1);
   if (!isPowerOf2_64(V))
     fatal(toString(File) + ": section sh_addralign is not a power of 2");
   this->Alignment = V;
 }
 
+// Drop SHF_GROUP bit unless we are producing a re-linkable object file.
+// SHF_GROUP is a marker that a section belongs to some comdat group.
+// That flag doesn't make sense in an executable.
+static uint64_t getFlags(uint64_t Flags) {
+  Flags &= ~(uint64_t)SHF_INFO_LINK;
+  if (!Config->Relocatable)
+    Flags &= ~(uint64_t)SHF_GROUP;
+  return Flags;
+}
+
 // GNU assembler 2.24 and LLVM 4.0.0's MC (the newest release as of
 // March 2017) fail to infer section types for sections starting with
 // ".init_array." or ".fini_array.". They set SHT_PROGBITS instead of
 // SHF_INIT_ARRAY. As a result, the following assembler directive
 // creates ".init_array.100" with SHT_PROGBITS, for example.
 //
 //   .section .init_array.100, "aw"
 //
 // This function forces SHT_{INIT,FINI}_ARRAY so that we can handle
 // incorrect inputs as if they were correct from the beginning.
 static uint64_t getType(uint64_t Type, StringRef Name) {
   if (Type == SHT_PROGBITS && Name.startswith(".init_array."))
     return SHT_INIT_ARRAY;
   if (Type == SHT_PROGBITS && Name.startswith(".fini_array."))
     return SHT_FINI_ARRAY;
   return Type;
 }
 
 template <class ELFT>
 InputSectionBase::InputSectionBase(elf::ObjectFile<ELFT> *File,
                                    const typename ELFT::Shdr *Hdr,
                                    StringRef Name, Kind SectionKind)
-    : InputSectionBase(File, Hdr->sh_flags & ~SHF_INFO_LINK,
+    : InputSectionBase(File, getFlags(Hdr->sh_flags),
                        getType(Hdr->sh_type, Name), Hdr->sh_entsize,
                        Hdr->sh_link, Hdr->sh_info, Hdr->sh_addralign,
                        getSectionContents(File, Hdr), Name, SectionKind) {
   // We reject object files having insanely large alignments even though
   // they are allowed by the spec. I think 4GB is a reasonable limitation.
   // We might want to relax this in the future.
   if (Hdr->sh_addralign > UINT32_MAX)
     fatal(toString(File) + ": section sh_addralign is too large");

lld/ELF/OutputSections.cpp

 void OutputSectionFactory::addInputSec(InputSectionBase *IS,
                                        StringRef OutsecName,
                                        OutputSection *&Sec) {
   if (!IS->Live) {
     reportDiscarded(IS);
     return;
   }
 
-  uint64_t Flags = IS->Flags;
-  if (!Config->Relocatable)
-    Flags &= ~(uint64_t)SHF_GROUP;
-
   if (Sec) {
     if (getIncompatibleFlags(Sec->Flags) != getIncompatibleFlags(IS->Flags))
       error("incompatible section flags for " + Sec->Name +
             "\n>>> " + toString(IS) + ": 0x" + utohexstr(IS->Flags) +
             "\n>>> output section " + Sec->Name + ": 0x" +
             utohexstr(Sec->Flags));
     if (Sec->Type != IS->Type) {
       if (canMergeToProgbits(Sec->Type) && canMergeToProgbits(IS->Type))
         Sec->Type = SHT_PROGBITS;
       else
         error("section type mismatch for " + IS->Name +
               "\n>>> " + toString(IS) + ": " +
               getELFSectionTypeName(Config->EMachine, IS->Type) +
               "\n>>> output section " + Sec->Name + ": " +
               getELFSectionTypeName(Config->EMachine, Sec->Type));
     }
-    Sec->Flags |= Flags;
+    Sec->Flags |= IS->Flags;
   } else {
-    Sec = make<OutputSection>(OutsecName, IS->Type, Flags);
+    Sec = make<OutputSection>(OutsecName, IS->Type, IS->Flags);
     OutputSections.push_back(Sec);
   }
 
   Sec->addSection(cast<InputSection>(IS));
 }
 
 OutputSectionFactory::~OutputSectionFactory() {}
 

lld/test/ELF/arm-icf-exidx.s

 g:
  .fnstart
  bx      lr
  .fnend
 
  .section .text.h
  .global __aeabi_unwind_cpp_pr0
 __aeabi_unwind_cpp_pr0:
+ nop
  bx lr
 
 // CHECK: Disassembly of section .text:
 // CHECK-NEXT: f:
 // CHECK-NEXT:    11000:        1e ff 2f e1     bx      lr
 // CHECK: __aeabi_unwind_cpp_pr0:
-// CHECK-NEXT:    11004:        1e ff 2f e1     bx      lr
+// CHECK-NEXT:    11004:        00 f0 20 e3     nop
+// CHECK-NEXT:    11008:        1e ff 2f e1     bx      lr
 
 // CHECK: Contents of section .ARM.exidx:
 // CHECK-NEXT:  100d4 2c0f0000 b0b0b080 280f0000 01000000

lld/test/ELF/icf-comdat.s

+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: ld.lld %t -o %t2 --icf=all --verbose | FileCheck %s
+
+# CHECK: selected .text.f1
+# CHECK:   removed .text.f2
+
+.globl _start, f1, f2
+_start:
+  ret
+
+.section .text.f1,"ax"
+f1:
+  mov $60, %rax
+  mov $42, %rdi
+  syscall
+
+.section .text.f2,"axG",@progbits,foo,comdat
+f2:
+  mov $60, %rax
+  mov $42, %rdi
+  syscall