[LLD] [MinGW] Hook up --icf=safe to -opt:safeicf

[mstorsjo]

Back when the --icf= option was hooked up in the MinGW frontend in LLD, in 2017, lld-link didn't support safe ICF, and mapping it to noicf was suggested in review: https://reviews.llvm.org/D40019

Later in 2021, lld-link did get support for safe ICF in 5bdc5e7 / https://reviews.llvm.org/D97436.

Hook this up for the MinGW frontend as well.

[llvmbot]

@llvm/pr-subscribers-lld
@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-lld-coff

Author: Martin Storsjö (mstorsjo)

Changes

Back when the --icf= option was hooked up in the MinGW frontend in LLD, in 2017, lld-link didn't support safe ICF, and mapping it to noicf was suggested in review: https://reviews.llvm.org/D40019

Later in 2021, lld-link did get support for safe ICF in 5bdc5e7 / https://reviews.llvm.org/D97436.

Hook this up for the MinGW frontend as well.

---

Full diff: https://github.com/llvm/llvm-project/pull/70037.diff

2 Files Affected:

• (modified) lld/MinGW/Driver.cpp (+3-1)
• (modified) lld/test/MinGW/driver.test (+4-2)

diff --git a/lld/MinGW/Driver.cpp b/lld/MinGW/Driver.cpp
index 8fc9ac54d04a2d1..19bf2d1617057eb 100644
--- a/lld/MinGW/Driver.cpp
+++ b/lld/MinGW/Driver.cpp
@@ -375,7 +375,9 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
     StringRef s = a->getValue();
     if (s == "all")
       add("-opt:icf");
-    else if (s == "safe" || s == "none")
+    else if (s == "safe")
+      add("-opt:safeicf");
+    else if (s == "none")
       add("-opt:noicf");
     else
       error("unknown parameter: --icf=" + s);
diff --git a/lld/test/MinGW/driver.test b/lld/test/MinGW/driver.test
index 1fdd494754af4bf..a07c95edb580da1 100644
--- a/lld/test/MinGW/driver.test
+++ b/lld/test/MinGW/driver.test
@@ -205,10 +205,12 @@ GC-SECTIONS: -opt:ref
 RUN: ld.lld -### -m i386pep foo.o 2>&1 | FileCheck -check-prefix ICF-NONE %s
 RUN: ld.lld -### -m i386pep foo.o --icf=none 2>&1 | FileCheck -check-prefix ICF-NONE %s
 RUN: ld.lld -### -m i386pep foo.o -icf=none 2>&1 | FileCheck -check-prefix ICF-NONE %s
-RUN: ld.lld -### -m i386pep foo.o --icf=safe 2>&1 | FileCheck -check-prefix ICF-NONE %s
-RUN: ld.lld -### -m i386pep foo.o -icf=safe 2>&1 | FileCheck -check-prefix ICF-NONE %s
 ICF-NONE: -opt:noicf
 
+RUN: ld.lld -### -m i386pep foo.o --icf=safe 2>&1 | FileCheck -check-prefix ICF-SAFE %s
+RUN: ld.lld -### -m i386pep foo.o -icf=safe 2>&1 | FileCheck -check-prefix ICF-SAFE %s
+ICF-SAFE: -opt:safeicf
+
 RUN: ld.lld -### -m i386pep foo.o --icf=all 2>&1 | FileCheck -check-prefix ICF %s
 RUN: ld.lld -### -m i386pep foo.o -icf=all 2>&1 | FileCheck -check-prefix ICF %s
 RUN: ld.lld -### -m i386pep foo.o --icf all 2>&1 | FileCheck -check-prefix ICF %s

[mstorsjo]

@rnk, back in 2017, https://reviews.llvm.org/D40019?id=122818#inline-349280, you wrote:

I don't think LLD has support for safe ICF. Safe ICF leveraged certain x86_32 ELF relocations that COFF lacks, and failing that, relies on knowledge about the Itanium C++ name mangling rules. LLD doesn't implement anything like that.

As LLD/COFF now does have support for safe ICF since 5bdc5e7, I presume it's safe to use it for the MinGW target as well - as we have the addrsig information encoded in COFF object files these days as well, right? Even if LLD doesn't have any knowledge about the Itanium C++ name mangling (at this level).

[ZequanWu]

Under safe ICF, it currently only checks Microsoft mangled vtable name. You may also want to update that as well to keep the behaviour consistent.

llvm-project/lld/COFF/ICF.cpp

Lines 96 to 98 in 8edcfa0

	// So are vtables.
	if (c->sym && c->sym->getName().starts_with("??_7"))
	return true;

[rnk]

Looks good!

The 2017 comments predate the addrsig tracking mechanisms that @pcc implemented, so this change makes sense today.