Merge pull request #4083 from ethereum/variable_stack_slot

[BREAKING] Allocate local variables according to their scope
This commit is contained in:
chriseth 2018-07-12 13:01:15 +02:00 committed by GitHub
commit 576f3ef18c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 446 additions and 44 deletions

View File

@ -55,6 +55,7 @@ Compiler Features:
* C API (``libsolc``): Export the ``solidity_license``, ``solidity_version`` and ``solidity_compile`` methods.
* Type Checker: Show named argument in case of error.
* Tests: Determine transaction status during IPC calls.
* Code Generator: Allocate and free local variables according to their scope.
Bugfixes:
* Tests: Fix chain parameters to make ipc tests work with newer versions of cpp-ethereum.

View File

@ -127,10 +127,14 @@ void CompilerContext::addVariable(VariableDeclaration const& _declaration,
unsigned _offsetToCurrent)
{
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);
}
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 +142,25 @@ void CompilerContext::removeVariable(VariableDeclaration const& _declaration)
m_localVariables.erase(&_declaration);
}
void CompilerContext::removeVariablesAboveStackHeight(unsigned _stackHeight)
{
vector<Declaration const*> toRemove;
for (auto _var: m_localVariables)
{
solAssert(!_var.second.empty(), "");
solAssert(_var.second.back() <= stackHeight(), "");
if (_var.second.back() >= _stackHeight)
toRemove.push_back(_var.first);
}
for (auto _var: toRemove)
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);

View File

@ -71,7 +71,11 @@ 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);
/// Returns the number of currently allocated local variables.
unsigned numberOfLocalVariables() const;
void setCompiledContracts(std::map<ContractDefinition const*, eth::Assembly const*> const& _contracts) { m_compiledContracts = _contracts; }
eth::Assembly const& compiledContract(ContractDefinition const& _contract) const;

View File

@ -1170,6 +1170,15 @@ void CompilerUtils::popStackSlots(size_t _amount)
m_context << Instruction::POP;
}
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);
m_context.adjustStackOffset(amount);
}
unsigned CompilerUtils::sizeOnStack(vector<shared_ptr<Type const>> const& _variableTypes)
{
unsigned size = 0;

View File

@ -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 <class T>
static unsigned sizeOnStack(std::vector<T> const& _variables);

View File

@ -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,8 +441,6 @@ bool ContractCompiler::visit(FunctionDefinition const& _function)
for (ASTPointer<VariableDeclaration const> 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<ContractDefinition const&>(*_function.scope())))
@ -451,12 +449,11 @@ bool ContractCompiler::visit(FunctionDefinition const& _function)
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 +464,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<int> stackLayout;
stackLayout.push_back(c_returnValuesSize); // target of return address
stackLayout += vector<int>(c_argumentsSize, -1); // discard all arguments
for (unsigned i = 0; i < c_returnValuesSize; ++i)
stackLayout.push_back(i);
stackLayout += vector<int>(c_localVariablesSize, -1);
if (stackLayout.size() > 17)
BOOST_THROW_EXCEPTION(
@ -493,18 +488,23 @@ 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<VariableDeclaration const> 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);
if (!_function.isConstructor())
{
solAssert(m_context.numberOfLocalVariables() == 0, "");
if (!_function.isFallback())
m_context.appendJump(eth::AssemblyItem::JumpType::OutOfFunction);
}
return false;
}
@ -669,14 +669,14 @@ 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()});
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 +687,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 +712,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);
storeStackHeight(&_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 +739,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 +757,7 @@ bool ContractCompiler::visit(Continue const& _continueStatement)
{
CompilerContext::LocationSetter locationSetter(m_context, _continueStatement);
solAssert(!m_continueTags.empty(), "");
m_context.appendJumpTo(m_continueTags.back());
CompilerUtils(m_context).popAndJump(m_continueTags.back().second, m_continueTags.back().first);
return false;
}
@ -758,7 +765,7 @@ bool ContractCompiler::visit(Break const& _breakStatement)
{
CompilerContext::LocationSetter locationSetter(m_context, _breakStatement);
solAssert(!m_breakTags.empty(), "");
m_context.appendJumpTo(m_breakTags.back());
CompilerUtils(m_context).popAndJump(m_breakTags.back().second, m_breakTags.back().first);
return false;
}
@ -784,10 +791,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);
CompilerUtils(m_context).popAndJump(m_returnTags.back().second, m_returnTags.back().first);
return false;
}
@ -810,8 +815,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 +873,18 @@ bool ContractCompiler::visit(PlaceholderStatement const& _placeholderStatement)
return true;
}
bool ContractCompiler::visit(Block const& _block)
{
storeStackHeight(&_block);
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 +940,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 +999,20 @@ eth::AssemblyPointer ContractCompiler::cloneRuntime() const
a << u256(0x20) << u256(0) << Instruction::RETURN;
return make_shared<eth::Assembly>(a);
}
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_scopeStackHeight[m_modifierDepth].erase(_node);
if (m_scopeStackHeight[m_modifierDepth].size() == 0)
m_scopeStackHeight.erase(m_modifierDepth);
}
void ContractCompiler::storeStackHeight(ASTNode const* _node)
{
m_scopeStackHeight[m_modifierDepth][_node] = m_context.stackHeight();
}

