Skip to content

Commit

Permalink
[BOLT] Fix getDynoStats to handle BCs with no functions
Browse files Browse the repository at this point in the history
Address fuzzer crash

Reviewed By: yota9

Differential Revision: https://reviews.llvm.org/D120696
  • Loading branch information
aaupov committed Jun 30, 2022
1 parent 24b5f8e commit 66b01a8
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 9 deletions.
11 changes: 5 additions & 6 deletions bolt/include/bolt/Core/DynoStats.h
Expand Up @@ -146,8 +146,7 @@ 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;
Expand All @@ -160,16 +159,16 @@ inline DynoStats getDynoStats(const FuncsType &Funcs) {
/// 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"
Expand Down
3 changes: 2 additions & 1 deletion bolt/include/bolt/Passes/BinaryPasses.h
Expand Up @@ -71,7 +71,8 @@ class DynoStatsPrintPass : public BinaryFunctionPass {
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"
Expand Down
5 changes: 3 additions & 2 deletions bolt/lib/Rewrite/BinaryPassManager.cpp
Expand Up @@ -268,7 +268,7 @@ void BinaryFunctionPassManager::runPasses() {
TimerGroupDesc, TimeOpts);

callWithDynoStats([this, &Pass] { Pass->runOnFunctions(BC); }, BFs,
Pass->getName(), opts::DynoStatsAll);
Pass->getName(), opts::DynoStatsAll, BC.isAArch64());

if (opts::VerifyCFG &&
!std::accumulate(
Expand Down Expand Up @@ -307,7 +307,8 @@ void BinaryFunctionPassManager::runPasses() {
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());
Expand Down
1 change: 1 addition & 0 deletions bolt/unittests/Core/CMakeLists.txt
Expand Up @@ -8,6 +8,7 @@ set(LLVM_LINK_COMPONENTS
add_bolt_unittest(CoreTests
BinaryContext.cpp
MCPlusBuilder.cpp
DynoStats.cpp
)

target_link_libraries(CoreTests
Expand Down
24 changes: 24 additions & 0 deletions bolt/unittests/Core/DynoStats.cpp
@@ -0,0 +1,24 @@
//===- 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);
}

0 comments on commit 66b01a8

Please sign in to comment.