Merge remote-tracking branch 'origin/main' into CovrtIssue

Author: zahiraam

bolt/include/bolt/Core/BinaryFunction.h

@@ -527,6 +527,11 @@ class BinaryFunction {
   /// fragment of the function.
   SmallVector<MCSymbol *, 0> LSDASymbols;
 
+  /// Each function fragment may have another fragment containing all landing
+  /// pads for it. If that's the case, the LP fragment will be stored in the
+  /// vector below with indexing starting with the main fragment.
+  SmallVector<std::optional<FragmentNum>, 0> LPFragments;
+
   /// Map to discover which CFIs are attached to a given instruction offset.
   /// Maps an instruction offset into a FrameInstructions offset.
   /// This is only relevant to the buildCFG phase and is discarded afterwards.
@@ -1885,6 +1890,42 @@ class BinaryFunction {
     return LSDASymbols[F.get()];
   }
 
+  /// If all landing pads for the function fragment \p F are located in fragment
+  /// \p LPF, designate \p LPF as a landing-pad fragment for \p F. Passing
+  /// std::nullopt in LPF, means that landing pads for \p F are located in more
+  /// than one fragment.
+  void setLPFragment(const FragmentNum F, std::optional<FragmentNum> LPF) {
+    if (F.get() >= LPFragments.size())
+      LPFragments.resize(F.get() + 1);
+
+    LPFragments[F.get()] = LPF;
+  }
+
+  /// If function fragment \p F has a designated landing pad fragment, i.e. a
+  /// fragment that contains all landing pads for throwers in \p F, then return
+  /// that landing pad fragment number. If \p F does not need landing pads,
+  /// return \p F. Return nullptr if landing pads for \p F are scattered among
+  /// several function fragments.
+  std::optional<FragmentNum> getLPFragment(const FragmentNum F) {
+    if (!isSplit()) {
+      assert(F == FragmentNum::main() && "Invalid fragment number");
+      return FragmentNum::main();
+    }
+
+    if (F.get() >= LPFragments.size())
+      return std::nullopt;
+
+    return LPFragments[F.get()];
+  }
+
+  /// Return a symbol corresponding to a landing pad fragment for fragment \p F.
+  /// See getLPFragment().
+  MCSymbol *getLPStartSymbol(const FragmentNum F) {
+    if (std::optional<FragmentNum> LPFragment = getLPFragment(F))
+      return getSymbol(*LPFragment);
+    return nullptr;
+  }
+
   void setOutputDataAddress(uint64_t Address) { OutputDataOffset = Address; }
 
   uint64_t getOutputDataAddress() const { return OutputDataOffset; }

bolt/lib/Core/BinaryEmitter.cpp

@@ -140,7 +140,7 @@ class BinaryEmitter {
 
   void emitCFIInstruction(const MCCFIInstruction &Inst) const;
 
-  /// Emit exception handling ranges for the function.
+  /// Emit exception handling ranges for the function fragment.
   void emitLSDA(BinaryFunction &BF, const FunctionFragment &FF);
 
   /// Emit line number information corresponding to \p NewLoc. \p PrevLoc
@@ -915,17 +915,29 @@ void BinaryEmitter::emitLSDA(BinaryFunction &BF, const FunctionFragment &FF) {
   // Emit the LSDA header.
 
   // If LPStart is omitted, then the start of the FDE is used as a base for
-  // landing pad displacements. Then, if a cold fragment starts with a landing
-  // pad, this means that the first landing pad offset will be 0. However, C++
-  // runtime treats 0 as if there is no landing pad present, thus we *must* emit
-  // non-zero offsets for all valid LPs.
+  // landing pad displacements. Then, if a cold fragment starts with
+  // a landing pad, this means that the first landing pad offset will be 0.
+  // However, C++ runtime will treat 0 as if there is no landing pad, thus we
+  // cannot emit LP offset as 0.
   //
   // As a solution, for fixed-address binaries we set LPStart to 0, and for
-  // position-independent binaries we set LP start to FDE start minus one byte
-  // for FDEs that start with a landing pad.
-  const bool NeedsLPAdjustment = !FF.empty() && FF.front()->isLandingPad();
+  // position-independent binaries we offset LP start by one byte.
+  bool NeedsLPAdjustment = false;
   std::function<void(const MCSymbol *)> emitLandingPad;
-  if (BC.HasFixedLoadAddress) {
+
+  // Check if there's a symbol associated with a landing pad fragment.
+  const MCSymbol *LPStartSymbol = BF.getLPStartSymbol(FF.getFragmentNum());
+  if (!LPStartSymbol) {
+    // Since landing pads are not in the same fragment, we fall back to emitting
+    // absolute addresses for this FDE.
+    if (opts::Verbosity >= 2) {
+      BC.outs() << "BOLT-INFO: falling back to generating absolute-address "
+                << "exception ranges for " << BF << '\n';
+    }
+
+    assert(BC.HasFixedLoadAddress &&
+           "Cannot emit absolute-address landing pads for PIE/DSO");
+
     Streamer.emitIntValue(dwarf::DW_EH_PE_udata4, 1); // LPStart format
     Streamer.emitIntValue(0, 4);                      // LPStart
     emitLandingPad = [&](const MCSymbol *LPSymbol) {
@@ -935,17 +947,23 @@ void BinaryEmitter::emitLSDA(BinaryFunction &BF, const FunctionFragment &FF) {
         Streamer.emitIntValue(0, 4);
     };
   } else {
-    if (NeedsLPAdjustment) {
-      // Use relative LPStart format and emit LPStart as [SymbolStart - 1].
+    std::optional<FragmentNum> LPFN = BF.getLPFragment(FF.getFragmentNum());
+    const FunctionFragment &LPFragment = BF.getLayout().getFragment(*LPFN);
+    NeedsLPAdjustment =
+        (!LPFragment.empty() && LPFragment.front()->isLandingPad());
+
+    // Emit LPStart encoding and optionally LPStart.
+    if (NeedsLPAdjustment || LPStartSymbol != StartSymbol) {
       Streamer.emitIntValue(dwarf::DW_EH_PE_pcrel | dwarf::DW_EH_PE_sdata4, 1);
       MCSymbol *DotSymbol = BC.Ctx->createTempSymbol("LPBase");
       Streamer.emitLabel(DotSymbol);
 
       const MCExpr *LPStartExpr = MCBinaryExpr::createSub(
-          MCSymbolRefExpr::create(StartSymbol, *BC.Ctx),
+          MCSymbolRefExpr::create(LPStartSymbol, *BC.Ctx),
           MCSymbolRefExpr::create(DotSymbol, *BC.Ctx), *BC.Ctx);
-      LPStartExpr = MCBinaryExpr::createSub(
-          LPStartExpr, MCConstantExpr::create(1, *BC.Ctx), *BC.Ctx);
+      if (NeedsLPAdjustment)
+        LPStartExpr = MCBinaryExpr::createSub(
+            LPStartExpr, MCConstantExpr::create(1, *BC.Ctx), *BC.Ctx);
       Streamer.emitValue(LPStartExpr, 4);
     } else {
       // DW_EH_PE_omit means FDE start (StartSymbol) will be used as LPStart.
@@ -955,13 +973,13 @@ void BinaryEmitter::emitLSDA(BinaryFunction &BF, const FunctionFragment &FF) {
       if (LPSymbol) {
         const MCExpr *LPOffsetExpr = MCBinaryExpr::createSub(
             MCSymbolRefExpr::create(LPSymbol, *BC.Ctx),
-            MCSymbolRefExpr::create(StartSymbol, *BC.Ctx), *BC.Ctx);
+            MCSymbolRefExpr::create(LPStartSymbol, *BC.Ctx), *BC.Ctx);
         if (NeedsLPAdjustment)
           LPOffsetExpr = MCBinaryExpr::createAdd(
               LPOffsetExpr, MCConstantExpr::create(1, *BC.Ctx), *BC.Ctx);
-        Streamer.emitValue(LPOffsetExpr, 4);
+        Streamer.emitULEB128Value(LPOffsetExpr);
       } else {
-        Streamer.emitIntValue(0, 4);
+        Streamer.emitULEB128IntValue(0);
       }
     };
   }
@@ -976,10 +994,12 @@ void BinaryEmitter::emitLSDA(BinaryFunction &BF, const FunctionFragment &FF) {
     Streamer.emitLabel(TTBaseRefLabel);
   }
 
-  // Emit the landing pad call site table. We use signed data4 since we can emit
-  // a landing pad in a different part of the split function that could appear
-  // earlier in the address space than LPStart.
-  Streamer.emitIntValue(dwarf::DW_EH_PE_sdata4, 1);
+  // Emit encoding of entries in the call site table. The format is used for the
+  // call site start, length, and corresponding landing pad.
+  if (!LPStartSymbol)
+    Streamer.emitIntValue(dwarf::DW_EH_PE_sdata4, 1);
+  else
+    Streamer.emitIntValue(dwarf::DW_EH_PE_uleb128, 1);
 
   MCSymbol *CSTStartLabel = BC.Ctx->createTempSymbol("CSTStart");
   MCSymbol *CSTEndLabel = BC.Ctx->createTempSymbol("CSTEnd");
@@ -996,8 +1016,13 @@ void BinaryEmitter::emitLSDA(BinaryFunction &BF, const FunctionFragment &FF) {
 
     // Start of the range is emitted relative to the start of current
     // function split part.
-    Streamer.emitAbsoluteSymbolDiff(BeginLabel, StartSymbol, 4);
-    Streamer.emitAbsoluteSymbolDiff(EndLabel, BeginLabel, 4);
+    if (!LPStartSymbol) {
+      Streamer.emitAbsoluteSymbolDiff(BeginLabel, StartSymbol, 4);
+      Streamer.emitAbsoluteSymbolDiff(EndLabel, BeginLabel, 4);
+    } else {
+      Streamer.emitAbsoluteSymbolDiffAsULEB128(BeginLabel, StartSymbol);
+      Streamer.emitAbsoluteSymbolDiffAsULEB128(EndLabel, BeginLabel);
+    }
     emitLandingPad(CallSite.LP);
     Streamer.emitULEB128IntValue(CallSite.Action);
   }

bolt/lib/Passes/SplitFunctions.cpp

@@ -901,8 +901,47 @@ void SplitFunctions::splitFunction(BinaryFunction &BF, SplitStrategy &S) {
   // have to be placed in the same fragment. When we split them, create
   // trampoline landing pads that will redirect the execution to real LPs.
   TrampolineSetType Trampolines;
-  if (!BC.HasFixedLoadAddress && BF.hasEHRanges() && BF.isSplit())
-    Trampolines = createEHTrampolines(BF);
+  if (BF.hasEHRanges() && BF.isSplit()) {
+    // If all landing pads for this fragment are grouped in one (potentially
+    // different) fragment, we can set LPStart to the start of that fragment
+    // and avoid trampoline code.
+    bool NeedsTrampolines = false;
+    for (FunctionFragment &FF : BF.getLayout().fragments()) {
+      // Vector of fragments that contain landing pads for this fragment.
+      SmallVector<FragmentNum, 4> LandingPadFragments;
+      for (const BinaryBasicBlock *BB : FF)
+        for (const BinaryBasicBlock *LPB : BB->landing_pads())
+          LandingPadFragments.push_back(LPB->getFragmentNum());
+
+      // Eliminate duplicate entries from the vector.
+      llvm::sort(LandingPadFragments);
+      auto Last = llvm::unique(LandingPadFragments);
+      LandingPadFragments.erase(Last, LandingPadFragments.end());
+
+      if (LandingPadFragments.size() == 0) {
+        // If the fragment has no landing pads, we can safely set itself as its
+        // landing pad fragment.
+        BF.setLPFragment(FF.getFragmentNum(), FF.getFragmentNum());
+      } else if (LandingPadFragments.size() == 1) {
+        BF.setLPFragment(FF.getFragmentNum(), LandingPadFragments.front());
+      } else {
+        if (!BC.HasFixedLoadAddress) {
+          NeedsTrampolines = true;
+          break;
+        } else {
+          BF.setLPFragment(FF.getFragmentNum(), std::nullopt);
+        }
+      }
+    }
+
+    // Trampolines guarantee that all landing pads for any given fragment will
+    // be contained in the same fragment.
+    if (NeedsTrampolines) {
+      for (FunctionFragment &FF : BF.getLayout().fragments())
+        BF.setLPFragment(FF.getFragmentNum(), FF.getFragmentNum());
+      Trampolines = createEHTrampolines(BF);
+    }
+  }
 
   // Check the new size to see if it's worth splitting the function.
   if (BC.isX86() && LayoutUpdated) {
@@ -933,6 +972,10 @@ void SplitFunctions::splitFunction(BinaryFunction &BF, SplitStrategy &S) {
     }
   }
 
+  // Restore LP fragment for the main fragment if the splitting was undone.
+  if (BF.hasEHRanges() && !BF.isSplit())
+    BF.setLPFragment(FragmentNum::main(), FragmentNum::main());
+
   // Fix branches if the splitting decision of the pass after function
   // reordering is different from that of the pass before function reordering.
   if (LayoutUpdated && BC.HasFinalizedFunctionOrder)

bolt/test/X86/exceptions-compact.s

@@ -0,0 +1,75 @@
+## Check that llvm-bolt is able to overwrite LSDA in ULEB128 format in-place for
+## all types of binaries.
+
+# REQUIRES: system-linux
+
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-linux %s -o %t.o
+# RUN: ld.lld --no-pie %t.o -o %t.exe -q
+# RUN: ld.lld --pie %t.o -o %t.pie -q
+# RUN: ld.lld --shared %t.o -o %t.so -q
+# RUN: llvm-bolt %t.exe -o %t.bolt --strict \
+# RUN:   | FileCheck --check-prefix=CHECK-BOLT %s
+# RUN: llvm-bolt %t.pie -o %t.pie.bolt --strict \
+# RUN:   | FileCheck --check-prefix=CHECK-BOLT %s
+# RUN: llvm-bolt %t.so -o %t.so.bolt --strict \
+# RUN:   | FileCheck --check-prefix=CHECK-BOLT %s
+
+# CHECK-BOLT: rewriting .gcc_except_table in-place
+
+# RUN: llvm-readelf -WS %t.bolt | FileCheck --check-prefix=CHECK-ELF %s
+# RUN: llvm-readelf -WS %t.pie.bolt | FileCheck --check-prefix=CHECK-ELF %s
+# RUN: llvm-readelf -WS %t.so.bolt | FileCheck --check-prefix=CHECK-ELF %s
+
+# CHECK-ELF-NOT: .bolt.org.gcc_except_table
+
+  .text
+  .global foo
+  .type foo, %function
+foo:
+  .cfi_startproc
+  ret
+  .cfi_endproc
+  .size foo, .-foo
+
+  .globl _start
+  .type _start, %function
+_start:
+.Lfunc_begin0:
+  .cfi_startproc
+  .cfi_lsda 27, .Lexception0
+  call foo
+.Ltmp0:
+  call foo
+.Ltmp1:
+  ret
+
+## Landing pads.
+.LLP1:
+  ret
+.LLP0:
+  ret
+
+  .cfi_endproc
+.Lfunc_end0:
+  .size _start, .-_start
+
+## EH table.
+  .section  .gcc_except_table,"a",@progbits
+  .p2align  2
+GCC_except_table0:
+.Lexception0:
+  .byte 255                             # @LPStart Encoding = omit
+  .byte 255                             # @TType Encoding = omit
+  .byte 1                               # Call site Encoding = uleb128
+  .uleb128 .Lcst_end0-.Lcst_begin0
+.Lcst_begin0:
+  .uleb128 .Lfunc_begin0-.Lfunc_begin0  # >> Call Site 1 <<
+  .uleb128 .Ltmp0-.Lfunc_begin0         #   Call between .Lfunc_begin0 and .Ltmp0
+  .uleb128 .LLP0-.Lfunc_begin0          #   jumps to .LLP0
+  .byte 0                               #   On action: cleanup
+  .uleb128 .Ltmp0-.Lfunc_begin0         # >> Call Site 2 <<
+  .uleb128 .Ltmp1-.Ltmp0                #   Call between .Ltmp0 and .Ltmp1
+  .uleb128 .LLP1-.Lfunc_begin0          #     jumps to .LLP1
+  .byte 0                               #   On action: cleanup
+.Lcst_end0:
+