Preserve nops that are SDT markers in binaries and disable SDT confli…

…cting optimizations

Summary:
SDT markers that appears as nops in the assembly, are preserved and not eliminated.
Functions with SDT markers are also flagged. Inlining and folding are disabled for
functions that have SDT markers.

Reviewed By: maksfb

Differential Revision: D15379799

fbshipit-source-id: c76dc135888

src/BinaryContext.h

@@ -395,8 +395,8 @@ class BinaryContext {
   /// List of functions that always trap.
   std::vector<const BinaryFunction *> TrappedFunctions;
 
-  /// List of the SDT markers in the elf file
-  std::vector<SDTMarkerInfo> SDTMarkers;
+  /// Map SDT locations to SDT markers info
+  std::unordered_map<uint64_t, SDTMarkerInfo> SDTMarkers;
 
   BinaryContext(std::unique_ptr<MCContext> Ctx,
                 std::unique_ptr<DWARFContext> DwCtx,

src/BinaryFunction.cpp

@@ -1744,10 +1744,17 @@ bool BinaryFunction::buildCFG() {
       if (PrevBB)
         updateOffset(LastInstrOffset);
     }
-    // Ignore nops. We use nops to derive alignment of the next basic block.
-    // It will not always work, as some blocks are naturally aligned, but
-    // it's just part of heuristic for block alignment.
-    if (MIB->isNoop(Instr) && !PreserveNops) {
+
+    bool IsSDTMarker =
+        MIB->isNoop(Instr) && BC.SDTMarkers.count(I->first + Address);
+
+    if (IsSDTMarker)
+      HasSDTMarker = true;
+
+    // Ignore nops except SDT markers. We use nops to derive alignment of the
+    // next basic block. It will not always work, as some blocks are naturally
+    // aligned, but it's just part of heuristic for block alignment.
+    if (MIB->isNoop(Instr) && !PreserveNops && !IsSDTMarker) {
       IsLastInstrNop = true;
       continue;
     }

src/BinaryFunction.h

@@ -309,6 +309,9 @@ class BinaryFunction {
   /// Function order for streaming into the destination binary.
   uint32_t Index{-1U};
 
+  /// Indicate that the function body has SDT marker
+  bool HasSDTMarker{false};
+
   /// Get basic block index assuming it belongs to this function.
   unsigned getIndex(const BinaryBasicBlock *BB) const {
     assert(BB->getIndex() < BasicBlocks.size());
@@ -1244,6 +1247,9 @@ class BinaryFunction {
     return !JumpTables.empty();
   }
 
+  /// Return true if the function has SDT marker
+  bool hasSDTMarker() const { return HasSDTMarker; }
+
   const JumpTable *getJumpTable(const MCInst &Inst) const {
     const auto Address = BC.MIB->getJumpTable(Inst);
     return getJumpTableContainingAddress(Address);

src/Passes/IdenticalCodeFolding.cpp

@@ -312,7 +312,7 @@ void IdenticalCodeFolding::runOnFunctions(BinaryContext &BC) {
                      KeyHash, KeyCongruent> CongruentBuckets;
   for (auto &BFI : BC.getBinaryFunctions()) {
     auto &BF = BFI.second;
-    if (!shouldOptimize(BF) || BF.isFolded())
+    if (!shouldOptimize(BF) || BF.isFolded() || BF.hasSDTMarker())
       continue;
 
     // Make sure indices are in-order.

src/Passes/Inliner.cpp

@@ -180,6 +180,9 @@ Inliner::InliningInfo Inliner::getInliningInfo(const BinaryFunction &BF) const {
 
   // Perform necessary checks unless the option overrides it.
   if (!opts::mustConsider(BF)) {
+    if (BF.hasSDTMarker())
+      return INL_NONE;
+
     if (BF.hasEHRanges())
       return INL_NONE;
 

src/RewriteInstance.cpp

@@ -919,8 +919,7 @@ void RewriteInstance::parseSDTNotes() {
     Marker.Args = DE.getCStr(&Offset);
 
     Offset = alignTo(Offset, 4);
-
-    BC->SDTMarkers.push_back(Marker);
+    BC->SDTMarkers[Marker.PC] = Marker;
   }
 
   if (opts::PrintSDTMarkers)
@@ -930,7 +929,8 @@ void RewriteInstance::parseSDTNotes() {
 void RewriteInstance::printSDTMarkers() {
   outs() << "BOLT-INFO: Number of SDT markers is " << BC->SDTMarkers.size()
          << "\n";
-  for (auto &Marker : BC->SDTMarkers) {
+  for (auto It : BC->SDTMarkers) {
+    auto &Marker = It.second;
     outs() << "BOLT-INFO: PC: " << utohexstr(Marker.PC)
            << ", Base: " << utohexstr(Marker.Base)
            << ", Semaphore: " << utohexstr(Marker.Semaphore)