[BOLT][NFC] Log through JournalingStreams (llvm#81524)

Make core BOLT functionality more friendly to being used as a
library instead of in our standalone driver llvm-bolt. To
accomplish this, we augment BinaryContext with journaling streams
that are to be used by most BOLT code whenever something needs to
be logged to the screen. Users of the library can decide if logs
should be printed to a file, no file or to the screen, as
before. To illustrate this, this patch adds a new option
`--log-file` that allows the user to redirect BOLT logging to a
file on disk or completely hide it by using
`--log-file=/dev/null`. Future BOLT code should now use
`BinaryContext::outs()` for printing important messages instead of
`llvm::outs()`. A new test log.test enforces this by verifying that
no strings are print to screen once the `--log-file` option is
used.

In previous patches we also added a new BOLTError class to report
common and fatal errors, so code shouldn't call exit(1) now. To
easily handle problems as before (by quitting with exit(1)),
callers can now use
`BinaryContext::logBOLTErrorsAndQuitOnFatal(Error)` whenever code
needs to deal with BOLT errors. To test this, we have fatal.s
that checks we are correctly quitting and printing a fatal error
to the screen.

Because this is a significant change by itself, not all code was
yet ported. Code from Profiler libs (DataAggregator and friends)
still print errors directly to screen.

Co-authored-by: Rafael Auler <[email protected]>

Test Plan: NFC

Author: aaupov

bolt/include/bolt/Core/BinaryContext.h

@@ -165,6 +165,12 @@ class BOLTError : public ErrorInfo<BOLTError> {
   std::string Msg;
 };
 
+/// Streams used by BOLT to log regular or error events
+struct JournalingStreams {
+  raw_ostream &Out;
+  raw_ostream &Err;
+};
+
 Error createNonFatalBOLTError(const Twine &S);
 Error createFatalBOLTError(const Twine &S);
 
@@ -260,7 +266,8 @@ class BinaryContext {
 public:
   static Expected<std::unique_ptr<BinaryContext>>
   createBinaryContext(const ObjectFile *File, bool IsPIC,
-                      std::unique_ptr<DWARFContext> DwCtx);
+                      std::unique_ptr<DWARFContext> DwCtx,
+                      JournalingStreams Logger);
 
   /// Superset of compiler units that will contain overwritten code that needs
   /// new debug info. In a few cases, functions may end up not being
@@ -628,6 +635,10 @@ class BinaryContext {
 
   std::unique_ptr<MCAsmBackend> MAB;
 
+  /// Allows BOLT to print to log whenever it is necessary (with or without
+  /// const references)
+  mutable JournalingStreams Logger;
+
   /// Indicates if the binary is Linux kernel.
   bool IsLinuxKernel{false};
 
@@ -760,7 +771,8 @@ class BinaryContext {
                 std::unique_ptr<const MCInstrAnalysis> MIA,
                 std::unique_ptr<MCPlusBuilder> MIB,
                 std::unique_ptr<const MCRegisterInfo> MRI,
-                std::unique_ptr<MCDisassembler> DisAsm);
+                std::unique_ptr<MCDisassembler> DisAsm,
+                JournalingStreams Logger);
 
   ~BinaryContext();
 
@@ -1372,8 +1384,12 @@ class BinaryContext {
     return Offset;
   }
 
-  void exitWithBugReport(StringRef Message,
-                         const BinaryFunction &Function) const;
+  /// Log BOLT errors to journaling streams and quit process with non-zero error
+  /// code 1 if error is fatal.
+  void logBOLTErrorsAndQuitOnFatal(Error E);
+
+  std::string generateBugReportMessage(StringRef Message,
+                                       const BinaryFunction &Function) const;
 
   struct IndependentCodeEmitter {
     std::unique_ptr<MCObjectFileInfo> LocalMOFI;
@@ -1421,6 +1437,10 @@ class BinaryContext {
     assert(IOAddressMap && "Address map not set yet");
     return *IOAddressMap;
   }
+
+  raw_ostream &outs() const { return Logger.Out; }
+
+  raw_ostream &errs() const { return Logger.Err; }
 };
 
 template <typename T, typename = std::enable_if_t<sizeof(T) == 1>>

bolt/include/bolt/Core/DIEBuilder.h

@@ -15,6 +15,7 @@
 #ifndef BOLT_CORE_DIE_BUILDER_H
 #define BOLT_CORE_DIE_BUILDER_H
 
+#include "bolt/Core/BinaryContext.h"
 #include "llvm/CodeGen/DIE.h"
 #include "llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h"
 #include "llvm/DebugInfo/DWARF/DWARFDie.h"
@@ -32,6 +33,7 @@
 namespace llvm {
 
 namespace bolt {
+
 class DIEStreamer;
 class DebugStrOffsetsWriter;
 
@@ -120,6 +122,7 @@ class DIEBuilder {
   std::unique_ptr<State> BuilderState;
   FoldingSet<DIEAbbrev> AbbreviationsSet;
   std::vector<std::unique_ptr<DIEAbbrev>> Abbreviations;
+  BinaryContext &BC;
   DWARFContext *DwarfContext{nullptr};
   bool IsDWO{false};
   uint64_t UnitSize{0};
@@ -219,9 +222,10 @@ class DIEBuilder {
     if (getState().CloneUnitCtxMap[UnitId].DieInfoVector.size() > DIEId)
       return *getState().CloneUnitCtxMap[UnitId].DieInfoVector[DIEId].get();
 
-    errs() << "BOLT-WARNING: [internal-dwarf-error]: The DIE is not allocated "
-              "before looking up, some"
-           << "unexpected corner cases happened.\n";
+    BC.errs()
+        << "BOLT-WARNING: [internal-dwarf-error]: The DIE is not allocated "
+           "before looking up, some"
+        << "unexpected corner cases happened.\n";
     return *getState().CloneUnitCtxMap[UnitId].DieInfoVector.front().get();
   }
 
@@ -261,7 +265,7 @@ class DIEBuilder {
   DIE *constructDIEFast(DWARFDie &DDie, DWARFUnit &U, uint32_t UnitId);
 
 public:
-  DIEBuilder(DWARFContext *DwarfContext, bool IsDWO = false);
+  DIEBuilder(BinaryContext &BC, DWARFContext *DwarfContext, bool IsDWO = false);
 
   /// Returns enum to what we are currently processing.
   ProcessingType getCurrentProcessingState() { return getState().Type; }
@@ -295,8 +299,9 @@ class DIEBuilder {
     if (getState().TypeDIEMap.count(&DU))
       return getState().TypeDIEMap[&DU];
 
-    errs() << "BOLT-ERROR: unable to find TypeUnit for Type Unit at offset 0x"
-           << DU.getOffset() << "\n";
+    BC.errs()
+        << "BOLT-ERROR: unable to find TypeUnit for Type Unit at offset 0x"
+        << DU.getOffset() << "\n";
     return nullptr;
   }
 

bolt/include/bolt/Core/DynoStats.h

@@ -159,8 +159,9 @@ inline DynoStats getDynoStats(FuncsType &Funcs, bool IsAArch64) {
 
 /// Call a function with optional before and after dynostats printing.
 template <typename FnType, typename FuncsType>
-inline void callWithDynoStats(FnType &&Func, FuncsType &Funcs, StringRef Phase,
-                              const bool Flag, bool IsAArch64) {
+inline void callWithDynoStats(raw_ostream &OS, FnType &&Func, FuncsType &Funcs,
+                              StringRef Phase, const bool Flag,
+                              bool IsAArch64) {
   DynoStats DynoStatsBefore(IsAArch64);
   if (Flag)
     DynoStatsBefore = getDynoStats(Funcs, IsAArch64);
@@ -170,12 +171,12 @@ inline void callWithDynoStats(FnType &&Func, FuncsType &Funcs, StringRef Phase,
   if (Flag) {
     const DynoStats DynoStatsAfter = getDynoStats(Funcs, IsAArch64);
     const bool Changed = (DynoStatsAfter != DynoStatsBefore);
-    outs() << "BOLT-INFO: program-wide dynostats after running " << Phase
-           << (Changed ? "" : " (no change)") << ":\n\n"
-           << DynoStatsBefore << '\n';
+    OS << "BOLT-INFO: program-wide dynostats after running " << Phase
+       << (Changed ? "" : " (no change)") << ":\n\n"
+       << DynoStatsBefore << '\n';
     if (Changed)
-      DynoStatsAfter.print(outs(), &DynoStatsBefore);
-    outs() << '\n';
+      DynoStatsAfter.print(OS, &DynoStatsBefore);
+    OS << '\n';
   }
 }
 

bolt/include/bolt/Core/Exceptions.h

@@ -30,13 +30,14 @@ class FDE;
 
 namespace bolt {
 
+class BinaryContext;
 class BinaryFunction;
 
 /// \brief Wraps up information to read all CFI instructions and feed them to a
 /// BinaryFunction, as well as rewriting CFI sections.
 class CFIReaderWriter {
 public:
-  explicit CFIReaderWriter(const DWARFDebugFrame &EHFrame);
+  explicit CFIReaderWriter(BinaryContext &BC, const DWARFDebugFrame &EHFrame);
 
   bool fillCFIInfoFor(BinaryFunction &Function) const;
 
@@ -59,6 +60,7 @@ class CFIReaderWriter {
   const FDEsMap &getFDEs() const { return FDEs; }
 
 private:
+  BinaryContext &BC;
   FDEsMap FDEs;
 };
 

bolt/include/bolt/Passes/BinaryPasses.h

@@ -50,7 +50,6 @@ class BinaryFunctionPass {
   /// this pass is completed (printPass() must have returned true).
   virtual bool shouldPrint(const BinaryFunction &BF) const;
 
-  /// Execute this pass on the given functions.
   virtual Error runOnFunctions(BinaryContext &BC) = 0;
 };
 
@@ -74,14 +73,14 @@ class DynoStatsPrintPass : public BinaryFunctionPass {
     const DynoStats NewDynoStats =
         getDynoStats(BC.getBinaryFunctions(), BC.isAArch64());
     const bool Changed = (NewDynoStats != PrevDynoStats);
-    outs() << "BOLT-INFO: program-wide dynostats " << Title
-           << (Changed ? "" : " (no change)") << ":\n\n"
-           << PrevDynoStats;
+    BC.outs() << "BOLT-INFO: program-wide dynostats " << Title
+              << (Changed ? "" : " (no change)") << ":\n\n"
+              << PrevDynoStats;
     if (Changed) {
-      outs() << '\n';
-      NewDynoStats.print(outs(), &PrevDynoStats, BC.InstPrinter.get());
+      BC.outs() << '\n';
+      NewDynoStats.print(BC.outs(), &PrevDynoStats, BC.InstPrinter.get());
     }
-    outs() << '\n';
+    BC.outs() << '\n';
     return Error::success();
   }
 };

bolt/include/bolt/Passes/CMOVConversion.h

@@ -64,7 +64,7 @@ class CMOVConversion : public BinaryFunctionPass {
     }
     double getMPRatio() { return (double)RemovedMP / PossibleMP; }
 
-    void dump();
+    void dumpTo(raw_ostream &OS);
   };
   // BinaryContext-wide stats
   Stats Global;

bolt/include/bolt/Passes/CacheMetrics.h

@@ -17,12 +17,16 @@
 #include <vector>
 
 namespace llvm {
+
+class raw_ostream;
+
 namespace bolt {
 class BinaryFunction;
 namespace CacheMetrics {
 
 /// Calculate and print various metrics related to instruction cache performance
-void printAll(const std::vector<BinaryFunction *> &BinaryFunctions);
+void printAll(raw_ostream &OS,
+              const std::vector<BinaryFunction *> &BinaryFunctions);
 
 } // namespace CacheMetrics
 } // namespace bolt

bolt/include/bolt/Passes/ReorderData.h

@@ -35,7 +35,8 @@ class ReorderData : public BinaryFunctionPass {
   sortedByFunc(BinaryContext &BC, const BinarySection &Section,
                std::map<uint64_t, BinaryFunction> &BFs) const;
 
-  void printOrder(const BinarySection &Section, DataOrder::const_iterator Begin,
+  void printOrder(BinaryContext &BC, const BinarySection &Section,
+                  DataOrder::const_iterator Begin,
                   DataOrder::const_iterator End) const;
 
   /// Set the ordering of the section with \p SectionName.  \p NewOrder is a

bolt/include/bolt/Passes/ReorderFunctions.h

@@ -20,10 +20,10 @@ class Cluster;
 class ReorderFunctions : public BinaryFunctionPass {
   BinaryFunctionCallGraph Cg;
 
-  void reorder(std::vector<Cluster> &&Clusters,
+  void reorder(BinaryContext &BC, std::vector<Cluster> &&Clusters,
                std::map<uint64_t, BinaryFunction> &BFs);
 
-  void printStats(const std::vector<Cluster> &Clusters,
+  void printStats(BinaryContext &BC, const std::vector<Cluster> &Clusters,
                   const std::vector<uint64_t> &FuncAddr);
 
 public:

bolt/include/bolt/Passes/ShrinkWrapping.h

@@ -523,7 +523,7 @@ class ShrinkWrapping {
 
   Expected<bool> perform(bool HotOnly = false);
 
-  static void printStats();
+  static void printStats(BinaryContext &BC);
 };
 
 } // end namespace bolt

bolt/include/bolt/Profile/BoltAddressTranslation.h

@@ -85,7 +85,7 @@ class BoltAddressTranslation {
 
   /// Read the serialized address translation tables and load them internally
   /// in memory. Return a parse error if failed.
-  std::error_code parse(StringRef Buf);
+  std::error_code parse(raw_ostream &OS, StringRef Buf);
 
   /// Dump the parsed address translation tables
   void dump(raw_ostream &OS);

bolt/include/bolt/Rewrite/RewriteInstance.h

@@ -47,11 +47,14 @@ class RewriteInstance {
   // construction. Constructors can’t return errors, so clients must test \p Err
   // after the object is constructed. Use `create` method instead.
   RewriteInstance(llvm::object::ELFObjectFileBase *File, const int Argc,
-                  const char *const *Argv, StringRef ToolPath, Error &Err);
+                  const char *const *Argv, StringRef ToolPath,
+                  raw_ostream &Stdout, raw_ostream &Stderr, Error &Err);
 
   static Expected<std::unique_ptr<RewriteInstance>>
   create(llvm::object::ELFObjectFileBase *File, const int Argc,
-         const char *const *Argv, StringRef ToolPath);
+         const char *const *Argv, StringRef ToolPath,
+         raw_ostream &Stdout = llvm::outs(),
+         raw_ostream &Stderr = llvm::errs());
   ~RewriteInstance();
 
   /// Assign profile from \p Filename to this instance.

bolt/lib/Core/BinaryBasicBlock.cpp

@@ -92,8 +92,8 @@ bool BinaryBasicBlock::validateSuccessorInvariants() {
         // Work on the assumption that jump table blocks don't
         // have a conditional successor.
         Valid = false;
-        errs() << "BOLT-WARNING: Jump table successor " << Succ->getName()
-               << " not contained in the jump table.\n";
+        BC.errs() << "BOLT-WARNING: Jump table successor " << Succ->getName()
+                  << " not contained in the jump table.\n";
       }
     }
     // If there are any leftover entries in the jump table, they
@@ -103,8 +103,8 @@ bool BinaryBasicBlock::validateSuccessorInvariants() {
         Valid &= (Sym == Function->getFunctionEndLabel() ||
                   Sym == Function->getFunctionEndLabel(getFragmentNum()));
         if (!Valid) {
-          errs() << "BOLT-WARNING: Jump table contains illegal entry: "
-                 << Sym->getName() << "\n";
+          BC.errs() << "BOLT-WARNING: Jump table contains illegal entry: "
+                    << Sym->getName() << "\n";
         }
       }
     }
@@ -141,11 +141,11 @@ bool BinaryBasicBlock::validateSuccessorInvariants() {
     }
   }
   if (!Valid) {
-    errs() << "BOLT-WARNING: CFG invalid in " << *getFunction() << " @ "
-           << getName() << "\n";
+    BC.errs() << "BOLT-WARNING: CFG invalid in " << *getFunction() << " @ "
+              << getName() << "\n";
     if (JT) {
-      errs() << "Jump Table instruction addr = 0x"
-             << Twine::utohexstr(BC.MIB->getJumpTable(*Inst)) << "\n";
+      BC.errs() << "Jump Table instruction addr = 0x"
+                << Twine::utohexstr(BC.MIB->getJumpTable(*Inst)) << "\n";
       JT->print(errs());
     }
     getFunction()->dump();
@@ -520,9 +520,9 @@ uint32_t BinaryBasicBlock::getNumPseudos() const {
       ++N;
 
   if (N != NumPseudos) {
-    errs() << "BOLT-ERROR: instructions for basic block " << getName()
-           << " in function " << *Function << ": calculated pseudos " << N
-           << ", set pseudos " << NumPseudos << ", size " << size() << '\n';
+    BC.errs() << "BOLT-ERROR: instructions for basic block " << getName()
+              << " in function " << *Function << ": calculated pseudos " << N
+              << ", set pseudos " << NumPseudos << ", size " << size() << '\n';
     llvm_unreachable("pseudos mismatch");
   }
 #endif
@@ -559,18 +559,18 @@ BinaryBasicBlock::getBranchStats(const BinaryBasicBlock *Succ) const {
 void BinaryBasicBlock::dump() const {
   BinaryContext &BC = Function->getBinaryContext();
   if (Label)
-    outs() << Label->getName() << ":\n";
-  BC.printInstructions(outs(), Instructions.begin(), Instructions.end(),
+    BC.outs() << Label->getName() << ":\n";
+  BC.printInstructions(BC.outs(), Instructions.begin(), Instructions.end(),
                        getOffset(), Function);
-  outs() << "preds:";
+  BC.outs() << "preds:";
   for (auto itr = pred_begin(); itr != pred_end(); ++itr) {
-    outs() << " " << (*itr)->getName();
+    BC.outs() << " " << (*itr)->getName();
   }
-  outs() << "\nsuccs:";
+  BC.outs() << "\nsuccs:";
   for (auto itr = succ_begin(); itr != succ_end(); ++itr) {
-    outs() << " " << (*itr)->getName();
+    BC.outs() << " " << (*itr)->getName();
   }
-  outs() << "\n";
+  BC.outs() << "\n";
 }
 
 uint64_t BinaryBasicBlock::estimateSize(const MCCodeEmitter *Emitter) const {