From 5bdc5e7efda4100c4d11085c2da8f1fb932ccce4 Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Fri, 26 Feb 2021 16:38:24 -0800 Subject: [PATCH] [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 | 9 ++++++++- lld/COFF/Driver.cpp | 35 +++++++++++++++++------------------ lld/COFF/ICF.cpp | 11 ++++++++--- lld/COFF/ICF.h | 3 ++- lld/test/COFF/icf-safe.s | 38 +++++++++++++++++++++++++++++++++++++- 5 files changed, 72 insertions(+), 24 deletions(-) diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h index 65ddc32..ba1a579 100644 --- a/lld/COFF/Config.h +++ b/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; diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp index 69ed292..9c13d4a 100644 --- a/lld/COFF/Driver.cpp +++ b/lld/COFF/Driver.cpp @@ -1552,8 +1552,9 @@ void LinkerDriver::linkerMain(ArrayRef 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 = 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 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 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 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 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 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. diff --git a/lld/COFF/ICF.cpp b/lld/COFF/ICF.cpp index 386f861..7326469 100644 --- a/lld/COFF/ICF.cpp +++ b/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 v); private: @@ -62,6 +63,7 @@ private: std::vector chunks; int cnt = 0; std::atomic 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 vec) { } // Entry point to ICF. -void doICF(ArrayRef chunks) { ICF().run(chunks); } +void doICF(ArrayRef chunks, ICFLevel icfLevel) { + ICF(icfLevel).run(chunks); +} } // namespace coff } // namespace lld diff --git a/lld/COFF/ICF.h b/lld/COFF/ICF.h index 0b3c8fa..f8cc807 100644 --- a/lld/COFF/ICF.h +++ b/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 chunks); +void doICF(ArrayRef chunks, ICFLevel); } // namespace coff } // namespace lld diff --git a/lld/test/COFF/icf-safe.s b/lld/test/COFF/icf-safe.s index d16f21f..e3a20ed 100644 --- a/lld/test/COFF/icf-safe.s +++ b/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 -- 2.7.4