View File

@ -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,31 @@ 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);
/// Sets the stack height for the visited loop.
void storeStackHeight(ASTNode const* _node);
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<eth::AssemblyItem> m_breakTags; ///< tag to jump to for a "break" statement
std::vector<eth::AssemblyItem> 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<eth::AssemblyItem> m_returnTags;
/// Tag to jump to for a "break" statement and the stack height after freeing the local loop variables.
std::vector<std::pair<eth::AssemblyItem, unsigned>> m_breakTags;
/// Tag to jump to for a "continue" statement and the stack height after freeing the local loop variables.
std::vector<std::pair<eth::AssemblyItem, unsigned>> 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<std::pair<eth::AssemblyItem, unsigned>> 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<FunctionDefinition const*, ASTNode const*> const* m_baseArguments;
/// Stores the variables that were declared inside a specific scope, for each modifier depth.
std::map<unsigned, std::map<ASTNode const*, unsigned>> m_scopeStackHeight;
};
}

View File

@ -523,6 +523,96 @@ BOOST_AUTO_TEST_CASE(do_while_loop_continue)
ABI_CHECK(callContractFunction("f()"), encodeArgs(42));
}
BOOST_AUTO_TEST_CASE(array_multiple_local_vars)
{
char const* sourceCode = R"(
contract test {
function f(uint256[] seq) external pure returns (uint256) {
uint i = 0;
uint sum = 0;
while (i < seq.length)
{
uint idx = i;
if (idx >= 10) break;
uint x = seq[idx];
if (x >= 1000) {
uint n = i + 1;
i = n;
continue;
}
else {
uint y = sum + x;
sum = y;
}
if (sum >= 500) return sum;
i++;
}
return sum;
}
}
)";
compileAndRun(sourceCode);
ABI_CHECK(callContractFunction("f(uint256[])", 32, 3, u256(1000), u256(1), u256(2)), encodeArgs(3));
ABI_CHECK(callContractFunction("f(uint256[])", 32, 3, u256(100), u256(500), u256(300)), encodeArgs(600));
ABI_CHECK(callContractFunction(
"f(uint256[])", 32, 11,
u256(1), u256(2), u256(3), u256(4), u256(5), u256(6), u256(7), u256(8), u256(9), u256(10), u256(111)
), encodeArgs(55));
}
BOOST_AUTO_TEST_CASE(do_while_loop_multiple_local_vars)
{
char const* sourceCode = R"(
contract test {
function f(uint x) public pure returns(uint r) {
uint i = 0;
do
{
uint z = x * 2;
if (z < 4) break;
else {
uint k = z + 1;
if (k < 8) {
x++;
continue;
}
}
if (z > 12) return 0;
x++;
i++;
} while (true);
return 42;
}
}
)";
compileAndRun(sourceCode);
auto do_while = [](u256 n) -> u256
{
u256 i = 0;
do
{
u256 z = n * 2;
if (z < 4) break;
else {
u256 k = z + 1;
if (k < 8) {
n++;
continue;
}
}
if (z > 12) return 0;
n++;
i++;
} while (true);
return 42;
};
testContractAgainstCppOnRange("f(uint256)", do_while, 0, 12);
}
BOOST_AUTO_TEST_CASE(nested_loops)
{
// tests that break and continue statements in nested loops jump to the correct place
@ -574,6 +664,178 @@ BOOST_AUTO_TEST_CASE(nested_loops)
testContractAgainstCppOnRange("f(uint256)", nested_loops_cpp, 0, 12);
}
BOOST_AUTO_TEST_CASE(nested_loops_multiple_local_vars)
{
// tests that break and continue statements in nested loops jump to the correct place
// and free local variables properly
char const* sourceCode = R"(
contract test {
function f(uint x) returns(uint y) {
while (x > 0) {
uint z = x + 10;
uint k = z + 1;
if (k > 20) {
break;
uint p = 100;
k += p;
}
if (k > 15) {
x--;
continue;
uint t = 1000;
x += t;
}
while (k > 10) {
uint m = k - 1;
if (m == 10) return x;
return k;
uint h = 10000;
z += h;
}
x--;
break;
}
return x;
}
}
)";
compileAndRun(sourceCode);
auto nested_loops_cpp = [](u256 n) -> u256
{
while (n > 0)
{
u256 z = n + 10;
u256 k = z + 1;
if (k > 20) break;
if (k > 15) {
n--;
continue;
}
while (k > 10)
{
u256 m = k - 1;
if (m == 10) return n;
return k;
}
n--;
break;
}
return n;
};
testContractAgainstCppOnRange("f(uint256)", nested_loops_cpp, 0, 12);
}
BOOST_AUTO_TEST_CASE(for_loop_multiple_local_vars)
{
char const* sourceCode = R"(
contract test {
function f(uint x) public pure returns(uint r) {
for (uint i = 0; i < 12; i++)
{
uint z = x + 1;
if (z < 4) break;
else {
uint k = z * 2;
if (i + k < 10) {
x++;
continue;
}
}
if (z > 8) return 0;
x++;
}
return 42;
}
}
)";
compileAndRun(sourceCode);
auto for_loop = [](u256 n) -> u256
{
for (u256 i = 0; i < 12; i++)
{
u256 z = n + 1;
if (z < 4) break;
else {
u256 k = z * 2;
if (i + k < 10) {
n++;
continue;
}
}
if (z > 8) return 0;
n++;
}
return 42;
};
testContractAgainstCppOnRange("f(uint256)", for_loop, 0, 12);
}
BOOST_AUTO_TEST_CASE(nested_for_loop_multiple_local_vars)
{
char const* sourceCode = R"(
contract test {
function f(uint x) public pure returns(uint r) {
for (uint i = 0; i < 5; i++)
{
uint z = x + 1;
if (z < 3) {
break;
uint p = z + 2;
}
for (uint j = 0; j < 5; j++)
{
uint k = z * 2;
if (j + k < 8) {
x++;
continue;
uint t = z * 3;
}
x++;
if (x > 20) {
return 84;
uint h = x + 42;
}
}
if (x > 30) {
return 42;
uint b = 0xcafe;
}
}
return 42;
}
}
)";
compileAndRun(sourceCode);
auto for_loop = [](u256 n) -> u256
{
for (u256 i = 0; i < 5; i++)
{
u256 z = n + 1;
if (z < 3) break;
for (u256 j = 0; j < 5; j++)
{
u256 k = z * 2;
if (j + k < 8) {
n++;
continue;
}
n++;
if (n > 20) return 84;
}
if (n > 30) return 42;
}
return 42;
};
testContractAgainstCppOnRange("f(uint256)", for_loop, 0, 12);
}
BOOST_AUTO_TEST_CASE(for_loop)
{
char const* sourceCode = R"(
@ -9232,7 +9494,9 @@ BOOST_AUTO_TEST_CASE(break_in_modifier)
}
}
function f() run {
x++;
uint k = x;
uint t = k + 1;
x = t;
}
}
)";
@ -9242,6 +9506,54 @@ BOOST_AUTO_TEST_CASE(break_in_modifier)
ABI_CHECK(callContractFunction("x()"), encodeArgs(u256(1)));
}
BOOST_AUTO_TEST_CASE(continue_in_modifier)
{
char const* sourceCode = R"(
contract C {
uint public x;
modifier run() {
for (uint i = 0; i < 10; i++) {
if (i % 2 == 1) continue;
_;
}
}
function f() run {
uint k = x;
uint t = k + 1;
x = t;
}
}
)";
compileAndRun(sourceCode, 0, "C");
ABI_CHECK(callContractFunction("x()"), encodeArgs(u256(0)));
ABI_CHECK(callContractFunction("f()"), encodeArgs());
ABI_CHECK(callContractFunction("x()"), encodeArgs(u256(5)));
}
BOOST_AUTO_TEST_CASE(return_in_modifier)
{
char const* sourceCode = R"(
contract C {
uint public x;
modifier run() {
for (uint i = 1; i < 10; i++) {
if (i == 5) return;
_;
}
}
function f() run {
uint k = x;
uint t = k + 1;
x = t;
}
}
)";
compileAndRun(sourceCode, 0, "C");
ABI_CHECK(callContractFunction("x()"), encodeArgs(u256(0)));
ABI_CHECK(callContractFunction("f()"), encodeArgs());
ABI_CHECK(callContractFunction("x()"), encodeArgs(u256(4)));
}
BOOST_AUTO_TEST_CASE(stacked_return_with_modifiers)
{
char const* sourceCode = R"(
@ -9254,7 +9566,9 @@ BOOST_AUTO_TEST_CASE(stacked_return_with_modifiers)
}
}
function f() run {
x++;
uint k = x;
uint t = k + 1;
x = t;
}
}
)";