From 04a4591f5c500cb5b34ff0c6bf2785bc03e0f553 Mon Sep 17 00:00:00 2001 From: Alessandro Di Federico Date: Wed, 1 Mar 2017 17:04:54 +0100 Subject: [PATCH] When splitting a basic block, retranslate This commit should fix some bugs due to the fact that when we're splitting a basic block we don't retranslate the basic block at the split point but preserve the existing code. This lead to problems, in particular in x86-64 where certain QEMU local variables were not available. This change should fix it. Basically, every time we split a basic block in `JumpTargetManager::registerJT` we note down that the new basic block must be purged, and in `JumpTargetManager::harvest` we perform the purge. `harvest` has been chosen since it's a particularly quiet moment, i.e., there should be no pending references/iterator to code we have to delete. --- codegenerator.cpp | 1 + functionboundariesdetection.cpp | 1 + ir-helpers.h | 17 ++++++++ jumptargetmanager.cpp | 69 +++++++++++++++++++++++++++++---- jumptargetmanager.h | 42 ++++++++++++++++++++ 5 files changed, 122 insertions(+), 8 deletions(-) diff --git a/codegenerator.cpp b/codegenerator.cpp index 6ce95db5d..d9dcc7300 100644 --- a/codegenerator.cpp +++ b/codegenerator.cpp @@ -632,6 +632,7 @@ void CodeGenerator::translate(uint64_t VirtualAddress) { while (Entry != nullptr) { Builder.SetInsertPoint(Entry); + // TODO: what if create a new instance of an InstructionTranslator here? Translator.reset(); // TODO: rename this type diff --git a/functionboundariesdetection.cpp b/functionboundariesdetection.cpp index 2bbf6bfd7..ffcda4efb 100644 --- a/functionboundariesdetection.cpp +++ b/functionboundariesdetection.cpp @@ -249,6 +249,7 @@ void FBD::collectReturnInstructions() { bool IsReturn = true; for (BasicBlock *Successor : Terminator->successors()) { + assert(!Successor->empty()); // A return instruction must jump to JTM->anyPC, while all the other // successors (if any) must be registered returns addresses diff --git a/ir-helpers.h b/ir-helpers.h index 5cca5870c..3ad26764a 100644 --- a/ir-helpers.h +++ b/ir-helpers.h @@ -469,4 +469,21 @@ static inline llvm::Instruction *getNext(llvm::Instruction *I) { return &*It; } +/// \brief Check whether the instruction/basic block is the first in its +/// container or not +template +static inline bool isFirst(T *I) { + assert(I != nullptr); + return I == &*I->getParent()->begin(); +} + +/// \brief Check if among \p BB's predecessors there's \p Target +static inline bool hasPredecessor(llvm::BasicBlock *BB, + llvm::BasicBlock *Target) { + for (llvm::BasicBlock *Predecessor : predecessors(BB)) + if (Predecessor == Target) + return true; + return false; +} + #endif // _IRHELPERS_H diff --git a/jumptargetmanager.cpp b/jumptargetmanager.cpp index 573a74a42..d20326519 100644 --- a/jumptargetmanager.cpp +++ b/jumptargetmanager.cpp @@ -166,7 +166,6 @@ bool TranslateDirectBranchesPass::pinConstantStore(Function &F) { if (Address != nullptr) { // Compute the actual PC and get the associated BasicBlock uint64_t TargetPC = Address->getSExtValue(); - // TODO: can we switch to getBlockAt()? auto *TargetBlock = JTM->registerJT(TargetPC, JumpTargetManager::DirectJump); @@ -962,6 +961,11 @@ void JumpTargetManager::translateIndirectJumps() { JumpTargetManager::BlockWithAddress JumpTargetManager::peek() { harvest(); + // Purge all the partial translations we know might be wrong + for (BasicBlock *BB : ToPurge) + purgeTranslation(BB); + ToPurge.clear(); + if (Unexplored.empty()) return NoMoreTargets; else { @@ -1002,6 +1006,47 @@ BasicBlock *JumpTargetManager::getBlockAt(uint64_t PC) { return TargetIt->second.head(); } +void JumpTargetManager::purgeTranslation(BasicBlock *Start) { + OnceQueue Queue; + Queue.insert(Start); + + // Collect all the descendats, except if we meet a jump target + while (!Queue.empty()) { + BasicBlock *BB = Queue.pop(); + for (BasicBlock *Successor : successors(BB)) { + if (isTranslatedBB(Successor) + && !isJumpTarget(Successor) + && !hasPredecessor(Successor, Dispatcher)) { + Queue.insert(Successor); + } + } + } + + // Purge (but do not erase) the starting basic block + while (!Start->empty()) + eraseInstruction(&*(--Start->end())); + + // Erase all the visited basic blocks + std::set Visited = Queue.visited(); + Visited.erase(Start); + for (BasicBlock *BB : Visited) { + while (!BB->empty()) + eraseInstruction(&*(--BB->end())); + } + + for (BasicBlock *BB : Visited) { + // We might have some predecessorless basic blocks jumping to us, purge them + // TODO: why this? + for (BasicBlock *Predecessor : predecessors(BB)) { + assert(pred_empty(Predecessor)); + Predecessor->eraseFromParent(); + } + + assert(BB->use_empty()); + BB->eraseFromParent(); + } +} + // TODO: register Reason BasicBlock *JumpTargetManager::registerJT(uint64_t PC, JTReason Reason) { if (!isExecutableAddress(PC) || !isInstructionAligned(PC)) @@ -1024,22 +1069,30 @@ BasicBlock *JumpTargetManager::registerJT(uint64_t PC, JTReason Reason) { if (InstrIt != OriginalInstructionAddresses.end()) { // Case 2: the address has already been met, but needs to be promoted to // BasicBlock level. - BasicBlock *ContainingBlock = InstrIt->second->getParent(); - if (InstrIt->second == &*ContainingBlock->begin()) + Instruction *I = InstrIt->second; + BasicBlock *ContainingBlock = I->getParent(); + if (isFirst(I)) { NewBlock = ContainingBlock; - else { - assert(InstrIt->second != nullptr - && InstrIt->second != ContainingBlock->end()); - NewBlock = ContainingBlock->splitBasicBlock(InstrIt->second); + } else { + assert(I != nullptr && I != ContainingBlock->end()); + NewBlock = ContainingBlock->splitBasicBlock(I); } + + // Register the basic block and all of its descendants to be purged so that + // we can retranslate this PC + // TODO: this might create a problem if QEMU generates control flow that + // crosses an instruction boundary + ToPurge.insert(NewBlock); + unvisit(NewBlock); } else { // Case 3: the address has never been met, create a temporary one, register // it for future exploration and return it NewBlock = BasicBlock::Create(Context, "", TheFunction); - Unexplored.push_back(BlockWithAddress(PC, NewBlock)); } + Unexplored.push_back(BlockWithAddress(PC, NewBlock)); + if (NewBlock->getName().empty()) { std::stringstream Name; Name << "bb." << nameForAddress(PC); diff --git a/jumptargetmanager.h b/jumptargetmanager.h index da57315d8..b72aeba95 100644 --- a/jumptargetmanager.h +++ b/jumptargetmanager.h @@ -282,6 +282,18 @@ public: return JumpTargets.count(PC); } + /// \brief Return true if the given basic block corresponds to a jump target + bool isJumpTarget(llvm::BasicBlock *BB) { + if (BB->empty()) + return false; + + uint64_t PC = getPCFromNewPCCall(&*BB->begin()); + if (PC != 0) + return isJumpTarget(PC); + + return false; + } + /// \brief Return true if \p PC is in an executable segment bool isExecutableAddress(uint64_t PC) const { for (std::pair Range : ExecutableRanges) @@ -467,6 +479,35 @@ public: private: + /// \brief Helper function to check if an instruction is a call to `newpc` + /// + /// \return 0 if \p I is not a call to `newpc`, otherwise the PC address of + /// associated to the call to `newpc` + uint64_t getPCFromNewPCCall(llvm::Instruction *I) { + if (auto *CallNewPC = llvm::dyn_cast(I)) { + if (CallNewPC->getCalledFunction() == nullptr + || CallNewPC->getCalledFunction()->getName() != "newpc") + return 0; + + return getLimitedValue(CallNewPC->getArgOperand(0)); + } + + return 0; + } + + /// \brief Erase \p I, and deregister it in case it's a call to `newpc` + void eraseInstruction(llvm::Instruction *I) { + assert(I->use_empty()); + + uint64_t PC = getPCFromNewPCCall(I); + if (PC != 0) + OriginalInstructionAddresses.erase(PC); + I->eraseFromParent(); + } + + /// \brief Drop \p Start and all the descendants, stopping when a JT is met + void purgeTranslation(llvm::BasicBlock *Start); + /// \brief Check if \p BB has at least a predecessor, excluding the dispatcher bool hasPredecessors(llvm::BasicBlock *BB) const; @@ -532,6 +573,7 @@ private: boost::icl::interval_map SymbolMap; CFGForm CurrentCFGForm; + std::set ToPurge; }; template<>