[BOLT] Preserve original LSDA type encoding

[treapster]

Summary

In non-pie binaries BOLT unconditionally converted type encoding from indirect to absptr, which broke std exceptions since pointers to their typeinfo were only assigned at runtime in .data section. In this patch we preserve original encoding so that indirect remains indirect and can be resolved at runtime, and absolute remains absolute.

[treapster]

clang-format

[rafauler]

Interesting.. Thanks for presenting this testcase and providing a fix!

@maksfb Do you recall why were we hardcoding the encoding of types?

I'm taking a guess that we wanted to restrict the encoding to the ones we know how to emit in BinaryEmitter, but we do support emitting both pic/non-pic - meaning, indirect + pcrel, as well as absolute. Taking a look at BinaryEmitter.cpp:1024, which is responsible for actually encoding the type tables, what if we don't support outputting the format seen in the input? Will we crash with "unsupported TTypeEncoding"? Or are we covering every possible input type encoding?

bolt/include/bolt/Core/BinaryFunction.h

194

I think it makes more sense to move this line closer to "LSDAAddress" declaration in this same class, and define a getter member functions, since this will be used outside of this class (in binary emitter)

bolt/lib/Core/BinaryEmitter.cpp

979–980

This comment should be moved inside the if

bolt/lib/Core/Exceptions.cpp

123–124

The changes to this function will generate this->LSDATypeEncoding access all over the parser (accessing a heap object).

I guess we can keep the original code, which is a const variable, and just add a new line here setting LSDATypeEncoding to TTypeEncoding.

[treapster]

clang-format

[treapster]

bolt/lib/Core/BinaryContext.cpp

183

BTW, is this still relevant? What prevents us from using absptr on X86(at least for non-pie)?

[treapster]

Gentle ping

[maksfb]

This looks good to me. Thanks! I don't think there was any strong reason to enforce the type of encoding in the first place other than simplicity of the implementation.

bolt/include/bolt/Core/BinaryContext.h

