From 1f77deada1abc9ca1e635ab13d45707e852a5628 Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Thu, 3 May 2018 16:40:33 +0200 Subject: [050] Reserving and popping local vars in their scope --- libsolidity/codegen/CompilerContext.cpp | 15 ++++- libsolidity/codegen/CompilerContext.h | 4 +- libsolidity/codegen/ContractCompiler.cpp | 104 +++++++++++++++++++++---------- libsolidity/codegen/ContractCompiler.h | 28 +++++++-- 4 files changed, 110 insertions(+), 41 deletions(-) (limited to 'libsolidity/codegen') diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp index a35eea73..9c5dc89c 100644 --- a/libsolidity/codegen/CompilerContext.cpp +++ b/libsolidity/codegen/CompilerContext.cpp @@ -130,7 +130,7 @@ void CompilerContext::addVariable(VariableDeclaration const& _declaration, m_localVariables[&_declaration].push_back(unsigned(m_asm->deposit()) - _offsetToCurrent); } -void CompilerContext::removeVariable(VariableDeclaration const& _declaration) +void CompilerContext::removeVariable(Declaration const& _declaration) { solAssert(m_localVariables.count(&_declaration) && !m_localVariables[&_declaration].empty(), ""); m_localVariables[&_declaration].pop_back(); @@ -138,6 +138,19 @@ void CompilerContext::removeVariable(VariableDeclaration const& _declaration) m_localVariables.erase(&_declaration); } +void CompilerContext::removeVariablesAboveStackHeight(unsigned _stackHeight) +{ + vector toRemove; + for (auto _var: m_localVariables) + { + solAssert(!_var.second.empty(), ""); + if (_var.second.back() >= _stackHeight) + toRemove.push_back(_var.first); + } + for (auto _var: toRemove) + removeVariable(*_var); +} + eth::Assembly const& CompilerContext::compiledContract(const ContractDefinition& _contract) const { auto ret = m_compiledContracts.find(&_contract); diff --git a/libsolidity/codegen/CompilerContext.h b/libsolidity/codegen/CompilerContext.h index 5776b5d1..f3934615 100644 --- a/libsolidity/codegen/CompilerContext.h +++ b/libsolidity/codegen/CompilerContext.h @@ -71,7 +71,9 @@ public: void addStateVariable(VariableDeclaration const& _declaration, u256 const& _storageOffset, unsigned _byteOffset); void addVariable(VariableDeclaration const& _declaration, unsigned _offsetToCurrent = 0); - void removeVariable(VariableDeclaration const& _declaration); + void removeVariable(Declaration const& _declaration); + /// Removes all local variables currently allocated above _stackHeight. + void removeVariablesAboveStackHeight(unsigned _stackHeight); void setCompiledContracts(std::map const& _contracts) { m_compiledContracts = _contracts; } eth::Assembly const& compiledContract(ContractDefinition const& _contract) const; diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 81aba21e..def4a878 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -427,7 +427,7 @@ bool ContractCompiler::visit(FunctionDefinition const& _function) m_context.startFunction(_function); // stack upon entry: [return address] [arg0] [arg1] ... [argn] - // reserve additional slots: [retarg0] ... [retargm] [localvar0] ... [localvarp] + // reserve additional slots: [retarg0] ... [retargm] unsigned parametersSize = CompilerUtils::sizeOnStack(_function.parameters()); if (!_function.isConstructor()) @@ -441,22 +441,18 @@ bool ContractCompiler::visit(FunctionDefinition const& _function) for (ASTPointer const& variable: _function.returnParameters()) appendStackVariableInitialisation(*variable); - for (VariableDeclaration const* localVariable: _function.localVariables()) - appendStackVariableInitialisation(*localVariable); if (_function.isConstructor()) if (auto c = m_context.nextConstructor(dynamic_cast(*_function.scope()))) appendBaseConstructor(*c); - solAssert(m_returnTags.empty(), ""); m_breakTags.clear(); m_continueTags.clear(); - m_stackCleanupForReturn = 0; m_currentFunction = &_function; m_modifierDepth = -1; + m_scopeStackHeight.clear(); appendModifierOrFunctionCode(); - solAssert(m_returnTags.empty(), ""); // Now we need to re-shuffle the stack. For this we keep a record of the stack layout @@ -467,14 +463,12 @@ bool ContractCompiler::visit(FunctionDefinition const& _function) unsigned const c_argumentsSize = CompilerUtils::sizeOnStack(_function.parameters()); unsigned const c_returnValuesSize = CompilerUtils::sizeOnStack(_function.returnParameters()); - unsigned const c_localVariablesSize = CompilerUtils::sizeOnStack(_function.localVariables()); vector stackLayout; stackLayout.push_back(c_returnValuesSize); // target of return address stackLayout += vector(c_argumentsSize, -1); // discard all arguments for (unsigned i = 0; i < c_returnValuesSize; ++i) stackLayout.push_back(i); - stackLayout += vector(c_localVariablesSize, -1); if (stackLayout.size() > 17) BOOST_THROW_EXCEPTION( @@ -493,18 +487,19 @@ bool ContractCompiler::visit(FunctionDefinition const& _function) m_context << swapInstruction(stackLayout.size() - stackLayout.back() - 1); swap(stackLayout[stackLayout.back()], stackLayout.back()); } - //@todo assert that everything is in place now + for (int i = 0; i < int(stackLayout.size()); ++i) + if (stackLayout[i] != i) + solAssert(false, "Invalid stack layout on cleanup."); for (ASTPointer const& variable: _function.parameters() + _function.returnParameters()) m_context.removeVariable(*variable); - for (VariableDeclaration const* localVariable: _function.localVariables()) - m_context.removeVariable(*localVariable); m_context.adjustStackOffset(-(int)c_returnValuesSize); /// The constructor and the fallback function doesn't to jump out. if (!_function.isConstructor() && !_function.isFallback()) m_context.appendJump(eth::AssemblyItem::JumpType::OutOfFunction); + return false; } @@ -669,14 +664,16 @@ bool ContractCompiler::visit(WhileStatement const& _whileStatement) eth::AssemblyItem loopStart = m_context.newTag(); eth::AssemblyItem loopEnd = m_context.newTag(); - m_breakTags.push_back(loopEnd); + m_breakTags.push_back({loopEnd, m_context.stackHeight()}); + + visitLoop(&_whileStatement); m_context << loopStart; if (_whileStatement.isDoWhile()) { eth::AssemblyItem condition = m_context.newTag(); - m_continueTags.push_back(condition); + m_continueTags.push_back({condition, m_context.stackHeight()}); _whileStatement.body().accept(*this); @@ -687,7 +684,7 @@ bool ContractCompiler::visit(WhileStatement const& _whileStatement) } else { - m_continueTags.push_back(loopStart); + m_continueTags.push_back({loopStart, m_context.stackHeight()}); compileExpression(_whileStatement.condition()); m_context << Instruction::ISZERO; m_context.appendConditionalJumpTo(loopEnd); @@ -712,12 +709,14 @@ bool ContractCompiler::visit(ForStatement const& _forStatement) eth::AssemblyItem loopStart = m_context.newTag(); eth::AssemblyItem loopEnd = m_context.newTag(); eth::AssemblyItem loopNext = m_context.newTag(); - m_continueTags.push_back(loopNext); - m_breakTags.push_back(loopEnd); + + visitLoop(&_forStatement); if (_forStatement.initializationExpression()) _forStatement.initializationExpression()->accept(*this); + m_breakTags.push_back({loopEnd, m_context.stackHeight()}); + m_continueTags.push_back({loopNext, m_context.stackHeight()}); m_context << loopStart; // if there is no terminating condition in for, default is to always be true @@ -737,11 +736,16 @@ bool ContractCompiler::visit(ForStatement const& _forStatement) _forStatement.loopExpression()->accept(*this); m_context.appendJumpTo(loopStart); + m_context << loopEnd; m_continueTags.pop_back(); m_breakTags.pop_back(); + // For the case where no break/return is executed: + // loop initialization variables have to be freed + popScopedVariables(&_forStatement); + checker.check(); return false; } @@ -750,7 +754,7 @@ bool ContractCompiler::visit(Continue const& _continueStatement) { CompilerContext::LocationSetter locationSetter(m_context, _continueStatement); solAssert(!m_continueTags.empty(), ""); - m_context.appendJumpTo(m_continueTags.back()); + popAndJump(m_context.stackHeight() - m_continueTags.back().second, m_continueTags.back().first); return false; } @@ -758,7 +762,7 @@ bool ContractCompiler::visit(Break const& _breakStatement) { CompilerContext::LocationSetter locationSetter(m_context, _breakStatement); solAssert(!m_breakTags.empty(), ""); - m_context.appendJumpTo(m_breakTags.back()); + popAndJump(m_context.stackHeight() - m_breakTags.back().second, m_breakTags.back().first); return false; } @@ -784,10 +788,8 @@ bool ContractCompiler::visit(Return const& _return) for (auto const& retVariable: boost::adaptors::reverse(returnParameters)) CompilerUtils(m_context).moveToStackVariable(*retVariable); } - for (unsigned i = 0; i < m_stackCleanupForReturn; ++i) - m_context << Instruction::POP; - m_context.appendJumpTo(m_returnTags.back()); - m_context.adjustStackOffset(m_stackCleanupForReturn); + + popAndJump(m_context.stackHeight() - m_returnTags.back().second, m_returnTags.back().first); return false; } @@ -810,8 +812,15 @@ bool ContractCompiler::visit(EmitStatement const& _emit) bool ContractCompiler::visit(VariableDeclarationStatement const& _variableDeclarationStatement) { - StackHeightChecker checker(m_context); CompilerContext::LocationSetter locationSetter(m_context, _variableDeclarationStatement); + + // Local variable slots are reserved when their declaration is visited, + // and freed in the end of their scope. + for (auto _decl: _variableDeclarationStatement.declarations()) + if (_decl) + appendStackVariableInitialisation(*_decl); + + StackHeightChecker checker(m_context); if (Expression const* expression = _variableDeclarationStatement.initialValue()) { CompilerUtils utils(m_context); @@ -861,6 +870,18 @@ bool ContractCompiler::visit(PlaceholderStatement const& _placeholderStatement) return true; } +bool ContractCompiler::visit(Block const& _block) +{ + m_scopeStackHeight[m_modifierDepth][&_block] = m_context.stackHeight(); + return true; +} + +void ContractCompiler::endVisit(Block const& _block) +{ + // Frees local variables declared in the scope of this block. + popScopedVariables(&_block); +} + void ContractCompiler::appendMissingFunctions() { while (Declaration const* function = m_context.nextFunctionToCompile()) @@ -916,27 +937,19 @@ void ContractCompiler::appendModifierOrFunctionCode() modifier.parameters()[i]->annotation().type ); } - for (VariableDeclaration const* localVariable: modifier.localVariables()) - { - addedVariables.push_back(localVariable); - appendStackVariableInitialisation(*localVariable); - } - stackSurplus = - CompilerUtils::sizeOnStack(modifier.parameters()) + - CompilerUtils::sizeOnStack(modifier.localVariables()); + stackSurplus = CompilerUtils::sizeOnStack(modifier.parameters()); codeBlock = &modifier.body(); } } if (codeBlock) { - m_returnTags.push_back(m_context.newTag()); - + m_returnTags.push_back({m_context.newTag(), m_context.stackHeight()}); codeBlock->accept(*this); solAssert(!m_returnTags.empty(), ""); - m_context << m_returnTags.back(); + m_context << m_returnTags.back().first; m_returnTags.pop_back(); CompilerUtils(m_context).popStackSlots(stackSurplus); @@ -983,3 +996,26 @@ eth::AssemblyPointer ContractCompiler::cloneRuntime() const a << u256(0x20) << u256(0) << Instruction::RETURN; return make_shared(a); } + +void ContractCompiler::popScopedVariables(ASTNode const* _node) +{ + unsigned blockHeight = m_scopeStackHeight[m_modifierDepth][_node]; + unsigned stackDiff = m_context.stackHeight() - blockHeight; + CompilerUtils(m_context).popStackSlots(stackDiff); + m_context.removeVariablesAboveStackHeight(blockHeight); + m_scopeStackHeight[m_modifierDepth].erase(_node); + if (m_scopeStackHeight[m_modifierDepth].size() == 0) + m_scopeStackHeight.erase(m_modifierDepth); +} + +void ContractCompiler::visitLoop(BreakableStatement const* _loop) +{ + m_scopeStackHeight[m_modifierDepth][_loop] = m_context.stackHeight(); +} + +void ContractCompiler::popAndJump(unsigned _amount, eth::AssemblyItem const& _jumpTo) +{ + CompilerUtils(m_context).popStackSlots(_amount); + m_context.appendJumpTo(_jumpTo); + m_context.adjustStackOffset(_amount); +} diff --git a/libsolidity/codegen/ContractCompiler.h b/libsolidity/codegen/ContractCompiler.h index 02a3452f..72f46495 100644 --- a/libsolidity/codegen/ContractCompiler.h +++ b/libsolidity/codegen/ContractCompiler.h @@ -109,6 +109,8 @@ private: virtual bool visit(VariableDeclarationStatement const& _variableDeclarationStatement) override; virtual bool visit(ExpressionStatement const& _expressionStatement) override; virtual bool visit(PlaceholderStatement const&) override; + virtual bool visit(Block const& _block) override; + virtual void endVisit(Block const& _block) override; /// Repeatedly visits all function which are referenced but which are not compiled yet. void appendMissingFunctions(); @@ -123,19 +125,35 @@ private: /// @returns the runtime assembly for clone contracts. eth::AssemblyPointer cloneRuntime() const; + /// Frees the variables of a certain scope (to be used when leaving). + void popScopedVariables(ASTNode const* _node); + + /// Pops _amount slots from the stack and jumps to _jumpTo. + /// Also readjusts the stack offset to the original value. + void popAndJump(unsigned _amount, eth::AssemblyItem const& _jumpTo); + + /// Sets the stack height for the visited loop. + void visitLoop(BreakableStatement const* _loop); + bool const m_optimise; /// Pointer to the runtime compiler in case this is a creation compiler. ContractCompiler* m_runtimeCompiler = nullptr; CompilerContext& m_context; - std::vector m_breakTags; ///< tag to jump to for a "break" statement - std::vector m_continueTags; ///< tag to jump to for a "continue" statement - /// Tag to jump to for a "return" statement, needs to be stacked because of modifiers. - std::vector m_returnTags; + /// Tag to jump to for a "break" statement and the stack height after freeing the local loop variables. + std::vector> m_breakTags; + /// Tag to jump to for a "continue" statement and the stack height after freeing the local loop variables. + std::vector> m_continueTags; + /// Tag to jump to for a "return" statement and the stack height after freeing the local function or modifier variables. + /// Needs to be stacked because of modifiers. + std::vector> m_returnTags; unsigned m_modifierDepth = 0; FunctionDefinition const* m_currentFunction = nullptr; - unsigned m_stackCleanupForReturn = 0; ///< this number of stack elements need to be removed before jump to m_returnTag + // arguments for base constructors, filled in derived-to-base order std::map const* m_baseArguments; + + /// Stores the variables that were declared inside a specific scope, for each modifier depth. + std::map> m_scopeStackHeight; }; } -- cgit v1.2.3 From 9d895e002d0e5e1e3435d03ac82277caa397bbec Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Wed, 4 Jul 2018 14:14:41 +0200 Subject: Added tests and review suggestions --- libsolidity/codegen/CompilerUtils.cpp | 8 ++++++++ libsolidity/codegen/CompilerUtils.h | 4 ++++ libsolidity/codegen/ContractCompiler.cpp | 26 +++++++++----------------- libsolidity/codegen/ContractCompiler.h | 6 +----- 4 files changed, 22 insertions(+), 22 deletions(-) (limited to 'libsolidity/codegen') diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp index 2f45765a..92c45227 100644 --- a/libsolidity/codegen/CompilerUtils.cpp +++ b/libsolidity/codegen/CompilerUtils.cpp @@ -1170,6 +1170,14 @@ void CompilerUtils::popStackSlots(size_t _amount) m_context << Instruction::POP; } +void CompilerUtils::popAndJump(unsigned _toHeight, eth::AssemblyItem const& _jumpTo) +{ + unsigned amount = m_context.stackHeight() - _toHeight; + popStackSlots(amount); + m_context.appendJumpTo(_jumpTo); + m_context.adjustStackOffset(amount); +} + unsigned CompilerUtils::sizeOnStack(vector> const& _variableTypes) { unsigned size = 0; diff --git a/libsolidity/codegen/CompilerUtils.h b/libsolidity/codegen/CompilerUtils.h index 0ff3ad7c..26df4765 100644 --- a/libsolidity/codegen/CompilerUtils.h +++ b/libsolidity/codegen/CompilerUtils.h @@ -241,6 +241,10 @@ public: void popStackElement(Type const& _type); /// Removes element from the top of the stack _amount times. void popStackSlots(size_t _amount); + /// Pops slots from the stack such that its height is _toHeight. + /// Adds jump to _jumpTo. + /// Readjusts the stack offset to the original value. + void popAndJump(unsigned _toHeight, eth::AssemblyItem const& _jumpTo); template static unsigned sizeOnStack(std::vector const& _variables); diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index def4a878..474d2b68 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -446,6 +446,7 @@ bool ContractCompiler::visit(FunctionDefinition const& _function) if (auto c = m_context.nextConstructor(dynamic_cast(*_function.scope()))) appendBaseConstructor(*c); + solAssert(m_returnTags.empty(), ""); m_breakTags.clear(); m_continueTags.clear(); m_currentFunction = &_function; @@ -666,8 +667,6 @@ bool ContractCompiler::visit(WhileStatement const& _whileStatement) eth::AssemblyItem loopEnd = m_context.newTag(); m_breakTags.push_back({loopEnd, m_context.stackHeight()}); - visitLoop(&_whileStatement); - m_context << loopStart; if (_whileStatement.isDoWhile()) @@ -710,7 +709,7 @@ bool ContractCompiler::visit(ForStatement const& _forStatement) eth::AssemblyItem loopEnd = m_context.newTag(); eth::AssemblyItem loopNext = m_context.newTag(); - visitLoop(&_forStatement); + storeStackHeight(&_forStatement); if (_forStatement.initializationExpression()) _forStatement.initializationExpression()->accept(*this); @@ -754,7 +753,7 @@ bool ContractCompiler::visit(Continue const& _continueStatement) { CompilerContext::LocationSetter locationSetter(m_context, _continueStatement); solAssert(!m_continueTags.empty(), ""); - popAndJump(m_context.stackHeight() - m_continueTags.back().second, m_continueTags.back().first); + CompilerUtils(m_context).popAndJump(m_continueTags.back().second, m_continueTags.back().first); return false; } @@ -762,7 +761,7 @@ bool ContractCompiler::visit(Break const& _breakStatement) { CompilerContext::LocationSetter locationSetter(m_context, _breakStatement); solAssert(!m_breakTags.empty(), ""); - popAndJump(m_context.stackHeight() - m_breakTags.back().second, m_breakTags.back().first); + CompilerUtils(m_context).popAndJump(m_breakTags.back().second, m_breakTags.back().first); return false; } @@ -789,7 +788,7 @@ bool ContractCompiler::visit(Return const& _return) CompilerUtils(m_context).moveToStackVariable(*retVariable); } - popAndJump(m_context.stackHeight() - m_returnTags.back().second, m_returnTags.back().first); + CompilerUtils(m_context).popAndJump(m_returnTags.back().second, m_returnTags.back().first); return false; } @@ -872,7 +871,7 @@ bool ContractCompiler::visit(PlaceholderStatement const& _placeholderStatement) bool ContractCompiler::visit(Block const& _block) { - m_scopeStackHeight[m_modifierDepth][&_block] = m_context.stackHeight(); + storeStackHeight(&_block); return true; } @@ -999,7 +998,7 @@ eth::AssemblyPointer ContractCompiler::cloneRuntime() const void ContractCompiler::popScopedVariables(ASTNode const* _node) { - unsigned blockHeight = m_scopeStackHeight[m_modifierDepth][_node]; + unsigned blockHeight = m_scopeStackHeight.at(m_modifierDepth).at(_node); unsigned stackDiff = m_context.stackHeight() - blockHeight; CompilerUtils(m_context).popStackSlots(stackDiff); m_context.removeVariablesAboveStackHeight(blockHeight); @@ -1008,14 +1007,7 @@ void ContractCompiler::popScopedVariables(ASTNode const* _node) m_scopeStackHeight.erase(m_modifierDepth); } -void ContractCompiler::visitLoop(BreakableStatement const* _loop) -{ - m_scopeStackHeight[m_modifierDepth][_loop] = m_context.stackHeight(); -} - -void ContractCompiler::popAndJump(unsigned _amount, eth::AssemblyItem const& _jumpTo) +void ContractCompiler::storeStackHeight(ASTNode const* _node) { - CompilerUtils(m_context).popStackSlots(_amount); - m_context.appendJumpTo(_jumpTo); - m_context.adjustStackOffset(_amount); + m_scopeStackHeight[m_modifierDepth][_node] = m_context.stackHeight(); } diff --git a/libsolidity/codegen/ContractCompiler.h b/libsolidity/codegen/ContractCompiler.h index 72f46495..8516ec2c 100644 --- a/libsolidity/codegen/ContractCompiler.h +++ b/libsolidity/codegen/ContractCompiler.h @@ -128,12 +128,8 @@ private: /// Frees the variables of a certain scope (to be used when leaving). void popScopedVariables(ASTNode const* _node); - /// Pops _amount slots from the stack and jumps to _jumpTo. - /// Also readjusts the stack offset to the original value. - void popAndJump(unsigned _amount, eth::AssemblyItem const& _jumpTo); - /// Sets the stack height for the visited loop. - void visitLoop(BreakableStatement const* _loop); + void storeStackHeight(ASTNode const* _node); bool const m_optimise; /// Pointer to the runtime compiler in case this is a creation compiler. -- cgit v1.2.3 From b750ca9741aba47cc8ba650a04dd620725ed4610 Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Fri, 6 Jul 2018 09:56:27 +0200 Subject: Add more tests and assertions --- libsolidity/codegen/CompilerContext.cpp | 2 ++ libsolidity/codegen/CompilerUtils.cpp | 1 + libsolidity/codegen/ContractCompiler.cpp | 1 + 3 files changed, 4 insertions(+) (limited to 'libsolidity/codegen') diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp index 9c5dc89c..51f76399 100644 --- a/libsolidity/codegen/CompilerContext.cpp +++ b/libsolidity/codegen/CompilerContext.cpp @@ -127,6 +127,8 @@ void CompilerContext::addVariable(VariableDeclaration const& _declaration, unsigned _offsetToCurrent) { solAssert(m_asm->deposit() >= 0 && unsigned(m_asm->deposit()) >= _offsetToCurrent, ""); + unsigned sizeOnStack = _declaration.annotation().type->sizeOnStack(); + solAssert(sizeOnStack == 1 || sizeOnStack == 2, ""); m_localVariables[&_declaration].push_back(unsigned(m_asm->deposit()) - _offsetToCurrent); } diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp index 92c45227..5adce4a0 100644 --- a/libsolidity/codegen/CompilerUtils.cpp +++ b/libsolidity/codegen/CompilerUtils.cpp @@ -1172,6 +1172,7 @@ void CompilerUtils::popStackSlots(size_t _amount) void CompilerUtils::popAndJump(unsigned _toHeight, eth::AssemblyItem const& _jumpTo) { + solAssert(m_context.stackHeight() >= _toHeight, ""); unsigned amount = m_context.stackHeight() - _toHeight; popStackSlots(amount); m_context.appendJumpTo(_jumpTo); diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 474d2b68..207e0af7 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -999,6 +999,7 @@ eth::AssemblyPointer ContractCompiler::cloneRuntime() const void ContractCompiler::popScopedVariables(ASTNode const* _node) { unsigned blockHeight = m_scopeStackHeight.at(m_modifierDepth).at(_node); + solAssert(m_context.stackHeight() >= blockHeight, ""); unsigned stackDiff = m_context.stackHeight() - blockHeight; CompilerUtils(m_context).popStackSlots(stackDiff); m_context.removeVariablesAboveStackHeight(blockHeight); -- cgit v1.2.3 From 0c5e0e0d59dc55fcfe5b95a8c649a7a769ad3400 Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Tue, 10 Jul 2018 18:39:26 +0200 Subject: Added assertion and tests suggestions --- libsolidity/codegen/CompilerContext.cpp | 8 ++++++++ libsolidity/codegen/CompilerContext.h | 2 ++ libsolidity/codegen/ContractCompiler.cpp | 10 +++++++--- 3 files changed, 17 insertions(+), 3 deletions(-) (limited to 'libsolidity/codegen') diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp index 51f76399..3b1b4ec0 100644 --- a/libsolidity/codegen/CompilerContext.cpp +++ b/libsolidity/codegen/CompilerContext.cpp @@ -128,6 +128,8 @@ void CompilerContext::addVariable(VariableDeclaration const& _declaration, { solAssert(m_asm->deposit() >= 0 && unsigned(m_asm->deposit()) >= _offsetToCurrent, ""); unsigned sizeOnStack = _declaration.annotation().type->sizeOnStack(); + // Variables should not have stack size other than [1, 2], + // but that might change when new types are introduced. solAssert(sizeOnStack == 1 || sizeOnStack == 2, ""); m_localVariables[&_declaration].push_back(unsigned(m_asm->deposit()) - _offsetToCurrent); } @@ -146,6 +148,7 @@ void CompilerContext::removeVariablesAboveStackHeight(unsigned _stackHeight) for (auto _var: m_localVariables) { solAssert(!_var.second.empty(), ""); + solAssert(_var.second.back() <= stackHeight(), ""); if (_var.second.back() >= _stackHeight) toRemove.push_back(_var.first); } @@ -153,6 +156,11 @@ void CompilerContext::removeVariablesAboveStackHeight(unsigned _stackHeight) removeVariable(*_var); } +unsigned CompilerContext::numberOfLocalVariables() const +{ + return m_localVariables.size(); +} + eth::Assembly const& CompilerContext::compiledContract(const ContractDefinition& _contract) const { auto ret = m_compiledContracts.find(&_contract); diff --git a/libsolidity/codegen/CompilerContext.h b/libsolidity/codegen/CompilerContext.h index f3934615..3f357821 100644 --- a/libsolidity/codegen/CompilerContext.h +++ b/libsolidity/codegen/CompilerContext.h @@ -74,6 +74,8 @@ public: void removeVariable(Declaration const& _declaration); /// Removes all local variables currently allocated above _stackHeight. void removeVariablesAboveStackHeight(unsigned _stackHeight); + /// Returns the number of currently allocated local variables. + unsigned numberOfLocalVariables() const; void setCompiledContracts(std::map const& _contracts) { m_compiledContracts = _contracts; } eth::Assembly const& compiledContract(ContractDefinition const& _contract) const; diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 207e0af7..93f698bc 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -498,8 +498,12 @@ bool ContractCompiler::visit(FunctionDefinition const& _function) m_context.adjustStackOffset(-(int)c_returnValuesSize); /// The constructor and the fallback function doesn't to jump out. - if (!_function.isConstructor() && !_function.isFallback()) - m_context.appendJump(eth::AssemblyItem::JumpType::OutOfFunction); + if (!_function.isConstructor()) + { + solAssert(m_context.numberOfLocalVariables() == 0, ""); + if (!_function.isFallback()) + m_context.appendJump(eth::AssemblyItem::JumpType::OutOfFunction); + } return false; } @@ -999,10 +1003,10 @@ eth::AssemblyPointer ContractCompiler::cloneRuntime() const void ContractCompiler::popScopedVariables(ASTNode const* _node) { unsigned blockHeight = m_scopeStackHeight.at(m_modifierDepth).at(_node); + m_context.removeVariablesAboveStackHeight(blockHeight); solAssert(m_context.stackHeight() >= blockHeight, ""); unsigned stackDiff = m_context.stackHeight() - blockHeight; CompilerUtils(m_context).popStackSlots(stackDiff); - m_context.removeVariablesAboveStackHeight(blockHeight); m_scopeStackHeight[m_modifierDepth].erase(_node); if (m_scopeStackHeight[m_modifierDepth].size() == 0) m_scopeStackHeight.erase(m_modifierDepth); -- cgit v1.2.3