[lld-link] Add safe icf mode to lld-link, which does safe icf for all sections.

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

lld/COFF/Config.h

@@ -80,6 +80,13 @@ enum class GuardCFLevel {
   Full,      // Enable all protections.
 };
 
+enum class ICFLevel {
+  None,
+  Safe, // Safe ICF for all sections.
+  All,  // Aggressive ICF for code, but safe ICF for data, similar to MSVC's
+        // behavior.
+};
+
 // Global configuration.
 struct Configuration {
   enum ManifestKind { SideBySide, Embed, No };
@@ -95,7 +102,7 @@ struct Configuration {
   std::string importName;
   bool demangle = true;
   bool doGC = true;
-  bool doICF = true;
+  ICFLevel doICF = ICFLevel::None;
   bool tailMerge;
   bool relocatable = true;
   bool forceMultiple = false;

lld/COFF/Driver.cpp

@@ -1552,8 +1552,9 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
 
   // Handle /opt.
   bool doGC = debug == DebugKind::None || args.hasArg(OPT_profile);
-  unsigned icfLevel =
-      args.hasArg(OPT_profile) ? 0 : 1; // 0: off, 1: limited, 2: on
+  Optional<ICFLevel> icfLevel = None;
+  if (args.hasArg(OPT_profile))
+    icfLevel = ICFLevel::None;
   unsigned tailMerge = 1;
   bool ltoNewPM = LLVM_ENABLE_NEW_PASS_MANAGER;
   bool ltoDebugPM = false;
@@ -1567,9 +1568,11 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
       } else if (s == "noref") {
         doGC = false;
       } else if (s == "icf" || s.startswith("icf=")) {
-        icfLevel = 2;
+        icfLevel = ICFLevel::All;
+      } else if (s == "safeicf") {
+        icfLevel = ICFLevel::Safe;
       } else if (s == "noicf") {
-        icfLevel = 0;
+        icfLevel = ICFLevel::None;
       } else if (s == "lldtailmerge") {
         tailMerge = 2;
       } else if (s == "nolldtailmerge") {
@@ -1601,16 +1604,12 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
     }
   }
 
-  // Limited ICF is enabled if GC is enabled and ICF was never mentioned
-  // explicitly.
-  // FIXME: LLD only implements "limited" ICF, i.e. it only merges identical
-  // code. If the user passes /OPT:ICF explicitly, LLD should merge identical
-  // comdat readonly data.
-  if (icfLevel == 1 && !doGC)
-    icfLevel = 0;
+  if (!icfLevel)
+    icfLevel = doGC ? ICFLevel::All : ICFLevel::None;
   config->doGC = doGC;
-  config->doICF = icfLevel > 0;
-  config->tailMerge = (tailMerge == 1 && config->doICF) || tailMerge == 2;
+  config->doICF = icfLevel.getValue();
+  config->tailMerge =
+      (tailMerge == 1 && config->doICF != ICFLevel::None) || tailMerge == 2;
   config->ltoNewPassManager = ltoNewPM;
   config->ltoDebugPassManager = ltoDebugPM;
 
@@ -1719,8 +1718,8 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
       args.hasFlag(OPT_allowisolation, OPT_allowisolation_no, true);
   config->incremental =
       args.hasFlag(OPT_incremental, OPT_incremental_no,
-                   !config->doGC && !config->doICF && !args.hasArg(OPT_order) &&
-                       !args.hasArg(OPT_profile));
+                   !config->doGC && config->doICF == ICFLevel::None &&
+                       !args.hasArg(OPT_order) && !args.hasArg(OPT_profile));
   config->integrityCheck =
       args.hasFlag(OPT_integritycheck, OPT_integritycheck_no, false);
   config->cetCompat = args.hasFlag(OPT_cetcompat, OPT_cetcompat_no, false);
@@ -1769,7 +1768,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
     config->incremental = false;
   }
 
