From bbb745efa9a3124607c13711140ec8461655836c Mon Sep 17 00:00:00 2001 From: Maksim Panchenko Date: Tue, 19 Jan 2016 00:20:06 -0800 Subject: [PATCH] Don't create empty basic blocks. Fix CFI bug. Summary: Some basic blocks were created empty because they only contained alignment nop's. Ignore such nop's before basic block gets created. Fixed intermittent aborts related to CFI update. (cherry picked from FBD2844465) --- bolt/BinaryFunction.cpp | 20 +++++++++++--------- bolt/RewriteInstance.cpp | 14 ++++++++------ 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/bolt/BinaryFunction.cpp b/bolt/BinaryFunction.cpp index 76cbbf05e1d4..ee79308f4c11 100644 --- a/bolt/BinaryFunction.cpp +++ b/bolt/BinaryFunction.cpp @@ -542,6 +542,13 @@ bool BinaryFunction::buildCFG() { InsertBB = addBasicBlock(LI->first, LI->second, /* DeriveAlignment = */ IsLastInstrNop); } + // 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 (MIA->isNoop(InstrInfo.second)) { + IsLastInstrNop = true; + continue; + } if (!InsertBB) { // It must be a fallthrough or unreachable code. Create a new block unless // we see an unconditional branch following a conditional one. @@ -561,13 +568,6 @@ bool BinaryFunction::buildCFG() { // Add associated CFI pseudos in the first offset (0) addCFIPlaceholders(0, InsertBB); } - // 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 (MIA->isNoop(InstrInfo.second)) { - IsLastInstrNop = true; - continue; - } IsLastInstrNop = false; InsertBB->addInstruction(InstrInfo.second); @@ -930,16 +930,18 @@ bool BinaryFunction::fixCFIState() { } } auto Pos = BB->begin(); - while (MCCFIInstruction *CFI = getCFIFor(*Pos++)) { + while (Pos != BB->end() && BC.MIA->isCFI(*Pos)) { + auto CFI = getCFIFor(*Pos); if (CFI->getOperation() == MCCFIInstruction::OpRememberState) ++StackOffset; if (CFI->getOperation() == MCCFIInstruction::OpRestoreState) --StackOffset; + ++Pos; } if (StackOffset != 0) { errs() << " FLO-WARNING: not possible to remember/recover state" - << "without corrupting CFI state stack in function " + << " without corrupting CFI state stack in function " << getName() << "\n"; return false; } diff --git a/bolt/RewriteInstance.cpp b/bolt/RewriteInstance.cpp index b46b631b5b87..71d2a9b33626 100644 --- a/bolt/RewriteInstance.cpp +++ b/bolt/RewriteInstance.cpp @@ -690,7 +690,7 @@ void RewriteInstance::disassembleFunctions() { errs() << "FLO-INFO: " << ProfiledFunctions.size() << " functions out of " << NumSimpleFunctions << " simple functions (" - << format("%.f", + << format("%.1f", ProfiledFunctions.size() / (float) NumSimpleFunctions * 100.0) << "%) have non-empty execution profile.\n"; @@ -775,17 +775,19 @@ void RewriteInstance::runOptimizationPasses() { // Post-processing passes. - // Update exception handling information. - Function.updateEHRanges(); - if (opts::PrintAll || opts::PrintEHRanges) - Function.print(errs(), "after updating EH ranges"); - // Fix the CFI state. if (!Function.fixCFIState()) { errs() << "FLO-WARNING: unable to fix CFI state for function " << Function.getName() << ". Skipping.\n"; Function.setSimple(false); + continue; } + + // Update exception handling information. + Function.updateEHRanges(); + if (opts::PrintAll || opts::PrintEHRanges) + Function.print(errs(), "after updating EH ranges"); + } }