[BOLT] Add support for safe-icf

bolt/include/bolt/Core/BinaryFunction.h

@@ -2103,9 +2103,9 @@ class BinaryFunction {
   // adjustments.
   void handleAArch64IndirectCall(MCInst &Instruction, const uint64_t Offset);
 
-  /// Processes code section to identify function references.
-  void processInstructionsForFuncReferences(BinaryContext &BC,
-                                            const MCInst &Inst);
+  /// Analyze instruction to identify a function references.
+  void analyzeInstructionForFuncReference(BinaryContext &BC,
+                                          const MCInst &Inst);
 
   /// Scan function for references to other functions. In relocation mode,
   /// add relocations for external references. In non-relocation mode, detect

bolt/include/bolt/Passes/IdenticalCodeFolding.h

@@ -52,7 +52,11 @@ class IdenticalCodeFolding : public BinaryFunctionPass {
     return VtableBitVector.test(Address / 8);
   }
   /// Initialize bit vector of memory addresses of vtables.
-  void initVtable() { VtableBitVector.resize((((uint64_t)1) << 32) / 8); }
+  void initVtable() {
+    constexpr uint64_t PotentialRelocationAddressRange =
+        (((uint64_t)1) << 32) / 8;
+    VtableBitVector.resize(PotentialRelocationAddressRange);
+  }
   /// Mark memory address of vtable as used.
   void setAddressUsedInVTable(uint64_t Address) {
     VtableBitVector.set(Address / 8);

bolt/lib/Core/BinaryFunction.cpp

@@ -1513,17 +1513,17 @@ MCSymbol *BinaryFunction::registerBranch(uint64_t Src, uint64_t Dst) {
   return Target;
 }
 
-void BinaryFunction::processInstructionsForFuncReferences(BinaryContext &BC,
-                                                          const MCInst &Inst) {
+void BinaryFunction::analyzeInstructionForFuncReference(BinaryContext &BC,
+                                                        const MCInst &Inst) {
   for (const MCOperand &Op : MCPlus::primeOperands(Inst)) {
     if (!Op.isExpr())
       continue;
     const MCExpr &Expr = *Op.getExpr();
-    if (Expr.getKind() == MCExpr::SymbolRef) {
-      const MCSymbol &Symbol = cast<MCSymbolRefExpr>(Expr).getSymbol();
-      if (BinaryFunction *BF = BC.getFunctionForSymbol(&Symbol))
-        BF->setHasAddressTaken(true);
-    }
+    if (Expr.getKind() != MCExpr::SymbolRef)
+      continue;
+    const MCSymbol &Symbol = cast<MCSymbolRefExpr>(Expr).getSymbol();
+    if (BinaryFunction *BF = BC.getFunctionForSymbol(&Symbol))
+      BF->setHasAddressTaken(true);
   }
 }
 
@@ -1648,7 +1648,7 @@ bool BinaryFunction::scanExternalRefs() {
       // Skip assembly if the instruction may not have any symbolic operands.
       continue;
     } else {
-      processInstructionsForFuncReferences(BC, Instruction);
+      analyzeInstructionForFuncReference(BC, Instruction);
     }
 
     // Emit the instruction using temp emitter and generate relocations.

bolt/lib/Passes/IdenticalCodeFolding.cpp

@@ -13,7 +13,6 @@
 #include "bolt/Passes/IdenticalCodeFolding.h"
 #include "bolt/Core/HashUtilities.h"
 #include "bolt/Core/ParallelUtilities.h"
-#include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ThreadPool.h"
@@ -49,6 +48,8 @@ cl::opt<bolt::IdenticalCodeFolding::ICFLevel> ICF(
     cl::init(bolt::IdenticalCodeFolding::ICFLevel::None),
     cl::values(clEnumValN(bolt::IdenticalCodeFolding::ICFLevel::All, "all",
                           "Enable identical code folding"),
+               clEnumValN(bolt::IdenticalCodeFolding::ICFLevel::All, "1",
+                          "Enable identical code folding"),
                clEnumValN(bolt::IdenticalCodeFolding::ICFLevel::All, "",
                           "Enable identical code folding"),
                clEnumValN(bolt::IdenticalCodeFolding::ICFLevel::None, "none",
@@ -366,43 +367,43 @@ void IdenticalCodeFolding::initVTableReferences(const BinaryContext &BC) {
 }
 void IdenticalCodeFolding::analyzeDataRelocations(BinaryContext &BC) {
   initVTableReferences(BC);
+  // For static relocations there should be a symbol for function references.
   for (const BinarySection &Sec : BC.sections()) {
     if (!Sec.hasSectionRef() || !Sec.isData())
       continue;
     for (const auto &Rel : Sec.relocations()) {
       const uint64_t RelAddr = Rel.Offset + Sec.getAddress();
       if (isAddressInVTable(RelAddr))
         continue;
-      BinaryFunction *BF = BC.getFunctionForSymbol(Rel.Symbol);
-      if (!BF)
-        continue;
-      BF->setHasAddressTaken(true);
+      if (BinaryFunction *BF = BC.getFunctionForSymbol(Rel.Symbol))
+        BF->setHasAddressTaken(true);
     }
+    // For dyanmic relocations there are two cases:
+    // 1: No symbol and only addend.
+    // 2: There is symbol, but it references undefined symbol, or things like
+    // type information. As the result only using addend to lookup BF is a valid
+    // case.
     for (const auto &Rel : Sec.dynamicRelocations()) {
       const uint64_t RelAddr = Rel.Offset + Sec.getAddress();
       if (isAddressInVTable(RelAddr))
         continue;
-      BinaryFunction *BF =
-          BC.getBinaryFunctionContainingAddress(Rel.Addend,
-                                                /*CheckPastEnd*/ false,
-                                                /*UseMaxSize*/ true);
-      if (!BF)
-        continue;
-      BF->setHasAddressTaken(true);
+      if (BinaryFunction *BF =
+              BC.getBinaryFunctionContainingAddress(Rel.Addend,
+                                                    /*CheckPastEnd*/ false,
+                                                    /*UseMaxSize*/ true))
+        BF->setHasAddressTaken(true);
     }
   }
 }
 void IdenticalCodeFolding::analyzeFunctions(BinaryContext &BC) {
   ParallelUtilities::WorkFuncTy WorkFun = [&](BinaryFunction &BF) {
-    for (const BinaryBasicBlock *BB : BF.getLayout().blocks())
-      for (const MCInst &Inst : *BB)
+    for (const BinaryBasicBlock &BB : BF)
+      for (const MCInst &Inst : BB)
         if (!(BC.MIB->isCall(Inst) || BC.MIB->isBranch(Inst)))
-          BF.processInstructionsForFuncReferences(BC, Inst);
+          BF.analyzeInstructionForFuncReference(BC, Inst);
   };
   ParallelUtilities::PredicateTy SkipFunc =
-      [&](const BinaryFunction &BF) -> bool {
-    return BF.getState() != BinaryFunction::State::CFG;
-  };
+      [&](const BinaryFunction &BF) -> bool { return !BF.hasCFG(); };
   ParallelUtilities::runOnEachFunction(
       BC, ParallelUtilities::SchedulingPolicy::SP_INST_LINEAR, WorkFun,
       SkipFunc, "markUnsafe", /*ForceSequential*/ false, 2);
@@ -411,8 +412,8 @@ void IdenticalCodeFolding::analyzeFunctions(BinaryContext &BC) {
     for (auto &BFIter : BC.getBinaryFunctions()) {
       if (!BFIter.second.hasAddressTaken())
         continue;
-      dbgs() << "BOLT-DEBUG: skipping function " << BFIter.second.getOneName()
-             << '\n';
+      dbgs() << "BOLT-DEBUG: skipping function with reference taken "
+             << BFIter.second.getOneName() << '\n';
     }
   });
 }

bolt/test/X86/icf-safe-icp.test

@@ -1,20 +1,20 @@
 ## Check that BOLT handles correctly folding functions with --icf=safe
 ## that can be referenced through a non control flow instruction when ICP optimization is enabled.
-## This tests also checks that destructors are not folded.
+## This tests also checks that destructors are folded.
 
-# REQUIRES: system-linux
+# REQUIRES: system-linux, asserts
 # RUN: llvm-mc -filetype=obj -triple x86_64-unknown-linux %s -o %t1.o
 # RUN: %clang %cflags %t1.o -o %t.exe -Wl,-q
-# RUN: llvm-bolt --no-threads %t.exe --icf      -debug -debug-only=bolt-icf -o %t.bolt 2>&1 | FileCheck --check-prefix=ICFCHECK %s
-# RUN: llvm-bolt --no-threads %t.exe --icf=safe -debug -debug-only=bolt-icf -o %t.bolt 2>&1 | FileCheck --check-prefix=SAFEICFCHECK %s
+# RUN: llvm-bolt --no-threads %t.exe --icf      -debug-only=bolt-icf -o %t.bolt 2>&1 | FileCheck --check-prefix=ICFCHECK %s
+# RUN: llvm-bolt --no-threads %t.exe --icf=safe -debug-only=bolt-icf -o %t.bolt 2>&1 | FileCheck --check-prefix=SAFEICFCHECK %s
 
 # ICFCHECK:      ICF iteration 1
-# ICFCHECK-NEXT: folding _ZN8Derived3D0Ev into _ZN8Derived2D0Ev
-# ICFCHECK-NEXT: folding _ZNK8Derived34funcEii into _ZNK8Derived24funcEii
+# ICFCHECK-NEXT: folding Derived3Destructor into Derived2Destructor
+# ICFCHECK-NEXT: folding Derived3Func into Derived2Func
 
-# SAFEICFCHECK: skipping function _ZNK8Derived34funcEii
+# SAFEICFCHECK: skipping function with reference taken Derived3Func
 # SAFEICFCHECK-NEXT: ICF iteration 1
-# SAFEICFCHECK-NEXT: folding _ZN8Derived3D0Ev into _ZN8Derived2D0Ev
+# SAFEICFCHECK-NEXT: folding Derived3Destructor into Derived2Destructor
 # SAFEICFCHECK-NEXT: ===---------
 
 
@@ -43,7 +43,6 @@
 ##   virtual ~Base() {};
 ## };
 ##
-## //namespace {
 ## class Derived2 : public Base {
 ##   int c = 5;
 ## public:
@@ -58,7 +57,6 @@
 ##   __attribute__((noinline)) int func(int a, int b) const override { return a * (a - b) + this->c; }
 ##   ~Derived3() {}
 ## };
-## //} // namespace//
 ##
 ## __attribute__((noinline)) Base *createType(int a) {
 ##     Base *base = nullptr;
@@ -91,56 +89,51 @@
 	.globl	main
 	.type	main,@function
 main:
-	leaq	_ZNK8Derived34funcEii(%rip), %rcx
-	callq	_ZNK8Derived34funcEii
+	leaq	Derived3Func(%rip), %rcx
+	callq	Derived3Func
 	.size	main, .-main
 
-	.section	.text.hot._ZNK8Derived24funcEii,"axG",@progbits,_ZNK8Derived24funcEii,comdat
-	.weak	_ZNK8Derived24funcEii
-	.type	_ZNK8Derived24funcEii,@function
-_ZNK8Derived24funcEii:
+	.weak	Derived2Func
+	.type	Derived2Func,@function
+Derived2Func:
 	imull	%esi, %eax
 	retq
-	.size	_ZNK8Derived24funcEii, .-_ZNK8Derived24funcEii
+	.size	Derived2Func, .-Derived2Func
 
-	.section	.text.unlikely._ZN8Derived2D0Ev,"axG",@progbits,_ZN8Derived2D0Ev,comdat
-	.weak	_ZN8Derived2D0Ev
-	.type	_ZN8Derived2D0Ev,@function
-_ZN8Derived2D0Ev:
+	.weak	Derived2Destructor
+	.type	Derived2Destructor,@function
+Derived2Destructor:
 	jmp	_ZdlPvm@PLT
-	.size	_ZN8Derived2D0Ev, .-_ZN8Derived2D0Ev
+	.size	Derived2Destructor, .-Derived2Destructor
 
-	.section	.text.hot._ZNK8Derived34funcEii,"axG",@progbits,_ZNK8Derived34funcEii,comdat
-	.weak	_ZNK8Derived34funcEii
-	.type	_ZNK8Derived34funcEii,@function
-_ZNK8Derived34funcEii:
+	.weak	Derived3Func
+	.type	Derived3Func,@function
+Derived3Func:
 	imull	%esi, %eax
 	retq
-	.size	_ZNK8Derived34funcEii, .-_ZNK8Derived34funcEii
+	.size	Derived3Func, .-Derived3Func
 
-	.section	.text.unlikely._ZN4BaseD2Ev,"axG",@progbits,_ZN4BaseD2Ev,comdat
 	.weak	_ZN4BaseD2Ev
 	.type	_ZN4BaseD2Ev,@function
 _ZN4BaseD2Ev:
 	retq
 	.size	_ZN4BaseD2Ev, .-_ZN4BaseD2Ev
 
-	.section	.text.unlikely._ZN8Derived3D0Ev,"axG",@progbits,_ZN8Derived3D0Ev,comdat
-	.weak	_ZN8Derived3D0Ev
-	.type	_ZN8Derived3D0Ev,@function
-_ZN8Derived3D0Ev:
+	.weak	Derived3Destructor
+	.type	Derived3Destructor,@function
+Derived3Destructor:
 	jmp	_ZdlPvm@PLT
-	.size	_ZN8Derived3D0Ev, .-_ZN8Derived3D0Ev
+	.size	Derived3Destructor, .-Derived3Destructor
 
 	.type	_ZTV8Derived2,@object
 	.section	.data.rel.ro._ZTV8Derived2,"awG",@progbits,_ZTV8Derived2,comdat
 	.weak	_ZTV8Derived2
 _ZTV8Derived2:
 	.quad	0
 	.quad	_ZTI8Derived2
-	.quad	_ZNK8Derived24funcEii
+	.quad	Derived2Func
 	.quad	_ZN4BaseD2Ev
-	.quad	_ZN8Derived2D0Ev
+	.quad	Derived2Destructor
 	.size	_ZTV8Derived2, 40
 
 	.type	_ZTV8Derived3,@object
@@ -149,7 +142,7 @@ _ZTV8Derived2:
 _ZTV8Derived3:
 	.quad	0
 	.quad	_ZTI8Derived3
-	.quad	_ZNK8Derived34funcEii
+	.quad	Derived3Func
 	.quad	_ZN4BaseD2Ev
-	.quad	_ZN8Derived3D0Ev
+	.quad	Derived3Destructor
 	.size	_ZTV8Derived3, 40

bolt/test/X86/icf-safe-process-rela-data.test

@@ -1,16 +1,16 @@
-## Check that BOLT handles correctly folding functions with --icf=safe that are only reference in a .rela.data section.
+## Check that BOLT handles correctly folding functions with --icf=safe that are only referenced from a .rela.data section.
 
-# REQUIRES: system-linux
+# REQUIRES: system-linux, asserts
 # RUN: llvm-mc -filetype=obj -triple x86_64-unknown-linux %s -o %t1.o
 # RUN: %clang %cflags %t1.o -o %t.exe -Wl,-q -no-pie
-# RUN: llvm-bolt --no-threads %t.exe --icf -debug -debug-only=bolt-icf -o %t.bolt 2>&1 | FileCheck --check-prefix=ICFCHECK %s
-# RUN: llvm-bolt --no-threads %t.exe --icf=safe -debug -debug-only=bolt-icf -o %t.bolt 2>&1 | FileCheck --check-prefix=SAFEICFCHECK %s
+# RUN: llvm-bolt --no-threads %t.exe --icf -debug-only=bolt-icf -o %t.bolt 2>&1 | FileCheck --check-prefix=ICFCHECK %s
+# RUN: llvm-bolt --no-threads %t.exe --icf=safe -debug-only=bolt-icf -o %t.bolt 2>&1 | FileCheck --check-prefix=SAFEICFCHECK %s
 
 # ICFCHECK:      ICF iteration 1
 # ICFCHECK-NEXT: folding barAddFunc into fooAddFunc
 
-# SAFEICFCHECK:      skipping function fooAddFunc
-# SAFEICFCHECK-NEXT: skipping function barAddFunc
+# SAFEICFCHECK:      skipping function with reference taken fooAddFunc
+# SAFEICFCHECK-NEXT: skipping function with reference taken barAddFunc
 # SAFEICFCHECK-NEXT: ICF iteration 1
 # SAFEICFCHECK-NEXT: ===---------
 
@@ -23,8 +23,6 @@
 ## }
 ## Extra assembly removed.
 
-	.text
-	.file	"main.cpp"
 	.globl	fooAddFunc
 	.type	fooAddFunc,@function
 fooAddFunc:

bolt/test/X86/icf-safe-test1-no-relocs.test

@@ -1,9 +1,9 @@
-## Check that BOLT handles correctly a binary with no relocations with the --icf=safe option.
+## Check that BOLT reports an error for a binary with no relocations with the --icf=safe option.
 
-# REQUIRES: system-linux
+# REQUIRES: system-linux, asserts
 # RUN: llvm-mc -filetype=obj -triple x86_64-unknown-linux %s -o %t1.o
 # RUN: %clang %cflags %t1.o -o %t.exe
-# RUN: not llvm-bolt --no-threads %t.exe --icf=safe -debug -debug-only=bolt-icf -o %t.bolt 2>&1 | FileCheck --check-prefix=SAFEICFCHECK %s
+# RUN: not llvm-bolt --no-threads %t.exe --icf=safe -o %t.bolt 2>&1 | FileCheck --check-prefix=SAFEICFCHECK %s
 
 # SAFEICFCHECK: BOLT-ERROR: binary built without relocations. Safe ICF is not supported
 

bolt/test/X86/icf-safe-test1.test

@@ -1,26 +1,26 @@
 ## Check that BOLT handles correctly folding functions with --icf=safe that can be referenced by non-control flow instructions.
 ## It invokes BOLT twice first testing CFG path, and second when functions have to be disassembled.
 
-# REQUIRES: system-linux
+# REQUIRES: system-linux, asserts
 # RUN: llvm-mc -filetype=obj -triple x86_64-unknown-linux %s -o %t1.o
 # RUN: %clang %cflags %t1.o -o %t.exe -Wl,-q
-# RUN: llvm-bolt --no-threads %t.exe --icf -debug -debug-only=bolt-icf \
+# RUN: llvm-bolt --no-threads %t.exe --icf -debug-only=bolt-icf \
 # RUN:        -o %t.bolt 2>&1 | FileCheck --check-prefix=ICFCHECK %s
-# RUN: llvm-bolt --no-threads %t.exe --icf=safe -debug -debug-only=bolt-icf \
+# RUN: llvm-bolt --no-threads %t.exe --icf=safe -debug-only=bolt-icf \
 # RUN:        -o %t.bolt 2>&1 | FileCheck --check-prefix=SAFEICFCHECK %s
-# RUN: llvm-bolt --no-threads %t.exe --icf=safe -debug -debug-only=bolt-icf \
+# RUN: llvm-bolt --no-threads %t.exe --icf=safe -debug-only=bolt-icf \
 # RUN:        --skip-funcs=helper1Func,main -o %t.bolt 2>&1 | FileCheck --check-prefix=SAFEICFCHECKNOCFG %s
 
 # ICFCHECK:      ICF iteration 1
 # ICFCHECK-NEXT: folding barAddFunc into fooAddFunc
 # ICFCHECK-NEXT: folding barSubFunc into fooSubFunc
 
-# SAFEICFCHECK: skipping function barAddFunc
+# SAFEICFCHECK: skipping function with reference taken barAddFunc
 # SAFEICFCHECK-NEXT: ICF iteration 1
 # SAFEICFCHECK-NEXT: folding barSubFunc into fooSubFunc
 # SAFEICFCHECK-NEXT: ===---------
 
-# SAFEICFCHECKNOCFG: skipping function barAddFunc
+# SAFEICFCHECKNOCFG: skipping function with reference taken barAddFunc
 # SAFEICFCHECKNOCFG-NEXT: ICF iteration 1
 # SAFEICFCHECKNOCFG-NEXT: folding barSubFunc into fooSubFunc
 # SAFEICFCHECKNOCFG-NEXT: ===---------

bolt/test/X86/icf-safe-test2GlobalConstPtrNoPic.test

@@ -1,19 +1,19 @@
 ## Check that BOLT handles correctly folding functions with --icf=safe that can be referenced by non-control flow instructions,
-## when binary is built with -fno-PIC/-fno-PIE mode.
+## when binary is built with -fno-PIC/-fno-PIE.
 
-# REQUIRES: system-linux
+# REQUIRES: system-linux, asserts
 # RUN: llvm-mc -filetype=obj -triple x86_64-unknown-linux %s -o %t1.o
 # RUN: %clang %cflags %t1.o -o %t.exe -Wl,-q -no-pie
-# RUN: llvm-bolt --no-threads %t.exe --icf -debug -debug-only=bolt-icf -o %t.bolt 2>&1 | FileCheck --check-prefix=ICFCHECK %s
-# RUN: llvm-bolt --no-threads %t.exe --icf=safe -debug -debug-only=bolt-icf -o %t.bolt 2>&1 | FileCheck --check-prefix=SAFEICFCHECK %s
+# RUN: llvm-bolt --no-threads %t.exe --icf -debug-only=bolt-icf -o %t.bolt 2>&1 | FileCheck --check-prefix=ICFCHECK %s
+# RUN: llvm-bolt --no-threads %t.exe --icf=safe -debug-only=bolt-icf -o %t.bolt 2>&1 | FileCheck --check-prefix=SAFEICFCHECK %s
 
 # ICFCHECK:      ICF iteration 1
 # ICFCHECK-NEXT: folding barAddFunc into fooAddFunc
 # ICFCHECK-NEXT: folding barMulFunc into fooMulFunc
 
-# SAFEICFCHECK:      skipping function fooMulFunc
-# SAFEICFCHECK-NEXT: skipping function barMulFunc
-# SAFEICFCHECK-NEXT: skipping function barAddFunc
+# SAFEICFCHECK:      skipping function with reference taken fooMulFunc
+# SAFEICFCHECK-NEXT: skipping function with reference taken barMulFunc
+# SAFEICFCHECK-NEXT: skipping function with reference taken barAddFunc
 # SAFEICFCHECK-NEXT: ICF iteration 1
 # SAFEICFCHECK-NEXT: ===---------
 

bolt/test/X86/shared_object_entry.s

@@ -2,7 +2,7 @@
 # RUN: ld.lld %t.o -o %t.so --shared --entry=func1.cold.1 --emit-relocs
 # RUN: llvm-bolt -relocs %t.so -o %t -reorder-functions=hfsort+ \
 # RUN:    -split-functions -reorder-blocks=ext-tsp -split-all-cold \
-# RUN:    -dyno-stats -icf -use-gnu-stack
+# RUN:    -dyno-stats -icf=1 -use-gnu-stack
 
 ## Check that an entry point is a cold symbol
 # RUN: llvm-readelf -h %t.so > %t.log