From 1c837b5fb192bc54f9be5600fbaf2045e43b0f8e Mon Sep 17 00:00:00 2001
From: Rui Ueyama <ruiu@google.com>
Date: Sat, 10 Jun 2017 00:38:55 +0000
Subject: [PATCH] [ICF] Ignore SHF_GROUP flag when comparing two sections.

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.

Differential Revision: https://reviews.llvm.org/D34074

llvm-svn: 305134
---
 lld/ELF/InputSection.cpp     | 12 +++++++++++-
 lld/ELF/OutputSections.cpp   |  8 ++------
 lld/test/ELF/arm-icf-exidx.s |  4 +++-
 lld/test/ELF/icf-comdat.s    | 23 +++++++++++++++++++++++
 4 files changed, 39 insertions(+), 8 deletions(-)
 create mode 100644 lld/test/ELF/icf-comdat.s

diff --git a/lld/ELF/InputSection.cpp b/lld/ELF/InputSection.cpp
index 085b7504c222..e82f8c3016fa 100644
--- a/lld/ELF/InputSection.cpp
+++ b/lld/ELF/InputSection.cpp
@@ -73,6 +73,16 @@ InputSectionBase::InputSectionBase(InputFile *File, uint64_t Flags,
   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
@@ -95,7 +105,7 @@ 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) {
diff --git a/lld/ELF/OutputSections.cpp b/lld/ELF/OutputSections.cpp
index 47042b6eb66d..008871fd3889 100644
--- a/lld/ELF/OutputSections.cpp
+++ b/lld/ELF/OutputSections.cpp
@@ -313,10 +313,6 @@ void OutputSectionFactory::addInputSec(InputSectionBase *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 +
@@ -333,9 +329,9 @@ void OutputSectionFactory::addInputSec(InputSectionBase *IS,
               "\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);
   }
 
diff --git a/lld/test/ELF/arm-icf-exidx.s b/lld/test/ELF/arm-icf-exidx.s
index cb801b7f2420..6af30285db67 100644
--- a/lld/test/ELF/arm-icf-exidx.s
+++ b/lld/test/ELF/arm-icf-exidx.s
@@ -19,13 +19,15 @@ g:
  .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
diff --git a/lld/test/ELF/icf-comdat.s b/lld/test/ELF/icf-comdat.s
new file mode 100644
index 000000000000..28c0a586bf03
--- /dev/null
+++ b/lld/test/ELF/icf-comdat.s
@@ -0,0 +1,23 @@
+# 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