-  if (config->incremental && config->doICF) {
+  if (config->incremental && config->doICF != ICFLevel::None) {
     warn("ignoring '/incremental' because ICF is enabled; use '/opt:noicf' to "
          "disable");
     config->incremental = false;
@@ -2212,9 +2211,9 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   convertResources();
 
   // Identify identical COMDAT sections to merge them.
-  if (config->doICF) {
+  if (config->doICF != ICFLevel::None) {
     findKeepUniqueSections();
-    doICF(symtab->getChunks());
+    doICF(symtab->getChunks(), config->doICF);
   }
 
   // Write the result.

lld/COFF/ICF.cpp

@@ -40,6 +40,7 @@ static Timer icfTimer("ICF", Timer::root());
 
 class ICF {
 public:
+  ICF(ICFLevel icfLevel) : icfLevel(icfLevel){};
   void run(ArrayRef<Chunk *> v);
 
 private:
@@ -62,6 +63,7 @@ private:
   std::vector<SectionChunk *> chunks;
   int cnt = 0;
   std::atomic<bool> repeat = {false};
+  ICFLevel icfLevel = ICFLevel::All;
 };
 
 // Returns true if section S is subject of ICF.
@@ -81,8 +83,9 @@ bool ICF::isEligible(SectionChunk *c) {
   if (!c->isCOMDAT() || !c->live || writable)
     return false;
 
-  // Code sections are eligible.
-  if (c->getOutputCharacteristics() & llvm::COFF::IMAGE_SCN_MEM_EXECUTE)
+  // Under regular (not safe) ICF, all code sections are eligible.
+  if ((icfLevel == ICFLevel::All) &&
+      c->getOutputCharacteristics() & llvm::COFF::IMAGE_SCN_MEM_EXECUTE)
     return true;
 
   // .pdata and .xdata unwind info sections are eligible.
@@ -314,7 +317,9 @@ void ICF::run(ArrayRef<Chunk *> vec) {
 }
 
 // Entry point to ICF.
-void doICF(ArrayRef<Chunk *> chunks) { ICF().run(chunks); }
+void doICF(ArrayRef<Chunk *> chunks, ICFLevel icfLevel) {
+  ICF(icfLevel).run(chunks);
+}
 
 } // namespace coff
 } // namespace lld

lld/COFF/ICF.h

@@ -9,6 +9,7 @@
 #ifndef LLD_COFF_ICF_H
 #define LLD_COFF_ICF_H
 
+#include "Config.h"
 #include "lld/Common/LLVM.h"
 #include "llvm/ADT/ArrayRef.h"
 
@@ -17,7 +18,7 @@ namespace coff {
 
 class Chunk;
 
-void doICF(ArrayRef<Chunk *> chunks);
+void doICF(ArrayRef<Chunk *> chunks, ICFLevel);
 
 } // namespace coff
 } // namespace lld

lld/test/COFF/icf-safe.s

@@ -3,14 +3,28 @@
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-win32 %S/Inputs/icf-safe.s -o %t2.obj
 # RUN: lld-link /dll /noentry /out:%t.dll /verbose /opt:noref,icf %t1.obj %t2.obj 2>&1 | FileCheck %s
 # RUN: lld-link /dll /noentry /out:%t.dll /verbose /opt:noref,icf /export:g3 /export:g4 %t1.obj %t2.obj 2>&1 | FileCheck --check-prefix=EXPORT %s
+# RUN: lld-link /dll /noentry /out:%t.dll /verbose /opt:noref,safeicf %t1.obj %t2.obj 2>&1 | FileCheck %s --check-prefix=SAFEICF
 
 # CHECK-NOT: Selected
 # CHECK: Selected g3
 # CHECK-NEXT:   Removed g4
+# CHECK: Selected f1
+# CHECK-NEXT:   Removed f2
+# CHECK-NEXT:   Removed f3
+# CHECK-NEXT:   Removed f4
 # CHECK-NOT: Removed
 # CHECK-NOT: Selected
 
-# EXPORT-NOT: Selected
+# EXPORT-NOT: Selected g3
+# EXPORT-NOT: Selected g4
+
+# SAFEICF-NOT: Selected
+# SAFEICF: Selected g3
+# SAFEICF-NEXT:   Removed g4
+# SAFEICF: Selected f3
+# SAFEICF-NEXT:   Removed f4
+# SAFEICF-NOT: Removed
+# SAFEICF-NOT: Selected
 
 .section .rdata,"dr",one_only,g1
 .globl g1
@@ -32,6 +46,28 @@ g3:
 g4:
 .byte 2
 
+.section .text,"xr",one_only,f1
+.globl f1
+f1:
+ nop
+
+.section .text,"xr",one_only,f2
+.globl f2
+f2:
+ nop
+
+.section .text,"xr",one_only,f3
+.globl f3
+f3:
+ nop
+
+.section .text,"xr",one_only,f4
+.globl f4
+f4:
+ nop
+
 .addrsig
 .addrsig_sym g1
 .addrsig_sym g2
+.addrsig_sym f1
+.addrsig_sym f2