Skip to content

Commit fa7dd49

Browse files
authored
[BOLT][NFC] Add BOLTError and return it from passes (1/2) (#81522)
As part of the effort to refactor old error handling code that would directly call exit(1), in this patch we add a new class BOLTError and auxiliary functions `createFatalBOLTError()` and `createNonFatalBOLTError()` that allow BOLT code to bubble up the problem to the caller by using the Error class as a return type (or Expected). Also changes passes to use these. Co-authored-by: Rafael Auler <[email protected]> Test Plan: NFC
1 parent a5f3d1a commit fa7dd49

8 files changed

+71
-15
lines changed

Diff for: bolt/include/bolt/Core/BinaryContext.h

+23
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,29 @@ class FilterIterator {
145145
}
146146
};
147147

148+
/// BOLT-exclusive errors generated in core BOLT libraries, optionally holding a
149+
/// string message and whether it is fatal or not. In case it is fatal and if
150+
/// BOLT is running as a standalone process, the process might be killed as soon
151+
/// as the error is checked.
152+
class BOLTError : public ErrorInfo<BOLTError> {
153+
public:
154+
static char ID;
155+
156+
BOLTError(bool IsFatal, const Twine &S = Twine());
157+
void log(raw_ostream &OS) const override;
158+
bool isFatal() const { return IsFatal; }
159+
160+
const std::string &getMessage() const { return Msg; }
161+
std::error_code convertToErrorCode() const override;
162+
163+
private:
164+
bool IsFatal;
165+
std::string Msg;
166+
};
167+
168+
Error createNonFatalBOLTError(const Twine &S);
169+
Error createFatalBOLTError(const Twine &S);
170+
148171
class BinaryContext {
149172
BinaryContext() = delete;
150173

Diff for: bolt/lib/Core/BinaryContext.cpp

+31
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,37 @@ cl::opt<std::string> CompDirOverride(
8383
namespace llvm {
8484
namespace bolt {
8585

86+
char BOLTError::ID = 0;
87+
88+
BOLTError::BOLTError(bool IsFatal, const Twine &S)
89+
: IsFatal(IsFatal), Msg(S.str()) {}
90+
91+
void BOLTError::log(raw_ostream &OS) const {
92+
if (IsFatal)
93+
OS << "FATAL ";
94+
StringRef ErrMsg = StringRef(Msg);
95+
// Prepend our error prefix if it is missing
96+
if (ErrMsg.empty()) {
97+
OS << "BOLT-ERROR\n";
98+
} else {
99+
if (!ErrMsg.starts_with("BOLT-ERROR"))
100+
OS << "BOLT-ERROR: ";
101+
OS << ErrMsg << "\n";
102+
}
103+
}
104+
105+
std::error_code BOLTError::convertToErrorCode() const {
106+
return inconvertibleErrorCode();
107+
}
108+
109+
Error createNonFatalBOLTError(const Twine &S) {
110+
return make_error<BOLTError>(/*IsFatal*/ false, S);
111+
}
112+
113+
Error createFatalBOLTError(const Twine &S) {
114+
return make_error<BOLTError>(/*IsFatal*/ true, S);
115+
}
116+
86117
BinaryContext::BinaryContext(std::unique_ptr<MCContext> Ctx,
87118
std::unique_ptr<DWARFContext> DwCtx,
88119
std::unique_ptr<Triple> TheTriple,

Diff for: bolt/lib/Passes/ADRRelaxationPass.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ Error ADRRelaxationPass::runOnFunctions(BinaryContext &BC) {
110110
"ADRRelaxationPass");
111111

112112
if (PassFailed)
113-
exit(1);
113+
return createFatalBOLTError("");
114114
return Error::success();
115115
}
116116

Diff for: bolt/lib/Passes/BinaryPasses.cpp

+5-1
Original file line numberDiff line numberDiff line change
@@ -528,12 +528,14 @@ Error FixupBranches::runOnFunctions(BinaryContext &BC) {
528528
}
529529

530530
Error FinalizeFunctions::runOnFunctions(BinaryContext &BC) {
531+
std::atomic<bool> HasFatal{false};
531532
ParallelUtilities::WorkFuncTy WorkFun = [&](BinaryFunction &BF) {
532533
if (!BF.finalizeCFIState()) {
533534
if (BC.HasRelocations) {
534535
errs() << "BOLT-ERROR: unable to fix CFI state for function " << BF
535536
<< ". Exiting.\n";
536-
exit(1);
537+
HasFatal = true;
538+
return;
537539
}
538540
BF.setSimple(false);
539541
return;
@@ -552,6 +554,8 @@ Error FinalizeFunctions::runOnFunctions(BinaryContext &BC) {
552554
ParallelUtilities::runOnEachFunction(
553555
BC, ParallelUtilities::SchedulingPolicy::SP_CONSTANT, WorkFun,
554556
SkipPredicate, "FinalizeFunctions");
557+
if (HasFatal)
558+
return createFatalBOLTError("finalize CFI state failure");
555559
return Error::success();
556560
}
557561

Diff for: bolt/lib/Passes/Instrumentation.cpp

+6-8
Original file line numberDiff line numberDiff line change
@@ -567,10 +567,9 @@ Error Instrumentation::runOnFunctions(BinaryContext &BC) {
567567

568568
ErrorOr<BinarySection &> SetupSection =
569569
BC.getUniqueSectionByName("I__setup");
570-
if (!SetupSection) {
571-
llvm::errs() << "Cannot find I__setup section\n";
572-
exit(1);
573-
}
570+
if (!SetupSection)
571+
return createFatalBOLTError("Cannot find I__setup section\n");
572+
574573
MCSymbol *Target = BC.registerNameAtAddress(
575574
"__bolt_instr_setup", SetupSection->getAddress(), 0, 0);
576575
MCInst NewInst;
@@ -586,10 +585,9 @@ Error Instrumentation::runOnFunctions(BinaryContext &BC) {
586585
BinaryBasicBlock &BB = Ctor->front();
587586
ErrorOr<BinarySection &> FiniSection =
588587
BC.getUniqueSectionByName("I__fini");
589-
if (!FiniSection) {
590-
llvm::errs() << "Cannot find I__fini section\n";
591-
exit(1);
592-
}
588+
if (!FiniSection)
589+
return createFatalBOLTError("Cannot find I__fini section");
590+
593591
MCSymbol *Target = BC.registerNameAtAddress(
594592
"__bolt_instr_fini", FiniSection->getAddress(), 0, 0);
595593
auto IsLEA = [&BC](const MCInst &Inst) { return BC.MIB->isLEA64r(Inst); };

Diff for: bolt/lib/Passes/PatchEntries.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ Error PatchEntries::runOnFunctions(BinaryContext &BC) {
103103
if (opts::ForcePatch) {
104104
errs() << "BOLT-ERROR: unable to patch entries in " << Function
105105
<< "\n";
106-
exit(1);
106+
return createFatalBOLTError("");
107107
}
108108

109109
continue;

Diff for: bolt/lib/Passes/ReorderFunctions.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ Error ReorderFunctions::runOnFunctions(BinaryContext &BC) {
444444
if (!FuncsFile) {
445445
errs() << "BOLT-ERROR: ordered functions file "
446446
<< opts::GenerateFunctionOrderFile << " cannot be opened\n";
447-
exit(1);
447+
return createFatalBOLTError("");
448448
}
449449
}
450450

@@ -455,7 +455,7 @@ Error ReorderFunctions::runOnFunctions(BinaryContext &BC) {
455455
if (!LinkSectionsFile) {
456456
errs() << "BOLT-ERROR: link sections file " << opts::LinkSectionsFile
457457
<< " cannot be opened\n";
458-
exit(1);
458+
return createFatalBOLTError("");
459459
}
460460
}
461461

Diff for: bolt/lib/Passes/VeneerElimination.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ Error VeneerElimination::runOnFunctions(BinaryContext &BC) {
7979
VeneerCallers++;
8080
if (!BC.MIB->replaceBranchTarget(
8181
Instr, VeneerDestinations[TargetSymbol], BC.Ctx.get())) {
82-
errs() << "BOLT-ERROR: updating veneer call destination failed\n";
83-
exit(1);
82+
return createFatalBOLTError(
83+
"BOLT-ERROR: updating veneer call destination failed\n");
8484
}
8585
}
8686
}

0 commit comments

Comments
 (0)