[BOLT] Fix getDynoStats to handle BCs with no functions

[Amir]

Summary

Address fuzzer crash

[yota9]

LGTM

[Amir]

Pass bool instead of BC to getDynoStats, replace binary test with a unit test

[Amir]

clang-format

bolt/include/bolt/Core/DynoStats.h

 ///
 /// The function relies on branch instructions being in-sync with CFG for
 /// branch instructions stats. Thus it is better to call it after
 /// fixBranches().
 DynoStats getDynoStats(const BinaryFunction &BF);
 
 /// Return program-wide dynostats.
 template <typename FuncsType>
-inline DynoStats getDynoStats(const FuncsType &Funcs) {
-  bool IsAArch64 = Funcs.begin()->second.getBinaryContext().isAArch64();
+inline DynoStats getDynoStats(const FuncsType &Funcs, bool IsAArch64) {
   DynoStats dynoStats(IsAArch64);
   for (auto &BFI : Funcs) {
     auto &BF = BFI.second;
     if (BF.isSimple())
       dynoStats += getDynoStats(BF);
   }
   return dynoStats;
 }
 
 /// Call a function with optional before and after dynostats printing.
 template <typename FnType, typename FuncsType>
 inline void callWithDynoStats(FnType &&Func, const FuncsType &Funcs,
-                              StringRef Phase, const bool Flag) {
-  bool IsAArch64 = Funcs.begin()->second.getBinaryContext().isAArch64();
+                              StringRef Phase, const bool Flag,
+                              bool IsAArch64) {
   DynoStats DynoStatsBefore(IsAArch64);
   if (Flag)
-    DynoStatsBefore = getDynoStats(Funcs);
+    DynoStatsBefore = getDynoStats(Funcs, IsAArch64);
 
   Func();
 
   if (Flag) {
-    const DynoStats DynoStatsAfter = getDynoStats(Funcs);
+    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';
     if (Changed)
       DynoStatsAfter.print(outs(), &DynoStatsBefore);
     outs() << '\n';
   }
 }
 
 } // namespace bolt
 } // namespace llvm
 
 #endif

bolt/include/bolt/Passes/BinaryPasses.h

public:
 
   const char *getName() const override {
     return "print dyno-stats after optimizations";
   }
 
   bool shouldPrint(const BinaryFunction &BF) const override { return false; }
 
   void runOnFunctions(BinaryContext &BC) override {
-    const DynoStats NewDynoStats = getDynoStats(BC.getBinaryFunctions());
+    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;
     if (Changed) {
       outs() << '\n';
       NewDynoStats.print(outs(), &PrevDynoStats, BC.InstPrinter.get());
     }

bolt/lib/Rewrite/BinaryPassManager.cpp

for (size_t PassIdx = 0; PassIdx < Passes.size(); PassIdx++) {
 
     if (opts::Verbosity > 0)
       outs() << "BOLT-INFO: Starting pass: " << Pass->getName() << "\n";
 
     NamedRegionTimer T(Pass->getName(), Pass->getName(), TimerGroupName,
                        TimerGroupDesc, TimeOpts);
 
     callWithDynoStats([this, &Pass] { Pass->runOnFunctions(BC); }, BFs,
-                      Pass->getName(), opts::DynoStatsAll);
+                      Pass->getName(), opts::DynoStatsAll, BC.isAArch64());
 
     if (opts::VerifyCFG &&
         !std::accumulate(
             BFs.begin(), BFs.end(), true,
             [](const bool Valid,
                const std::pair<const uint64_t, BinaryFunction> &It) {
               return Valid && It.second.validateCFG();
             })) {
for (auto &It : BFs) {
         Function.dumpGraphForPass(PassIdName);
     }
   }
 }
 
 void BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) {
   BinaryFunctionPassManager Manager(BC);
 
-  const DynoStats InitialDynoStats = getDynoStats(BC.getBinaryFunctions());
+  const DynoStats InitialDynoStats =
+      getDynoStats(BC.getBinaryFunctions(), BC.isAArch64());
 
   Manager.registerPass(std::make_unique<AsmDumpPass>(),
                        opts::AsmDump.getNumOccurrences());
 
   if (opts::Instrument)
     Manager.registerPass(std::make_unique<Instrumentation>(NeverPrint));
 
   // Here we manage dependencies/order manually, since passes are run in the

bolt/unittests/Core/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   DebugInfoDWARF
   Object
   MC
   ${LLVM_TARGETS_TO_BUILD}
   )
 
 add_bolt_unittest(CoreTests
   BinaryContext.cpp
   MCPlusBuilder.cpp
+  DynoStats.cpp
   )
 
 target_link_libraries(CoreTests
   PRIVATE
   LLVMBOLTCore
   LLVMBOLTRewrite
   )
 

bolt/unittests/Core/DynoStats.cpp

+//===- llvm/unittest/MC/MCInstPrinter.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
+//
+//===----------------------------------------------------------------------===//
+
+#include "bolt/Core/DynoStats.h"
+#include "bolt/Core/BinaryFunction.h"
+#include "gtest/gtest.h"
+#include <map>
+
+using namespace llvm::bolt;
+
+TEST(DynoStatsTest, emptyFuncs) {
+  std::map<uint64_t, BinaryFunction> BinaryFunctions;
+  DynoStats DynoStatsAArch64 =
+      getDynoStats(BinaryFunctions, /* BC.isAArch64() = */ true);
+  DynoStats DynoStatsNonAArch64 =
+      getDynoStats(BinaryFunctions, /* BC.isAArch64() = */ false);
+  // Both should be null
+  ASSERT_EQ(DynoStatsAArch64, DynoStatsNonAArch64);
+}