COFF ICF: Merge only functions. Do not merge read-only data.
authorRui Ueyama <ruiu@google.com>
Thu, 27 Apr 2017 23:03:22 +0000 (23:03 +0000)
committerRui Ueyama <ruiu@google.com>
Thu, 27 Apr 2017 23:03:22 +0000 (23:03 +0000)
This seems to be the behavior of the MSVC linker. Previously, this
incompatibility caused nasty issues in chromium build a few times.

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

llvm-svn: 301598

lld/COFF/ICF.cpp
lld/test/COFF/icf-data.test [new file with mode: 0644]

index 19468c0..fe59de6 100644 (file)
@@ -71,10 +71,18 @@ uint32_t ICF::getHash(SectionChunk *C) {
 }
 
 // Returns true if section S is subject of ICF.
+//
+// Microsoft's documentation
+// (https://msdn.microsoft.com/en-us/library/bxwfs976.aspx; visited April
+// 2017) says that /opt:icf folds both functions and read-only data.
+// Despite that, the MSVC linker folds only functions. We found
+// a few instances of programs that are not safe for data merging.
+// Therefore, we merge only functions just like the MSVC tool.
 bool ICF::isEligible(SectionChunk *C) {
   bool Global = C->Sym && C->Sym->isExternal();
+  bool Executable = C->getPermissions() & llvm::COFF::IMAGE_SCN_MEM_EXECUTE;
   bool Writable = C->getPermissions() & llvm::COFF::IMAGE_SCN_MEM_WRITE;
-  return C->isCOMDAT() && C->isLive() && Global && !Writable;
+  return C->isCOMDAT() && C->isLive() && Global && Executable && !Writable;
 }
 
 // Split a range into smaller ranges by recoloring sections
diff --git a/lld/test/COFF/icf-data.test b/lld/test/COFF/icf-data.test
new file mode 100644 (file)
index 0000000..20bdeca
--- /dev/null
@@ -0,0 +1,61 @@
+# RUN: yaml2obj < %s > %t.obj
+# RUN: lld-link /entry:foo /out:%t.exe /subsystem:console /include:bar \
+# RUN:   /verbose %t.obj > %t.log 2>&1
+# RUN: FileCheck %s < %t.log
+
+# CHECK-NOT: Removed foo
+# CHECK-NOT: Removed bar
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: []
+sections:
+  - Name:            '.text$mn'
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     4883EC28E8000000004883C428C3
+  - Name:            '.text$mn'
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     4883EC28E8000000004883C428C3
+symbols:
+  - Name:            '.text$mn'
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          14
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        1682752513
+      Number:          0
+      Selection:       IMAGE_COMDAT_SELECT_NODUPLICATES
+  - Name:            '.text$mn'
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          14
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        1682752513
+      Number:          0
+      Selection:       IMAGE_COMDAT_SELECT_NODUPLICATES
+  - Name:            foo
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            bar
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...