JumpTable *getJumpTableContainingAddress(uint64_t Address) {
     if (JTI->first + JTI->second->getSize() > Address)
       return JTI->second;
     if (JTI->second->getSize() == 0 && JTI->first == Address)
       return JTI->second;
     return nullptr;
   }
 
   unsigned getDWARFEncodingSize(unsigned Encoding) {
+    if (Encoding == dwarf::DW_EH_PE_omit)
+      return 0;
     switch (Encoding & 0x0f) {
     default:
       llvm_unreachable("unknown encoding");
     case dwarf::DW_EH_PE_absptr:
     case dwarf::DW_EH_PE_signed:
       return AsmInfo->getCodePointerSize();
     case dwarf::DW_EH_PE_udata2:
     case dwarf::DW_EH_PE_sdata2:
public:
   /// and are referenced from BinaryFunction.
   std::list<std::pair<BinaryFunction *, uint64_t>> InterproceduralReferences;
 
   /// PseudoProbe decoder
   MCPseudoProbeDecoder ProbeDecoder;
 
   /// DWARF encoding. Available encoding types defined in BinaryFormat/Dwarf.h
   /// enum Constants, e.g. DW_EH_PE_omit.
-  unsigned TTypeEncoding = dwarf::DW_EH_PE_omit;
   unsigned LSDAEncoding = dwarf::DW_EH_PE_omit;
 
   BinaryContext(std::unique_ptr<MCContext> Ctx,
                 std::unique_ptr<DWARFContext> DwCtx,
                 std::unique_ptr<Triple> TheTriple, const Target *TheTarget,
                 std::string TripleName, std::unique_ptr<MCCodeEmitter> MCE,
                 std::unique_ptr<MCObjectFileInfo> MOFI,
                 std::unique_ptr<const MCAsmInfo> AsmInfo,

bolt/include/bolt/Core/BinaryFunction.h

public:
 
   static const char TimerGroupName[];
   static const char TimerGroupDesc[];
 
   using BasicBlockOrderType = SmallVector<BinaryBasicBlock *, 0>;
 
   /// Mark injected functions
   bool IsInjected = false;
 
   using LSDATypeTableTy = SmallVector<uint64_t, 0>;
 
   /// List of DWARF CFI instructions. Original CFI from the binary must be
   /// sorted w.r.t. offset that it appears. We rely on this to replay CFIs
   /// if needed (to fix state after reordering BBs).
   using CFIInstrMapType = SmallVector<MCCFIInstruction, 0>;
   using cfi_iterator = CFIInstrMapType::iterator;
   using const_cfi_iterator = CFIInstrMapType::const_iterator;
private:
 
   /// Score of the function (estimated number of instructions executed,
   /// according to profile data). -1 if the score has not been calculated yet.
   mutable int64_t FunctionScore{-1};
 
   /// Original LSDA address for the function.
   uint64_t LSDAAddress{0};
 
+  /// Original LSDA type encoding
+  unsigned LSDATypeEncoding{dwarf::DW_EH_PE_omit};
+
   /// Containing compilation unit for the function.
   DWARFUnit *DwarfUnit{nullptr};
 
   /// Last computed hash value. Note that the value could be recomputed using
   /// different parameters by every pass.
   mutable uint64_t Hash{0};
 
   /// For PLT functions it contains a symbol associated with a function
public:
   const CallSitesType &getCallSites() const { return CallSites; }
 
   const CallSitesType &getColdCallSites() const { return ColdCallSites; }
 
   ArrayRef<uint8_t> getLSDAActionTable() const { return LSDAActionTable; }
 
   const LSDATypeTableTy &getLSDATypeTable() const { return LSDATypeTable; }
 
+  unsigned getLSDATypeEncoding() const { return LSDATypeEncoding; }
+
   const LSDATypeTableTy &getLSDATypeAddressTable() const {
     return LSDATypeAddressTable;
   }
 
   ArrayRef<uint8_t> getLSDATypeIndexTable() const { return LSDATypeIndexTable; }
 
   const LabelsMapType &getLabels() const { return Labels; }
 

bolt/lib/Core/BinaryContext.cpp

return createStringError(
         make_error_code(std::errc::not_supported),
         Twine("BOLT-ERROR: no instruction info for target ", TripleName));
 
   std::unique_ptr<MCContext> Ctx(
       new MCContext(*TheTriple, AsmInfo.get(), MRI.get(), STI.get()));
   std::unique_ptr<MCObjectFileInfo> MOFI(
       TheTarget->createMCObjectFileInfo(*Ctx, IsPIC));
   Ctx->setObjectFileInfo(MOFI.get());
   // We do not support X86 Large code model. Change this in the future.
   bool Large = false;
   if (TheTriple->getArch() == llvm::Triple::aarch64)
     Large = true;
   unsigned LSDAEncoding =
       Large ? dwarf::DW_EH_PE_absptr : dwarf::DW_EH_PE_udata4;
-  unsigned TTypeEncoding =
-      Large ? dwarf::DW_EH_PE_absptr : dwarf::DW_EH_PE_udata4;
   if (IsPIC) {
     LSDAEncoding = dwarf::DW_EH_PE_pcrel |
                    (Large ? dwarf::DW_EH_PE_sdata8 : dwarf::DW_EH_PE_sdata4);
-    TTypeEncoding = dwarf::DW_EH_PE_indirect | dwarf::DW_EH_PE_pcrel |
-                    (Large ? dwarf::DW_EH_PE_sdata8 : dwarf::DW_EH_PE_sdata4);
   }
 
   std::unique_ptr<MCDisassembler> DisAsm(
       TheTarget->createMCDisassembler(*STI, *Ctx));
 
   if (!DisAsm)
     return createStringError(
         make_error_code(std::errc::not_supported),
BinaryContext::createBinaryContext(const ObjectFile *File, bool IsPIC,
 
   auto BC = std::make_unique<BinaryContext>(
       std::move(Ctx), std::move(DwCtx), std::move(TheTriple), TheTarget,
       std::string(TripleName), std::move(MCE), std::move(MOFI),
       std::move(AsmInfo), std::move(MII), std::move(STI),
       std::move(InstructionPrinter), std::move(MIA), nullptr, std::move(MRI),
       std::move(DisAsm));
 
-  BC->TTypeEncoding = TTypeEncoding;
   BC->LSDAEncoding = LSDAEncoding;
 
   BC->MAB = std::unique_ptr<MCAsmBackend>(
       BC->TheTarget->createMCAsmBackend(*BC->STI, *BC->MRI, MCTargetOptions()));
 
   BC->setFilename(File->getFileName());
 
   BC->HasFixedLoadAddress = !IsPIC;

bolt/lib/Core/BinaryEmitter.cpp

void BinaryEmitter::emitLSDA(BinaryFunction &BF, bool EmitColdPart) {
   //
   //  sizeof(dwarf::DW_EH_PE_data4) * 3 + sizeof(uleb128(action))
   uint64_t CallSiteTableLength = Sites->size() * 4 * 3;
   for (const BinaryFunction::CallSite &CallSite : *Sites)
     CallSiteTableLength += getULEB128Size(CallSite.Action);
 
   Streamer.switchSection(BC.MOFI->getLSDASection());
 
-  const unsigned TTypeEncoding = BC.TTypeEncoding;
+  const unsigned TTypeEncoding = BF.getLSDATypeEncoding();
   const unsigned TTypeEncodingSize = BC.getDWARFEncodingSize(TTypeEncoding);
   const uint16_t TTypeAlignment = 4;
 
   // Type tables have to be aligned at 4 bytes.
   Streamer.emitValueToAlignment(TTypeAlignment);
 
   // Emit the LSDA label.
   MCSymbol *LSDASymbol = EmitColdPart
unsigned TTypeBaseOffset =
       BF.getLSDAActionTable().size() + // Actions table size
       BF.getLSDATypeTable().size() * TTypeEncodingSize; // Types table size
   unsigned TTypeBaseOffsetSize = getULEB128Size(TTypeBaseOffset);
   unsigned TotalSize = sizeof(int8_t) +      // LPStart format
                        sizeof(int8_t) +      // TType format
                        TTypeBaseOffsetSize + // TType base offset size
                        TTypeBaseOffset;      // TType base offset
   unsigned SizeAlign = (4 - TotalSize) & 3;
 
+  if (TTypeEncoding != dwarf::DW_EH_PE_omit)
     // Account for any extra padding that will be added to the call site table
     // length.
     Streamer.emitULEB128IntValue(TTypeBaseOffset,
                                  /*PadTo=*/TTypeBaseOffsetSize + SizeAlign);
 
   // 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);
   Streamer.emitULEB128IntValue(CallSiteTableLength);
 
   for (const BinaryFunction::CallSite &CallSite : *Sites) {

bolt/lib/Core/Exceptions.cpp

void BinaryFunction::parseLSDA(ArrayRef<uint8_t> LSDASectionData,
   uint8_t LPStartEncoding = Data.getU8(&Offset);
   uint64_t LPStart = 0;
   // Convert to offset if LPStartEncoding is typed absptr DW_EH_PE_absptr
   if (Optional<uint64_t> MaybeLPStart = Data.getEncodedPointer(
           &Offset, LPStartEncoding, Offset + LSDASectionAddress))
     LPStart = (LPStartEncoding && 0xFF == 0) ? *MaybeLPStart
                                              : *MaybeLPStart - Address;
 
   const uint8_t TTypeEncoding = Data.getU8(&Offset);
+  LSDATypeEncoding = TTypeEncoding;
   size_t TTypeEncodingSize = 0;
   uintptr_t TTypeEnd = 0;
   if (TTypeEncoding != DW_EH_PE_omit) {
     TTypeEnd = Data.getULEB128(&Offset);
     TTypeEncodingSize = BC.getDWARFEncodingSize(TTypeEncoding);
   }
 
   if (opts::PrintExceptions) {

bolt/test/runtime/exceptions-no-pie.cpp

+// REQUIRES: system-linux
+// RUN: %clangxx -no-pie -Wl,-q %s -o %t.exe
+// RUN: llvm-bolt %t.exe -o %t.bolt.exe -lite=false
+// RUN: not --crash %t.bolt.exe 2>&1 | FileCheck %s --check-prefix=CHECK-FAIL
+// CHECK-FAIL: Should pass one argument
+// RUN: not %t.bolt.exe -1 | FileCheck %s --check-prefix=CHECK-BAD
+// CHECK-BAD: Bad value
+// RUN: not %t.bolt.exe 0 | FileCheck %s --check-prefix=CHECK-ZERO
+// CHECK-ZERO: Value is zero
+// RUN: %t.bolt.exe 1 | FileCheck %s --check-prefix=CHECK-GOOD
+// CHECK-GOOD: Good value
+#include <exception>
+#include <iostream>
+
+struct ValIsZero {
+  const char *error = "Value is zero\n";
+};
+int dummy(int arg) {
+  if (arg == 0)
+    throw ValIsZero();
+  if (arg > 0)
+    return 0;
+  else
+    throw std::out_of_range("Bad value");
+}
+
+int main(int argc, char **argv) {
+  if (argc != 2)
+    throw std::invalid_argument("Should pass one argument");
+  try {
+    dummy(std::strtol(argv[1], nullptr, 10));
+  } catch (std::out_of_range &e) {
+    std::cout << e.what() << "\n";
+    return 1;
+  } catch (ValIsZero &e) {
+    std::cout << e.error;
+    return 1;
+  }
+  std::cout << "Good value\n";
+  return 0;
+}