[BOLT][NFCI] Use MetadataRewriter interface to update SDT markers
authorMaksim Panchenko <maks@fb.com>
Wed, 28 Jun 2023 05:56:47 +0000 (22:56 -0700)
committerMaksim Panchenko <maks@fb.com>
Thu, 6 Jul 2023 18:17:17 +0000 (11:17 -0700)
Migrate SDT markers processing to the new MetadataRewriter interface.

Depends on D154020

Reviewed By: Amir

Differential Revision: https://reviews.llvm.org/D154021

bolt/include/bolt/Core/BinaryBasicBlock.h
bolt/include/bolt/Core/BinaryContext.h
bolt/include/bolt/Core/BinaryFunction.h
bolt/include/bolt/Core/BinarySection.h
bolt/include/bolt/Rewrite/MetadataRewriters.h
bolt/include/bolt/Rewrite/RewriteInstance.h
bolt/lib/Core/BinaryFunction.cpp
bolt/lib/Rewrite/CMakeLists.txt
bolt/lib/Rewrite/RewriteInstance.cpp
bolt/lib/Rewrite/SDTRewriter.cpp [new file with mode: 0644]

index c9694ab..02be9c1 100644 (file)
@@ -103,7 +103,7 @@ private:
   /// After output/codegen, map output offsets of instructions in this basic
   /// block to instruction offsets in the original function. Note that the
   /// output basic block could be different from the input basic block.
-  /// We only map instruction of interest, such as calls, and sdt markers.
+  /// We only map instruction of interest, such as calls and markers.
   ///
   /// We store the offset array in a basic block to facilitate BAT tables
   /// generation. Otherwise, the mapping could be done at function level.
index feefd06..bbe385f 100644 (file)
@@ -673,9 +673,6 @@ public:
   /// List of functions that always trap.
   std::vector<const BinaryFunction *> TrappedFunctions;
 
-  /// Map SDT locations to SDT markers info
-  std::unordered_map<uint64_t, SDTMarkerInfo> SDTMarkers;
-
   /// Map linux kernel program locations/instructions to their pointers in
   /// special linux kernel sections
   std::unordered_map<uint64_t, std::vector<LKInstructionMarkerInfo>> LKMarkers;
index c698319..d0927af 100644 (file)
@@ -1702,6 +1702,8 @@ public:
   /// Indicate that another function body was merged with this function.
   void setHasFunctionsFoldedInto() { HasFunctionsFoldedInto = true; }
 
