From 854b8b65b525aa62d08b5e3126253759875e24d1 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Thu, 9 Sep 2021 16:49:36 +0200 Subject: [PATCH] Better source locations in Yul ControlFlowGraph and OptimizedEVMCodeTransform. --- libyul/AST.h | 14 +-- libyul/backends/evm/ControlFlowGraph.h | 13 +- .../backends/evm/ControlFlowGraphBuilder.cpp | 118 ++++++++++-------- libyul/backends/evm/ControlFlowGraphBuilder.h | 19 +-- .../evm/OptimizedEVMCodeTransform.cpp | 23 ++-- .../backends/evm/OptimizedEVMCodeTransform.h | 3 +- 6 files changed, 108 insertions(+), 82 deletions(-) diff --git a/libyul/AST.h b/libyul/AST.h index 486e11faa..01eece34d 100644 --- a/libyul/AST.h +++ b/libyul/AST.h @@ -95,18 +95,16 @@ template inline langutil::SourceLocation locationOf(std::variant return std::visit([](auto const& _arg) { return locationOf(_arg); }, _node); } -struct DebugDataExtractor +/// Extracts the debug data from a Yul node. +template inline std::shared_ptr debugDataOf(T const& _node) { - template std::shared_ptr const& operator()(T const& _node) const - { - return _node.debugData; - } -}; + return _node.debugData; +} /// Extracts the debug data from a Yul node. -template inline std::shared_ptr const& debugDataOf(T const& _node) +template inline std::shared_ptr debugDataOf(std::variant const& _node) { - return std::visit(DebugDataExtractor(), _node); + return std::visit([](auto const& _arg) { return debugDataOf(_arg); }, _node); } } diff --git a/libyul/backends/evm/ControlFlowGraph.h b/libyul/backends/evm/ControlFlowGraph.h index b78a37843..0099b9924 100644 --- a/libyul/backends/evm/ControlFlowGraph.h +++ b/libyul/backends/evm/ControlFlowGraph.h @@ -172,18 +172,25 @@ struct CFG struct MainExit {}; struct ConditionalJump { + std::shared_ptr debugData; StackSlot condition; BasicBlock* nonZero = nullptr; BasicBlock* zero = nullptr; }; struct Jump { + std::shared_ptr debugData; BasicBlock* target = nullptr; /// The only backwards jumps are jumps from loop post to loop condition. bool backwards = false; }; - struct FunctionReturn { CFG::FunctionInfo* info = nullptr; }; + struct FunctionReturn + { + std::shared_ptr debugData; + CFG::FunctionInfo* info = nullptr; + }; struct Terminated {}; + std::shared_ptr debugData; std::vector entries; std::vector operations; std::variant exit = MainExit{}; @@ -216,9 +223,9 @@ struct CFG /// the switch case literals when transforming the control flow of a switch to a sequence of conditional jumps. std::list ghostCalls; - BasicBlock& makeBlock() + BasicBlock& makeBlock(std::shared_ptr _debugData) { - return blocks.emplace_back(BasicBlock{}); + return blocks.emplace_back(BasicBlock{move(_debugData), {}, {}}); } }; diff --git a/libyul/backends/evm/ControlFlowGraphBuilder.cpp b/libyul/backends/evm/ControlFlowGraphBuilder.cpp index f4077f177..fd5b4682d 100644 --- a/libyul/backends/evm/ControlFlowGraphBuilder.cpp +++ b/libyul/backends/evm/ControlFlowGraphBuilder.cpp @@ -134,7 +134,7 @@ std::unique_ptr ControlFlowGraphBuilder::build( ) { auto result = std::make_unique(); - result->entry = &result->makeBlock(); + result->entry = &result->makeBlock(debugDataOf(_block)); ControlFlowGraphBuilder builder(*result, _analysisInfo, _dialect); builder.m_currentBlock = result->entry; @@ -233,7 +233,7 @@ void ControlFlowGraphBuilder::operator()(ExpressionStatement const& _exprStmt) if (builtin->controlFlowSideEffects.terminates) { m_currentBlock->exit = CFG::BasicBlock::Terminated{}; - m_currentBlock = &m_graph.makeBlock(); + m_currentBlock = &m_graph.makeBlock(debugDataOf(*m_currentBlock)); } } @@ -246,15 +246,19 @@ void ControlFlowGraphBuilder::operator()(Block const& _block) void ControlFlowGraphBuilder::operator()(If const& _if) { - auto&& [ifBranch, afterIf] = makeConditionalJump(std::visit(*this, *_if.condition)); - m_currentBlock = ifBranch; + auto& ifBranch = m_graph.makeBlock(debugDataOf(_if.body)); + auto& afterIf = m_graph.makeBlock(debugDataOf(*m_currentBlock)); + makeConditionalJump(debugDataOf(_if), std::visit(*this, *_if.condition), ifBranch, afterIf); + m_currentBlock = &ifBranch; (*this)(_if.body); - jump(*afterIf); + jump(debugDataOf(_if.body), afterIf); } void ControlFlowGraphBuilder::operator()(Switch const& _switch) { yulAssert(m_currentBlock, ""); + shared_ptr preSwitchDebugData = debugDataOf(_switch); + auto ghostVariableId = m_graph.ghostVariables.size(); YulString ghostVariableName("GHOST[" + to_string(ghostVariableId) + "]"); auto& ghostVar = m_graph.ghostVariables.emplace_back(Scope::Variable{""_yulstring, ghostVariableName}); @@ -273,43 +277,46 @@ void ControlFlowGraphBuilder::operator()(Switch const& _switch) // Artificially generate: // eq(, ) - auto makeValueCompare = [&](Literal const& _value) { + auto makeValueCompare = [&](Case const& _case) { yul::FunctionCall const& ghostCall = m_graph.ghostCalls.emplace_back(yul::FunctionCall{ - _value.debugData, + debugDataOf(_case), yul::Identifier{{}, "eq"_yulstring}, - {_value, Identifier{{}, ghostVariableName}} + {*_case.value, Identifier{{}, ghostVariableName}} }); CFG::Operation& operation = m_currentBlock->operations.emplace_back(CFG::Operation{ - Stack{ghostVarSlot, LiteralSlot{valueOfLiteral(_value), _value.debugData}}, + Stack{ghostVarSlot, LiteralSlot{valueOfLiteral(*_case.value), debugDataOf(*_case.value)}}, Stack{TemporarySlot{ghostCall, 0}}, - CFG::BuiltinCall{_switch.debugData, *equalityBuiltin, ghostCall, 2}, + CFG::BuiltinCall{debugDataOf(_case), *equalityBuiltin, ghostCall, 2}, }); return operation.output.front(); }; - CFG::BasicBlock& afterSwitch = m_graph.makeBlock(); + CFG::BasicBlock& afterSwitch = m_graph.makeBlock(preSwitchDebugData); yulAssert(!_switch.cases.empty(), ""); for (auto const& switchCase: _switch.cases | ranges::views::drop_last(1)) { yulAssert(switchCase.value, ""); - auto&& [caseBranch, elseBranch] = makeConditionalJump(makeValueCompare(*switchCase.value)); - m_currentBlock = caseBranch; + auto& caseBranch = m_graph.makeBlock(debugDataOf(switchCase.body)); + auto& elseBranch = m_graph.makeBlock(debugDataOf(_switch)); + makeConditionalJump(debugDataOf(switchCase), makeValueCompare(switchCase), caseBranch, elseBranch); + m_currentBlock = &caseBranch; (*this)(switchCase.body); - jump(afterSwitch); - m_currentBlock = elseBranch; + jump(debugDataOf(switchCase.body), afterSwitch); + m_currentBlock = &elseBranch; } Case const& switchCase = _switch.cases.back(); if (switchCase.value) { - CFG::BasicBlock& caseBranch = m_graph.makeBlock(); - makeConditionalJump(makeValueCompare(*switchCase.value), caseBranch, afterSwitch); + CFG::BasicBlock& caseBranch = m_graph.makeBlock(debugDataOf(switchCase.body)); + makeConditionalJump(debugDataOf(switchCase), makeValueCompare(switchCase), caseBranch, afterSwitch); m_currentBlock = &caseBranch; } (*this)(switchCase.body); - jump(afterSwitch); + jump(debugDataOf(switchCase.body), afterSwitch); } void ControlFlowGraphBuilder::operator()(ForLoop const& _loop) { + shared_ptr preLoopDebugData = debugDataOf(_loop); ScopedSaveAndRestore scopeRestore(m_scope, m_info.scopes.at(&_loop.pre).get()); (*this)(_loop.pre); @@ -317,10 +324,10 @@ void ControlFlowGraphBuilder::operator()(ForLoop const& _loop) if (auto const* literalCondition = get_if(_loop.condition.get())) constantCondition = valueOfLiteral(*literalCondition) != 0; - CFG::BasicBlock& loopCondition = m_graph.makeBlock(); - CFG::BasicBlock& loopBody = m_graph.makeBlock(); - CFG::BasicBlock& post = m_graph.makeBlock(); - CFG::BasicBlock& afterLoop = m_graph.makeBlock(); + CFG::BasicBlock& loopCondition = m_graph.makeBlock(debugDataOf(*_loop.condition)); + CFG::BasicBlock& loopBody = m_graph.makeBlock(debugDataOf(_loop.body)); + CFG::BasicBlock& post = m_graph.makeBlock(debugDataOf(_loop.post)); + CFG::BasicBlock& afterLoop = m_graph.makeBlock(preLoopDebugData); ScopedSaveAndRestore scopedSaveAndRestore(m_forLoopInfo, ForLoopInfo{afterLoop, post}); @@ -328,48 +335,49 @@ void ControlFlowGraphBuilder::operator()(ForLoop const& _loop) { if (*constantCondition) { - jump(loopBody); + jump(debugDataOf(_loop.pre), loopBody); (*this)(_loop.body); - jump(post); + jump(debugDataOf(_loop.body), post); (*this)(_loop.post); - jump(loopBody, true); + jump(debugDataOf(_loop.post), loopBody, true); } else - jump(afterLoop); + jump(debugDataOf(_loop.pre), afterLoop); } else { - jump(loopCondition); - makeConditionalJump(std::visit(*this, *_loop.condition), loopBody, afterLoop); + jump(debugDataOf(_loop.pre), loopCondition); + makeConditionalJump(debugDataOf(*_loop.condition), std::visit(*this, *_loop.condition), loopBody, afterLoop); m_currentBlock = &loopBody; (*this)(_loop.body); - jump(post); + jump(debugDataOf(_loop.body), post); (*this)(_loop.post); - jump(loopCondition, true); + jump(debugDataOf(_loop.post), loopCondition, true); } m_currentBlock = &afterLoop; } -void ControlFlowGraphBuilder::operator()(Break const&) +void ControlFlowGraphBuilder::operator()(Break const& _break) { yulAssert(m_forLoopInfo.has_value(), ""); - jump(m_forLoopInfo->afterLoop); - m_currentBlock = &m_graph.makeBlock(); + jump(debugDataOf(_break), m_forLoopInfo->afterLoop); + m_currentBlock = &m_graph.makeBlock(debugDataOf(*m_currentBlock)); } -void ControlFlowGraphBuilder::operator()(Continue const&) +void ControlFlowGraphBuilder::operator()(Continue const& _continue) { yulAssert(m_forLoopInfo.has_value(), ""); - jump(m_forLoopInfo->post); - m_currentBlock = &m_graph.makeBlock(); + jump(debugDataOf(_continue), m_forLoopInfo->post); + m_currentBlock = &m_graph.makeBlock(debugDataOf(*m_currentBlock)); } -void ControlFlowGraphBuilder::operator()(Leave const&) +// '_leave' and '__leave' are reserved in VisualStudio +void ControlFlowGraphBuilder::operator()(Leave const& leave_) { - yulAssert(m_currentFunctionExit.has_value(), ""); - m_currentBlock->exit = *m_currentFunctionExit; - m_currentBlock = &m_graph.makeBlock(); + yulAssert(m_currentFunction.has_value(), ""); + m_currentBlock->exit = CFG::BasicBlock::FunctionReturn{debugDataOf(leave_), *m_currentFunction}; + m_currentBlock = &m_graph.makeBlock(debugDataOf(*m_currentBlock)); } void ControlFlowGraphBuilder::operator()(FunctionDefinition const& _function) @@ -386,7 +394,7 @@ void ControlFlowGraphBuilder::operator()(FunctionDefinition const& _function) auto&& [it, inserted] = m_graph.functionInfo.emplace(std::make_pair(&function, CFG::FunctionInfo{ _function.debugData, function, - &m_graph.makeBlock(), + &m_graph.makeBlock(debugDataOf(_function.body)), _function.parameters | ranges::views::transform([&](auto const& _param) { return VariableSlot{ std::get(virtualFunctionScope->identifiers.at(_param.name)), @@ -404,10 +412,10 @@ void ControlFlowGraphBuilder::operator()(FunctionDefinition const& _function) CFG::FunctionInfo& functionInfo = it->second; ControlFlowGraphBuilder builder{m_graph, m_info, m_dialect}; - builder.m_currentFunctionExit = CFG::BasicBlock::FunctionReturn{&functionInfo}; + builder.m_currentFunction = &functionInfo; builder.m_currentBlock = functionInfo.entry; builder(_function.body); - builder.m_currentBlock->exit = CFG::BasicBlock::FunctionReturn{&functionInfo}; + builder.m_currentBlock->exit = CFG::BasicBlock::FunctionReturn{debugDataOf(_function), &functionInfo}; } @@ -497,18 +505,16 @@ Scope::Variable const& ControlFlowGraphBuilder::lookupVariable(YulString _name) yulAssert(false, "External identifier access unimplemented."); } -std::pair ControlFlowGraphBuilder::makeConditionalJump(StackSlot _condition) -{ - CFG::BasicBlock& nonZero = m_graph.makeBlock(); - CFG::BasicBlock& zero = m_graph.makeBlock(); - makeConditionalJump(move(_condition), nonZero, zero); - return {&nonZero, &zero}; -} - -void ControlFlowGraphBuilder::makeConditionalJump(StackSlot _condition, CFG::BasicBlock& _nonZero, CFG::BasicBlock& _zero) +void ControlFlowGraphBuilder::makeConditionalJump( + shared_ptr _debugData, + StackSlot _condition, + CFG::BasicBlock& _nonZero, + CFG::BasicBlock& _zero +) { yulAssert(m_currentBlock, ""); m_currentBlock->exit = CFG::BasicBlock::ConditionalJump{ + move(_debugData), move(_condition), &_nonZero, &_zero @@ -518,10 +524,14 @@ void ControlFlowGraphBuilder::makeConditionalJump(StackSlot _condition, CFG::Bas m_currentBlock = nullptr; } -void ControlFlowGraphBuilder::jump(CFG::BasicBlock& _target, bool backwards) +void ControlFlowGraphBuilder::jump( + shared_ptr _debugData, + CFG::BasicBlock& _target, + bool backwards +) { yulAssert(m_currentBlock, ""); - m_currentBlock->exit = CFG::BasicBlock::Jump{&_target, backwards}; + m_currentBlock->exit = CFG::BasicBlock::Jump{move(_debugData), &_target, backwards}; _target.entries.emplace_back(m_currentBlock); m_currentBlock = &_target; } diff --git a/libyul/backends/evm/ControlFlowGraphBuilder.h b/libyul/backends/evm/ControlFlowGraphBuilder.h index 2a70a9472..f99a879f6 100644 --- a/libyul/backends/evm/ControlFlowGraphBuilder.h +++ b/libyul/backends/evm/ControlFlowGraphBuilder.h @@ -62,13 +62,18 @@ private: Scope::Function const& lookupFunction(YulString _name) const; Scope::Variable const& lookupVariable(YulString _name) const; - /// @returns a pair of newly created blocks, the first element being the non-zero branch, the second element the - /// zero branch. /// Resets m_currentBlock to enforce a subsequent explicit reassignment. - std::pair makeConditionalJump(StackSlot _condition); - /// Resets m_currentBlock to enforce a subsequent explicit reassignment. - void makeConditionalJump(StackSlot _condition, CFG::BasicBlock& _nonZero, CFG::BasicBlock& _zero); - void jump(CFG::BasicBlock& _target, bool _backwards = false); + void makeConditionalJump( + std::shared_ptr _debugData, + StackSlot _condition, + CFG::BasicBlock& _nonZero, + CFG::BasicBlock& _zero + ); + void jump( + std::shared_ptr _debugData, + CFG::BasicBlock& _target, + bool _backwards = false + ); CFG& m_graph; AsmAnalysisInfo const& m_info; Dialect const& m_dialect; @@ -80,7 +85,7 @@ private: std::reference_wrapper post; }; std::optional m_forLoopInfo; - std::optional m_currentFunctionExit; + std::optional m_currentFunction; }; } diff --git a/libyul/backends/evm/OptimizedEVMCodeTransform.cpp b/libyul/backends/evm/OptimizedEVMCodeTransform.cpp index 91f19ad62..3aedfec3a 100644 --- a/libyul/backends/evm/OptimizedEVMCodeTransform.cpp +++ b/libyul/backends/evm/OptimizedEVMCodeTransform.cpp @@ -57,7 +57,7 @@ vector OptimizedEVMCodeTransform::run( stackLayout ); // Create initial entry layout. - optimizedCodeTransform.createStackLayout(stackLayout.blockInfos.at(dfg->entry).entryLayout); + optimizedCodeTransform.createStackLayout(debugDataOf(*dfg->entry), stackLayout.blockInfos.at(dfg->entry).entryLayout); optimizedCodeTransform(*dfg->entry); for (Scope::Function const* function: dfg->functions) optimizedCodeTransform(dfg->functionInfo.at(function)); @@ -220,7 +220,7 @@ void OptimizedEVMCodeTransform::validateSlot(StackSlot const& _slot, Expression }, _expression); } -void OptimizedEVMCodeTransform::createStackLayout(Stack _targetStack) +void OptimizedEVMCodeTransform::createStackLayout(std::shared_ptr _debugData, Stack _targetStack) { static constexpr auto slotVariableName = [](StackSlot const& _slot) { return std::visit(util::GenericVisitor{ @@ -231,6 +231,8 @@ void OptimizedEVMCodeTransform::createStackLayout(Stack _targetStack) yulAssert(m_assembly.stackHeight() == static_cast(m_stack.size()), ""); // ::createStackLayout asserts that it has successfully achieved the target layout. + langutil::SourceLocation sourceLocation = _debugData ? _debugData->location : langutil::SourceLocation{}; + m_assembly.setSourceLocation(sourceLocation); ::createStackLayout( m_stack, _targetStack | ranges::to, @@ -299,6 +301,7 @@ void OptimizedEVMCodeTransform::createStackLayout(Stack _targetStack) { m_assembly.setSourceLocation(locationOf(_literal)); m_assembly.appendConstant(_literal.value); + m_assembly.setSourceLocation(sourceLocation); }, [&](FunctionReturnLabelSlot const&) { @@ -310,6 +313,7 @@ void OptimizedEVMCodeTransform::createStackLayout(Stack _targetStack) m_returnLabels[&_returnLabel.call.get()] = m_assembly.newLabelId(); m_assembly.setSourceLocation(locationOf(_returnLabel.call.get())); m_assembly.appendLabelReference(m_returnLabels.at(&_returnLabel.call.get())); + m_assembly.setSourceLocation(sourceLocation); }, [&](VariableSlot const& _variable) { @@ -317,6 +321,7 @@ void OptimizedEVMCodeTransform::createStackLayout(Stack _targetStack) { m_assembly.setSourceLocation(locationOf(_variable)); m_assembly.appendConstant(0); + m_assembly.setSourceLocation(sourceLocation); return; } yulAssert(false, "Variable not found on stack."); @@ -346,6 +351,7 @@ void OptimizedEVMCodeTransform::operator()(CFG::BasicBlock const& _block) // Assert that this is the first visit of the block and mark as generated. yulAssert(m_generated.insert(&_block).second, ""); + m_assembly.setSourceLocation(locationOf(_block)); auto const& blockInfo = m_stackLayout.blockInfos.at(&_block); // Assert that the stack is valid for entering the block. @@ -360,7 +366,7 @@ void OptimizedEVMCodeTransform::operator()(CFG::BasicBlock const& _block) for (auto const& operation: _block.operations) { // Create required layout for entering the operation. - createStackLayout(m_stackLayout.operationEntryLayout.at(&operation)); + createStackLayout(debugDataOf(operation.operation), m_stackLayout.operationEntryLayout.at(&operation)); // Assert that we have the inputs of the operation on stack top. yulAssert(static_cast(m_stack.size()) == m_assembly.stackHeight(), ""); @@ -385,6 +391,7 @@ void OptimizedEVMCodeTransform::operator()(CFG::BasicBlock const& _block) } // Exit the block. + m_assembly.setSourceLocation(locationOf(_block)); std::visit(util::GenericVisitor{ [&](CFG::BasicBlock::MainExit const&) { @@ -393,7 +400,7 @@ void OptimizedEVMCodeTransform::operator()(CFG::BasicBlock const& _block) [&](CFG::BasicBlock::Jump const& _jump) { // Create the stack expected at the jump target. - createStackLayout(m_stackLayout.blockInfos.at(_jump.target).entryLayout); + createStackLayout(debugDataOf(_jump), m_stackLayout.blockInfos.at(_jump.target).entryLayout); // If this is the only jump to the block, we do not need a label and can directly continue with the target block. if (!m_blockLabels.count(_jump.target) && _jump.target->entries.size() == 1) @@ -417,7 +424,7 @@ void OptimizedEVMCodeTransform::operator()(CFG::BasicBlock const& _block) [&](CFG::BasicBlock::ConditionalJump const& _conditionalJump) { // Create the shared entry layout of the jump targets, which is stored as exit layout of the current block. - createStackLayout(blockInfo.exitLayout); + createStackLayout(debugDataOf(_conditionalJump), blockInfo.exitLayout); // Create labels for the targets, if not already present. if (!m_blockLabels.count(_conditionalJump.nonZero)) @@ -468,9 +475,7 @@ void OptimizedEVMCodeTransform::operator()(CFG::BasicBlock const& _block) exitStack.emplace_back(FunctionReturnLabelSlot{_functionReturn.info->function}); // Create the function return layout and jump. - m_assembly.setSourceLocation(locationOf(*m_currentFunctionInfo)); - createStackLayout(exitStack); - m_assembly.setSourceLocation(locationOf(*m_currentFunctionInfo)); + createStackLayout(debugDataOf(_functionReturn), exitStack); m_assembly.appendJump(0, AbstractAssembly::JumpType::OutOfFunction); }, [&](CFG::BasicBlock::Terminated const&) @@ -505,7 +510,7 @@ void OptimizedEVMCodeTransform::operator()(CFG::FunctionInfo const& _functionInf m_assembly.appendLabel(getFunctionLabel(_functionInfo.function)); // Create the entry layout of the function body block and visit. - createStackLayout(m_stackLayout.blockInfos.at(_functionInfo.entry).entryLayout); + createStackLayout(debugDataOf(_functionInfo), m_stackLayout.blockInfos.at(_functionInfo.entry).entryLayout); (*this)(*_functionInfo.entry); m_stack.clear(); diff --git a/libyul/backends/evm/OptimizedEVMCodeTransform.h b/libyul/backends/evm/OptimizedEVMCodeTransform.h index bbbfcc363..48819251a 100644 --- a/libyul/backends/evm/OptimizedEVMCodeTransform.h +++ b/libyul/backends/evm/OptimizedEVMCodeTransform.h @@ -77,7 +77,8 @@ private: static void validateSlot(StackSlot const& _slot, Expression const& _expression); /// Shuffles m_stack to the desired @a _targetStack while emitting the shuffling code to m_assembly. - void createStackLayout(Stack _targetStack); + /// Sets the source locations to the one in @a _debugData. + void createStackLayout(std::shared_ptr _debugData, Stack _targetStack); /// Generate code for the given block @a _block. /// Expects the current stack layout m_stack to be a stack layout that is compatible with the