From 832a8d0baa4e9e48ef11aab89b8692e276ca218f Mon Sep 17 00:00:00 2001 From: Ivan Krysak Date: Mon, 9 May 2022 16:38:23 +0300 Subject: [PATCH] Use TTG-based assembly to weaken dependency on efa --- include/revng/Yield/BasicBlock.h | 6 ++ include/revng/Yield/HTML.h | 18 ++-- include/revng/Yield/Plain.h | 18 ++-- lib/Pipes/Yield/Assembly.cpp | 2 +- lib/Yield/Assembly/HTML.cpp | 172 ++++++++++++++++++++----------- lib/Yield/Assembly/Plain.cpp | 118 +++++++++++++++------ 6 files changed, 221 insertions(+), 113 deletions(-) diff --git a/include/revng/Yield/BasicBlock.h b/include/revng/Yield/BasicBlock.h index 421c89677..19e7038c7 100644 --- a/include/revng/Yield/BasicBlock.h +++ b/include/revng/Yield/BasicBlock.h @@ -42,6 +42,12 @@ fields: elementType: yield::Instruction - name: IsLabelAlwaysRequired + doc: | + This flag is set to `false` for basic blocks that are never directly + pointed to, i.e. blocks that are only ever entered from the previous + instructions and such. + This lets us dynamically decide whether we want to show labels like this + or not. type: bool key: diff --git a/include/revng/Yield/HTML.h b/include/revng/Yield/HTML.h index bf09e6881..282dac261 100644 --- a/include/revng/Yield/HTML.h +++ b/include/revng/Yield/HTML.h @@ -6,27 +6,23 @@ #include +class MetaAddress; namespace model { class Binary; } -namespace efa { -class FunctionMetadata; -} namespace yield { -class BasicBlock; class Function; -} // namespace yield +} namespace yield { namespace html { -std::string assembly(const yield::BasicBlock &BasicBlock, - const efa::FunctionMetadata &Metadata, - const model::Binary &Binary); -std::string assembly(const yield::Function &Function, - const efa::FunctionMetadata &Metadata, - const model::Binary &Binary); +std::string functionAssembly(const yield::Function &InternalFunction, + const model::Binary &Binary); +std::string controlFlowNode(const MetaAddress &BasicBlockAddress, + const yield::Function &Function, + const model::Binary &Binary); } // namespace html diff --git a/include/revng/Yield/Plain.h b/include/revng/Yield/Plain.h index eaebf7137..2b407fdbd 100644 --- a/include/revng/Yield/Plain.h +++ b/include/revng/Yield/Plain.h @@ -6,27 +6,23 @@ #include +class MetaAddress; namespace model { class Binary; } -namespace efa { -class FunctionMetadata; -} namespace yield { -class BasicBlock; class Function; -} // namespace yield +} namespace yield { namespace plain { -std::string assembly(const yield::BasicBlock &BasicBlock, - const efa::FunctionMetadata &Metadata, - const model::Binary &Binary); -std::string assembly(const yield::Function &Function, - const efa::FunctionMetadata &Metadata, - const model::Binary &Binary); +std::string functionAssembly(const yield::Function &InternalFunction, + const model::Binary &Binary); +std::string controlFlowNode(const MetaAddress &BasicBlockAddress, + const yield::Function &Function, + const model::Binary &Binary); } // namespace plain diff --git a/lib/Pipes/Yield/Assembly.cpp b/lib/Pipes/Yield/Assembly.cpp index dc8acf574..0927f3ebf 100644 --- a/lib/Pipes/Yield/Assembly.cpp +++ b/lib/Pipes/Yield/Assembly.cpp @@ -45,7 +45,7 @@ void YieldAssemblyPipe::run(pipeline::Context &Context, const auto &Func = *ModelFunctionIterator; auto Disassembled = Helper.disassemble(Func, *Metadata, BinaryView, *Model); - auto HTML = yield::html::assembly(Disassembled, *Metadata, *Model); + auto HTML = yield::html::functionAssembly(Disassembled, *Model); Output.insert_or_assign(Metadata->Entry, std::move(HTML)); } } diff --git a/lib/Yield/Assembly/HTML.cpp b/lib/Yield/Assembly/HTML.cpp index d83732e01..def5a6ab6 100644 --- a/lib/Yield/Assembly/HTML.cpp +++ b/lib/Yield/Assembly/HTML.cpp @@ -7,21 +7,16 @@ #include "llvm/Support/FormatVariadic.h" -#include "revng/EarlyFunctionAnalysis/FunctionMetadata.h" #include "revng/Model/Binary.h" #include "revng/Yield/Function.h" #include "revng/Yield/HTML.h" -namespace yield::html { - namespace tags { -static constexpr auto NormalBasicBlockLabel = "basic-block-label"; -static constexpr auto HiddenBasicBlockLabel = "hidden-basic-block-label"; -static constexpr auto GraphOnlyBasicBlockLabel = "graph-only-basic-block-label"; - static constexpr auto Function = "function"; static constexpr auto BasicBlock = "basic-block"; +static constexpr auto FunctionLabel = "function-label"; +static constexpr auto BasicBlockLabel = "basic-block-label"; static constexpr auto Instruction = "instruction"; static constexpr auto InstructionAddress = "instruction-address"; @@ -88,7 +83,7 @@ static std::string instructionID(const MetaAddress &Address) { } static std::string link(const MetaAddress &Target, - const efa::FunctionMetadata &Metadata, + const yield::Function &Function, const model::Binary &Binary, llvm::StringRef CustomName = "") { if (auto Iterator = Binary.Functions.find(Target); @@ -101,28 +96,25 @@ static std::string link(const MetaAddress &Target, tags::FunctionLink, linkAddress(Target) + ".html#" + basicBlockID(Target), std::move(FinalName)); - } else if (auto Iterator = Metadata.ControlFlowGraph.find(Target); - Iterator != Metadata.ControlFlowGraph.end()) { + } else if (auto Iterator = Function.ControlFlowGraph.find(Target); + Iterator != Function.ControlFlowGraph.end()) { // The target is a basic block std::string FinalName = CustomName.str(); if (FinalName.empty()) { - // Deduce the function current basic block belongs to based on - // the received metadata and use that as a part of its name - std::string FunctionName = "unknown_function_"; - if (auto FunctionIterator = Binary.Functions.find(Metadata.Entry); - FunctionIterator != Binary.Functions.end()) { - FunctionName = FunctionIterator->name().str().str() + "_"; - } + auto FunctionIterator = Binary.Functions.find(Function.Entry); + revng_assert(FunctionIterator != Binary.Functions.end()); + std::string FunctionPrefix = FunctionIterator->name().str().str() + "_"; std::string BlockOwnerName = llvm::formatv(templates::Span, tags::BasicBlockOwner, - std::move(FunctionName)); + std::move(FunctionPrefix)); + std::string BlockName = "basic_block_at_" + linkAddress(Target); FinalName = std::move(BlockOwnerName) + std::move(BlockName); } return llvm::formatv(templates::Link, tags::BasicBlockLink, - linkAddress(Metadata.Entry) + ".html#" + linkAddress(Function.Entry) + ".html#" + basicBlockID(Target), std::move(FinalName)); } else if (Target.isValid()) { @@ -132,7 +124,7 @@ static std::string link(const MetaAddress &Target, FinalName = Target.toString(); return llvm::formatv(templates::Link, tags::InstructionLink, - linkAddress(Metadata.Entry) + ".html#" + linkAddress(Function.Entry) + ".html#" + instructionID(Target), std::move(FinalName)); } else { @@ -156,15 +148,13 @@ static std::string labelIndicator(model::Architecture::Values Architecture) { } static std::string label(const yield::BasicBlock &BasicBlock, - const efa::FunctionMetadata &Metadata, + const yield::Function &Function, const model::Binary &Binary) { - std::string_view LabelTag = tags::NormalBasicBlockLabel; - if (BasicBlock.IsLabelAlwaysRequired) - LabelTag = tags::GraphOnlyBasicBlockLabel; - - std::string Link = link(BasicBlock.Start, Metadata, Binary); + std::string Link = link(BasicBlock.Start, Function, Binary); return llvm::formatv(templates::SimpleDiv, - std::move(LabelTag), + Function.Entry == BasicBlock.Start ? + tags::FunctionLabel : + tags::BasicBlockLabel, std::move(Link += labelIndicator(Binary.Architecture))); } @@ -261,9 +251,9 @@ static size_t countTargets(const auto &Targets) { static bool areTargetsAdjacent(const MetaAddress &CurrentAddress, const MetaAddress &TargetAddress, - const efa::FunctionMetadata &Metadata) { - auto CurrentIterator = Metadata.ControlFlowGraph.find(CurrentAddress); - if (CurrentIterator == Metadata.ControlFlowGraph.end()) + const yield::Function &Function) { + auto CurrentIterator = Function.ControlFlowGraph.find(CurrentAddress); + if (CurrentIterator == Function.ControlFlowGraph.end()) return false; return CurrentIterator->End == TargetAddress; @@ -271,25 +261,25 @@ static bool areTargetsAdjacent(const MetaAddress &CurrentAddress, static std::string targetLink(const MetaAddress &Target, const yield::BasicBlock &BasicBlock, - const efa::FunctionMetadata &Metadata, + const yield::Function &Function, const model::Binary &Binary) { - if (areTargetsAdjacent(BasicBlock.Start, Target, Metadata)) + if (areTargetsAdjacent(BasicBlock.Start, Target, Function)) return llvm::formatv(templates::Span, tags::InstructionTarget, link(Target, - Metadata, + Function, Binary, "the next instruction")); else return llvm::formatv(templates::Span, tags::InstructionTarget, - link(Target, Metadata, Binary)); + link(Target, Function, Binary)); } static std::string targets(const yield::BasicBlock &BasicBlock, - const efa::FunctionMetadata &Metadata, + const yield::Function &Function, const model::Binary &Binary, - size_t TailOffset) { + size_t TailOffset = 0) { size_t TargetCount = BasicBlock.Successors.size(); if (TargetCount == 0) return ""; // We know nothing about the targets. @@ -303,7 +293,7 @@ static std::string targets(const yield::BasicBlock &BasicBlock, "always goes to " + targetLink(Successor.Destination, BasicBlock, - Metadata, + Function, Binary)); } } else { @@ -311,7 +301,7 @@ static std::string targets(const yield::BasicBlock &BasicBlock, bool HasInvalidTargets = false; for (size_t Counter = 0; const auto &Edge : BasicBlock.Successors) { if (Edge->Destination.isValid()) { - auto Link = targetLink(Edge->Destination, BasicBlock, Metadata, Binary); + auto Link = targetLink(Edge->Destination, BasicBlock, Function, Binary); if (BasicBlock.Successors.size() == TargetCount && Counter != TargetCount - 1) { Link = std::move(Link) + ","; @@ -461,8 +451,9 @@ static std::string taggedText(const yield::Instruction &Instruction) { static std::string instruction(const yield::Instruction &Instruction, bool IsInDelayedSlot, bool NeedsToPrintTargets, + bool ShouldUseVerticalLayout, const yield::BasicBlock &BasicBlock, - const efa::FunctionMetadata &Metadata, + const yield::Function &Function, const model::Binary &Binary) { // MetaAddress of the instruction. std::string Result = blockComment(tags::InstructionAddress, @@ -484,7 +475,7 @@ static std::string instruction(const yield::Instruction &Instruction, "llvm Opcode: " + Instruction.OpcodeIdentifier); // Error message (Vertical layout only). - if (!Instruction.Error.empty()) + if (ShouldUseVerticalLayout == true && !Instruction.Error.empty()) Result += error(Binary, "Error: " + Instruction.Error + "\n"); // Tagged instruction body. @@ -499,7 +490,7 @@ static std::string instruction(const yield::Instruction &Instruction, } // Delayed slot notice if applicable. - if (IsInDelayedSlot) { + if (ShouldUseVerticalLayout == false && IsInDelayedSlot) { if (HasTailComments == true) Result += comment(Binary, "delayed", Tail, true); else @@ -508,7 +499,7 @@ static std::string instruction(const yield::Instruction &Instruction, } // An error message if present. - if (!Instruction.Error.empty()) { + if (ShouldUseVerticalLayout == false && !Instruction.Error.empty()) { if (HasTailComments == true) Result += error(Binary, "Error: " + Instruction.Error, Tail, true); else @@ -524,7 +515,7 @@ static std::string instruction(const yield::Instruction &Instruction, Result += newLine() + whitespace(Tail); } - Result += targets(BasicBlock, Metadata, Binary, Tail); + Result += targets(BasicBlock, Function, Binary, Tail); } return llvm::formatv(templates::BlockDiv, @@ -533,11 +524,30 @@ static std::string instruction(const yield::Instruction &Instruction, std::move(Result)); } +template static std::string basicBlock(const yield::BasicBlock &BasicBlock, - const efa::FunctionMetadata &Metadata, + const yield::Function &Function, const model::Binary &Binary) { - std::string Result; + // Blocks are strung together if there's no reason to keep them separate. + // This determines whether this is the last block in the current string + // (if `NextBlock` is `nullptr`) or if there's continuation. + const yield::BasicBlock *NextBlock = nullptr; + for (const auto &Successor : BasicBlock.Successors) { + if (const MetaAddress &Target = Successor->Destination; + Target.isValid() && Target == BasicBlock.End) { + auto Iterator = Function.ControlFlowGraph.find(Target); + if (Iterator != Function.ControlFlowGraph.end()) { + if (!Iterator->IsLabelAlwaysRequired) { + revng_assert(NextBlock == nullptr, + "Multiple targets with the same address"); + NextBlock = &*Iterator; + } + } + } + } + // Compile the list of delayed instructions so the corresponding comment + // can be emited. llvm::SmallVector DelayedList; bool IsNextInstructionDelayed = false; for (const auto &Instruction : BasicBlock.Instructions) { @@ -548,6 +558,8 @@ static std::string basicBlock(const yield::BasicBlock &BasicBlock, revng_assert(IsNextInstructionDelayed == false, "Last instruction has an unfilled delayed slot."); + // Determine the last "proper" instruction. This is the instruction "targets" + // get printed for if this is the last basic block in a string. MetaAddress LastNotDelayedInstruction = MetaAddress::invalid(); for (const auto &Instruction : llvm::reverse(BasicBlock.Instructions)) { if (!llvm::is_contained(DelayedList, Instruction.Address)) { @@ -555,14 +567,22 @@ static std::string basicBlock(const yield::BasicBlock &BasicBlock, break; } } + revng_assert(LastNotDelayedInstruction.isValid()); - Result += label(BasicBlock, Metadata, Binary); + // String the results together. + std::string Result; for (const auto &Instruction : BasicBlock.Instructions) { + bool PrintTargets = LastNotDelayedInstruction == Instruction.Address; + PrintTargets = PrintTargets && !UseVerticalTargetLayout; + if (NextBlock != nullptr) + PrintTargets = PrintTargets && BasicBlock.Successors.size() > 1; + Result += instruction(Instruction, llvm::is_contained(DelayedList, Instruction.Address), - LastNotDelayedInstruction == Instruction.Address, + PrintTargets, + UseVerticalTargetLayout, BasicBlock, - Metadata, + Function, Binary); } @@ -571,26 +591,62 @@ static std::string basicBlock(const yield::BasicBlock &BasicBlock, Result += error(Binary, std::move(DelayedError), 2, true); } + if (NextBlock != nullptr) { + return Result += basicBlock(*NextBlock, + Function, + Binary); + } else { + if constexpr (UseVerticalTargetLayout == true) { + auto Targets = targets(BasicBlock, Function, Binary); + if (!Targets.empty()) + Result += newLine() + std::move(Targets); + } + + return Result; + } +} + +template +static std::string basicBlockString(const yield::BasicBlock &BasicBlock, + const yield::Function &Function, + const model::Binary &Binary) { + // Blocks that are merged into other block strings cannot start a new one. + if (!BasicBlock.IsLabelAlwaysRequired) + return ""; + + std::string Result; + Result += label(BasicBlock, Function, Binary); + Result += basicBlock(BasicBlock, Function, Binary); + return llvm::formatv(templates::BlockDiv, tags::BasicBlock, basicBlockID(BasicBlock.Start), std::move(Result)); } -std::string assembly(const yield::BasicBlock &BasicBlock, - const efa::FunctionMetadata &Metadata, - const model::Binary &Binary) { - return basicBlock(BasicBlock, Metadata, Binary); -} -std::string assembly(const yield::Function &Function, - const efa::FunctionMetadata &Metadata, - const model::Binary &Binary) { +std::string yield::html::functionAssembly(const yield::Function &Function, + const model::Binary &Binary) { std::string Result; for (const auto &BasicBlock : Function.ControlFlowGraph) - Result += basicBlock(BasicBlock, Metadata, Binary); + Result += basicBlockString(BasicBlock, Function, Binary); return Result; } -} // namespace yield::html +std::string yield::html::controlFlowNode(const MetaAddress &Address, + const yield::Function &Function, + const model::Binary &Binary) { + if (auto Iterator = Function.ControlFlowGraph.find(Address); + Iterator != Function.ControlFlowGraph.end()) { + auto Result = basicBlockString(*Iterator, Function, Binary); + revng_assert(!Result.empty()); + + return Result; + } else { + revng_assert(Binary.Functions.find(Address) != Binary.Functions.end()); + return link(Address, Function, Binary); + } +} diff --git a/lib/Yield/Assembly/Plain.cpp b/lib/Yield/Assembly/Plain.cpp index 0715d5e53..bb3b8ba91 100644 --- a/lib/Yield/Assembly/Plain.cpp +++ b/lib/Yield/Assembly/Plain.cpp @@ -7,13 +7,10 @@ #include "llvm/Support/FormatVariadic.h" -#include "revng/EarlyFunctionAnalysis/FunctionMetadata.h" #include "revng/Model/Binary.h" #include "revng/Yield/Function.h" #include "revng/Yield/Plain.h" -namespace yield::plain { - static std::string linkAddress(const MetaAddress &Address) { std::string Result = Address.toString(); @@ -28,15 +25,15 @@ static std::string linkAddress(const MetaAddress &Address) { return Result; } -static std::string link(const MetaAddress &Target, - const efa::FunctionMetadata &Metadata, - const model::Binary &Binary) { +static std::string deduceName(const MetaAddress &Target, + const yield::Function &Function, + const model::Binary &Binary) { if (auto Iterator = Binary.Functions.find(Target); Iterator != Binary.Functions.end()) { // The target is a function return Iterator->name().str().str(); - } else if (auto Iterator = Metadata.ControlFlowGraph.find(Target); - Iterator != Metadata.ControlFlowGraph.end()) { + } else if (auto Iterator = Function.ControlFlowGraph.find(Target); + Iterator != Function.ControlFlowGraph.end()) { // The target is a basic block // TODO: maybe there's something better than the address to put here. @@ -46,49 +43,106 @@ static std::string link(const MetaAddress &Target, return "instruction_at_" + linkAddress(Target); } else { // The target is impossible to deduce, it's an indirect call or the like. - return "(unknown)"; + return "(error)"; } } static std::string label(const yield::BasicBlock &BasicBlock, - const efa::FunctionMetadata &Metadata, + const yield::Function &Function, const model::Binary &Binary) { - if (BasicBlock.IsLabelAlwaysRequired) - return ""; + std::string Result = deduceName(BasicBlock.Start, Function, Binary); - return link(BasicBlock.Start, Metadata, Binary) + ":\n"; + namespace Arch = model::Architecture; + auto LabelIndicator = Arch::getAssemblyLabelIndicator(Binary.Architecture); + return (Result += LabelIndicator) += "\n"; } -static std::string instruction(const yield::Instruction &Instruction) { - return Instruction.Disassembled + '\n'; -} +static std::string instruction(const yield::Instruction &Instruction, + const yield::BasicBlock &BasicBlock, + const model::Binary &Binary) { + std::string Result = Instruction.Disassembled; -static std::string basicBlock(const yield::BasicBlock &BasicBlock, - const efa::FunctionMetadata &Metadata, - const model::Binary &Binary) { - std::string Result; + namespace A = model::Architecture; + auto CommentIndicator = A::getAssemblyCommentIndicator(Binary.Architecture); - Result += label(BasicBlock, Metadata, Binary); - for (const auto &Instruction : BasicBlock.Instructions) - Result += instruction(Instruction); + if (!Instruction.Error.empty()) { + Result += ' '; + Result += CommentIndicator; + Result += " Error: "; + Result += Instruction.Error; + } else if (!Instruction.Comment.empty()) { + Result += ' '; + Result += CommentIndicator; + Result += ' '; + Result += Instruction.Comment; + } return Result; } -std::string assembly(const yield::BasicBlock &BasicBlock, - const efa::FunctionMetadata &Metadata, - const model::Binary &Binary) { - return basicBlock(BasicBlock, Metadata, Binary); +template +static std::string basicBlock(const yield::BasicBlock &BasicBlock, + const yield::Function &Function, + const model::Binary &Binary) { + // Blocks are strung together if there's no reason to keep them separate. + // This determines whether this is the last block in the current string + // (if `NextBlock` is `nullptr`) or if there's continuation. + const yield::BasicBlock *NextBlock = nullptr; + for (const auto &Successor : BasicBlock.Successors) { + if (Successor->Destination == BasicBlock.End) { + auto Iterator = Function.ControlFlowGraph.find(Successor->Destination); + revng_assert(Iterator != Function.ControlFlowGraph.end()); + if (!Iterator->IsLabelAlwaysRequired) { + NextBlock = &*Iterator; + } + } + } + + // String the results together. + std::string Result; + for (const auto &Instruction : BasicBlock.Instructions) + Result += instruction(Instruction, BasicBlock, Binary); + + if (NextBlock != nullptr) + return Result += basicBlock(*NextBlock, + Function, + Binary); + else + return Result; } -std::string assembly(const yield::Function &Function, - const efa::FunctionMetadata &Metadata, - const model::Binary &Binary) { + +template +static std::string basicBlockString(const yield::BasicBlock &BasicBlock, + const yield::Function &Function, + const model::Binary &Binary) { + // Blocks that are merged into other block strings cannot start a new one. + if (!BasicBlock.IsLabelAlwaysRequired) + return ""; + + return label(BasicBlock, Function, Binary) + + basicBlock(BasicBlock, + Function, + Binary); +} + +std::string yield::plain::functionAssembly(const yield::Function &Function, + const model::Binary &Binary) { std::string Result; for (const auto &BasicBlock : Function.ControlFlowGraph) - Result += basicBlock(BasicBlock, Metadata, Binary); + Result += basicBlockString(BasicBlock, Function, Binary); return Result; } -} // namespace yield::plain +std::string yield::plain::controlFlowNode(const MetaAddress &BasicBlockAddress, + const yield::Function &Function, + const model::Binary &Binary) { + auto Iterator = Function.ControlFlowGraph.find(BasicBlockAddress); + revng_assert(Iterator != Function.ControlFlowGraph.end()); + + auto Result = basicBlockString(*Iterator, Function, Binary); + revng_assert(!Result.empty()); + + return Result; +}