From 0bec8d9fcd29ca1182d91fa041c9d842794a1912 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 31 Mar 2015 14:59:38 +0200 Subject: [PATCH] Some cleanup concerning byte arrays. --- libsolidity/ArrayUtils.cpp | 118 ++++++++++++++-------------------- libsolidity/Compiler.cpp | 4 -- libsolidity/CompilerUtils.cpp | 2 +- libsolidity/LValue.cpp | 4 +- libsolidity/Types.h | 2 +- 5 files changed, 51 insertions(+), 79 deletions(-) diff --git a/libsolidity/ArrayUtils.cpp b/libsolidity/ArrayUtils.cpp index 58031390f..1a91b0537 100644 --- a/libsolidity/ArrayUtils.cpp +++ b/libsolidity/ArrayUtils.cpp @@ -388,10 +388,7 @@ void ArrayUtils::convertLengthToSize(ArrayType const& _arrayType, bool _pad) con { if (_arrayType.getLocation() == ArrayType::Location::Storage) { - if (_arrayType.isByteArray()) - m_context << u256(31) << eth::Instruction::ADD - << u256(32) << eth::Instruction::SWAP1 << eth::Instruction::DIV; - else if (_arrayType.getBaseType()->getStorageSize() <= 1) + if (_arrayType.getBaseType()->getStorageSize() <= 1) { unsigned baseBytes = _arrayType.getBaseType()->getStorageBytes(); if (baseBytes == 0) @@ -465,82 +462,61 @@ void ArrayUtils::accessIndex(ArrayType const& _arrayType) const m_context << legalAccess; // stack: - if (_arrayType.isByteArray()) - switch (location) - { - case ArrayType::Location::Storage: - // byte array index storage lvalue on stack (goal): - // = - m_context << u256(32) << eth::Instruction::SWAP2; + m_context << eth::Instruction::SWAP1; + if (_arrayType.isDynamicallySized()) + { + if (location == ArrayType::Location::Storage) CompilerUtils(m_context).computeHashStatic(); - // stack: 32 index data_ref + else if (location == ArrayType::Location::Memory) + m_context << u256(32) << eth::Instruction::ADD; + } + // stack: + switch (location) + { + case ArrayType::Location::CallData: + if (!_arrayType.isByteArray()) + m_context + << eth::Instruction::SWAP1 << _arrayType.getBaseType()->getCalldataEncodedSize() + << eth::Instruction::MUL; + m_context << eth::Instruction::ADD; + if (_arrayType.getBaseType()->isValueType()) + CompilerUtils(m_context).loadFromMemoryDynamic( + *_arrayType.getBaseType(), + true, + !_arrayType.isByteArray(), + false + ); + break; + case ArrayType::Location::Storage: + m_context << eth::Instruction::SWAP1; + if (_arrayType.getBaseType()->getStorageBytes() <= 16) + { + // stack: + // goal: + // = <(index % itemsPerSlot) * byteSize> + unsigned byteSize = _arrayType.getBaseType()->getStorageBytes(); + solAssert(byteSize != 0, ""); + unsigned itemsPerSlot = 32 / byteSize; + m_context << u256(itemsPerSlot) << eth::Instruction::SWAP2; + // stack: itemsPerSlot index data_ref m_context << eth::Instruction::DUP3 << eth::Instruction::DUP3 << eth::Instruction::DIV << eth::Instruction::ADD - // stack: 32 index (data_ref + index / 32) + // stack: itemsPerSlot index (data_ref + index / itemsPerSlot) << eth::Instruction::SWAP2 << eth::Instruction::SWAP1 << eth::Instruction::MOD; - break; - case ArrayType::Location::CallData: - // no lvalue, just retrieve the value - m_context - << eth::Instruction::ADD << eth::Instruction::CALLDATALOAD - << ((u256(0xff) << (256 - 8))) << eth::Instruction::AND; - break; - case ArrayType::Location::Memory: - solAssert(false, "Memory lvalues not yet implemented."); - } - else - { - // stack: - m_context << eth::Instruction::SWAP1; - if (_arrayType.isDynamicallySized()) - { - if (location == ArrayType::Location::Storage) - CompilerUtils(m_context).computeHashStatic(); - else if (location == ArrayType::Location::Memory) - m_context << u256(32) << eth::Instruction::ADD; + if (byteSize != 1) + m_context << u256(byteSize) << eth::Instruction::MUL; } - // stack: - switch (location) + else { - case ArrayType::Location::CallData: - m_context - << eth::Instruction::SWAP1 << _arrayType.getBaseType()->getCalldataEncodedSize() - << eth::Instruction::MUL << eth::Instruction::ADD; - if (_arrayType.getBaseType()->isValueType()) - CompilerUtils(m_context).loadFromMemoryDynamic(*_arrayType.getBaseType(), true, true, false); - break; - case ArrayType::Location::Storage: - m_context << eth::Instruction::SWAP1; - if (_arrayType.getBaseType()->getStorageBytes() <= 16) - { - // stack: - // goal: - // = <(index % itemsPerSlot) * byteSize> - unsigned byteSize = _arrayType.getBaseType()->getStorageBytes(); - solAssert(byteSize != 0, ""); - unsigned itemsPerSlot = 32 / byteSize; - m_context << u256(itemsPerSlot) << eth::Instruction::SWAP2; - // stack: itemsPerSlot index data_ref - m_context - << eth::Instruction::DUP3 << eth::Instruction::DUP3 - << eth::Instruction::DIV << eth::Instruction::ADD - // stack: itemsPerSlot index (data_ref + index / itemsPerSlot) - << eth::Instruction::SWAP2 << eth::Instruction::SWAP1 - << eth::Instruction::MOD - << u256(byteSize) << eth::Instruction::MUL; - } - else - { - if (_arrayType.getBaseType()->getStorageSize() != 1) - m_context << _arrayType.getBaseType()->getStorageSize() << eth::Instruction::MUL; - m_context << eth::Instruction::ADD << u256(0); - } - break; - case ArrayType::Location::Memory: - solAssert(false, "Memory lvalues not yet implemented."); + if (_arrayType.getBaseType()->getStorageSize() != 1) + m_context << _arrayType.getBaseType()->getStorageSize() << eth::Instruction::MUL; + m_context << eth::Instruction::ADD << u256(0); } + break; + case ArrayType::Location::Memory: + solAssert(false, "Memory lvalues not yet implemented."); } } diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index 8e2634499..886565cb4 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -254,7 +254,6 @@ void Compiler::appendCalldataUnpacker(TypePointers const& _typeParameters, bool void Compiler::appendReturnValuePacker(TypePointers const& _typeParameters) { - //@todo this can be also done more efficiently unsigned dataOffset = 0; unsigned stackDepth = 0; for (TypePointer const& type: _typeParameters) @@ -303,9 +302,6 @@ bool Compiler::visit(VariableDeclaration const& _variableDeclaration) bool Compiler::visit(FunctionDefinition const& _function) { CompilerContext::LocationSetter locationSetter(m_context, _function); - //@todo to simplify this, the calling convention could by changed such that - // caller puts: [retarg0] ... [retargm] [return address] [arg0] ... [argn] - // although note that this reduces the size of the visible stack m_context.startFunction(_function); diff --git a/libsolidity/CompilerUtils.cpp b/libsolidity/CompilerUtils.cpp index 454951147..8d3e9d2a2 100644 --- a/libsolidity/CompilerUtils.cpp +++ b/libsolidity/CompilerUtils.cpp @@ -93,7 +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::POP; // remove offset, arrays always start new slot 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/LValue.cpp b/libsolidity/LValue.cpp index 234072bce..02e6cbca5 100644 --- a/libsolidity/LValue.cpp +++ b/libsolidity/LValue.cpp @@ -225,7 +225,8 @@ void StorageItem::setToZero(SourceLocation const&, bool _removeReference) const else if (m_dataType.getCategory() == Type::Category::Struct) { // stack layout: storage_key storage_offset - // @todo this can be improved for packed types + // @todo this can be improved: use StorageItem for non-value types, and just store 0 in + // all slots that contain value types later. auto const& structType = dynamic_cast(m_dataType); for (auto const& member: structType.getMembers()) { @@ -245,7 +246,6 @@ void StorageItem::setToZero(SourceLocation const&, bool _removeReference) const else { solAssert(m_dataType.isValueType(), "Clearing of unsupported type requested: " + m_dataType.toString()); - // @todo actually use offset if (!_removeReference) CompilerUtils(m_context).copyToStackTop(sizeOnStack(), sizeOnStack()); if (m_dataType.getStorageBytes() == 32) diff --git a/libsolidity/Types.h b/libsolidity/Types.h index e1852bc7f..e6d32d175 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -345,7 +345,7 @@ public: explicit ArrayType(Location _location): m_location(_location), m_isByteArray(true), - m_baseType(std::make_shared(8)) + m_baseType(std::make_shared(1)) {} /// Constructor for a dynamically sized array type ("type[]") ArrayType(Location _location, const TypePointer &_baseType):