Merge pull request #5 from Xilinx/dominik.llvm_oct_31.FXML-1067

Bump LLVM to green commit 74fb770 from Oct. 31

Author: gargaroff

bolt/include/bolt/Core/BinaryContext.h

@@ -1236,10 +1236,10 @@ class BinaryContext {
     return Size;
   }
 
-  /// Verify that assembling instruction \p Inst results in the same sequence of
-  /// bytes as \p Encoding.
-  bool validateEncoding(const MCInst &Instruction,
-                        ArrayRef<uint8_t> Encoding) const;
+  /// Validate that disassembling the \p Sequence of bytes into an instruction
+  /// and assembling the instruction again, results in a byte sequence identical
+  /// to the original one.
+  bool validateInstructionEncoding(ArrayRef<uint8_t> Sequence) const;
 
   /// Return a function execution count threshold for determining whether
   /// the function is 'hot'. Consider it hot if count is above the average exec

bolt/lib/Core/BinaryContext.cpp

@@ -2287,19 +2287,25 @@ BinaryContext::calculateEmittedSize(BinaryFunction &BF, bool FixBranches) {
   return std::make_pair(HotSize, ColdSize);
 }
 
-bool BinaryContext::validateEncoding(const MCInst &Inst,
-                                     ArrayRef<uint8_t> InputEncoding) const {
+bool BinaryContext::validateInstructionEncoding(
+    ArrayRef<uint8_t> InputSequence) const {
+  MCInst Inst;
+  uint64_t InstSize;
+  DisAsm->getInstruction(Inst, InstSize, InputSequence, 0, nulls());
+  assert(InstSize == InputSequence.size() &&
+         "Disassembled instruction size does not match the sequence.");
+
   SmallString<256> Code;
   SmallVector<MCFixup, 4> Fixups;
   raw_svector_ostream VecOS(Code);
 
   MCE->encodeInstruction(Inst, VecOS, Fixups, *STI);
-  auto EncodedData = ArrayRef<uint8_t>((uint8_t *)Code.data(), Code.size());
-  if (InputEncoding != EncodedData) {
+  auto OutputSequence = ArrayRef<uint8_t>((uint8_t *)Code.data(), Code.size());
+  if (InputSequence != OutputSequence) {
     if (opts::Verbosity > 1) {
       errs() << "BOLT-WARNING: mismatched encoding detected\n"
-             << "      input: " << InputEncoding << '\n'
-             << "     output: " << EncodedData << '\n';
+             << "      input: " << InputSequence << '\n'
+             << "     output: " << OutputSequence << '\n';
     }
     return false;
   }

bolt/lib/Core/BinaryFunction.cpp

@@ -1217,7 +1217,7 @@ bool BinaryFunction::disassemble() {
     // Check integrity of LLVM assembler/disassembler.
     if (opts::CheckEncoding && !BC.MIB->isBranch(Instruction) &&
         !BC.MIB->isCall(Instruction) && !BC.MIB->isNoop(Instruction)) {
-      if (!BC.validateEncoding(Instruction, FunctionData.slice(Offset, Size))) {
+      if (!BC.validateInstructionEncoding(FunctionData.slice(Offset, Size))) {
         errs() << "BOLT-WARNING: mismatching LLVM encoding detected in "
                << "function " << *this << " for instruction :\n";
         BC.printInstruction(errs(), Instruction, AbsoluteInstrAddr);
@@ -1233,15 +1233,10 @@ bool BinaryFunction::disassemble() {
         break;
       }
 
-      // Disassemble again without the symbolizer and check that the disassembly
-      // matches the assembler output.
-      MCInst TempInst;
-      BC.DisAsm->getInstruction(TempInst, Size, FunctionData.slice(Offset),
-                                AbsoluteInstrAddr, nulls());
-      if (!BC.validateEncoding(TempInst, FunctionData.slice(Offset, Size))) {
+      if (!BC.validateInstructionEncoding(FunctionData.slice(Offset, Size))) {
         errs() << "BOLT-WARNING: internal assembler/disassembler error "
                   "detected for AVX512 instruction:\n";
-        BC.printInstruction(errs(), TempInst, AbsoluteInstrAddr);
+        BC.printInstruction(errs(), Instruction, AbsoluteInstrAddr);
         errs() << " in function " << *this << '\n';
         setIgnored();
         break;

bolt/lib/Rewrite/DWARFRewriter.cpp

@@ -498,20 +498,20 @@ void DWARFRewriter::updateUnitDebugInfo(
         Optional<uint64_t> Address = AttrVal.V.getAsAddress();
         const BinaryFunction *Function =
             BC.getBinaryFunctionContainingAddress(*Address);
-        uint32_t Index = 0;
-        // Preserving original address instead of using whatever ends up at this
-        // index.
-        if (!Function) {
-          Index = AddrWriter->getIndexFromAddress(*Address, Unit);
-        } else {
-          const uint64_t UpdatedAddress =
-              Function->translateInputToOutputAddress(*Address);
-          Index = AddrWriter->getIndexFromAddress(UpdatedAddress, Unit);
-        }
-        if (AttrVal.V.getForm() == dwarf::DW_FORM_addrx)
+        uint64_t UpdatedAddress = *Address;
+        if (Function)
+          UpdatedAddress =
+              Function->translateInputToOutputAddress(UpdatedAddress);
+
+        if (AttrVal.V.getForm() == dwarf::DW_FORM_addrx) {
+          const uint32_t Index =
+              AddrWriter->getIndexFromAddress(UpdatedAddress, Unit);
           DebugInfoPatcher.addUDataPatch(AttrVal.Offset, Index, AttrVal.Size);
-        else
+        } else if (AttrVal.V.getForm() == dwarf::DW_FORM_addr) {
+          DebugInfoPatcher.addLE32Patch(AttrVal.Offset, UpdatedAddress);
+        } else {
           errs() << "BOLT-ERROR: unsupported form for " << Entry << "\n";
+        }
       };
 
       if (Optional<AttrInfo> AttrVal =

bolt/lib/Rewrite/RewriteInstance.cpp

@@ -407,14 +407,6 @@ Error RewriteInstance::discoverStorage() {
   NamedRegionTimer T("discoverStorage", "discover storage", TimerGroupName,
                      TimerGroupDesc, opts::TimeRewrite);
 
-  // Stubs are harmful because RuntimeDyld may try to increase the size of
-  // sections accounting for stubs when we need those sections to match the
-  // same size seen in the input binary, in case this section is a copy
-  // of the original one seen in the binary.
-  BC->EFMM.reset(new ExecutableFileMemoryManager(*BC, /*AllowStubs*/ false));
-  BC->EFMM->setNewSecPrefix(getNewSecPrefix());
-  BC->EFMM->setOrgSecPrefix(getOrgSecPrefix());
-
   auto ELF64LEFile = dyn_cast<ELF64LEObjectFile>(InputFile);
   const ELFFile<ELF64LE> &Obj = ELF64LEFile->getELFFile();
 
@@ -607,10 +599,19 @@ void RewriteInstance::parsePseudoProbe() {
     errs() << "BOLT-WARNING: fail in building GUID2FuncDescMap\n";
     return;
   }
+
+  MCPseudoProbeDecoder::Uint64Set GuidFilter;
+  MCPseudoProbeDecoder::Uint64Map FuncStartAddrs;
+  for (const BinaryFunction *F : BC->getAllBinaryFunctions()) {
+    for (const MCSymbol *Sym : F->getSymbols()) {
+      FuncStartAddrs[Function::getGUID(NameResolver::restore(Sym->getName()))] =
+          F->getAddress();
+    }
+  }
   Contents = PseudoProbeSection->getContents();
   if (!BC->ProbeDecoder.buildAddress2ProbeMap(
-          reinterpret_cast<const uint8_t *>(Contents.data()),
-          Contents.size())) {
+          reinterpret_cast<const uint8_t *>(Contents.data()), Contents.size(),
+          GuidFilter, FuncStartAddrs)) {
     BC->ProbeDecoder.getAddress2ProbesMap().clear();
     errs() << "BOLT-WARNING: fail in building Address2ProbeMap\n";
     return;
@@ -974,7 +975,19 @@ void RewriteInstance::discoverFileObjects() {
     if (Name.empty()) {
       UniqueName = "ANONYMOUS." + std::to_string(AnonymousId++);
     } else if (cantFail(Symbol.getFlags()) & SymbolRef::SF_Global) {
-      assert(!BC->getBinaryDataByName(Name) && "global name not unique");
+      if (const BinaryData *BD = BC->getBinaryDataByName(Name)) {
+        if (BD->getSize() == ELFSymbolRef(Symbol).getSize() &&
+            BD->getAddress() == Address) {
+          if (opts::Verbosity > 1)
+            errs() << "BOLT-WARNING: ignoring duplicate global symbol " << Name
+                   << "\n";
+          // Ignore duplicate entry - possibly a bug in the linker
+          continue;
+        }
+        errs() << "BOLT-ERROR: bad input binary, global symbol \"" << Name
+               << "\" is not unique\n";
+        exit(1);
+      }
       UniqueName = Name;
     } else {
       // If we have a local file name, we should create 2 variants for the
@@ -1068,6 +1081,7 @@ void RewriteInstance::discoverFileObjects() {
         if (opts::Verbosity >= 1)
           outs() << "BOLT-INFO: skipping possibly another entry for function "
                  << *PreviousFunction << " : " << UniqueName << '\n';
+        registerName(SymbolSize);
       } else {
         outs() << "BOLT-INFO: using " << UniqueName << " as another entry to "
                << "function " << *PreviousFunction << '\n';
@@ -1084,7 +1098,6 @@ void RewriteInstance::discoverFileObjects() {
         assert(SI->second == Symbol && "wrong symbol found");
         FileSymRefs.erase(SI);
       }
-      registerName(SymbolSize);
       continue;
     }
 
@@ -1273,9 +1286,12 @@ void RewriteInstance::createPLTBinaryFunction(uint64_t TargetAddress,
 
   ErrorOr<BinarySection &> Section = BC->getSectionForAddress(EntryAddress);
   assert(Section && "cannot get section for address");
-  BF = BC->createBinaryFunction(Rel->Symbol->getName().str() + "@PLT", *Section,
-                                EntryAddress, 0, EntrySize,
-                                Section->getAlignment());
+  if (!BF)
+    BF = BC->createBinaryFunction(Rel->Symbol->getName().str() + "@PLT",
+                                  *Section, EntryAddress, 0, EntrySize,
+                                  Section->getAlignment());
+  else
+    BF->addAlternativeName(Rel->Symbol->getName().str() + "@PLT");
   setPLTSymbol(BF, Rel->Symbol->getName());
 }
 
@@ -1407,15 +1423,19 @@ void RewriteInstance::disassemblePLT() {
       continue;
 
     analyzeOnePLTSection(Section, PLTSI->EntrySize);
-    // If we did not register any function at the start of the section,
-    // then it must be a general PLT entry. Add a function at the location.
-    if (BC->getBinaryFunctions().find(Section.getAddress()) ==
-        BC->getBinaryFunctions().end()) {
-      BinaryFunction *BF = BC->createBinaryFunction(
+
+    BinaryFunction *PltBF;
+    auto BFIter = BC->getBinaryFunctions().find(Section.getAddress());
+    if (BFIter != BC->getBinaryFunctions().end()) {
+      PltBF = &BFIter->second;
+    } else {
+      // If we did not register any function at the start of the section,
+      // then it must be a general PLT entry. Add a function at the location.
+      PltBF = BC->createBinaryFunction(
           "__BOLT_PSEUDO_" + Section.getName().str(), Section,
           Section.getAddress(), 0, PLTSI->EntrySize, Section.getAlignment());
-      BF->setPseudo(true);
     }
+    PltBF->setPseudo(true);
   }
 }
 
@@ -3180,6 +3200,14 @@ void RewriteInstance::emitAndLink() {
   MCAsmLayout FinalLayout(
       static_cast<MCObjectStreamer *>(Streamer.get())->getAssembler());
 
+  // Disable stubs because RuntimeDyld may try to increase the size of
+  // sections accounting for stubs. We need those sections to match the
+  // same size seen in the input binary, in case this section is a copy
+  // of the original one seen in the binary.
+  BC->EFMM.reset(new ExecutableFileMemoryManager(*BC, /*AllowStubs=*/false));
+  BC->EFMM->setNewSecPrefix(getNewSecPrefix());
+  BC->EFMM->setOrgSecPrefix(getOrgSecPrefix());
+
   RTDyld.reset(new decltype(RTDyld)::element_type(*BC->EFMM, Resolver));
   RTDyld->setProcessAllSections(false);
   RTDyld->loadObject(*Obj);
@@ -3407,6 +3435,8 @@ void RewriteInstance::encodePseudoProbes() {
   // Address of the first probe is absolute.
   // Other probes' address are represented by delta
   auto EmitDecodedPseudoProbe = [&](MCDecodedPseudoProbe *&CurProbe) {
+    assert(!isSentinelProbe(CurProbe->getAttributes()) &&
+           "Sentinel probes should not be emitted");
     EmitULEB128IntValue(CurProbe->getIndex());
     uint8_t PackedType = CurProbe->getType() | (CurProbe->getAttributes() << 4);
     uint8_t Flag =
@@ -3511,9 +3541,17 @@ void RewriteInstance::encodePseudoProbes() {
         reinterpret_cast<const uint8_t *>(DescContents.data()),
         DescContents.size());
     StringRef ProbeContents = PseudoProbeSection->getOutputContents();
+    MCPseudoProbeDecoder::Uint64Set GuidFilter;
+    MCPseudoProbeDecoder::Uint64Map FuncStartAddrs;
+    for (const BinaryFunction *F : BC->getAllBinaryFunctions()) {
+      const uint64_t Addr =
+          F->isEmitted() ? F->getOutputAddress() : F->getAddress();
+      FuncStartAddrs[Function::getGUID(
+          NameResolver::restore(F->getOneName()))] = Addr;
+    }
     DummyDecoder.buildAddress2ProbeMap(
         reinterpret_cast<const uint8_t *>(ProbeContents.data()),
-        ProbeContents.size());
+        ProbeContents.size(), GuidFilter, FuncStartAddrs);
     DummyDecoder.printProbesForAllAddresses(outs());
   }
 }

bolt/test/AArch64/double_jump.cpp

@@ -3,7 +3,7 @@
 // RUN: %clang %cflags -O0 %s -o %t.exe
 // RUN: llvm-bolt %t.exe -o %t.bolt --peepholes=double-jumps | \
 // RUN:   FileCheck %s -check-prefix=CHECKBOLT
-// RUN: llvm-objdump -d %t.bolt | FileCheck %s
+// RUN: llvm-objdump --no-print-imm-hex -d %t.bolt | FileCheck %s
 
 // CHECKBOLT: BOLT-INFO: Peephole: 1 double jumps patched.