[PATCH] D30363: COFF ICF: Merge only functions. Do not merge read-only data.

[Rui Ueyama via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL301598: COFF ICF: Merge only functions. Do not merge read-only data. (authored by ruiu).

Changed prior to commit:
  https://reviews.llvm.org/D30363?vs=97015&id=97016#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30363

Files:
  lld/trunk/COFF/ICF.cpp
  lld/trunk/test/COFF/icf-data.test


Index: lld/trunk/test/COFF/icf-data.test
===================================================================
--- lld/trunk/test/COFF/icf-data.test
+++ lld/trunk/test/COFF/icf-data.test
@@ -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
+...
Index: lld/trunk/COFF/ICF.cpp
===================================================================
--- lld/trunk/COFF/ICF.cpp
+++ lld/trunk/COFF/ICF.cpp
@@ -71,10 +71,18 @@
 }
 
 // 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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D30363.97016.patch
Type: text/x-patch
Size: 3289 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170427/0900bd19/attachment.bin>