From fed44efdce5a4f9051175186d6bdb6e1fca6f2c5 Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 11 Mar 2015 18:09:35 +0100 Subject: [PATCH] Enlarge storage references to two stack slots. --- libsolidity/ArrayUtils.cpp | 68 ++++++++++----- libsolidity/ArrayUtils.h | 12 +-- libsolidity/CompilerUtils.cpp | 1 + libsolidity/ExpressionCompiler.cpp | 72 ++++++++++------ libsolidity/ExpressionCompiler.h | 1 - libsolidity/LValue.cpp | 128 ++++++++++++----------------- libsolidity/LValue.h | 10 +-- libsolidity/Types.cpp | 3 + libsolidity/Types.h | 7 +- test/SolidityEndToEndTest.cpp | 23 +++++- test/SolidityOptimizer.cpp | 2 +- 11 files changed, 191 insertions(+), 136 deletions(-) diff --git a/libsolidity/ArrayUtils.cpp b/libsolidity/ArrayUtils.cpp index a064b2f57..2596e4afa 100644 --- a/libsolidity/ArrayUtils.cpp +++ b/libsolidity/ArrayUtils.cpp @@ -34,8 +34,10 @@ using namespace solidity; void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType const& _sourceType) const { - // stack layout: [source_ref] target_ref (top) - // need to leave target_ref on the stack at the end + // this copies source to target and also clears target if it was larger + // need to leave "target_ref target_byte_off" on the stack at the end + + // stack layout: [source_ref] [source_byte_off] [source length] target_ref target_byte_off (top) solAssert(_targetType.getLocation() == ArrayType::Location::Storage, ""); solAssert( _sourceType.getLocation() == ArrayType::Location::CallData || @@ -47,14 +49,20 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons Type const* targetBaseType = _targetType.isByteArray() ? &uint256 : &(*_targetType.getBaseType()); Type const* sourceBaseType = _sourceType.isByteArray() ? &uint256 : &(*_sourceType.getBaseType()); - // this copies source to target and also clears target if it was larger - // TODO unroll loop for small sizes - // stack: source_ref [source_length] target_ref + bool sourceIsStorage = _sourceType.getLocation() == ArrayType::Location::Storage; + + // stack: source_ref [source_byte_off] [source_length] target_ref target_byte_off // store target_ref + m_context << eth::Instruction::POP; //@todo for (unsigned i = _sourceType.getSizeOnStack(); i > 0; --i) m_context << eth::swapInstruction(i); + // stack: target_ref source_ref [source_byte_off] [source_length] + if (sourceIsStorage) + m_context << eth::Instruction::POP; //@todo + // stack: target_ref source_ref [source_length] + // retrieve source length if (_sourceType.getLocation() != ArrayType::Location::CallData || !_sourceType.isDynamicallySized()) retrieveLength(_sourceType); // otherwise, length is already there // stack: target_ref source_ref source_length @@ -73,6 +81,7 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons m_context << eth::Instruction::POP << eth::Instruction::POP << eth::Instruction::POP << eth::Instruction::POP; + m_context << u256(0); //@todo return; } // compute hashes (data positions) @@ -109,32 +118,37 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons // copy if (sourceBaseType->getCategory() == Type::Category::Array) { - m_context << eth::Instruction::DUP3 << eth::Instruction::DUP3; + //@todo + m_context << eth::Instruction::DUP3; + if (sourceIsStorage) + m_context << u256(0); + m_context << eth::dupInstruction(sourceIsStorage ? 4 : 3) << u256(0); copyArrayToStorage( dynamic_cast(*targetBaseType), dynamic_cast(*sourceBaseType) ); - m_context << eth::Instruction::POP; + m_context << eth::Instruction::POP << eth::Instruction::POP; } else { m_context << eth::Instruction::DUP3; if (_sourceType.getLocation() == ArrayType::Location::Storage) + { + m_context << u256(0); StorageItem(m_context, *sourceBaseType).retrieveValue(SourceLocation(), true); + } else if (sourceBaseType->isValueType()) CompilerUtils(m_context).loadFromMemoryDynamic(*sourceBaseType, true, true, false); else solAssert(false, "Copying of unknown type requested: " + sourceBaseType->toString()); solAssert(2 + sourceBaseType->getSizeOnStack() <= 16, "Stack too deep."); - m_context << eth::dupInstruction(2 + sourceBaseType->getSizeOnStack()); + m_context << eth::dupInstruction(2 + sourceBaseType->getSizeOnStack()) << u256(0); StorageItem(m_context, *targetBaseType).storeValue(*sourceBaseType, SourceLocation(), true); } // increment source m_context << eth::Instruction::SWAP2 - << (_sourceType.getLocation() == ArrayType::Location::Storage ? - sourceBaseType->getStorageSize() : - sourceBaseType->getCalldataEncodedSize()) + << (sourceIsStorage ? sourceBaseType->getStorageSize() : sourceBaseType->getCalldataEncodedSize()) << eth::Instruction::ADD << eth::Instruction::SWAP2; // increment target @@ -147,40 +161,48 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons m_context << copyLoopEnd; // zero-out leftovers in target - // stack: target_ref target_data_end source_data_pos target_data_pos source_data_end + // stack: target_ref target_data_end source_data_pos target_data_pos_updated source_data_end m_context << eth::Instruction::POP << eth::Instruction::SWAP1 << eth::Instruction::POP; // stack: target_ref target_data_end target_data_pos_updated clearStorageLoop(*targetBaseType); m_context << eth::Instruction::POP; + m_context << u256(0); //@todo } void ArrayUtils::clearArray(ArrayType const& _type) const { + unsigned stackHeightStart = m_context.getStackHeight(); solAssert(_type.getLocation() == ArrayType::Location::Storage, ""); if (_type.isDynamicallySized()) + { + m_context << eth::Instruction::POP; // remove byte offset clearDynamicArray(_type); + } else if (_type.getLength() == 0 || _type.getBaseType()->getCategory() == Type::Category::Mapping) - m_context << eth::Instruction::POP; + m_context << eth::Instruction::POP << eth::Instruction::POP; else if (_type.getLength() < 5) // unroll loop for small arrays @todo choose a good value { solAssert(!_type.isByteArray(), ""); for (unsigned i = 1; i < _type.getLength(); ++i) { StorageItem(m_context, *_type.getBaseType()).setToZero(SourceLocation(), false); + m_context << eth::Instruction::SWAP1; m_context << u256(_type.getBaseType()->getStorageSize()) << eth::Instruction::ADD; + m_context << eth::Instruction::SWAP1; } StorageItem(m_context, *_type.getBaseType()).setToZero(SourceLocation(), true); } else { solAssert(!_type.isByteArray(), ""); - m_context - << eth::Instruction::DUP1 << u256(_type.getLength()) - << u256(_type.getBaseType()->getStorageSize()) - << eth::Instruction::MUL << eth::Instruction::ADD << eth::Instruction::SWAP1; + m_context << eth::Instruction::SWAP1; + m_context << eth::Instruction::DUP1 << _type.getLength(); + convertLengthToSize(_type); + m_context << eth::Instruction::ADD << eth::Instruction::SWAP1; clearStorageLoop(*_type.getBaseType()); - m_context << eth::Instruction::POP; + m_context << eth::Instruction::POP << eth::Instruction::POP; } + solAssert(m_context.getStackHeight() == stackHeightStart - 2, ""); } void ArrayUtils::clearDynamicArray(ArrayType const& _type) const @@ -188,6 +210,7 @@ void ArrayUtils::clearDynamicArray(ArrayType const& _type) const solAssert(_type.getLocation() == ArrayType::Location::Storage, ""); solAssert(_type.isDynamicallySized(), ""); + unsigned stackHeightStart = m_context.getStackHeight(); // fetch length m_context << eth::Instruction::DUP1 << eth::Instruction::SLOAD; // set length to zero @@ -197,7 +220,7 @@ void ArrayUtils::clearDynamicArray(ArrayType const& _type) const // compute data positions m_context << eth::Instruction::SWAP1; CompilerUtils(m_context).computeHashStatic(); - // stack: len data_pos (len is in slots for byte array and in items for other arrays) + // stack: len data_pos m_context << eth::Instruction::SWAP1 << eth::Instruction::DUP2 << eth::Instruction::ADD << eth::Instruction::SWAP1; // stack: data_pos_end data_pos @@ -207,6 +230,7 @@ void ArrayUtils::clearDynamicArray(ArrayType const& _type) const clearStorageLoop(*_type.getBaseType()); // cleanup m_context << eth::Instruction::POP; + solAssert(m_context.getStackHeight() == stackHeightStart - 1, ""); } void ArrayUtils::resizeDynamicArray(const ArrayType& _type) const @@ -214,6 +238,7 @@ void ArrayUtils::resizeDynamicArray(const ArrayType& _type) const solAssert(_type.getLocation() == ArrayType::Location::Storage, ""); solAssert(_type.isDynamicallySized(), ""); + unsigned stackHeightStart = m_context.getStackHeight(); eth::AssemblyItem resizeEnd = m_context.newTag(); // stack: ref new_length @@ -249,10 +274,12 @@ void ArrayUtils::resizeDynamicArray(const ArrayType& _type) const m_context << resizeEnd; // cleanup m_context << eth::Instruction::POP << eth::Instruction::POP << eth::Instruction::POP; + solAssert(m_context.getStackHeight() == stackHeightStart - 2, ""); } void ArrayUtils::clearStorageLoop(Type const& _type) const { + unsigned stackHeightStart = m_context.getStackHeight(); if (_type.getCategory() == Type::Category::Mapping) { m_context << eth::Instruction::POP; @@ -267,13 +294,16 @@ void ArrayUtils::clearStorageLoop(Type const& _type) const eth::AssemblyItem zeroLoopEnd = m_context.newTag(); m_context.appendConditionalJumpTo(zeroLoopEnd); // delete + m_context << u256(0); //@todo StorageItem(m_context, _type).setToZero(SourceLocation(), false); + m_context << eth::Instruction::POP; // increment m_context << u256(1) << eth::Instruction::ADD; m_context.appendJumpTo(loopStart); // cleanup m_context << zeroLoopEnd; m_context << eth::Instruction::POP; + solAssert(m_context.getStackHeight() == stackHeightStart - 1, ""); } void ArrayUtils::convertLengthToSize(ArrayType const& _arrayType, bool _pad) const diff --git a/libsolidity/ArrayUtils.h b/libsolidity/ArrayUtils.h index 31cca8173..c7b05b6d3 100644 --- a/libsolidity/ArrayUtils.h +++ b/libsolidity/ArrayUtils.h @@ -41,19 +41,19 @@ public: /// Copies an array to an array in storage. The arrays can be of different types only if /// their storage representation is the same. - /// Stack pre: [source_reference] target_reference - /// Stack post: target_reference + /// Stack pre: source_reference [source_byte_offset/source_length] target_reference target_byte_offset + /// Stack post: target_reference target_byte_offset void copyArrayToStorage(ArrayType const& _targetType, ArrayType const& _sourceType) const; /// Clears the given dynamic or static array. - /// Stack pre: reference + /// Stack pre: storage_ref storage_byte_offset /// Stack post: void clearArray(ArrayType const& _type) const; /// Clears the length and data elements of the array referenced on the stack. - /// Stack pre: reference + /// Stack pre: reference (excludes byte offset) /// Stack post: void clearDynamicArray(ArrayType const& _type) const; /// Changes the size of a dynamic array and clears the tail if it is shortened. - /// Stack pre: reference new_length + /// Stack pre: reference (excludes byte offset) new_length /// Stack post: void resizeDynamicArray(ArrayType const& _type) const; /// Appends a loop that clears a sequence of storage slots of the given type (excluding end). @@ -67,7 +67,7 @@ public: void convertLengthToSize(ArrayType const& _arrayType, bool _pad = false) const; /// Retrieves the length (number of elements) of the array ref on the stack. This also /// works for statically-sized arrays. - /// Stack pre: reference + /// Stack pre: reference (excludes byte offset for dynamic storage arrays) /// Stack post: reference length void retrieveLength(ArrayType const& _arrayType) const; diff --git a/libsolidity/CompilerUtils.cpp b/libsolidity/CompilerUtils.cpp index e517e384d..5fc8bd8f2 100644 --- a/libsolidity/CompilerUtils.cpp +++ b/libsolidity/CompilerUtils.cpp @@ -93,6 +93,7 @@ void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBound else { solAssert(type.getLocation() == ArrayType::Location::Storage, "Memory arrays not yet implemented."); + m_context << eth::Instruction::POP; //@todo m_context << eth::Instruction::DUP1 << eth::Instruction::SLOAD; // stack here: memory_offset storage_offset length_bytes // jump to end if length is zero diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index 92fd70437..6f5331294 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -81,6 +81,7 @@ void ExpressionCompiler::appendStateVariableAccessor(VariableDeclaration const& returnType = dynamic_cast(*returnType).getValueType(); } + m_context << u256(0); // @todo unsigned retSizeOnStack = 0; solAssert(accessorType.getReturnParameterTypes().size() >= 1, ""); if (StructType const* structType = dynamic_cast(returnType.get())) @@ -90,15 +91,18 @@ void ExpressionCompiler::appendStateVariableAccessor(VariableDeclaration const& // struct for (size_t i = 0; i < names.size(); ++i) { - m_context << eth::Instruction::DUP1 - << structType->getStorageOffsetOfMember(names[i]) - << eth::Instruction::ADD; + if (types[i]->getCategory() == Type::Category::Mapping) + continue; + m_context + << eth::Instruction::DUP2 << structType->getStorageOffsetOfMember(names[i]) + << eth::Instruction::ADD; + m_context << u256(0); //@todo StorageItem(m_context, *types[i]).retrieveValue(SourceLocation(), true); solAssert(types[i]->getSizeOnStack() == 1, "Returning struct elements with stack size != 1 not yet implemented."); - m_context << eth::Instruction::SWAP1; + m_context << eth::Instruction::SWAP2 << eth::Instruction::SWAP1; retSizeOnStack += types[i]->getSizeOnStack(); } - m_context << eth::Instruction::POP; + m_context << eth::Instruction::POP << eth::Instruction::POP; } else { @@ -280,23 +284,24 @@ bool ExpressionCompiler::visit(UnaryOperation const& _unaryOperation) case Token::Dec: // -- (pre- or postfix) solAssert(!!m_currentLValue, "LValue not retrieved."); m_currentLValue->retrieveValue(_unaryOperation.getLocation()); - solAssert(m_currentLValue->sizeOnStack() <= 1, "Not implemented."); if (!_unaryOperation.isPrefixOperation()) { - if (m_currentLValue->sizeOnStack() == 1) - m_context << eth::Instruction::SWAP1 << eth::Instruction::DUP2; - else - m_context << eth::Instruction::DUP1; + // store value for later + solAssert(_unaryOperation.getType()->getSizeOnStack() == 1, "Stack size != 1 not implemented."); + m_context << eth::Instruction::DUP1; + if (m_currentLValue->sizeOnStack() > 0) + for (unsigned i = 1 + m_currentLValue->sizeOnStack(); i > 0; --i) + m_context << eth::swapInstruction(i); } m_context << u256(1); if (_unaryOperation.getOperator() == Token::Inc) m_context << eth::Instruction::ADD; else - m_context << eth::Instruction::SWAP1 << eth::Instruction::SUB; // @todo avoid the swap - // Stack for prefix: [ref] (*ref)+-1 - // Stack for postfix: *ref [ref] (*ref)+-1 - if (m_currentLValue->sizeOnStack() == 1) - m_context << eth::Instruction::SWAP1; + m_context << eth::Instruction::SWAP1 << eth::Instruction::SUB; + // Stack for prefix: [ref...] (*ref)+-1 + // Stack for postfix: *ref [ref...] (*ref)+-1 + for (unsigned i = m_currentLValue->sizeOnStack(); i > 0; --i) + m_context << eth::swapInstruction(i); m_currentLValue->storeValue( *_unaryOperation.getType(), _unaryOperation.getLocation(), !_unaryOperation.isPrefixOperation()); @@ -661,7 +666,10 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess) case Type::Category::Struct: { StructType const& type = dynamic_cast(*_memberAccess.getExpression().getType()); + m_context << eth::Instruction::POP; //@todo m_context << type.getStorageOffsetOfMember(member) << eth::Instruction::ADD; + //@todo + m_context << u256(0); setLValueToStorageItem(_memberAccess); break; } @@ -729,20 +737,22 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess) Type const& baseType = *_indexAccess.getBaseExpression().getType(); if (baseType.getCategory() == Type::Category::Mapping) { + // storage byte offset is ignored for mappings, it should be zero. + m_context << eth::Instruction::POP; + // stack: storage_base_ref Type const& keyType = *dynamic_cast(baseType).getKeyType(); - m_context << u256(0); + m_context << u256(0); // memory position solAssert(_indexAccess.getIndexExpression(), "Index expression expected."); appendExpressionCopyToMemory(keyType, *_indexAccess.getIndexExpression()); - solAssert(baseType.getSizeOnStack() == 1, - "Unexpected: Not exactly one stack slot taken by subscriptable expression."); m_context << eth::Instruction::SWAP1; appendTypeMoveToMemory(IntegerType(256)); m_context << u256(0) << eth::Instruction::SHA3; + m_context << u256(0); setLValueToStorageItem( _indexAccess); } else if (baseType.getCategory() == Type::Category::Array) { - // stack layout: [] + // stack layout: [storage_byte_offset] [] ArrayType const& arrayType = dynamic_cast(baseType); solAssert(_indexAccess.getIndexExpression(), "Index expression expected."); ArrayType::Location location = arrayType.getLocation(); @@ -758,9 +768,11 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess) else if (location == ArrayType::Location::CallData) // length is stored on the stack m_context << eth::Instruction::SWAP1; + else if (location == ArrayType::Location::Storage) + m_context << eth::Instruction::DUP3 << load; else m_context << eth::Instruction::DUP2 << load; - // stack: + // stack: [storage_byte_offset] // check out-of-bounds access m_context << eth::Instruction::DUP2 << eth::Instruction::LT; eth::AssemblyItem legalAccess = m_context.appendConditionalJump(); @@ -768,7 +780,7 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess) m_context << eth::Instruction::STOP; m_context << legalAccess; - // stack: + // stack: [storage_byte_offset] if (arrayType.isByteArray()) // byte array is packed differently, especially in storage switch (location) @@ -776,14 +788,15 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess) case ArrayType::Location::Storage: // byte array index storage lvalue on stack (goal): // = - m_context << u256(32) << eth::Instruction::SWAP2; + m_context << u256(32) << eth::Instruction::SWAP3; CompilerUtils(m_context).computeHashStatic(); - // stack: 32 index data_ref + // stack: 32 storage_byte_offset index data_ref m_context - << eth::Instruction::DUP3 << eth::Instruction::DUP3 + << eth::Instruction::DUP4 << eth::Instruction::DUP3 << eth::Instruction::DIV << eth::Instruction::ADD - // stack: 32 index (data_ref + index / 32) - << eth::Instruction::SWAP2 << eth::Instruction::SWAP1 << eth::Instruction::MOD; + // stack: 32 storage_byte_offset index (data_ref + index / 32) + << eth::Instruction::SWAP3 << eth::Instruction::SWAP2 + << eth::Instruction::POP << eth::Instruction::MOD; setLValue(_indexAccess); break; case ArrayType::Location::CallData: @@ -797,6 +810,10 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess) } else { + // stack: [storage_byte_offset] + if (location == ArrayType::Location::Storage) + //@todo use byte offset, remove it for now + m_context << eth::Instruction::SWAP1 << eth::Instruction::POP; u256 elementSize = location == ArrayType::Location::Storage ? arrayType.getBaseType()->getStorageSize() : @@ -822,6 +839,7 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess) CompilerUtils(m_context).loadFromMemoryDynamic(*arrayType.getBaseType(), true, true, false); break; case ArrayType::Location::Storage: + m_context << u256(0); // @todo setLValueToStorageItem(_indexAccess); break; case ArrayType::Location::Memory: @@ -1141,7 +1159,7 @@ void ExpressionCompiler::setLValueFromDeclaration(Declaration const& _declaratio if (m_context.isLocalVariable(&_declaration)) setLValue(_expression, _declaration); else if (m_context.isStateVariable(&_declaration)) - setLValue(_expression, _declaration); + setLValue(_expression, _declaration); else BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_sourceLocation(_expression.getLocation()) diff --git a/libsolidity/ExpressionCompiler.h b/libsolidity/ExpressionCompiler.h index 9cab757ea..e6caad747 100644 --- a/libsolidity/ExpressionCompiler.h +++ b/libsolidity/ExpressionCompiler.h @@ -140,7 +140,6 @@ void ExpressionCompiler::setLValue(Expression const& _expression, _Arguments con m_currentLValue = move(lvalue); else lvalue->retrieveValue(_expression.getLocation(), true); - } } diff --git a/libsolidity/LValue.cpp b/libsolidity/LValue.cpp index 68d6797eb..8e618e87f 100644 --- a/libsolidity/LValue.cpp +++ b/libsolidity/LValue.cpp @@ -77,7 +77,7 @@ void StackVariable::setToZero(SourceLocation const& _location, bool) const StorageItem::StorageItem(CompilerContext& _compilerContext, Declaration const& _declaration): StorageItem(_compilerContext, *_declaration.getType()) { - m_context << m_context.getStorageLocationOfVariable(_declaration); + m_context << m_context.getStorageLocationOfVariable(_declaration) << u256(0); } StorageItem::StorageItem(CompilerContext& _compilerContext, Type const& _type): @@ -86,62 +86,42 @@ StorageItem::StorageItem(CompilerContext& _compilerContext, Type const& _type): if (m_dataType.isValueType()) { solAssert(m_dataType.getStorageSize() == m_dataType.getSizeOnStack(), ""); - solAssert(m_dataType.getStorageSize() <= numeric_limits::max(), - "The storage size of " + m_dataType.toString() + " should fit in an unsigned"); - m_size = unsigned(m_dataType.getStorageSize()); + //@todo the meaning of getStorageSize() probably changes + solAssert(m_dataType.getStorageSize() == 1, "Invalid storage size."); } - else - m_size = 0; // unused } void StorageItem::retrieveValue(SourceLocation const&, bool _remove) const { + // stack: storage_key storage_offset if (!m_dataType.isValueType()) return; // no distinction between value and reference for non-value types if (!_remove) - m_context << eth::Instruction::DUP1; - if (m_size == 1) - m_context << eth::Instruction::SLOAD; - else - for (unsigned i = 0; i < m_size; ++i) - { - m_context << eth::Instruction::DUP1 << eth::Instruction::SLOAD << eth::Instruction::SWAP1; - if (i + 1 < m_size) - m_context << u256(1) << eth::Instruction::ADD; - else - m_context << eth::Instruction::POP; - } + CompilerUtils(m_context).copyToStackTop(sizeOnStack(), sizeOnStack()); + m_context + << eth::Instruction::SWAP1 << eth::Instruction::SLOAD << eth::Instruction::SWAP1 + << u256(2) << eth::Instruction::EXP << eth::Instruction::SWAP1 << eth::Instruction::DIV; + //@todo higher order bits might be dirty. Is this bad? + //@todo this does not work for types that are left-aligned on the stack. + // make those types right-aligned? } void StorageItem::storeValue(Type const& _sourceType, SourceLocation const& _location, bool _move) const { - // stack layout: value value ... value target_ref + // stack: value storage_key storage_offset if (m_dataType.isValueType()) { - if (!_move) // copy values - { - if (m_size + 1 > 16) - BOOST_THROW_EXCEPTION(CompilerError() - << errinfo_sourceLocation(_location) << errinfo_comment("Stack too deep.")); - for (unsigned i = 0; i < m_size; ++i) - m_context << eth::dupInstruction(m_size + 1) << eth::Instruction::SWAP1; - } - if (m_size > 1) // store high index value first - m_context << u256(m_size - 1) << eth::Instruction::ADD; - for (unsigned i = 0; i < m_size; ++i) - { - if (i + 1 >= m_size) - m_context << eth::Instruction::SSTORE; - else - // stack here: value value ... value value (target_ref+offset) - m_context << eth::Instruction::SWAP1 << eth::Instruction::DUP2 - << eth::Instruction::SSTORE - << u256(1) << eth::Instruction::SWAP1 << eth::Instruction::SUB; - } + //@todo OR the value into the storage like it is done for ByteArrayElement + m_context + << u256(2) << eth::Instruction::EXP << eth::Instruction::DUP3 << eth::Instruction::MUL + << eth::Instruction::SWAP1 << eth::Instruction::SSTORE; + if (_move) + m_context << eth::Instruction::POP; } else { - solAssert(_sourceType.getCategory() == m_dataType.getCategory(), + solAssert( + _sourceType.getCategory() == m_dataType.getCategory(), "Wrong type conversation for assignment."); if (m_dataType.getCategory() == Type::Category::Array) { @@ -149,40 +129,48 @@ void StorageItem::storeValue(Type const& _sourceType, SourceLocation const& _loc dynamic_cast(m_dataType), dynamic_cast(_sourceType)); if (_move) - m_context << eth::Instruction::POP; + CompilerUtils(m_context).popStackElement(_sourceType); } else if (m_dataType.getCategory() == Type::Category::Struct) { - // stack layout: source_ref target_ref + // stack layout: source_ref source_offset target_ref target_offset auto const& structType = dynamic_cast(m_dataType); solAssert(structType == _sourceType, "Struct assignment with conversion."); for (auto const& member: structType.getMembers()) { + //@todo actually use offsets // assign each member that is not a mapping TypePointer const& memberType = member.second; if (memberType->getCategory() == Type::Category::Mapping) continue; m_context << structType.getStorageOffsetOfMember(member.first) - << eth::Instruction::DUP3 << eth::Instruction::DUP2 << eth::Instruction::ADD; - // stack: source_ref target_ref member_offset source_member_ref + << eth::Instruction::DUP5 << eth::Instruction::DUP2 << eth::Instruction::ADD; + m_context << u256(0); // zero offset + // stack: source_ref source_off target_ref target_off member_offset source_member_ref source_member_off StorageItem(m_context, *memberType).retrieveValue(_location, true); - // stack: source_ref target_ref member_offset source_value... + // stack: source_ref source_off target_ref target_off member_offset source_value... solAssert(2 + memberType->getSizeOnStack() <= 16, "Stack too deep."); - m_context << eth::dupInstruction(2 + memberType->getSizeOnStack()) + m_context << eth::dupInstruction(3 + memberType->getSizeOnStack()) << eth::dupInstruction(2 + memberType->getSizeOnStack()) << eth::Instruction::ADD; - // stack: source_ref target_ref member_offset source_value... target_member_ref + // stack: source_ref source_off target_ref target_off member_offset source_value... target_member_ref + m_context << u256(0); // zero offset StorageItem(m_context, *memberType).storeValue(*memberType, _location, true); m_context << eth::Instruction::POP; } if (_move) - m_context << eth::Instruction::POP; + m_context + << eth::Instruction::POP << eth::Instruction::POP + << eth::Instruction::POP << eth::Instruction::POP; else - m_context << eth::Instruction::SWAP1; - m_context << eth::Instruction::POP; + m_context + << eth::Instruction::SWAP2 << eth::Instruction::POP + << eth::Instruction::SWAP2 << eth::Instruction::POP; } else - BOOST_THROW_EXCEPTION(InternalCompilerError() - << errinfo_sourceLocation(_location) << errinfo_comment("Invalid non-value type for assignment.")); + BOOST_THROW_EXCEPTION( + InternalCompilerError() + << errinfo_sourceLocation(_location) + << errinfo_comment("Invalid non-value type for assignment.")); } } @@ -191,12 +179,12 @@ void StorageItem::setToZero(SourceLocation const&, bool _removeReference) const if (m_dataType.getCategory() == Type::Category::Array) { if (!_removeReference) - m_context << eth::Instruction::DUP1; + CompilerUtils(m_context).copyToStackTop(sizeOnStack(), sizeOnStack()); ArrayUtils(m_context).clearArray(dynamic_cast(m_dataType)); } else if (m_dataType.getCategory() == Type::Category::Struct) { - // stack layout: ref + // stack layout: storage_key storage_offset auto const& structType = dynamic_cast(m_dataType); for (auto const& member: structType.getMembers()) { @@ -204,33 +192,23 @@ void StorageItem::setToZero(SourceLocation const&, bool _removeReference) const TypePointer const& memberType = member.second; if (memberType->getCategory() == Type::Category::Mapping) continue; - m_context << structType.getStorageOffsetOfMember(member.first) - << eth::Instruction::DUP2 << eth::Instruction::ADD; + // @todo actually use offset + m_context + << structType.getStorageOffsetOfMember(member.first) + << eth::Instruction::DUP3 << eth::Instruction::ADD; + m_context << u256(0); StorageItem(m_context, *memberType).setToZero(); } if (_removeReference) - m_context << eth::Instruction::POP; + m_context << eth::Instruction::POP << eth::Instruction::POP; } else { solAssert(m_dataType.isValueType(), "Clearing of unsupported type requested: " + m_dataType.toString()); - if (m_size == 0 && _removeReference) - m_context << eth::Instruction::POP; - else if (m_size == 1) - m_context - << u256(0) << (_removeReference ? eth::Instruction::SWAP1 : eth::Instruction::DUP2) - << eth::Instruction::SSTORE; - else - { - if (!_removeReference) - m_context << eth::Instruction::DUP1; - for (unsigned i = 0; i < m_size; ++i) - if (i + 1 >= m_size) - m_context << u256(0) << eth::Instruction::SWAP1 << eth::Instruction::SSTORE; - else - m_context << u256(0) << eth::Instruction::DUP2 << eth::Instruction::SSTORE - << u256(1) << eth::Instruction::ADD; - } + // @todo actually use offset + if (!_removeReference) + CompilerUtils(m_context).copyToStackTop(sizeOnStack(), sizeOnStack()); + m_context << eth::Instruction::POP << u256(0) << eth::Instruction::SWAP1 << eth::Instruction::SSTORE; } } @@ -300,6 +278,8 @@ StorageArrayLength::StorageArrayLength(CompilerContext& _compilerContext, const m_arrayType(_arrayType) { solAssert(m_arrayType.isDynamicallySized(), ""); + // storage byte offset must be zero + m_context << eth::Instruction::POP; } void StorageArrayLength::retrieveValue(SourceLocation const&, bool _remove) const diff --git a/libsolidity/LValue.h b/libsolidity/LValue.h index c57c80e37..ad6225162 100644 --- a/libsolidity/LValue.h +++ b/libsolidity/LValue.h @@ -98,7 +98,9 @@ private: }; /** - * Reference to some item in storage. The (starting) position of the item is stored on the stack. + * Reference to some item in storage. On the stack this is , + * where 0 <= offset_inside_value < 32 and an offset of i means that the value is multiplied + * by 2**i before storing it. */ class StorageItem: public LValue { @@ -107,6 +109,7 @@ public: StorageItem(CompilerContext& _compilerContext, Declaration const& _declaration); /// Constructs the LValue and assumes that the storage reference is already on the stack. StorageItem(CompilerContext& _compilerContext, Type const& _type); + virtual unsigned sizeOnStack() const { return 2; } virtual void retrieveValue(SourceLocation const& _location, bool _remove = false) const override; virtual void storeValue( Type const& _sourceType, @@ -117,11 +120,6 @@ public: SourceLocation const& _location = SourceLocation(), bool _removeReference = true ) const override; - -private: - /// Number of stack elements occupied by the value (not the reference). - /// Only used for value types. - unsigned m_size; }; /** diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 70cbec5d9..2764eb7b1 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -644,6 +644,9 @@ unsigned ArrayType::getSizeOnStack() const if (m_location == Location::CallData) // offset [length] (stack top) return 1 + (isDynamicallySized() ? 1 : 0); + else if (m_location == Location::Storage) + // storage_key storage_offset + return 2; else // offset return 1; diff --git a/libsolidity/Types.h b/libsolidity/Types.h index 853bd6889..4b0df694d 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -284,6 +284,9 @@ public: /** * The type of an array. The flavours are byte array (bytes), statically- ([]) * and dynamically-sized array ([]). + * In storage, all arrays are packed tightly (as long as more than one elementary type fits in + * one slot). Dynamically sized arrays (including byte arrays) start with their size as a uint and + * thus start on their own slot. */ class ArrayType: public Type { @@ -384,7 +387,7 @@ public: virtual bool operator==(Type const& _other) const override; virtual u256 getStorageSize() const override; virtual bool canLiveOutsideStorage() const override; - virtual unsigned getSizeOnStack() const override { return 1; /*@todo*/ } + virtual unsigned getSizeOnStack() const override { return 2; } virtual std::string toString() const override; virtual MemberList const& getMembers() const override; @@ -527,6 +530,7 @@ private: /** * The type of a mapping, there is one distinct type per key/value type pair. + * Mappings always occupy their own storage slot, but do not actually use it. */ class MappingType: public Type { @@ -537,6 +541,7 @@ public: virtual bool operator==(Type const& _other) const override; virtual std::string toString() const override; + virtual unsigned getSizeOnStack() const override { return 2; } virtual bool canLiveOutsideStorage() const override { return false; } TypePointer const& getKeyType() const { return m_keyType; } diff --git a/test/SolidityEndToEndTest.cpp b/test/SolidityEndToEndTest.cpp index 8ccf9b3fe..de6f43b77 100644 --- a/test/SolidityEndToEndTest.cpp +++ b/test/SolidityEndToEndTest.cpp @@ -534,6 +534,27 @@ BOOST_AUTO_TEST_CASE(empty_string_on_stack) BOOST_CHECK(callContractFunction("run(bytes0,uint8)", string(), byte(0x02)) == encodeArgs(0x2, string(""), string("abc\0"))); } +BOOST_AUTO_TEST_CASE(inc_dec_operators) +{ + char const* sourceCode = R"( + contract test { + uint8 x; + uint v; + function f() returns (uint r) { + uint a = 6; + r = a; + r += (a++) * 0x10; + r += (++a) * 0x100; + v = 3; + r += (v++) * 0x1000; + r += (++v) * 0x10000; + } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("f()") == encodeArgs(0x53866)); +} + BOOST_AUTO_TEST_CASE(state_smoke_test) { char const* sourceCode = "contract test {\n" @@ -2502,7 +2523,7 @@ BOOST_AUTO_TEST_CASE(struct_copy) contract c { struct Nested { uint x; uint y; } struct Struct { uint a; mapping(uint => Struct) b; Nested nested; uint c; } - mapping(uint => Struct) public data; + mapping(uint => Struct) data; function set(uint k) returns (bool) { data[k].a = 1; data[k].nested.x = 3; diff --git a/test/SolidityOptimizer.cpp b/test/SolidityOptimizer.cpp index 41ec1f90c..161d1f898 100644 --- a/test/SolidityOptimizer.cpp +++ b/test/SolidityOptimizer.cpp @@ -120,7 +120,7 @@ BOOST_AUTO_TEST_CASE(unused_expressions) data; } })"; - compileBothVersions(33, sourceCode); + compileBothVersions(29, sourceCode); compareVersions("f()"); }