+  void setHasSDTMarker(bool V) { HasSDTMarker = V; }
+
   BinaryFunction &setPersonalityFunction(uint64_t Addr) {
     assert(!PersonalityFunction && "can't set personality function twice");
     PersonalityFunction = BC.getOrCreateGlobalSymbol(Addr, "FUNCat");
index 3602f58..76363b4 100644 (file)
@@ -509,18 +509,6 @@ inline raw_ostream &operator<<(raw_ostream &OS, const BinarySection &Section) {
   return OS;
 }
 
-struct SDTMarkerInfo {
-  uint64_t PC;
-  uint64_t Base;
-  uint64_t Semaphore;
-  StringRef Provider;
-  StringRef Name;
-  StringRef Args;
-
-  /// The offset of PC within the note section
-  unsigned PCOffset;
-};
-
 /// Linux Kernel special sections point to a specific instruction in many cases.
 /// Unlike SDTMarkerInfo, these markers can come from different sections.
 struct LKInstructionMarkerInfo {
index 72f50d2..56de1fa 100644 (file)
@@ -15,9 +15,11 @@ namespace llvm {
 namespace bolt {
 
 class MetadataRewriter;
+class BinaryContext;
 
-/// The list of rewriter build functions (e.g. createNewMetadataRewriter()) that
-/// return std::unique_ptr<MetadataRewriter>.
+// The list of rewriter build functions.
+
+std::unique_ptr<MetadataRewriter> createSDTRewriter(BinaryContext &);
 
 } // namespace bolt
 } // namespace llvm
index ccd63d0..1fe5333 100644 (file)
@@ -198,9 +198,6 @@ private:
   /// Update debug and other auxiliary information in the file.
   void updateMetadata();
 
-  /// Update SDTMarkers' locations for the output binary.
-  void updateSDTMarkers();
-
   /// Update LKMarkers' locations for the output binary.
   void updateLKMarkers();
 
@@ -398,16 +395,10 @@ private:
   /// of appending contents to it.
   bool willOverwriteSection(StringRef SectionName);
 
-  /// Parse .note.stapsdt section
-  void parseSDTNotes();
-
   /// Parse .pseudo_probe_desc section and .pseudo_probe section
   /// Setup Pseudo probe decoder
   void parsePseudoProbe();
 
-  /// Print all SDT markers
-  void printSDTMarkers();
-
 public:
   /// Standard ELF sections we overwrite.
   static constexpr const char *SectionsToOverwrite[] = {
@@ -589,10 +580,6 @@ private:
   /// .note.gnu.build-id section.
   ErrorOr<BinarySection &> BuildIDSection{std::errc::bad_address};
 
-  /// .note.stapsdt section.
-  /// Contains information about statically defined tracing points
-  ErrorOr<BinarySection &> SDTSection{std::errc::bad_address};
-
   /// .pseudo_probe_desc section.
   /// Contains information about pseudo probe description, like its related
   /// function
index bde25ef..0e00a5f 100644 (file)
@@ -1988,19 +1988,20 @@ bool BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) {
         updateOffset(LastInstrOffset);
     }
 
-    const uint64_t InstrInputAddr = I->first + Address;
-    bool IsSDTMarker =
-        MIB->isNoop(Instr) && BC.SDTMarkers.count(InstrInputAddr);
-    bool IsLKMarker = BC.LKMarkers.count(InstrInputAddr);
+    bool IsLKMarker = BC.LKMarkers.count(I->first + Address);
     // Mark all nops with Offset for profile tracking purposes.
     if (MIB->isNoop(Instr) || IsLKMarker) {
-      if (!MIB->getOffset(Instr))
+      // If "Offset" annotation is not present, set it and mark the nop for
+      // deletion.
+      if (!MIB->getOffset(Instr)) {
         MIB->setOffset(Instr, static_cast<uint32_t>(Offset), AllocatorId);
-      if (IsSDTMarker || IsLKMarker)
-        HasSDTMarker = true;
-      else
         // Annotate ordinary nops, so we can safely delete them if required.
-        MIB->addAnnotation(Instr, "NOP", static_cast<uint32_t>(1), AllocatorId);
+        if (!IsLKMarker)
+          MIB->addAnnotation(Instr, "NOP", static_cast<uint32_t>(1),
+                             AllocatorId);
+      }
+      if (IsLKMarker)
+        HasSDTMarker = true;
     }
 
     if (!InsertBB) {
index fa058a2..7d633c4 100644 (file)
@@ -17,6 +17,7 @@ add_llvm_library(LLVMBOLTRewrite
   MachORewriteInstance.cpp
   MetadataManager.cpp
   RewriteInstance.cpp
+  SDTRewriter.cpp
 
   DISABLE_LLVM_LINK_LLVM_DYLIB
 
index 3e63bf9..6f4faa5 100644 (file)
@@ -196,10 +196,6 @@ static cl::opt<bool> PrintLoopInfo("print-loops",
                                    cl::desc("print loop related information"),
                                    cl::Hidden, cl::cat(BoltCategory));
 
-static cl::opt<bool> PrintSDTMarkers("print-sdt",
-                                     cl::desc("print all SDT markers"),
-                                     cl::Hidden, cl::cat(BoltCategory));
-
 enum PrintPseudoProbesOptions {
   PPP_None = 0,
   PPP_Probes_Section_Decode = 0x1,
@@ -634,51 +630,6 @@ Error RewriteInstance::discoverStorage() {
   return Error::success();
 }
 
-void RewriteInstance::parseSDTNotes() {
-  if (!SDTSection)
-    return;
-
-  StringRef Buf = SDTSection->getContents();
-  DataExtractor DE = DataExtractor(Buf, BC->AsmInfo->isLittleEndian(),
-                                   BC->AsmInfo->getCodePointerSize());
-  uint64_t Offset = 0;
-
-  while (DE.isValidOffset(Offset)) {
-    uint32_t NameSz = DE.getU32(&Offset);
-    DE.getU32(&Offset); // skip over DescSz
-    uint32_t Type = DE.getU32(&Offset);
-    Offset = alignTo(Offset, 4);
-
-    if (Type != 3)
-      errs() << "BOLT-WARNING: SDT note type \"" << Type
-             << "\" is not expected\n";
-
-    if (NameSz == 0)
-      errs() << "BOLT-WARNING: SDT note has empty name\n";
-
-    StringRef Name = DE.getCStr(&Offset);
-
-    if (!Name.equals("stapsdt"))
-      errs() << "BOLT-WARNING: SDT note name \"" << Name
-             << "\" is not expected\n";
-
-    // Parse description
-    SDTMarkerInfo Marker;
-    Marker.PCOffset = Offset;
-    Marker.PC = DE.getU64(&Offset);
-    Marker.Base = DE.getU64(&Offset);
-    Marker.Semaphore = DE.getU64(&Offset);
-    Marker.Provider = DE.getCStr(&Offset);
-    Marker.Name = DE.getCStr(&Offset);
-    Marker.Args = DE.getCStr(&Offset);
-    Offset = alignTo(Offset, 4);
-    BC->SDTMarkers[Marker.PC] = Marker;
-  }
-
-  if (opts::PrintSDTMarkers)
-    printSDTMarkers();
-}
-
 void RewriteInstance::parsePseudoProbe() {
   if (!PseudoProbeDescSection && !PseudoProbeSection) {
     // pesudo probe is not added to binary. It is normal and no warning needed.
@@ -728,19 +679,6 @@ void RewriteInstance::parsePseudoProbe() {
   }
 }
 
-void RewriteInstance::printSDTMarkers() {
-  outs() << "BOLT-INFO: Number of SDT markers is " << BC->SDTMarkers.size()
-         << "\n";
-  for (auto It : BC->SDTMarkers) {
-    SDTMarkerInfo &Marker = It.second;
-    outs() << "BOLT-INFO: PC: " << utohexstr(Marker.PC)
-           << ", Base: " << utohexstr(Marker.Base)
-           << ", Semaphore: " << utohexstr(Marker.Semaphore)
-           << ", Provider: " << Marker.Provider << ", Name: " << Marker.Name
-           << ", Args: " << Marker.Args << "\n";
-  }
-}
-
 void RewriteInstance::parseBuildID() {
   if (!BuildIDSection)
     return;
@@ -1853,7 +1791,6 @@ Error RewriteInstance::readSpecialSections() {
   LSDASection = BC->getUniqueSectionByName(".gcc_except_table");
   EHFrameSection = BC->getUniqueSectionByName(".eh_frame");
   BuildIDSection = BC->getUniqueSectionByName(".note.gnu.build-id");
-  SDTSection = BC->getUniqueSectionByName(".note.stapsdt");
   PseudoProbeDescSection = BC->getUniqueSectionByName(".pseudo_probe_desc");
   PseudoProbeSection = BC->getUniqueSectionByName(".pseudo_probe");
 
@@ -1910,8 +1847,6 @@ Error RewriteInstance::readSpecialSections() {
   if (std::optional<std::string> FileBuildID = getPrintableBuildID())
     BC->setFileBuildID(*FileBuildID);
 
-  parseSDTNotes();
-
   // Read .dynamic/PT_DYNAMIC.
   return readELFDynamic();
 }
@@ -3245,7 +3180,7 @@ void RewriteInstance::preprocessProfileData() {
 }
 
 void RewriteInstance::initializeMetadataManager() {
-  // TODO
+  MetadataManager.registerRewriter(createSDTRewriter(*BC));
 }
 
 void RewriteInstance::processMetadataPreCFG() {
@@ -3621,7 +3556,6 @@ void RewriteInstance::updateMetadata() {
   MetadataManager.runFinalizersAfterEmit();
 
   // TODO: use MetadataManager for updates.
-  updateSDTMarkers();
   updateLKMarkers();
   parsePseudoProbe();
   updatePseudoProbes();
@@ -3902,29 +3836,6 @@ void RewriteInstance::encodePseudoProbes() {
   }
 }
 
-void RewriteInstance::updateSDTMarkers() {
-  NamedRegionTimer T("updateSDTMarkers", "update SDT markers", TimerGroupName,
-                     TimerGroupDesc, opts::TimeRewrite);
-
-  if (!SDTSection)
-    return;
-  SDTSection->registerPatcher(std::make_unique<SimpleBinaryPatcher>());
-
-  SimpleBinaryPatcher *SDTNotePatcher =
-      static_cast<SimpleBinaryPatcher *>(SDTSection->getPatcher());
-  for (auto &SDTInfoKV : BC->SDTMarkers) {
-    const uint64_t OriginalAddress = SDTInfoKV.first;
-    SDTMarkerInfo &SDTInfo = SDTInfoKV.second;
-    const BinaryFunction *F =
-        BC->getBinaryFunctionContainingAddress(OriginalAddress);
-    if (!F)
-      continue;
-    const uint64_t NewAddress =
-        F->translateInputToOutputAddress(OriginalAddress);
-    SDTNotePatcher->addLE64Patch(SDTInfo.PCOffset, NewAddress);
-  }
-}
-
 void RewriteInstance::updateLKMarkers() {
   if (BC->LKMarkers.size() == 0)
     return;
diff --git a/bolt/lib/Rewrite/SDTRewriter.cpp b/bolt/lib/Rewrite/SDTRewriter.cpp
new file mode 100644 (file)
index 0000000..cc663b2
--- /dev/null
@@ -0,0 +1,178 @@
+//===- bolt/Rewrite/SDTRewriter.cpp ---------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Implement support for System Tap Statically-Defined Trace points stored in
+// .note.stapsdt section.
+//
+//===----------------------------------------------------------------------===//
+
+#include "bolt/Core/BinaryFunction.h"
+#include "bolt/Core/DebugData.h"
+#include "bolt/Rewrite/MetadataRewriter.h"
+#include "bolt/Rewrite/MetadataRewriters.h"
+#include "bolt/Utils/CommandLineOpts.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/Timer.h"
+
+using namespace llvm;
+using namespace bolt;
+
+namespace opts {
+static cl::opt<bool> PrintSDTMarkers("print-sdt",
+                                     cl::desc("print all SDT markers"),
+                                     cl::Hidden, cl::cat(BoltCategory));
+}
+
+namespace {
+class SDTRewriter final : public MetadataRewriter {
+  ErrorOr<BinarySection &> SDTSection{std::errc::bad_address};
+
+  struct SDTMarkerInfo {
+    uint64_t PC;
+    uint64_t Base;
+    uint64_t Semaphore;
+    StringRef Provider;
+    StringRef Name;
+    StringRef Args;
+
+    /// The offset of PC within the note section
+    unsigned PCOffset;
+  };
+
+  /// Map SDT locations to SDT markers info
+  using SDTMarkersListType = std::unordered_map<uint64_t, SDTMarkerInfo>;
+  SDTMarkersListType SDTMarkers;
+
+  /// Read section to populate SDTMarkers.
+  void readSection();
+
+  void printSDTMarkers() const;
+
+public:
+  SDTRewriter(StringRef Name, BinaryContext &BC) : MetadataRewriter(Name, BC) {}
+
+  Error preCFGInitializer() override;
+
+  Error postEmitFinalizer() override;
+};
+
+void SDTRewriter::readSection() {
+  SDTSection = BC.getUniqueSectionByName(".note.stapsdt");
+  if (!SDTSection)
+    return;
+
+  StringRef Buf = SDTSection->getContents();
+  DataExtractor DE = DataExtractor(Buf, BC.AsmInfo->isLittleEndian(),
+                                   BC.AsmInfo->getCodePointerSize());
+  uint64_t Offset = 0;
+
+  while (DE.isValidOffset(Offset)) {
+    uint32_t NameSz = DE.getU32(&Offset);
+    DE.getU32(&Offset); // skip over DescSz
+    uint32_t Type = DE.getU32(&Offset);
+    Offset = alignTo(Offset, 4);
+
+    if (Type != 3)
+      errs() << "BOLT-WARNING: SDT note type \"" << Type
+             << "\" is not expected\n";
+
+    if (NameSz == 0)
+      errs() << "BOLT-WARNING: SDT note has empty name\n";
+
+    StringRef Name = DE.getCStr(&Offset);
+
+    if (!Name.equals("stapsdt"))
+      errs() << "BOLT-WARNING: SDT note name \"" << Name
+             << "\" is not expected\n";
+
+    // Parse description
+    SDTMarkerInfo Marker;
+    Marker.PCOffset = Offset;
+    Marker.PC = DE.getU64(&Offset);
+    Marker.Base = DE.getU64(&Offset);
+    Marker.Semaphore = DE.getU64(&Offset);
+    Marker.Provider = DE.getCStr(&Offset);
+    Marker.Name = DE.getCStr(&Offset);
+    Marker.Args = DE.getCStr(&Offset);
+    Offset = alignTo(Offset, 4);
+    SDTMarkers[Marker.PC] = Marker;
+  }
+
+  if (opts::PrintSDTMarkers)
+    printSDTMarkers();
+}
+
+Error SDTRewriter::preCFGInitializer() {
+  // Populate SDTMarkers.
+  readSection();
+
+  // Mark nop instructions referenced by SDT and the containing function.
+  for (const uint64_t PC : llvm::make_first_range(SDTMarkers)) {
+    BinaryFunction *BF = BC.getBinaryFunctionContainingAddress(PC);
+
+    if (!BF || !BC.shouldEmit(*BF))
+      continue;
+
+    const uint64_t Offset = PC - BF->getAddress();
+    MCInst *Inst = BF->getInstructionAtOffset(Offset);
+    if (!Inst)
+      return createStringError(errc::executable_format_error,
+                               "no instruction matches SDT offset");
+
+    if (!BC.MIB->isNoop(*Inst))
+      return createStringError(std::make_error_code(std::errc::not_supported),
+                               "nop instruction expected at SDT offset");
+
+    BC.MIB->setOffset(*Inst, static_cast<uint32_t>(Offset));
+
+    BF->setHasSDTMarker(true);
+  }
+
+  return Error::success();
+}
+
+Error SDTRewriter::postEmitFinalizer() {
+  if (!SDTSection)
+    return Error::success();
+
+  SDTSection->registerPatcher(std::make_unique<SimpleBinaryPatcher>());
+
+  SimpleBinaryPatcher *SDTNotePatcher =
+      static_cast<SimpleBinaryPatcher *>(SDTSection->getPatcher());
+  for (auto &SDTInfoKV : SDTMarkers) {
+    const uint64_t OriginalAddress = SDTInfoKV.first;
+    const SDTMarkerInfo &SDTInfo = SDTInfoKV.second;
+    const BinaryFunction *F =
+        BC.getBinaryFunctionContainingAddress(OriginalAddress);
+    if (!F)
+      continue;
+    const uint64_t NewAddress =
+        F->translateInputToOutputAddress(OriginalAddress);
+    SDTNotePatcher->addLE64Patch(SDTInfo.PCOffset, NewAddress);
+  }
+
+  return Error::success();
+}
+
+void SDTRewriter::printSDTMarkers() const {
+  outs() << "BOLT-INFO: Number of SDT markers is " << SDTMarkers.size() << "\n";
+  for (const SDTMarkerInfo &Marker : llvm::make_second_range(SDTMarkers)) {
+    outs() << "BOLT-INFO: PC: " << utohexstr(Marker.PC)
+           << ", Base: " << utohexstr(Marker.Base)
+           << ", Semaphore: " << utohexstr(Marker.Semaphore)
+           << ", Provider: " << Marker.Provider << ", Name: " << Marker.Name
+           << ", Args: " << Marker.Args << "\n";
+  }
+}
+} // namespace
+
+std::unique_ptr<MetadataRewriter>
+llvm::bolt::createSDTRewriter(BinaryContext &BC) {
+  return std::make_unique<SDTRewriter>("sdt-rewriter", BC);
+}