From 4a1b58114c1c7fb38b7e302aecfbf2dccf047381 Mon Sep 17 00:00:00 2001 From: yann300 Date: Fri, 27 Feb 2015 16:54:18 +0100 Subject: [PATCH 01/16] - Add validation for deployment values. --- mix/qml/DeploymentDialog.qml | 38 +++++++++++++++++++++++++++++++++--- mix/qml/ProjectModel.qml | 1 + 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/mix/qml/DeploymentDialog.qml b/mix/qml/DeploymentDialog.qml index 5dbdd356e..703246998 100644 --- a/mix/qml/DeploymentDialog.qml +++ b/mix/qml/DeploymentDialog.qml @@ -80,6 +80,20 @@ Window { }); } + function stopForInputError(inError) + { + errorDialog.text = ""; + if (inError.length > 0) + { + errorDialog.text = qsTr("The length of a string cannot exceed 32 characters.\nPlease verify the following value(s):\n\n") + for (var k in inError) + errorDialog.text += inError[k] + "\n"; + errorDialog.open(); + return true; + } + return false; + } + function pad(h) { // TODO move this to QHashType class @@ -250,6 +264,12 @@ Window { icon: StandardIcon.Warning } + MessageDialog { + id: errorDialog + standardButtons: StandardButton.Ok + icon: StandardIcon.Error + } + RowLayout { anchors.bottom: parent.bottom @@ -259,7 +279,15 @@ Window { text: qsTr("Deploy contract / Package resources"); tooltip: qsTr("Deploy contract and package resources files.") onClicked: { - deployWarningDialog.open(); + var inError = []; + var ethUrl = ProjectModelCode.formatAppUrl(applicationUrlEth.text); + for (var k in ethUrl) + { + if (ethUrl[k].length > 32) + inError.push(qsTr("Member too long: " + ethUrl[k]) + "\n"); + } + if (!stopForInputError(inError)) + deployWarningDialog.open(); } } @@ -287,10 +315,14 @@ Window { if (applicationUrlHttp.text === "" || deploymentDialog.packageHash === "") { deployDialog.title = text; - deployDialog.text = qsTr("Please provide the link where the resources are stored and ensure the package is aleary built using the deployment step. ") + deployDialog.text = qsTr("Please provide the link where the resources are stored and ensure the package is aleary built using the deployment step.") deployDialog.open(); + return; } - else + var inError = []; + if (applicationUrlHttp.text.length > 32) + inError.push(qsTr(applicationUrlHttp.text)); + if (!stopForInputError(inError)) ProjectModelCode.registerToUrlHint(); } } diff --git a/mix/qml/ProjectModel.qml b/mix/qml/ProjectModel.qml index 7e4e8cb49..d3b4070aa 100644 --- a/mix/qml/ProjectModel.qml +++ b/mix/qml/ProjectModel.qml @@ -59,6 +59,7 @@ Item { function addExistingFiles(paths) { ProjectModelCode.doAddExistingFiles(paths); } function deployProject() { ProjectModelCode.deployProject(false); } function registerToUrlHint() { ProjectModelCode.registerToUrlHint(); } + function formatAppUrl() { ProjectModelCode.formatAppUrl(url); } Connections { target: appContext From 43892601e388e41f43f006d243ec54f70429a38b Mon Sep 17 00:00:00 2001 From: Christian Date: Wed, 25 Feb 2015 20:27:55 +0100 Subject: [PATCH 02/16] Shortening of dynamic arrays. --- libsolidity/ArrayUtils.cpp | 284 +++++++++++++++++++++++++++++ libsolidity/ArrayUtils.h | 72 ++++++++ libsolidity/CompilerUtils.cpp | 170 ----------------- libsolidity/CompilerUtils.h | 13 -- libsolidity/ExpressionCompiler.cpp | 8 +- libsolidity/LValue.cpp | 122 +++++++++---- libsolidity/LValue.h | 44 ++++- test/SolidityEndToEndTest.cpp | 90 +++++++++ 8 files changed, 567 insertions(+), 236 deletions(-) create mode 100644 libsolidity/ArrayUtils.cpp create mode 100644 libsolidity/ArrayUtils.h diff --git a/libsolidity/ArrayUtils.cpp b/libsolidity/ArrayUtils.cpp new file mode 100644 index 000000000..07949c79e --- /dev/null +++ b/libsolidity/ArrayUtils.cpp @@ -0,0 +1,284 @@ +/* + This file is part of cpp-ethereum. + + cpp-ethereum is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + cpp-ethereum is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with cpp-ethereum. If not, see . +*/ +/** + * @author Christian + * @date 2015 + * Code generation utils that handle arrays. + */ + +#include +#include +#include +#include +#include +#include +#include + +using namespace std; +using namespace dev; +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 + solAssert(_targetType.getLocation() == ArrayType::Location::Storage, ""); + solAssert(_targetType.isByteArray(), "Non byte arrays not yet implemented here."); + solAssert(_sourceType.isByteArray(), "Non byte arrays not yet implemented here."); + + switch (_sourceType.getLocation()) + { + case ArrayType::Location::CallData: + { + // This also assumes that after "length" we only have zeros, i.e. it cannot be used to + // slice a byte array from calldata. + + // stack: source_offset source_len target_ref + // fetch old length and convert to words + m_context << eth::Instruction::DUP1 << eth::Instruction::SLOAD; + convertLengthToSize(_targetType); + // stack here: source_offset source_len target_ref target_length_words + // actual array data is stored at SHA3(storage_offset) + m_context << eth::Instruction::DUP2; + CompilerUtils(m_context).computeHashStatic(); + // compute target_data_end + m_context << eth::Instruction::DUP1 << eth::Instruction::SWAP2 << eth::Instruction::ADD + << eth::Instruction::SWAP1; + // stack here: source_offset source_len target_ref target_data_end target_data_ref + // store length (in bytes) + m_context << eth::Instruction::DUP4 << eth::Instruction::DUP1 << eth::Instruction::DUP5 + << eth::Instruction::SSTORE; + // jump to end if length is zero + m_context << eth::Instruction::ISZERO; + eth::AssemblyItem copyLoopEnd = m_context.newTag(); + m_context.appendConditionalJumpTo(copyLoopEnd); + // store start offset + m_context << eth::Instruction::DUP5; + // stack now: source_offset source_len target_ref target_data_end target_data_ref calldata_offset + eth::AssemblyItem copyLoopStart = m_context.newTag(); + m_context << copyLoopStart + // copy from calldata and store + << eth::Instruction::DUP1 << eth::Instruction::CALLDATALOAD + << eth::Instruction::DUP3 << eth::Instruction::SSTORE + // increment target_data_ref by 1 + << eth::Instruction::SWAP1 << u256(1) << eth::Instruction::ADD + // increment calldata_offset by 32 + << eth::Instruction::SWAP1 << u256(32) << eth::Instruction::ADD + // check for loop condition + << eth::Instruction::DUP1 << eth::Instruction::DUP6 << eth::Instruction::GT; + m_context.appendConditionalJumpTo(copyLoopStart); + m_context << eth::Instruction::POP; + m_context << copyLoopEnd; + + // now clear leftover bytes of the old value + // stack now: source_offset source_len target_ref target_data_end target_data_ref + clearStorageLoop(IntegerType(256)); + // stack now: source_offset source_len target_ref target_data_end + + m_context << eth::Instruction::POP << eth::Instruction::SWAP2 + << eth::Instruction::POP << eth::Instruction::POP; + break; + } + case ArrayType::Location::Storage: + { + // this copies source to target and also clears target if it was larger + + // stack: source_ref target_ref + // store target_ref + m_context << eth::Instruction::SWAP1 << eth::Instruction::DUP2; + // fetch lengthes + m_context << eth::Instruction::DUP1 << eth::Instruction::SLOAD << eth::Instruction::SWAP2 + << eth::Instruction::DUP1 << eth::Instruction::SLOAD; + // stack: target_ref target_len_bytes target_ref source_ref source_len_bytes + // store new target length + m_context << eth::Instruction::DUP1 << eth::Instruction::DUP4 << eth::Instruction::SSTORE; + // compute hashes (data positions) + m_context << eth::Instruction::SWAP2; + CompilerUtils(m_context).computeHashStatic(); + m_context << eth::Instruction::SWAP1; + CompilerUtils(m_context).computeHashStatic(); + // stack: target_ref target_len_bytes source_len_bytes target_data_pos source_data_pos + // convert lengthes from bytes to storage slots + m_context << u256(31) << u256(32) << eth::Instruction::DUP1 << eth::Instruction::DUP3 + << eth::Instruction::DUP8 << eth::Instruction::ADD << eth::Instruction::DIV + << eth::Instruction::SWAP2 + << eth::Instruction::DUP6 << eth::Instruction::ADD << eth::Instruction::DIV; + // stack: target_ref target_len_bytes source_len_bytes target_data_pos source_data_pos target_len source_len + // @todo we might be able to go without a third counter + m_context << u256(0); + // stack: target_ref target_len_bytes source_len_bytes target_data_pos source_data_pos target_len source_len counter + eth::AssemblyItem copyLoopStart = m_context.newTag(); + m_context << copyLoopStart; + // check for loop condition + m_context << eth::Instruction::DUP1 << eth::Instruction::DUP3 + << eth::Instruction::GT << eth::Instruction::ISZERO; + eth::AssemblyItem copyLoopEnd = m_context.newTag(); + m_context.appendConditionalJumpTo(copyLoopEnd); + // copy + m_context << eth::Instruction::DUP4 << eth::Instruction::DUP2 << eth::Instruction::ADD + << eth::Instruction::SLOAD + << eth::Instruction::DUP6 << eth::Instruction::DUP3 << eth::Instruction::ADD + << eth::Instruction::SSTORE; + // increment + m_context << u256(1) << eth::Instruction::ADD; + m_context.appendJumpTo(copyLoopStart); + m_context << copyLoopEnd; + + // zero-out leftovers in target + // stack: target_ref target_len_bytes source_len_bytes target_data_pos source_data_pos target_len source_len counter + // add counter to target_data_pos + m_context << eth::Instruction::DUP5 << eth::Instruction::ADD + << eth::Instruction::SWAP5 << eth::Instruction::POP; + // stack: target_ref target_len_bytes target_data_pos_updated target_data_pos source_data_pos target_len source_len + // add length to target_data_pos to get target_data_end + m_context << eth::Instruction::POP << eth::Instruction::DUP3 << eth::Instruction::ADD + << eth::Instruction::SWAP4 + << eth::Instruction::POP << eth::Instruction::POP << eth::Instruction::POP; + // stack: target_ref target_data_end target_data_pos_updated + clearStorageLoop(IntegerType(256)); + m_context << eth::Instruction::POP; + break; + } + default: + solAssert(false, "Given byte array location not implemented."); + } +} + +void ArrayUtils::clearArray(ArrayType const& _type) const +{ + solAssert(_type.getLocation() == ArrayType::Location::Storage, ""); + if (_type.isDynamicallySized()) + clearDynamicArray(_type); + else if (_type.getLength() == 0) + m_context << eth::Instruction::POP; + else if (_type.getLength() < 5) // unroll loop for small arrays @todo choose a good value + { + for (unsigned i = 1; i < _type.getLength(); ++i) + { + StorageItem(m_context, *_type.getBaseType()).setToZero(SourceLocation(), false); + m_context << u256(_type.getBaseType()->getStorageSize()) << eth::Instruction::ADD; + } + StorageItem(m_context, *_type.getBaseType()).setToZero(SourceLocation(), true); + } + else + { + m_context + << eth::Instruction::DUP1 << u256(_type.getLength()) + << u256(_type.getBaseType()->getStorageSize()) + << eth::Instruction::MUL << eth::Instruction::ADD << eth::Instruction::SWAP1; + clearStorageLoop(*_type.getBaseType()); + m_context << eth::Instruction::POP; + } +} + +void ArrayUtils::clearDynamicArray(ArrayType const& _type) const +{ + solAssert(_type.getLocation() == ArrayType::Location::Storage, ""); + solAssert(_type.isDynamicallySized(), ""); + + // fetch length + m_context << eth::Instruction::DUP1 << eth::Instruction::SLOAD; + // set length to zero + m_context << u256(0) << eth::Instruction::DUP3 << eth::Instruction::SSTORE; + // stack: ref old_length + convertLengthToSize(_type); + // 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) + m_context << eth::Instruction::SWAP1 << eth::Instruction::DUP2 << eth::Instruction::ADD + << eth::Instruction::SWAP1; + // stack: data_pos_end data_pos + if (_type.isByteArray()) + clearStorageLoop(IntegerType(256)); + else + clearStorageLoop(*_type.getBaseType()); + // cleanup + m_context << eth::Instruction::POP; +} + +void ArrayUtils::resizeDynamicArray(const ArrayType& _type) const +{ + solAssert(_type.getLocation() == ArrayType::Location::Storage, ""); + solAssert(_type.isDynamicallySized(), ""); + + eth::AssemblyItem resizeEnd = m_context.newTag(); + + // stack: ref new_length + // fetch old length + m_context << eth::Instruction::DUP2 << eth::Instruction::SLOAD; + // stack: ref new_length old_length + // store new length + m_context << eth::Instruction::DUP2 << eth::Instruction::DUP4 << eth::Instruction::SSTORE; + // skip if size is not reduced + m_context << eth::Instruction::DUP2 << eth::Instruction::DUP2 + << eth::Instruction::ISZERO << eth::Instruction::GT; + m_context.appendConditionalJumpTo(resizeEnd); + + // size reduced, clear the end of the array + // stack: ref new_length old_length + convertLengthToSize(_type); + m_context << eth::Instruction::DUP2; + convertLengthToSize(_type); + // stack: ref new_length old_size new_size + // compute data positions + m_context << eth::Instruction::DUP4; + CompilerUtils(m_context).computeHashStatic(); + // stack: ref new_length old_size new_size data_pos + m_context << eth::Instruction::SWAP2 << eth::Instruction::DUP3 << eth::Instruction::ADD; + // stack: ref new_length data_pos new_size delete_end + m_context << eth::Instruction::SWAP2 << eth::Instruction::ADD; + // stack: ref new_length delete_end delete_start + if (_type.isByteArray()) + clearStorageLoop(IntegerType(256)); + else + clearStorageLoop(*_type.getBaseType()); + + m_context << resizeEnd; + // cleanup + m_context << eth::Instruction::POP << eth::Instruction::POP << eth::Instruction::POP; +} + +void ArrayUtils::clearStorageLoop(Type const& _type) const +{ + // stack: end_pos pos + eth::AssemblyItem loopStart = m_context.newTag(); + m_context << loopStart; + // check for loop condition + m_context << eth::Instruction::DUP1 << eth::Instruction::DUP3 + << eth::Instruction::GT << eth::Instruction::ISZERO; + eth::AssemblyItem zeroLoopEnd = m_context.newTag(); + m_context.appendConditionalJumpTo(zeroLoopEnd); + // delete + StorageItem(m_context, _type).setToZero(SourceLocation(), false); + // increment + m_context << u256(1) << eth::Instruction::ADD; + m_context.appendJumpTo(loopStart); + // cleanup + m_context << zeroLoopEnd; + m_context << eth::Instruction::POP; +} + +void ArrayUtils::convertLengthToSize(ArrayType const& _arrayType) const +{ + if (_arrayType.isByteArray()) + m_context << u256(31) << eth::Instruction::ADD + << u256(32) << eth::Instruction::SWAP1 << eth::Instruction::DIV; + else if (_arrayType.getBaseType()->getStorageSize() > 1) + m_context << _arrayType.getBaseType()->getStorageSize() << eth::Instruction::MUL; +} + diff --git a/libsolidity/ArrayUtils.h b/libsolidity/ArrayUtils.h new file mode 100644 index 000000000..f080dce66 --- /dev/null +++ b/libsolidity/ArrayUtils.h @@ -0,0 +1,72 @@ +/* + This file is part of cpp-ethereum. + + cpp-ethereum is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + cpp-ethereum is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with cpp-ethereum. If not, see . +*/ +/** + * @author Christian + * @date 2015 + * Code generation utils that handle arrays. + */ + +#pragma once + +namespace dev +{ +namespace solidity +{ + +class CompilerContext; +class Type; +class ArrayType; + +/** + * Class that provides code generation for handling arrays. + */ +class ArrayUtils +{ +public: + ArrayUtils(CompilerContext& _context): m_context(_context) {} + + /// Copies an array to an array in storage. + /// Stack pre: [source_reference] target_reference + /// Stack post: target_reference + void copyArrayToStorage(ArrayType const& _targetType, ArrayType const& _sourceType) const; + /// Clears the given dynamic or static array. + /// Stack pre: reference + /// 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 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 post: + void resizeDynamicArray(ArrayType const& _type) const; + /// Appends a loop that clears a sequence of storage slots of the given type (excluding end). + /// Stack pre: end_ref start_ref + /// Stack post: end_ref + void clearStorageLoop(Type const& _type) const; + /// Converts length to size (multiplies by size on stack), rounds up for byte arrays. + /// Stack pre: length + /// Stack post: size + void convertLengthToSize(ArrayType const& _arrayType) const; + +private: + CompilerContext& m_context; +}; + +} +} diff --git a/libsolidity/CompilerUtils.cpp b/libsolidity/CompilerUtils.cpp index c7ce94456..826651e61 100644 --- a/libsolidity/CompilerUtils.cpp +++ b/libsolidity/CompilerUtils.cpp @@ -164,134 +164,6 @@ void CompilerUtils::computeHashStatic(Type const& _type, bool _padToWordBoundari m_context << u256(length) << u256(0) << eth::Instruction::SHA3; } -void CompilerUtils::copyByteArrayToStorage( - 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 - solAssert(_targetType.getLocation() == ArrayType::Location::Storage, ""); - solAssert(_targetType.isByteArray(), "Non byte arrays not yet implemented here."); - solAssert(_sourceType.isByteArray(), "Non byte arrays not yet implemented here."); - - switch (_sourceType.getLocation()) - { - case ArrayType::Location::CallData: - { - // This also assumes that after "length" we only have zeros, i.e. it cannot be used to - // slice a byte array from calldata. - - // stack: source_offset source_len target_ref - // fetch old length and convert to words - m_context << eth::Instruction::DUP1 << eth::Instruction::SLOAD; - m_context << u256(31) << eth::Instruction::ADD - << u256(32) << eth::Instruction::SWAP1 << eth::Instruction::DIV; - // stack here: source_offset source_len target_ref target_length_words - // actual array data is stored at SHA3(storage_offset) - m_context << eth::Instruction::DUP2; - CompilerUtils(m_context).computeHashStatic(); - // compute target_data_end - m_context << eth::Instruction::DUP1 << eth::Instruction::SWAP2 << eth::Instruction::ADD - << eth::Instruction::SWAP1; - // stack here: source_offset source_len target_ref target_data_end target_data_ref - // store length (in bytes) - m_context << eth::Instruction::DUP4 << eth::Instruction::DUP1 << eth::Instruction::DUP5 - << eth::Instruction::SSTORE; - // jump to end if length is zero - m_context << eth::Instruction::ISZERO; - eth::AssemblyItem copyLoopEnd = m_context.newTag(); - m_context.appendConditionalJumpTo(copyLoopEnd); - // store start offset - m_context << eth::Instruction::DUP5; - // stack now: source_offset source_len target_ref target_data_end target_data_ref calldata_offset - eth::AssemblyItem copyLoopStart = m_context.newTag(); - m_context << copyLoopStart - // copy from calldata and store - << eth::Instruction::DUP1 << eth::Instruction::CALLDATALOAD - << eth::Instruction::DUP3 << eth::Instruction::SSTORE - // increment target_data_ref by 1 - << eth::Instruction::SWAP1 << u256(1) << eth::Instruction::ADD - // increment calldata_offset by 32 - << eth::Instruction::SWAP1 << u256(32) << eth::Instruction::ADD - // check for loop condition - << eth::Instruction::DUP1 << eth::Instruction::DUP6 << eth::Instruction::GT; - m_context.appendConditionalJumpTo(copyLoopStart); - m_context << eth::Instruction::POP; - m_context << copyLoopEnd; - - // now clear leftover bytes of the old value - // stack now: source_offset source_len target_ref target_data_end target_data_ref - clearStorageLoop(); - // stack now: source_offset source_len target_ref target_data_end - - m_context << eth::Instruction::POP << eth::Instruction::SWAP2 - << eth::Instruction::POP << eth::Instruction::POP; - break; - } - case ArrayType::Location::Storage: - { - // this copies source to target and also clears target if it was larger - - // stack: source_ref target_ref - // store target_ref - m_context << eth::Instruction::SWAP1 << eth::Instruction::DUP2; - // fetch lengthes - m_context << eth::Instruction::DUP1 << eth::Instruction::SLOAD << eth::Instruction::SWAP2 - << eth::Instruction::DUP1 << eth::Instruction::SLOAD; - // stack: target_ref target_len_bytes target_ref source_ref source_len_bytes - // store new target length - m_context << eth::Instruction::DUP1 << eth::Instruction::DUP4 << eth::Instruction::SSTORE; - // compute hashes (data positions) - m_context << eth::Instruction::SWAP2; - CompilerUtils(m_context).computeHashStatic(); - m_context << eth::Instruction::SWAP1; - CompilerUtils(m_context).computeHashStatic(); - // stack: target_ref target_len_bytes source_len_bytes target_data_pos source_data_pos - // convert lengthes from bytes to storage slots - m_context << u256(31) << u256(32) << eth::Instruction::DUP1 << eth::Instruction::DUP3 - << eth::Instruction::DUP8 << eth::Instruction::ADD << eth::Instruction::DIV - << eth::Instruction::SWAP2 - << eth::Instruction::DUP6 << eth::Instruction::ADD << eth::Instruction::DIV; - // stack: target_ref target_len_bytes source_len_bytes target_data_pos source_data_pos target_len source_len - // @todo we might be able to go without a third counter - m_context << u256(0); - // stack: target_ref target_len_bytes source_len_bytes target_data_pos source_data_pos target_len source_len counter - eth::AssemblyItem copyLoopStart = m_context.newTag(); - m_context << copyLoopStart; - // check for loop condition - m_context << eth::Instruction::DUP1 << eth::Instruction::DUP3 - << eth::Instruction::GT << eth::Instruction::ISZERO; - eth::AssemblyItem copyLoopEnd = m_context.newTag(); - m_context.appendConditionalJumpTo(copyLoopEnd); - // copy - m_context << eth::Instruction::DUP4 << eth::Instruction::DUP2 << eth::Instruction::ADD - << eth::Instruction::SLOAD - << eth::Instruction::DUP6 << eth::Instruction::DUP3 << eth::Instruction::ADD - << eth::Instruction::SSTORE; - // increment - m_context << u256(1) << eth::Instruction::ADD; - m_context.appendJumpTo(copyLoopStart); - m_context << copyLoopEnd; - - // zero-out leftovers in target - // stack: target_ref target_len_bytes source_len_bytes target_data_pos source_data_pos target_len source_len counter - // add counter to target_data_pos - m_context << eth::Instruction::DUP5 << eth::Instruction::ADD - << eth::Instruction::SWAP5 << eth::Instruction::POP; - // stack: target_ref target_len_bytes target_data_pos_updated target_data_pos source_data_pos target_len source_len - // add length to target_data_pos to get target_data_end - m_context << eth::Instruction::POP << eth::Instruction::DUP3 << eth::Instruction::ADD - << eth::Instruction::SWAP4 - << eth::Instruction::POP << eth::Instruction::POP << eth::Instruction::POP; - // stack: target_ref target_data_end target_data_pos_updated - clearStorageLoop(); - m_context << eth::Instruction::POP; - break; - } - default: - solAssert(false, "Given byte array location not implemented."); - } -} - unsigned CompilerUtils::loadFromMemoryHelper(Type const& _type, bool _fromCalldata, bool _padToWordBoundaries) { unsigned _encodedSize = _type.getCalldataEncodedSize(); @@ -316,28 +188,6 @@ unsigned CompilerUtils::loadFromMemoryHelper(Type const& _type, bool _fromCallda return numBytes; } -void CompilerUtils::clearByteArray(ArrayType const& _type) const -{ - solAssert(_type.getLocation() == ArrayType::Location::Storage, ""); - solAssert(_type.isByteArray(), "Non byte arrays not yet implemented here."); - - // fetch length - m_context << eth::Instruction::DUP1 << eth::Instruction::SLOAD; - // set length to zero - m_context << u256(0) << eth::Instruction::DUP3 << eth::Instruction::SSTORE; - // convert length from bytes to storage slots - m_context << u256(31) << eth::Instruction::ADD - << u256(32) << eth::Instruction::SWAP1 << eth::Instruction::DIV; - // compute data positions - m_context << eth::Instruction::SWAP1; - CompilerUtils(m_context).computeHashStatic(); - // stack: len data_pos - m_context << eth::Instruction::SWAP1 << eth::Instruction::DUP2 << eth::Instruction::ADD - << eth::Instruction::SWAP1; - clearStorageLoop(); - // cleanup - m_context << eth::Instruction::POP; -} unsigned CompilerUtils::prepareMemoryStore(Type const& _type, bool _padToWordBoundaries) const { @@ -356,25 +206,5 @@ unsigned CompilerUtils::prepareMemoryStore(Type const& _type, bool _padToWordBou return numBytes; } -void CompilerUtils::clearStorageLoop() const -{ - // stack: end_pos pos - eth::AssemblyItem loopStart = m_context.newTag(); - m_context << loopStart; - // check for loop condition - m_context << eth::Instruction::DUP1 << eth::Instruction::DUP3 - << eth::Instruction::GT << eth::Instruction::ISZERO; - eth::AssemblyItem zeroLoopEnd = m_context.newTag(); - m_context.appendConditionalJumpTo(zeroLoopEnd); - // zero out - m_context << u256(0) << eth::Instruction::DUP2 << eth::Instruction::SSTORE; - // increment - m_context << u256(1) << eth::Instruction::ADD; - m_context.appendJumpTo(loopStart); - // cleanup - m_context << zeroLoopEnd; - m_context << eth::Instruction::POP; -} - } } diff --git a/libsolidity/CompilerUtils.h b/libsolidity/CompilerUtils.h index 2fb97d808..2df85f11c 100644 --- a/libsolidity/CompilerUtils.h +++ b/libsolidity/CompilerUtils.h @@ -79,15 +79,6 @@ public: /// @note Only works for types of fixed size. void computeHashStatic(Type const& _type = IntegerType(256), bool _padToWordBoundaries = false); - /// Copies a byte array to a byte array in storage. - /// Stack pre: [source_reference] target_reference - /// Stack post: target_reference - void copyByteArrayToStorage(ArrayType const& _targetType, ArrayType const& _sourceType) const; - /// Clears the length and data elements of the byte array referenced on the stack. - /// Stack pre: reference - /// Stack post: - void clearByteArray(ArrayType const& _type) const; - /// Bytes we need to the start of call data. /// - The size in bytes of the function (hash) identifier. static const unsigned int dataStartOffset; @@ -97,10 +88,6 @@ private: unsigned prepareMemoryStore(Type const& _type, bool _padToWordBoundaries) const; /// Loads type from memory assuming memory offset is on stack top. unsigned loadFromMemoryHelper(Type const& _type, bool _fromCalldata, bool _padToWordBoundaries); - /// Appends a loop that clears a sequence of storage slots (excluding end). - /// Stack pre: end_ref start_ref - /// Stack post: end_ref - void clearStorageLoop() const; CompilerContext& m_context; }; diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index 430e46b06..cdf9436dc 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -93,7 +93,7 @@ void ExpressionCompiler::appendStateVariableAccessor(VariableDeclaration const& m_context << eth::Instruction::DUP1 << structType->getStorageOffsetOfMember(names[i]) << eth::Instruction::ADD; - StorageItem(m_context, types[i]).retrieveValue(SourceLocation(), true); + 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; retSizeOnStack += types[i]->getSizeOnStack(); @@ -104,7 +104,7 @@ void ExpressionCompiler::appendStateVariableAccessor(VariableDeclaration const& { // simple value solAssert(accessorType.getReturnParameterTypes().size() == 1, ""); - StorageItem(m_context, returnType).retrieveValue(SourceLocation(), true); + StorageItem(m_context, *returnType).retrieveValue(SourceLocation(), true); retSizeOnStack = returnType->getSizeOnStack(); } solAssert(retSizeOnStack <= 15, "Stack too deep."); @@ -680,7 +680,7 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess) m_context << eth::Instruction::SWAP1 << eth::Instruction::POP; break; case ArrayType::Location::Storage: - setLValueToStorageItem(_memberAccess); + setLValue(_memberAccess, type); break; default: solAssert(false, "Unsupported array location."); @@ -1044,7 +1044,7 @@ void ExpressionCompiler::setLValueFromDeclaration(Declaration const& _declaratio void ExpressionCompiler::setLValueToStorageItem(Expression const& _expression) { - setLValue(_expression, _expression.getType()); + setLValue(_expression, *_expression.getType()); } } diff --git a/libsolidity/LValue.cpp b/libsolidity/LValue.cpp index db59c41af..78154f427 100644 --- a/libsolidity/LValue.cpp +++ b/libsolidity/LValue.cpp @@ -32,15 +32,14 @@ using namespace solidity; StackVariable::StackVariable(CompilerContext& _compilerContext, Declaration const& _declaration): - LValue(_compilerContext, _declaration.getType()), + LValue(_compilerContext, *_declaration.getType()), m_baseStackOffset(m_context.getBaseStackOffsetOfVariable(_declaration)), - m_size(m_dataType->getSizeOnStack()) + m_size(m_dataType.getSizeOnStack()) { } -void StackVariable::retrieveValue(SourceLocation const& _location, bool _remove) const +void StackVariable::retrieveValue(SourceLocation const& _location, bool) const { - (void)_remove; unsigned stackPos = m_context.baseToCurrentStackOffset(m_baseStackOffset); if (stackPos >= 15) //@todo correct this by fetching earlier or moving to memory BOOST_THROW_EXCEPTION(CompilerError() @@ -49,9 +48,8 @@ void StackVariable::retrieveValue(SourceLocation const& _location, bool _remove) m_context << eth::dupInstruction(stackPos + 1); } -void StackVariable::storeValue(Type const& _sourceType, SourceLocation const& _location, bool _move) const +void StackVariable::storeValue(Type const&, SourceLocation const& _location, bool _move) const { - (void)_sourceType; unsigned stackDiff = m_context.baseToCurrentStackOffset(m_baseStackOffset) - m_size + 1; if (stackDiff > 16) BOOST_THROW_EXCEPTION(CompilerError() @@ -63,7 +61,7 @@ void StackVariable::storeValue(Type const& _sourceType, SourceLocation const& _l retrieveValue(_location); } -void StackVariable::setToZero(SourceLocation const& _location) const +void StackVariable::setToZero(SourceLocation const& _location, bool) const { unsigned stackDiff = m_context.baseToCurrentStackOffset(m_baseStackOffset); if (stackDiff > 16) @@ -77,20 +75,20 @@ void StackVariable::setToZero(SourceLocation const& _location) const StorageItem::StorageItem(CompilerContext& _compilerContext, Declaration const& _declaration): - StorageItem(_compilerContext, _declaration.getType()) + StorageItem(_compilerContext, *_declaration.getType()) { m_context << m_context.getStorageLocationOfVariable(_declaration); } -StorageItem::StorageItem(CompilerContext& _compilerContext, TypePointer const& _type): +StorageItem::StorageItem(CompilerContext& _compilerContext, Type const& _type): LValue(_compilerContext, _type) { - if (m_dataType->isValueType()) + 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()); + 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()); } else m_size = 0; // unused @@ -98,7 +96,7 @@ StorageItem::StorageItem(CompilerContext& _compilerContext, TypePointer const& _ void StorageItem::retrieveValue(SourceLocation const&, bool _remove) const { - if (!m_dataType->isValueType()) + if (!m_dataType.isValueType()) return; // no distinction between value and reference for non-value types if (!_remove) m_context << eth::Instruction::DUP1; @@ -118,7 +116,7 @@ void StorageItem::retrieveValue(SourceLocation const&, bool _remove) const void StorageItem::storeValue(Type const& _sourceType, SourceLocation const& _location, bool _move) const { // stack layout: value value ... value target_ref - if (m_dataType->isValueType()) + if (m_dataType.isValueType()) { if (!_move) // copy values { @@ -143,20 +141,20 @@ void StorageItem::storeValue(Type const& _sourceType, SourceLocation const& _loc } 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) + if (m_dataType.getCategory() == Type::Category::Array) { - CompilerUtils(m_context).copyByteArrayToStorage( - dynamic_cast(*m_dataType), + ArrayUtils(m_context).copyArrayToStorage( + dynamic_cast(m_dataType), dynamic_cast(_sourceType)); if (_move) m_context << eth::Instruction::POP; } - else if (m_dataType->getCategory() == Type::Category::Struct) + else if (m_dataType.getCategory() == Type::Category::Struct) { // stack layout: source_ref target_ref - auto const& structType = dynamic_cast(*m_dataType); + auto const& structType = dynamic_cast(m_dataType); solAssert(structType == _sourceType, "Struct assignment with conversion."); for (auto const& member: structType.getMembers()) { @@ -167,12 +165,12 @@ void StorageItem::storeValue(Type const& _sourceType, SourceLocation const& _loc 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 - StorageItem(m_context, memberType).retrieveValue(_location, true); + StorageItem(m_context, *memberType).retrieveValue(_location, true); // stack: source_ref target_ref member_offset source_value... m_context << eth::dupInstruction(2 + memberType->getSizeOnStack()) << eth::dupInstruction(2 + memberType->getSizeOnStack()) << eth::Instruction::ADD; // stack: source_ref target_ref member_offset source_value... target_member_ref - StorageItem(m_context, memberType).storeValue(*memberType, _location, true); + StorageItem(m_context, *memberType).storeValue(*memberType, _location, true); m_context << eth::Instruction::POP; } if (_move) @@ -187,16 +185,18 @@ void StorageItem::storeValue(Type const& _sourceType, SourceLocation const& _loc } } - -void StorageItem::setToZero(SourceLocation const& _location) const +void StorageItem::setToZero(SourceLocation const&, bool _removeReference) const { - (void)_location; - if (m_dataType->getCategory() == Type::Category::Array) - CompilerUtils(m_context).clearByteArray(dynamic_cast(*m_dataType)); - else if (m_dataType->getCategory() == Type::Category::Struct) + if (m_dataType.getCategory() == Type::Category::Array) + { + if (!_removeReference) + m_context << eth::Instruction::DUP1; + ArrayUtils(m_context).clearArray(dynamic_cast(m_dataType)); + } + else if (m_dataType.getCategory() == Type::Category::Struct) { // stack layout: ref - auto const& structType = dynamic_cast(*m_dataType); + auto const& structType = dynamic_cast(m_dataType); for (auto const& member: structType.getMembers()) { // zero each member that is not a mapping @@ -205,19 +205,61 @@ void StorageItem::setToZero(SourceLocation const& _location) const continue; m_context << structType.getStorageOffsetOfMember(member.first) << eth::Instruction::DUP2 << eth::Instruction::ADD; - StorageItem(m_context, memberType).setToZero(); + StorageItem(m_context, *memberType).setToZero(); } - m_context << eth::Instruction::POP; + if (_removeReference) + m_context << eth::Instruction::POP; } else { - if (m_size == 0) + if (m_size == 0 && _removeReference) m_context << eth::Instruction::POP; - 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; + 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; + } } } + + +StorageArrayLength::StorageArrayLength(CompilerContext& _compilerContext, const ArrayType& _arrayType): + LValue(_compilerContext, *_arrayType.getMemberType("length")), + m_arrayType(_arrayType) +{ + solAssert(m_arrayType.isDynamicallySized(), ""); +} + +void StorageArrayLength::retrieveValue(SourceLocation const& _location, bool _remove) const +{ + if (!_remove) + m_context << eth::Instruction::DUP1; + m_context << eth::Instruction::SLOAD; +} + +void StorageArrayLength::storeValue(Type const& _sourceType, SourceLocation const& _location, bool _move) const +{ + if (_move) + m_context << eth::Instruction::SWAP1; + else + m_context << eth::Instruction::DUP2; + ArrayUtils(m_context).resizeDynamicArray(m_arrayType); +} + +void StorageArrayLength::setToZero(SourceLocation const& _location, bool _removeReference) const +{ + if (!_removeReference) + m_context << eth::Instruction::DUP1; + ArrayUtils(m_context).clearDynamicArray(m_arrayType); +} + diff --git a/libsolidity/LValue.h b/libsolidity/LValue.h index bfa681a48..1ed0ae011 100644 --- a/libsolidity/LValue.h +++ b/libsolidity/LValue.h @@ -24,6 +24,7 @@ #include #include +#include namespace dev { @@ -32,6 +33,7 @@ namespace solidity class Declaration; class Type; +class ArrayType; class CompilerContext; /** @@ -40,7 +42,7 @@ class CompilerContext; class LValue { protected: - LValue(CompilerContext& _compilerContext, std::shared_ptr const& _dataType): + LValue(CompilerContext& _compilerContext, Type const& _dataType): m_context(_compilerContext), m_dataType(_dataType) {} public: @@ -56,13 +58,14 @@ public: /// Stack post: if !_move: value_of(lvalue_ref) virtual void storeValue(Type const& _sourceType, SourceLocation const& _location = SourceLocation(), bool _move = false) const = 0; - /// Stores zero in the lvalue. + /// Stores zero in the lvalue. Removes the reference from the stack if @a _removeReference is true. /// @a _location is the source location of the requested operation - virtual void setToZero(SourceLocation const& _location = SourceLocation()) const = 0; + virtual void setToZero( + SourceLocation const& _location = SourceLocation(), bool _removeReference = true) const = 0; protected: CompilerContext& m_context; - std::shared_ptr m_dataType; + Type const& m_dataType; }; /** @@ -71,13 +74,14 @@ protected: class StackVariable: public LValue { public: - explicit StackVariable(CompilerContext& _compilerContext, Declaration const& _declaration); + StackVariable(CompilerContext& _compilerContext, Declaration const& _declaration); virtual bool storesReferenceOnStack() const { return false; } virtual void retrieveValue(SourceLocation const& _location, bool _remove = false) const override; virtual void storeValue(Type const& _sourceType, SourceLocation const& _location = SourceLocation(), bool _move = false) const override; - virtual void setToZero(SourceLocation const& _location = SourceLocation()) const override; + virtual void setToZero( + SourceLocation const& _location = SourceLocation(), bool _removeReference = true) const override; private: /// Base stack offset (@see CompilerContext::getBaseStackOffsetOfVariable) of the local variable. @@ -93,14 +97,15 @@ class StorageItem: public LValue { public: /// Constructs the LValue and pushes the location of @a _declaration onto the stack. - explicit StorageItem(CompilerContext& _compilerContext, Declaration const& _declaration); + StorageItem(CompilerContext& _compilerContext, Declaration const& _declaration); /// Constructs the LValue and assumes that the storage reference is already on the stack. - explicit StorageItem(CompilerContext& _compilerContext, std::shared_ptr const& _type); + StorageItem(CompilerContext& _compilerContext, Type const& _type); virtual bool storesReferenceOnStack() const { return true; } virtual void retrieveValue(SourceLocation const& _location, bool _remove = false) const override; virtual void storeValue(Type const& _sourceType, SourceLocation const& _location = SourceLocation(), bool _move = false) const override; - virtual void setToZero(SourceLocation const& _location = SourceLocation()) const override; + virtual void setToZero( + SourceLocation const& _location = SourceLocation(), bool _removeReference = true) const override; private: /// Number of stack elements occupied by the value (not the reference). @@ -108,5 +113,26 @@ private: unsigned m_size; }; +/** + * Reference to the "length" member of a dynamically-sized array. This is an LValue with special + * semantics since assignments to it might reduce its length and thus arrays members have to be + * deleted. + */ +class StorageArrayLength: public LValue +{ +public: + /// Constructs the LValue, assumes that the reference to the array head is already on the stack. + StorageArrayLength(CompilerContext& _compilerContext, ArrayType const& _arrayType); + virtual bool storesReferenceOnStack() const { return true; } + virtual void retrieveValue(SourceLocation const& _location, bool _remove = false) const override; + virtual void storeValue(Type const& _sourceType, + SourceLocation const& _location = SourceLocation(), bool _move = false) const override; + virtual void setToZero( + SourceLocation const& _location = SourceLocation(), bool _removeReference = true) const override; + +private: + ArrayType const& m_arrayType; +}; + } } diff --git a/test/SolidityEndToEndTest.cpp b/test/SolidityEndToEndTest.cpp index f52b52d1c..5b99f72e0 100644 --- a/test/SolidityEndToEndTest.cpp +++ b/test/SolidityEndToEndTest.cpp @@ -2768,6 +2768,96 @@ BOOST_AUTO_TEST_CASE(dynamic_out_of_bounds_array_access) BOOST_CHECK(callContractFunction("length()") == encodeArgs(4)); } +BOOST_AUTO_TEST_CASE(fixed_array_cleanup) +{ + char const* sourceCode = R"( + contract c { + uint spacer1; + uint spacer2; + uint[20] data; + function fill() { + for (uint i = 0; i < data.length; ++i) data[i] = i+1; + } + function clear() { delete data; } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(m_state.storage(m_contractAddress).empty()); + BOOST_CHECK(callContractFunction("fill()") == bytes()); + BOOST_CHECK(!m_state.storage(m_contractAddress).empty()); + BOOST_CHECK(callContractFunction("clear()") == bytes()); + BOOST_CHECK(m_state.storage(m_contractAddress).empty()); +} + +BOOST_AUTO_TEST_CASE(short_fixed_array_cleanup) +{ + char const* sourceCode = R"( + contract c { + uint spacer1; + uint spacer2; + uint[3] data; + function fill() { + for (uint i = 0; i < data.length; ++i) data[i] = i+1; + } + function clear() { delete data; } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(m_state.storage(m_contractAddress).empty()); + BOOST_CHECK(callContractFunction("fill()") == bytes()); + BOOST_CHECK(!m_state.storage(m_contractAddress).empty()); + BOOST_CHECK(callContractFunction("clear()") == bytes()); + BOOST_CHECK(m_state.storage(m_contractAddress).empty()); +} + +BOOST_AUTO_TEST_CASE(dynamic_array_cleanup) +{ + char const* sourceCode = R"( + contract c { + uint[20] spacer; + uint[] dynamic; + function fill() { + dynamic.length = 21; + for (uint i = 0; i < dynamic.length; ++i) dynamic[i] = i+1; + } + function halfClear() { dynamic.length = 5; } + function fullClear() { delete dynamic; } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(m_state.storage(m_contractAddress).empty()); + BOOST_CHECK(callContractFunction("fill()") == bytes()); + BOOST_CHECK(!m_state.storage(m_contractAddress).empty()); + BOOST_CHECK(callContractFunction("halfClear()") == bytes()); + BOOST_CHECK(!m_state.storage(m_contractAddress).empty()); + BOOST_CHECK(callContractFunction("fullClear()") == bytes()); + BOOST_CHECK(m_state.storage(m_contractAddress).empty()); +} + +BOOST_AUTO_TEST_CASE(dynamic_multi_array_cleanup) +{ + char const* sourceCode = R"( + contract c { + struct s { uint[][] d; } + s[] data; + function fill() returns (uint) { + data.length = 3; + data[2].d.length = 4; + data[2].d[3].length = 5; + data[2].d[3][4] = 8; + return data[2].d[3][4]; + } + function clear() { delete data; } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(m_state.storage(m_contractAddress).empty()); + BOOST_CHECK(callContractFunction("fill()") == encodeArgs(8)); + BOOST_CHECK(!m_state.storage(m_contractAddress).empty()); + BOOST_CHECK(callContractFunction("clear()") == bytes()); + BOOST_CHECK(m_state.storage(m_contractAddress).empty()); +} + BOOST_AUTO_TEST_SUITE_END() } From 9252c02a63daeaf8042b09a30ee395a2469a560a Mon Sep 17 00:00:00 2001 From: Christian Date: Fri, 27 Feb 2015 14:50:06 +0100 Subject: [PATCH 03/16] Type checks for array assignment. --- libsolidity/Types.cpp | 19 ++++++++- test/SolidityNameAndTypeResolution.cpp | 55 ++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index adcd2e542..8cc1f2589 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -537,7 +537,19 @@ TypePointer ContractType::unaryOperatorResult(Token::Value _operator) const bool ArrayType::isImplicitlyConvertibleTo(const Type& _convertTo) const { - return _convertTo.getCategory() == getCategory(); + if (_convertTo.getCategory() != getCategory()) + return false; + auto& convertTo = dynamic_cast(_convertTo); + // let us not allow assignment to memory arrays for now + if (convertTo.getLocation() != Location::Storage) + return false; + if (convertTo.isByteArray() != isByteArray()) + return false; + if (!getBaseType()->isImplicitlyConvertibleTo(*convertTo.getBaseType())) + return false; + if (convertTo.isDynamicallySized()) + return true; + return !isDynamicallySized() && convertTo.getLength() >= getLength(); } TypePointer ArrayType::unaryOperatorResult(Token::Value _operator) const @@ -552,7 +564,10 @@ bool ArrayType::operator==(Type const& _other) const if (_other.getCategory() != getCategory()) return false; ArrayType const& other = dynamic_cast(_other); - return other.m_location == m_location; + if (other.m_location != m_location || other.isByteArray() != isByteArray() || + other.isDynamicallySized() != isDynamicallySized()) + return false; + return isDynamicallySized() || getLength() == other.getLength(); } u256 ArrayType::getStorageSize() const diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index 4809aac1e..ff9fa1655 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -1185,6 +1185,61 @@ BOOST_AUTO_TEST_CASE(array_with_nonconstant_length) BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); } +BOOST_AUTO_TEST_CASE(array_copy_with_different_types1) +{ + char const* text = R"( + contract c { + bytes a; + uint[] b; + function f() { b = a; } + })"; + BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); +} + +BOOST_AUTO_TEST_CASE(array_copy_with_different_types2) +{ + char const* text = R"( + contract c { + uint32[] a; + uint8[] b; + function f() { b = a; } + })"; + BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); +} + +BOOST_AUTO_TEST_CASE(array_copy_with_different_types_conversion_possible) +{ + char const* text = R"( + contract c { + uint32[] a; + uint8[] b; + function f() { a = b; } + })"; + BOOST_CHECK_NO_THROW(parseTextAndResolveNames(text)); +} + +BOOST_AUTO_TEST_CASE(array_copy_with_different_types_static_dynamic) +{ + char const* text = R"( + contract c { + uint32[] a; + uint8[80] b; + function f() { a = b; } + })"; + BOOST_CHECK_NO_THROW(parseTextAndResolveNames(text)); +} + +BOOST_AUTO_TEST_CASE(array_copy_with_different_types_dynamic_static) +{ + char const* text = R"( + contract c { + uint[] a; + uint[80] b; + function f() { b = a; } + })"; + BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); +} + BOOST_AUTO_TEST_SUITE_END() } From 96a50b3a23cb4cd85c88fb4ad41e33ff8c6dbba6 Mon Sep 17 00:00:00 2001 From: Christian Date: Fri, 27 Feb 2015 22:52:02 +0100 Subject: [PATCH 04/16] Array copy storage to storage. --- libsolidity/ArrayUtils.cpp | 72 +++++++++++++++++----------- libsolidity/ArrayUtils.h | 8 +++- test/SolidityEndToEndTest.cpp | 88 +++++++++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 27 deletions(-) diff --git a/libsolidity/ArrayUtils.cpp b/libsolidity/ArrayUtils.cpp index 07949c79e..43cc6fd49 100644 --- a/libsolidity/ArrayUtils.cpp +++ b/libsolidity/ArrayUtils.cpp @@ -37,13 +37,17 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons // stack layout: [source_ref] target_ref (top) // need to leave target_ref on the stack at the end solAssert(_targetType.getLocation() == ArrayType::Location::Storage, ""); - solAssert(_targetType.isByteArray(), "Non byte arrays not yet implemented here."); - solAssert(_sourceType.isByteArray(), "Non byte arrays not yet implemented here."); + + IntegerType uint256(256); + Type const* targetBaseType = _targetType.isByteArray() ? &uint256 : &(*_targetType.getBaseType()); + Type const* sourceBaseType = _sourceType.isByteArray() ? &uint256 : &(*_sourceType.getBaseType()); switch (_sourceType.getLocation()) { case ArrayType::Location::CallData: { + solAssert(_targetType.isByteArray(), "Non byte arrays not yet implemented here."); + solAssert(_sourceType.isByteArray(), "Non byte arrays not yet implemented here."); // This also assumes that after "length" we only have zeros, i.e. it cannot be used to // slice a byte array from calldata. @@ -97,30 +101,37 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons { // this copies source to target and also clears target if it was larger + solAssert(sourceBaseType->getStorageSize() == targetBaseType->getStorageSize(), + "Copying with different storage sizes not yet implemented."); // stack: source_ref target_ref // store target_ref m_context << eth::Instruction::SWAP1 << eth::Instruction::DUP2; + // stack: target_ref source_ref target_ref // fetch lengthes - m_context << eth::Instruction::DUP1 << eth::Instruction::SLOAD << eth::Instruction::SWAP2 - << eth::Instruction::DUP1 << eth::Instruction::SLOAD; - // stack: target_ref target_len_bytes target_ref source_ref source_len_bytes - // store new target length - m_context << eth::Instruction::DUP1 << eth::Instruction::DUP4 << eth::Instruction::SSTORE; + retrieveLength(_targetType); + m_context << eth::Instruction::SWAP2; + // stack: target_ref target_len target_ref source_ref + retrieveLength(_sourceType); + // stack: target_ref target_len target_ref source_ref source_len + if (_targetType.isDynamicallySized()) + // store new target length + m_context << eth::Instruction::DUP1 << eth::Instruction::DUP4 << eth::Instruction::SSTORE; // compute hashes (data positions) m_context << eth::Instruction::SWAP2; - CompilerUtils(m_context).computeHashStatic(); + if (_targetType.isDynamicallySized()) + CompilerUtils(m_context).computeHashStatic(); m_context << eth::Instruction::SWAP1; - CompilerUtils(m_context).computeHashStatic(); - // stack: target_ref target_len_bytes source_len_bytes target_data_pos source_data_pos - // convert lengthes from bytes to storage slots - m_context << u256(31) << u256(32) << eth::Instruction::DUP1 << eth::Instruction::DUP3 - << eth::Instruction::DUP8 << eth::Instruction::ADD << eth::Instruction::DIV - << eth::Instruction::SWAP2 - << eth::Instruction::DUP6 << eth::Instruction::ADD << eth::Instruction::DIV; - // stack: target_ref target_len_bytes source_len_bytes target_data_pos source_data_pos target_len source_len + if (_sourceType.isDynamicallySized()) + CompilerUtils(m_context).computeHashStatic(); + // stack: target_ref target_len source_len target_data_pos source_data_pos + m_context << eth::Instruction::DUP4; + convertLengthToSize(_sourceType); + m_context << eth::Instruction::DUP4; + convertLengthToSize(_sourceType); + // stack: target_ref target_len source_len target_data_pos source_data_pos target_size source_size // @todo we might be able to go without a third counter m_context << u256(0); - // stack: target_ref target_len_bytes source_len_bytes target_data_pos source_data_pos target_len source_len counter + // stack: target_ref target_len source_len target_data_pos source_data_pos target_size source_size counter eth::AssemblyItem copyLoopStart = m_context.newTag(); m_context << copyLoopStart; // check for loop condition @@ -129,27 +140,28 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons eth::AssemblyItem copyLoopEnd = m_context.newTag(); m_context.appendConditionalJumpTo(copyLoopEnd); // copy - m_context << eth::Instruction::DUP4 << eth::Instruction::DUP2 << eth::Instruction::ADD - << eth::Instruction::SLOAD - << eth::Instruction::DUP6 << eth::Instruction::DUP3 << eth::Instruction::ADD - << eth::Instruction::SSTORE; + m_context << eth::Instruction::DUP4 << eth::Instruction::DUP2 << eth::Instruction::ADD; + StorageItem(m_context, *sourceBaseType).retrieveValue(SourceLocation(), true); + m_context << eth::dupInstruction(5 + sourceBaseType->getSizeOnStack()) + << eth::dupInstruction(2 + sourceBaseType->getSizeOnStack()) << eth::Instruction::ADD; + StorageItem(m_context, *targetBaseType).storeValue(*sourceBaseType, SourceLocation(), true); // increment - m_context << u256(1) << eth::Instruction::ADD; + m_context << targetBaseType->getStorageSize() << eth::Instruction::ADD; m_context.appendJumpTo(copyLoopStart); m_context << copyLoopEnd; // zero-out leftovers in target - // stack: target_ref target_len_bytes source_len_bytes target_data_pos source_data_pos target_len source_len counter + // stack: target_ref target_len source_len target_data_pos source_data_pos target_size source_size counter // add counter to target_data_pos m_context << eth::Instruction::DUP5 << eth::Instruction::ADD << eth::Instruction::SWAP5 << eth::Instruction::POP; - // stack: target_ref target_len_bytes target_data_pos_updated target_data_pos source_data_pos target_len source_len - // add length to target_data_pos to get target_data_end + // stack: target_ref target_len target_data_pos_updated target_data_pos source_data_pos target_size source_size + // add size to target_data_pos to get target_data_end m_context << eth::Instruction::POP << eth::Instruction::DUP3 << eth::Instruction::ADD << eth::Instruction::SWAP4 << eth::Instruction::POP << eth::Instruction::POP << eth::Instruction::POP; // stack: target_ref target_data_end target_data_pos_updated - clearStorageLoop(IntegerType(256)); + clearStorageLoop(*targetBaseType); m_context << eth::Instruction::POP; break; } @@ -282,3 +294,11 @@ void ArrayUtils::convertLengthToSize(ArrayType const& _arrayType) const m_context << _arrayType.getBaseType()->getStorageSize() << eth::Instruction::MUL; } +void ArrayUtils::retrieveLength(ArrayType const& _arrayType) const +{ + if (_arrayType.isDynamicallySized()) + m_context << eth::Instruction::DUP1 << eth::Instruction::SLOAD; + else + m_context << _arrayType.getLength(); +} + diff --git a/libsolidity/ArrayUtils.h b/libsolidity/ArrayUtils.h index f080dce66..73e88340e 100644 --- a/libsolidity/ArrayUtils.h +++ b/libsolidity/ArrayUtils.h @@ -39,7 +39,8 @@ class ArrayUtils public: ArrayUtils(CompilerContext& _context): m_context(_context) {} - /// Copies an array to an array in storage. + /// 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 void copyArrayToStorage(ArrayType const& _targetType, ArrayType const& _sourceType) const; @@ -63,6 +64,11 @@ public: /// Stack pre: length /// Stack post: size void convertLengthToSize(ArrayType const& _arrayType) 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 post: reference length + void retrieveLength(ArrayType const& _arrayType) const; private: CompilerContext& m_context; diff --git a/test/SolidityEndToEndTest.cpp b/test/SolidityEndToEndTest.cpp index 5b99f72e0..f305e81e3 100644 --- a/test/SolidityEndToEndTest.cpp +++ b/test/SolidityEndToEndTest.cpp @@ -2858,6 +2858,94 @@ BOOST_AUTO_TEST_CASE(dynamic_multi_array_cleanup) BOOST_CHECK(m_state.storage(m_contractAddress).empty()); } +BOOST_AUTO_TEST_CASE(array_copy_storage_storage_dyn_dyn) +{ + char const* sourceCode = R"( + contract c { + uint[] data1; + uint[] data2; + function setData1(uint length, uint index, uint value) { + data1.length = length; if (index < length) data1[index] = value; + } + function copyStorageStorage() { data2 = data1; } + function getData2(uint index) returns (uint len, uint val) { + len = data2.length; if (index < len) val = data2[index]; + } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("setData1(uint256,uint256,uint256)", 10, 5, 4) == bytes()); + BOOST_CHECK(callContractFunction("copyStorageStorage()") == bytes()); + BOOST_CHECK(callContractFunction("getData2(uint256)", 5) == encodeArgs(10, 4)); + BOOST_CHECK(callContractFunction("setData1(uint256,uint256,uint256)", 0, 0, 0) == bytes()); + BOOST_CHECK(callContractFunction("copyStorageStorage()") == bytes()); + BOOST_CHECK(callContractFunction("getData2(uint256)", 0) == encodeArgs(0, 0)); + BOOST_CHECK(m_state.storage(m_contractAddress).empty()); +} + +BOOST_AUTO_TEST_CASE(array_copy_storage_storage_static_static) +{ + char const* sourceCode = R"( + contract c { + uint[40] data1; + uint[20] data2; + function test() returns (uint x, uint y){ + data1[30] = 4; + data1[2] = 7; + data1[3] = 9; + data2[3] = 8; + data1 = data2; + x = data1[3]; + y = data1[30]; // should be cleared + } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("test()") == encodeArgs(8, 0)); +} + +BOOST_AUTO_TEST_CASE(array_copy_storage_storage_static_dynamic) +{ + char const* sourceCode = R"( + contract c { + uint[9] data1; + uint[] data2; + function test() returns (uint x, uint y){ + data1[8] = 4; + data2 = data1; + x = data2.length; + y = data2[8]; + } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("test()") == encodeArgs(9, 4)); +} + +BOOST_AUTO_TEST_CASE(array_copy_storage_storage_struct) +{ + char const* sourceCode = R"( + contract c { + struct Data { uint x; uint y; } + Data[] data1; + Data[] data2; + function test() returns (uint x, uint y) { + data1.length = 9; + data1[8].x = 4; + data1[8].y = 5; + data2 = data1; + x = data2[8].x; + y = data2[8].y; + data1.length = 0; + data2 = data1; + } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("test()") == encodeArgs(4, 5)); + BOOST_CHECK(m_state.storage(m_contractAddress).empty()); +} + BOOST_AUTO_TEST_SUITE_END() } From 6045f1d23ad16946fa77243fc3d535257f87a507 Mon Sep 17 00:00:00 2001 From: Lu Guanqun Date: Sun, 1 Mar 2015 20:10:19 -0600 Subject: [PATCH 05/16] let the error output take tab into consideration --- libsolidity/SourceReferenceFormatter.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/libsolidity/SourceReferenceFormatter.cpp b/libsolidity/SourceReferenceFormatter.cpp index 489a676ed..b5e83b8c9 100644 --- a/libsolidity/SourceReferenceFormatter.cpp +++ b/libsolidity/SourceReferenceFormatter.cpp @@ -44,8 +44,14 @@ void SourceReferenceFormatter::printSourceLocation(ostream& _stream, tie(endLine, endColumn) = _scanner.translatePositionToLineColumn(_location.end); if (startLine == endLine) { - _stream << _scanner.getLineAtPosition(_location.start) << endl - << string(startColumn, ' ') << "^"; + string line = _scanner.getLineAtPosition(_location.start); + _stream << line << endl; + std::for_each(line.cbegin(), line.cbegin() + startColumn, + [&_stream](char const& ch) + { + _stream << (ch == '\t' ? '\t' : ' '); + }); + _stream << "^"; if (endColumn > startColumn + 2) _stream << string(endColumn - startColumn - 2, '-'); if (endColumn > startColumn + 1) From d7b4d98678957832235971ebca3e3ea9b2dfc207 Mon Sep 17 00:00:00 2001 From: yann300 Date: Mon, 2 Mar 2015 11:26:31 +0100 Subject: [PATCH 06/16] - Replace "Display Log" by an icon. --- mix/qml/StatusPane.qml | 47 ++++++++++++++++---------------- mix/qml/img/search_filled.png | Bin 0 -> 625 bytes mix/qml/js/TransactionHelper.js | 5 ++-- mix/res.qrc | 1 + 4 files changed, 28 insertions(+), 25 deletions(-) create mode 100644 mix/qml/img/search_filled.png diff --git a/mix/qml/StatusPane.qml b/mix/qml/StatusPane.qml index ff2173a22..414a051ec 100644 --- a/mix/qml/StatusPane.qml +++ b/mix/qml/StatusPane.qml @@ -75,7 +75,6 @@ Rectangle { height: 30 color: "#fcfbfc" - Text { anchors.verticalCenter: parent.verticalCenter anchors.horizontalCenter: parent.horizontalCenter @@ -116,27 +115,6 @@ Rectangle { } } - Button - { - anchors.verticalCenter: parent.verticalCenter - anchors.left: parent.right - anchors.leftMargin: 10 - width: 38 - height: 28 - visible: false - text: qsTr("Log") - objectName: "status" - id: logslink - action: displayLogAction - } - - Action { - id: displayLogAction - onTriggered: { - mainContent.displayCompilationErrorIfAny(); - } - } - Button { anchors.fill: parent @@ -157,6 +135,25 @@ Rectangle { } } + Button + { + id: logslink + anchors.left: statusContainer.right + anchors.leftMargin: 9 + visible: false + anchors.verticalCenter: parent.verticalCenter + action: displayLogAction + iconSource: "qrc:/qml/img/search_filled.png" + } + + Action { + id: displayLogAction + tooltip: qsTr("Display Log") + onTriggered: { + mainContent.displayCompilationErrorIfAny(); + } + } + Rectangle { color: "transparent" @@ -167,9 +164,13 @@ Rectangle { RowLayout { anchors.fill: parent - Rectangle { + anchors.top: statusHeader.top + anchors.right: statusHeader.right + Rectangle + { color: "transparent" anchors.fill: parent + Button { anchors.right: parent.right diff --git a/mix/qml/img/search_filled.png b/mix/qml/img/search_filled.png new file mode 100644 index 0000000000000000000000000000000000000000..c459e7cbddfec54248dcfeb1fc5c54289c95798f GIT binary patch literal 625 zcmV-%0*?KOP)?s#`6I|lk$QPz!rw5hS)Wq} zU3X_nY?;5omtItms6H;!XFKYalA`W(*5u56D6TwBMF+atEjATpJpwQWaI2&^a`m|) zEwd3X_43f!XKlljE8ETwsnw>=+kKqx(`M~rtIUC2CB;`ys&)(-$3vAgPY?M00000 LNkvXXu0mjfqsqml/WebPreviewStyle.qml qml/img/available_updates.png qml/DeploymentDialog.qml + qml/img/search_filled.png From 6d48abddd36fa28aa47e46f46bc725efd4077f2a Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Wed, 25 Feb 2015 16:32:04 +0100 Subject: [PATCH 07/16] Adding test for base class statevar accessors --- test/SolidityNameAndTypeResolution.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index 4809aac1e..e4764c5ef 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -720,6 +720,18 @@ BOOST_AUTO_TEST_CASE(private_state_variable) BOOST_CHECK_MESSAGE(function == nullptr, "Accessor function of an internal variable should not exist"); } +BOOST_AUTO_TEST_CASE(base_class_state_variable_accessor) +{ + // test for issue #1126 https://github.com/ethereum/cpp-ethereum/issues/1126 + char const* text = "contract Parent {\n" + " uint256 public m_aMember;\n" + "}\n" + "contract Child {\n" + " function foo() returns (uint256) { return Parent.m_aMember(); }\n" + "}\n"; + BOOST_CHECK_NO_THROW(parseTextAndResolveNamesWithChecks(text)); +} + BOOST_AUTO_TEST_CASE(fallback_function) { char const* text = R"( From 345e84baeaccaa810f163fe7e62c33e8d0fe749c Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Wed, 25 Feb 2015 20:35:55 +0100 Subject: [PATCH 08/16] Adding inheritable members to a contract --- libsolidity/AST.cpp | 27 +++++++++++++++++++++++++++ libsolidity/AST.h | 4 ++++ libsolidity/Types.cpp | 5 ++--- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index c37e8c374..b61eb011e 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -209,6 +209,33 @@ vector, FunctionTypePointer>> const& ContractDefinition::getIn return *m_interfaceFunctionList; } +vector> const& ContractDefinition::getInheritableMembers() const +{ + if (!m_inheritableMembers) + { + set memberSeen; + m_inheritableMembers.reset(new vector>()); + for (ContractDefinition const* contract: getLinearizedBaseContracts()) + { + for (ASTPointer const& f: contract->getDefinedFunctions()) + if (f->isPublic() && !f->isConstructor() && !f->getName().empty() + && memberSeen.count(f->getName()) == 0 && f->isVisibleInDerivedContracts()) + { + memberSeen.insert(f->getName()); + m_inheritableMembers->push_back(f); + } + + for (ASTPointer const& v: contract->getStateVariables()) + if (v->isPublic() && memberSeen.count(v->getName()) == 0) + { + memberSeen.insert(v->getName()); + m_inheritableMembers->push_back(v); + } + } + } + return *m_inheritableMembers; +} + TypePointer EnumValue::getType(ContractDefinition const*) const { EnumDefinition const* parentDef = dynamic_cast(getScope()); diff --git a/libsolidity/AST.h b/libsolidity/AST.h index dea0fba63..dd40e0239 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -247,6 +247,9 @@ public: /// as intended for use by the ABI. std::map, FunctionTypePointer> getInterfaceFunctions() const; + /// @returns a list of the inheritable members of this contract + std::vector> const& getInheritableMembers() const; + /// List of all (direct and indirect) base contracts in order from derived to base, including /// the contract itself. Available after name resolution std::vector const& getLinearizedBaseContracts() const { return m_linearizedBaseContracts; } @@ -273,6 +276,7 @@ private: std::vector m_linearizedBaseContracts; mutable std::unique_ptr, FunctionTypePointer>>> m_interfaceFunctionList; mutable std::unique_ptr>> m_interfaceEvents; + mutable std::unique_ptr>> m_inheritableMembers; }; class InheritanceSpecifier: public ASTNode diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index adcd2e542..1347c9ea8 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -1025,9 +1025,8 @@ MemberList const& TypeType::getMembers() const if (find(currentBases.begin(), currentBases.end(), &contract) != currentBases.end()) // We are accessing the type of a base contract, so add all public and protected // functions. Note that this does not add inherited functions on purpose. - for (ASTPointer const& f: contract.getDefinedFunctions()) - if (!f->isConstructor() && !f->getName().empty() && f->isVisibleInDerivedContracts()) - members.push_back(make_pair(f->getName(), make_shared(*f))); + for (ASTPointer const& decl: contract.getInheritableMembers()) + members.push_back(make_pair(decl->getName(), decl->getType())); } else if (m_actualType->getCategory() == Category::Enum) { From f2fdeb3599d3f885d046bc94f4ebb74c4d1258bf Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Thu, 26 Feb 2015 12:11:54 +0100 Subject: [PATCH 09/16] Add structs to inheritable members --- libsolidity/AST.cpp | 7 +++++++ libsolidity/Types.cpp | 2 +- test/SolidityNameAndTypeResolution.cpp | 4 ++-- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index b61eb011e..8c1944f96 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -231,6 +231,13 @@ vector> const& ContractDefinition::getInheritableMembers memberSeen.insert(v->getName()); m_inheritableMembers->push_back(v); } + + for (ASTPointer const& s: contract->getDefinedStructs()) + if (s->isPublic() && memberSeen.count(s->getName()) == 0) + { + memberSeen.insert(s->getName()); + m_inheritableMembers->push_back(s); + } } } return *m_inheritableMembers; diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 1347c9ea8..73367c85d 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -1024,7 +1024,7 @@ MemberList const& TypeType::getMembers() const vector currentBases = m_currentContract->getLinearizedBaseContracts(); if (find(currentBases.begin(), currentBases.end(), &contract) != currentBases.end()) // We are accessing the type of a base contract, so add all public and protected - // functions. Note that this does not add inherited functions on purpose. + // members. Note that this does not add inherited functions on purpose. for (ASTPointer const& decl: contract.getInheritableMembers()) members.push_back(make_pair(decl->getName(), decl->getType())); } diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index e4764c5ef..9db45b911 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -726,8 +726,8 @@ BOOST_AUTO_TEST_CASE(base_class_state_variable_accessor) char const* text = "contract Parent {\n" " uint256 public m_aMember;\n" "}\n" - "contract Child {\n" - " function foo() returns (uint256) { return Parent.m_aMember(); }\n" + "contract Child is Parent{\n" + " function foo() returns (uint256) { return Parent.m_aMember; }\n" "}\n"; BOOST_CHECK_NO_THROW(parseTextAndResolveNamesWithChecks(text)); } From b1dcc2a77ff7268c8a6a0b4f1eeadb509c6a499e Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Fri, 27 Feb 2015 10:08:14 +0100 Subject: [PATCH 10/16] VisibleInDerivedContracts() is now virtual() - Plus an extra test for internal visibility in a base class variable --- libsolidity/AST.cpp | 7 +++---- libsolidity/AST.h | 7 ++++++- libsolidity/Types.cpp | 3 +-- test/SolidityNameAndTypeResolution.cpp | 13 ++++++++++++- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 8c1944f96..d18a296d5 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -218,22 +218,21 @@ vector> const& ContractDefinition::getInheritableMembers for (ContractDefinition const* contract: getLinearizedBaseContracts()) { for (ASTPointer const& f: contract->getDefinedFunctions()) - if (f->isPublic() && !f->isConstructor() && !f->getName().empty() - && memberSeen.count(f->getName()) == 0 && f->isVisibleInDerivedContracts()) + if (memberSeen.count(f->getName()) == 0 && f->isVisibleInDerivedContracts()) { memberSeen.insert(f->getName()); m_inheritableMembers->push_back(f); } for (ASTPointer const& v: contract->getStateVariables()) - if (v->isPublic() && memberSeen.count(v->getName()) == 0) + if (memberSeen.count(v->getName()) == 0 && v->isVisibleInDerivedContracts()) { memberSeen.insert(v->getName()); m_inheritableMembers->push_back(v); } for (ASTPointer const& s: contract->getDefinedStructs()) - if (s->isPublic() && memberSeen.count(s->getName()) == 0) + if (memberSeen.count(s->getName()) == 0 && s->isVisibleInDerivedContracts()) { memberSeen.insert(s->getName()); m_inheritableMembers->push_back(s); diff --git a/libsolidity/AST.h b/libsolidity/AST.h index dd40e0239..97e85927b 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -144,7 +144,7 @@ public: Visibility getVisibility() const { return m_visibility == Visibility::Default ? getDefaultVisibility() : m_visibility; } bool isPublic() const { return getVisibility() >= Visibility::Public; } bool isVisibleInContract() const { return getVisibility() != Visibility::External; } - bool isVisibleInDerivedContracts() const { return isVisibleInContract() && getVisibility() >= Visibility::Internal; } + virtual bool isVisibleInDerivedContracts() const { return isVisibleInContract() && getVisibility() >= Visibility::Internal; } /// @returns the scope this declaration resides in. Can be nullptr if it is the global scope. /// Available only after name and type resolution step. @@ -409,6 +409,11 @@ public: ASTPointer const& getReturnParameterList() const { return m_returnParameters; } Block const& getBody() const { return *m_body; } + virtual bool isVisibleInDerivedContracts() const override + { + return !isConstructor() && !getName().empty() && isVisibleInContract() && + getVisibility() >= Visibility::Internal; + } virtual TypePointer getType(ContractDefinition const*) const override; /// Checks that all parameters have allowed types and calls checkTypeRequirements on the body. diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 73367c85d..84e029269 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -628,8 +628,7 @@ MemberList const& ContractType::getMembers() const { for (ContractDefinition const* base: m_contract.getLinearizedBaseContracts()) for (ASTPointer const& function: base->getDefinedFunctions()) - if (!function->isConstructor() && !function->getName().empty()&& - function->isVisibleInDerivedContracts()) + if (function->isVisibleInDerivedContracts()) members.push_back(make_pair(function->getName(), make_shared(*function, true))); } else diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index 9db45b911..58cebaebb 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -436,7 +436,7 @@ BOOST_AUTO_TEST_CASE(inheritance_diamond_basic) function g() { f(); rootFunction(); } } )"; - BOOST_CHECK_NO_THROW(parseTextAndResolveNames(text)); + BOOST_CHECK_NO_THROW(parseTextAndResolveNamesWithChecks(text)); } BOOST_AUTO_TEST_CASE(cyclic_inheritance) @@ -732,6 +732,17 @@ BOOST_AUTO_TEST_CASE(base_class_state_variable_accessor) BOOST_CHECK_NO_THROW(parseTextAndResolveNamesWithChecks(text)); } +BOOST_AUTO_TEST_CASE(base_class_state_variable_internal_member) +{ + char const* text = "contract Parent {\n" + " uint256 internal m_aMember;\n" + "}\n" + "contract Child is Parent{\n" + " function foo() returns (uint256) { return Parent.m_aMember; }\n" + "}\n"; + BOOST_CHECK_NO_THROW(parseTextAndResolveNamesWithChecks(text)); +} + BOOST_AUTO_TEST_CASE(fallback_function) { char const* text = R"( From 58005fd2034c0b63cb170f7f9a5140dff1825027 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Fri, 27 Feb 2015 11:35:25 +0100 Subject: [PATCH 11/16] Use lambda to avoid code duplication in inheritableMembers --- libsolidity/AST.cpp | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index d18a296d5..24a94a798 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -217,26 +217,23 @@ vector> const& ContractDefinition::getInheritableMembers m_inheritableMembers.reset(new vector>()); for (ContractDefinition const* contract: getLinearizedBaseContracts()) { - for (ASTPointer const& f: contract->getDefinedFunctions()) - if (memberSeen.count(f->getName()) == 0 && f->isVisibleInDerivedContracts()) + auto addInheritableMember = [&](ASTPointer const& _decl) + { + if (memberSeen.count(_decl->getName()) == 0 && _decl->isVisibleInDerivedContracts()) { - memberSeen.insert(f->getName()); - m_inheritableMembers->push_back(f); + memberSeen.insert(_decl->getName()); + m_inheritableMembers->push_back(_decl); } + }; + + for (ASTPointer const& f: contract->getDefinedFunctions()) + addInheritableMember(f); for (ASTPointer const& v: contract->getStateVariables()) - if (memberSeen.count(v->getName()) == 0 && v->isVisibleInDerivedContracts()) - { - memberSeen.insert(v->getName()); - m_inheritableMembers->push_back(v); - } + addInheritableMember(v); for (ASTPointer const& s: contract->getDefinedStructs()) - if (memberSeen.count(s->getName()) == 0 && s->isVisibleInDerivedContracts()) - { - memberSeen.insert(s->getName()); - m_inheritableMembers->push_back(s); - } + addInheritableMember(s); } } return *m_inheritableMembers; From 1681fbd8858f8e8b67c7a1413b2d85d1cccb4779 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Sat, 28 Feb 2015 18:30:33 +0100 Subject: [PATCH 12/16] getInheritableMembers() does not look at BaseContracts - Also adding tests for improper accessing members of other contracts. --- libsolidity/AST.cpp | 27 +++++++++++------------- test/SolidityNameAndTypeResolution.cpp | 29 ++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 24a94a798..e5aef4cbb 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -215,26 +215,23 @@ vector> const& ContractDefinition::getInheritableMembers { set memberSeen; m_inheritableMembers.reset(new vector>()); - for (ContractDefinition const* contract: getLinearizedBaseContracts()) + auto addInheritableMember = [&](ASTPointer const& _decl) { - auto addInheritableMember = [&](ASTPointer const& _decl) + if (memberSeen.count(_decl->getName()) == 0 && _decl->isVisibleInDerivedContracts()) { - if (memberSeen.count(_decl->getName()) == 0 && _decl->isVisibleInDerivedContracts()) - { - memberSeen.insert(_decl->getName()); - m_inheritableMembers->push_back(_decl); - } - }; + memberSeen.insert(_decl->getName()); + m_inheritableMembers->push_back(_decl); + } + }; - for (ASTPointer const& f: contract->getDefinedFunctions()) - addInheritableMember(f); + for (ASTPointer const& f: getDefinedFunctions()) + addInheritableMember(f); - for (ASTPointer const& v: contract->getStateVariables()) - addInheritableMember(v); + for (ASTPointer const& v: getStateVariables()) + addInheritableMember(v); - for (ASTPointer const& s: contract->getDefinedStructs()) - addInheritableMember(s); - } + for (ASTPointer const& s: getDefinedStructs()) + addInheritableMember(s); } return *m_inheritableMembers; } diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index 58cebaebb..81d58d0dd 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -743,6 +743,35 @@ BOOST_AUTO_TEST_CASE(base_class_state_variable_internal_member) BOOST_CHECK_NO_THROW(parseTextAndResolveNamesWithChecks(text)); } +BOOST_AUTO_TEST_CASE(state_variable_member_of_wrong_class1) +{ + char const* text = "contract Parent1 {\n" + " uint256 internal m_aMember1;\n" + "}\n" + "contract Parent2 is Parent1{\n" + " uint256 internal m_aMember2;\n" + "}\n" + "contract Child is Parent2{\n" + " function foo() returns (uint256) { return Parent2.m_aMember1; }\n" + "}\n"; + BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); +} + +BOOST_AUTO_TEST_CASE(state_variable_member_of_wrong_class2) +{ + char const* text = "contract Parent1 {\n" + " uint256 internal m_aMember1;\n" + "}\n" + "contract Parent2 is Parent1{\n" + " uint256 internal m_aMember2;\n" + "}\n" + "contract Child is Parent2{\n" + " function foo() returns (uint256) { return Child.m_aMember2; }\n" + " uint256 public m_aMember3;\n" + "}\n"; + BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); +} + BOOST_AUTO_TEST_CASE(fallback_function) { char const* text = R"( From de6e9f4f54f669369a521e45cb2d6d9af3dbadd1 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Mon, 2 Mar 2015 12:08:32 +0100 Subject: [PATCH 13/16] Using normal pointer in getInheritableMembers() --- libsolidity/AST.cpp | 12 ++++++------ libsolidity/AST.h | 4 ++-- libsolidity/Types.cpp | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index e5aef4cbb..4c6db6a20 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -209,13 +209,13 @@ vector, FunctionTypePointer>> const& ContractDefinition::getIn return *m_interfaceFunctionList; } -vector> const& ContractDefinition::getInheritableMembers() const +vector const& ContractDefinition::getInheritableMembers() const { if (!m_inheritableMembers) { set memberSeen; - m_inheritableMembers.reset(new vector>()); - auto addInheritableMember = [&](ASTPointer const& _decl) + m_inheritableMembers.reset(new vector()); + auto addInheritableMember = [&](Declaration const* _decl) { if (memberSeen.count(_decl->getName()) == 0 && _decl->isVisibleInDerivedContracts()) { @@ -225,13 +225,13 @@ vector> const& ContractDefinition::getInheritableMembers }; for (ASTPointer const& f: getDefinedFunctions()) - addInheritableMember(f); + addInheritableMember(f.get()); for (ASTPointer const& v: getStateVariables()) - addInheritableMember(v); + addInheritableMember(v.get()); for (ASTPointer const& s: getDefinedStructs()) - addInheritableMember(s); + addInheritableMember(s.get()); } return *m_inheritableMembers; } diff --git a/libsolidity/AST.h b/libsolidity/AST.h index 97e85927b..d028e8a76 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -248,7 +248,7 @@ public: std::map, FunctionTypePointer> getInterfaceFunctions() const; /// @returns a list of the inheritable members of this contract - std::vector> const& getInheritableMembers() const; + std::vector const& getInheritableMembers() const; /// List of all (direct and indirect) base contracts in order from derived to base, including /// the contract itself. Available after name resolution @@ -276,7 +276,7 @@ private: std::vector m_linearizedBaseContracts; mutable std::unique_ptr, FunctionTypePointer>>> m_interfaceFunctionList; mutable std::unique_ptr>> m_interfaceEvents; - mutable std::unique_ptr>> m_inheritableMembers; + mutable std::unique_ptr> m_inheritableMembers; }; class InheritanceSpecifier: public ASTNode diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 84e029269..dc72dcc49 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -1024,7 +1024,7 @@ MemberList const& TypeType::getMembers() const if (find(currentBases.begin(), currentBases.end(), &contract) != currentBases.end()) // We are accessing the type of a base contract, so add all public and protected // members. Note that this does not add inherited functions on purpose. - for (ASTPointer const& decl: contract.getInheritableMembers()) + for (Declaration const* decl: contract.getInheritableMembers()) members.push_back(make_pair(decl->getName(), decl->getType())); } else if (m_actualType->getCategory() == Category::Enum) From df4746d5e7cbef44726eea23cbf68f2a10232a7f Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Fri, 27 Feb 2015 17:41:22 +0100 Subject: [PATCH 14/16] Implemented passing arguments to the base constructor. --- libsolidity/AST.cpp | 31 ++++++--- libsolidity/AST.h | 5 +- libsolidity/Compiler.cpp | 108 ++++++++++++++++++++------------ libsolidity/Compiler.h | 7 ++- libsolidity/CompilerContext.cpp | 33 +++++++--- libsolidity/CompilerContext.h | 8 ++- test/SolidityEndToEndTest.cpp | 64 +++++++++++++++++++ 7 files changed, 196 insertions(+), 60 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 4c6db6a20..00660c5da 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -308,7 +308,9 @@ void FunctionDefinition::checkTypeRequirements() if (!var->getType()->canLiveOutsideStorage()) BOOST_THROW_EXCEPTION(var->createTypeError("Type is required to live outside storage.")); for (ASTPointer const& modifier: m_functionModifiers) - modifier->checkTypeRequirements(); + modifier->checkTypeRequirements(isConstructor() ? + dynamic_cast(*getScope()).getBaseContracts() : + vector>()); m_body->checkTypeRequirements(); } @@ -351,19 +353,34 @@ void ModifierDefinition::checkTypeRequirements() m_body->checkTypeRequirements(); } -void ModifierInvocation::checkTypeRequirements() +void ModifierInvocation::checkTypeRequirements(std::vector> const& _bases) { m_modifierName->checkTypeRequirements(); for (ASTPointer const& argument: m_arguments) argument->checkTypeRequirements(); - ModifierDefinition const* modifier = dynamic_cast(m_modifierName->getReferencedDeclaration()); - solAssert(modifier, "Function modifier not found."); - vector> const& parameters = modifier->getParameters(); - if (parameters.size() != m_arguments.size()) + auto declaration = m_modifierName->getReferencedDeclaration(); + vector> emptyParameterList; + vector> const* parameters = nullptr; + if (auto modifier = dynamic_cast(declaration)) + parameters = &modifier->getParameters(); + else + for (auto const& base: _bases) + if (declaration == base->getName()->getReferencedDeclaration()) + { + m_referencedConstructor = dynamic_cast(*declaration).getConstructor(); + if (m_referencedConstructor) + parameters = &m_referencedConstructor->getParameters(); + else + parameters = &emptyParameterList; + break; + } + if (!parameters) + BOOST_THROW_EXCEPTION(createTypeError("Referenced declaration is neither modifier nor base class.")); + if (parameters->size() != m_arguments.size()) BOOST_THROW_EXCEPTION(createTypeError("Wrong argument count for modifier invocation.")); for (size_t i = 0; i < m_arguments.size(); ++i) - if (!m_arguments[i]->getType()->isImplicitlyConvertibleTo(*parameters[i]->getType())) + if (!m_arguments[i]->getType()->isImplicitlyConvertibleTo(*(*parameters)[i]->getType())) BOOST_THROW_EXCEPTION(createTypeError("Invalid type for argument in modifier invocation.")); } diff --git a/libsolidity/AST.h b/libsolidity/AST.h index d028e8a76..967c6792d 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -510,7 +510,7 @@ private: }; /** - * Invocation/usage of a modifier in a function header. + * Invocation/usage of a modifier in a function header or a base constructor call. */ class ModifierInvocation: public ASTNode { @@ -525,11 +525,12 @@ public: ASTPointer const& getName() const { return m_modifierName; } std::vector> const& getArguments() const { return m_arguments; } - void checkTypeRequirements(); + void checkTypeRequirements(std::vector> const& _bases); private: ASTPointer m_modifierName; std::vector> m_arguments; + FunctionDefinition const* m_referencedConstructor = nullptr; }; /** diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index 2f75d2ea4..580fd5b62 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -58,7 +58,10 @@ void Compiler::compileContract(ContractDefinition const& _contract, while (!functions.empty()) { for (Declaration const* function: functions) + { + m_context.setStackOffset(0); function->accept(*this); + } functions = m_context.getFunctionsWithoutCode(); } @@ -79,37 +82,38 @@ void Compiler::initializeContext(ContractDefinition const& _contract, void Compiler::packIntoContractCreator(ContractDefinition const& _contract, CompilerContext const& _runtimeContext) { - // arguments for base constructors, filled in derived-to-base order - map> const*> baseArguments; - // Determine the arguments that are used for the base constructors. std::vector const& bases = _contract.getLinearizedBaseContracts(); for (ContractDefinition const* contract: bases) + { + if (FunctionDefinition const* constructor = contract->getConstructor()) + for (auto const& modifier: constructor->getModifiers()) + { + auto baseContract = dynamic_cast( + modifier->getName()->getReferencedDeclaration()); + if (baseContract) + if (m_baseArguments.count(baseContract->getConstructor()) == 0) + m_baseArguments[baseContract->getConstructor()] = &modifier->getArguments(); + } + for (ASTPointer const& base: contract->getBaseContracts()) { ContractDefinition const* baseContract = dynamic_cast( base->getName()->getReferencedDeclaration()); solAssert(baseContract, ""); - if (baseArguments.count(baseContract) == 0) - baseArguments[baseContract] = &base->getArguments(); - } - // Call constructors in base-to-derived order. - // The Constructor for the most derived contract is called later. - for (unsigned i = 1; i < bases.size(); i++) - { - ContractDefinition const* base = bases[bases.size() - i]; - solAssert(base, ""); - initializeStateVariables(*base); - FunctionDefinition const* baseConstructor = base->getConstructor(); - if (!baseConstructor) - continue; - solAssert(baseArguments[base], ""); - appendBaseConstructorCall(*baseConstructor, *baseArguments[base]); + if (m_baseArguments.count(baseContract->getConstructor()) == 0) + m_baseArguments[baseContract->getConstructor()] = &base->getArguments(); + } } - initializeStateVariables(_contract); - if (_contract.getConstructor()) - appendConstructorCall(*_contract.getConstructor()); + // Initialization of state variables in base-to-derived order. + for (ContractDefinition const* contract: boost::adaptors::reverse(bases)) + initializeStateVariables(*contract); + + if (FunctionDefinition const* constructor = _contract.getConstructor()) + appendConstructor(*constructor); + else if (auto c = m_context.getNextConstructor(_contract)) + appendBaseConstructor(*c); eth::AssemblyItem sub = m_context.addSubroutine(_runtimeContext.getAssembly()); // stack contains sub size @@ -126,22 +130,23 @@ void Compiler::packIntoContractCreator(ContractDefinition const& _contract, Comp } } -void Compiler::appendBaseConstructorCall(FunctionDefinition const& _constructor, - vector> const& _arguments) +void Compiler::appendBaseConstructor(FunctionDefinition const& _constructor) { CompilerContext::LocationSetter locationSetter(m_context, &_constructor); FunctionType constructorType(_constructor); - eth::AssemblyItem returnLabel = m_context.pushNewTag(); - for (unsigned i = 0; i < _arguments.size(); ++i) - compileExpression(*_arguments[i], constructorType.getParameterTypes()[i]); - m_context.appendJumpTo(m_context.getFunctionEntryLabel(_constructor)); - m_context << returnLabel; + if (!constructorType.getParameterTypes().empty()) + { + std::vector> const* arguments = m_baseArguments[&_constructor]; + solAssert(arguments, ""); + for (unsigned i = 0; i < arguments->size(); ++i) + compileExpression(*(arguments->at(i)), constructorType.getParameterTypes()[i]); + } + _constructor.accept(*this); } -void Compiler::appendConstructorCall(FunctionDefinition const& _constructor) +void Compiler::appendConstructor(FunctionDefinition const& _constructor) { CompilerContext::LocationSetter locationSetter(m_context, &_constructor); - eth::AssemblyItem returnTag = m_context.pushNewTag(); // copy constructor arguments from code to memory and then to stack, they are supplied after the actual program unsigned argumentSize = 0; for (ASTPointer const& var: _constructor.getParameters()) @@ -155,8 +160,7 @@ void Compiler::appendConstructorCall(FunctionDefinition const& _constructor) m_context << eth::Instruction::CODECOPY; appendCalldataUnpacker(FunctionType(_constructor).getParameterTypes(), true); } - m_context.appendJumpTo(m_context.getFunctionEntryLabel(_constructor)); - m_context << returnTag; + _constructor.accept(*this); } void Compiler::appendFunctionSelector(ContractDefinition const& _contract) @@ -296,28 +300,36 @@ bool Compiler::visit(FunctionDefinition const& _function) // although note that this reduces the size of the visible stack m_context.startFunction(_function); - m_returnTag = m_context.newTag(); - m_breakTags.clear(); - m_continueTags.clear(); - m_stackCleanupForReturn = 0; - m_currentFunction = &_function; - m_modifierDepth = 0; // stack upon entry: [return address] [arg0] [arg1] ... [argn] // reserve additional slots: [retarg0] ... [retargm] [localvar0] ... [localvarp] unsigned parametersSize = CompilerUtils::getSizeOnStack(_function.getParameters()); - m_context.adjustStackOffset(parametersSize); + if (!_function.isConstructor()) + // adding 1 for return address. + m_context.adjustStackOffset(parametersSize + 1); for (ASTPointer const& variable: _function.getParameters()) { m_context.addVariable(*variable, parametersSize); parametersSize -= variable->getType()->getSizeOnStack(); } + for (ASTPointer const& variable: _function.getReturnParameters()) m_context.addAndInitializeVariable(*variable); for (VariableDeclaration const* localVariable: _function.getLocalVariables()) m_context.addAndInitializeVariable(*localVariable); + if (_function.isConstructor()) + if (auto c = m_context.getNextConstructor(dynamic_cast(*_function.getScope()))) + appendBaseConstructor(*c); + + m_returnTag = m_context.newTag(); + m_breakTags.clear(); + m_continueTags.clear(); + m_stackCleanupForReturn = 0; + m_currentFunction = &_function; + m_modifierDepth = 0; + appendModifierOrFunctionCode(); m_context << m_returnTag; @@ -352,8 +364,14 @@ bool Compiler::visit(FunctionDefinition const& _function) } //@todo assert that everything is in place now - m_context << eth::Instruction::JUMP; + for (ASTPointer const& variable: _function.getParameters() + _function.getReturnParameters()) + m_context.removeVariable(*variable); + for (VariableDeclaration const* localVariable: _function.getLocalVariables()) + m_context.removeVariable(*localVariable); + m_context.adjustStackOffset(-c_returnValuesSize); + if (!_function.isConstructor()) + m_context << eth::Instruction::JUMP; return false; } @@ -515,6 +533,16 @@ void Compiler::appendModifierOrFunctionCode() else { ASTPointer const& modifierInvocation = m_currentFunction->getModifiers()[m_modifierDepth]; + + // constructor call should be excluded + if (dynamic_cast(modifierInvocation->getName()->getReferencedDeclaration())) + { + ++m_modifierDepth; + appendModifierOrFunctionCode(); + --m_modifierDepth; + return; + } + ModifierDefinition const& modifier = m_context.getFunctionModifier(modifierInvocation->getName()->getName()); CompilerContext::LocationSetter locationSetter(m_context, &modifier); solAssert(modifier.getParameters().size() == modifierInvocation->getArguments().size(), ""); diff --git a/libsolidity/Compiler.h b/libsolidity/Compiler.h index 3ad2d8c61..2804e8eca 100644 --- a/libsolidity/Compiler.h +++ b/libsolidity/Compiler.h @@ -55,9 +55,8 @@ private: /// Adds the code that is run at creation time. Should be run after exchanging the run-time context /// with a new and initialized context. Adds the constructor code. void packIntoContractCreator(ContractDefinition const& _contract, CompilerContext const& _runtimeContext); - void appendBaseConstructorCall(FunctionDefinition const& _constructor, - std::vector> const& _arguments); - void appendConstructorCall(FunctionDefinition const& _constructor); + void appendBaseConstructor(FunctionDefinition const& _constructor); + void appendConstructor(FunctionDefinition const& _constructor); void appendFunctionSelector(ContractDefinition const& _contract); /// Creates code that unpacks the arguments for the given function represented by a vector of TypePointers. /// From memory if @a _fromMemory is true, otherwise from call data. @@ -94,6 +93,8 @@ private: unsigned m_modifierDepth = 0; FunctionDefinition const* m_currentFunction; unsigned m_stackCleanupForReturn; ///< 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; }; } diff --git a/libsolidity/CompilerContext.cpp b/libsolidity/CompilerContext.cpp index 18be337fa..7184dc37d 100644 --- a/libsolidity/CompilerContext.cpp +++ b/libsolidity/CompilerContext.cpp @@ -51,8 +51,6 @@ void CompilerContext::addStateVariable(VariableDeclaration const& _declaration) void CompilerContext::startFunction(Declaration const& _function) { m_functionsWithCode.insert(&_function); - m_localVariables.clear(); - m_asm.setDeposit(0); *this << getFunctionEntryLabel(_function); } @@ -63,6 +61,12 @@ void CompilerContext::addVariable(VariableDeclaration const& _declaration, m_localVariables[&_declaration] = unsigned(m_asm.deposit()) - _offsetToCurrent; } +void CompilerContext::removeVariable(VariableDeclaration const& _declaration) +{ + solAssert(m_localVariables.count(&_declaration), ""); + m_localVariables.erase(&_declaration); +} + void CompilerContext::addAndInitializeVariable(VariableDeclaration const& _declaration) { LocationSetter locationSetter(*this, &_declaration); @@ -110,11 +114,8 @@ eth::AssemblyItem CompilerContext::getVirtualFunctionEntryLabel(FunctionDefiniti eth::AssemblyItem CompilerContext::getSuperFunctionEntryLabel(string const& _name, ContractDefinition const& _base) { - // search for first contract after _base - solAssert(!m_inheritanceHierarchy.empty(), "No inheritance hierarchy set."); - auto it = find(m_inheritanceHierarchy.begin(), m_inheritanceHierarchy.end(), &_base); - solAssert(it != m_inheritanceHierarchy.end(), "Base not found in inheritance hierarchy."); - for (++it; it != m_inheritanceHierarchy.end(); ++it) + auto it = getSuperContract(_base); + for (; it != m_inheritanceHierarchy.end(); ++it) for (ASTPointer const& function: (*it)->getDefinedFunctions()) if (!function->isConstructor() && function->getName() == _name) return getFunctionEntryLabel(*function); @@ -122,6 +123,16 @@ eth::AssemblyItem CompilerContext::getSuperFunctionEntryLabel(string const& _nam return m_asm.newTag(); // not reached } +FunctionDefinition const* CompilerContext::getNextConstructor(ContractDefinition const& _contract) const +{ + vector::const_iterator it = getSuperContract(_contract); + for (; it != m_inheritanceHierarchy.end(); it = getSuperContract(**it)) + if ((*it)->getConstructor()) + return (*it)->getConstructor(); + + return nullptr; +} + set CompilerContext::getFunctionsWithoutCode() { set functions; @@ -201,5 +212,13 @@ CompilerContext& CompilerContext::operator<<(bytes const& _data) return *this; } +vector::const_iterator CompilerContext::getSuperContract(ContractDefinition const& _contract) const +{ + solAssert(!m_inheritanceHierarchy.empty(), "No inheritance hierarchy set."); + auto it = find(m_inheritanceHierarchy.begin(), m_inheritanceHierarchy.end(), &_contract); + solAssert(it != m_inheritanceHierarchy.end(), "Base not found in inheritance hierarchy."); + return ++it; +} + } } diff --git a/libsolidity/CompilerContext.h b/libsolidity/CompilerContext.h index 94d6443e9..301ef1468 100644 --- a/libsolidity/CompilerContext.h +++ b/libsolidity/CompilerContext.h @@ -43,11 +43,13 @@ public: void addMagicGlobal(MagicVariableDeclaration const& _declaration); void addStateVariable(VariableDeclaration const& _declaration); void addVariable(VariableDeclaration const& _declaration, unsigned _offsetToCurrent = 0); + void removeVariable(VariableDeclaration const& _declaration); void addAndInitializeVariable(VariableDeclaration const& _declaration); void setCompiledContracts(std::map const& _contracts) { m_compiledContracts = _contracts; } bytes const& getCompiledContract(ContractDefinition const& _contract) const; + void setStackOffset(int _offset) { m_asm.setDeposit(_offset); } void adjustStackOffset(int _adjustment) { m_asm.adjustDeposit(_adjustment); } unsigned getStackHeight() const { solAssert(m_asm.deposit() >= 0, ""); return unsigned(m_asm.deposit()); } @@ -62,6 +64,8 @@ public: /// @returns the entry label of function with the given name from the most derived class just /// above _base in the current inheritance hierarchy. eth::AssemblyItem getSuperFunctionEntryLabel(std::string const& _name, ContractDefinition const& _base); + FunctionDefinition const* getNextConstructor(ContractDefinition const& _contract) const; + /// @returns the set of functions for which we still need to generate code std::set getFunctionsWithoutCode(); /// Resets function specific members, inserts the function entry label and marks the function @@ -126,9 +130,11 @@ public: LocationSetter(CompilerContext& _compilerContext, ASTNode const* _node): ScopeGuard(std::bind(&CompilerContext::popVisitedNodes, _compilerContext)) { _compilerContext.pushVisitedNodes(_node); } }; - eth::Assembly m_asm; + private: + std::vector::const_iterator getSuperContract(const ContractDefinition &_contract) const; + eth::Assembly m_asm; /// Magic global variables like msg, tx or this, distinguished by type. std::set m_magicGlobals; /// Other already compiled contracts to be used in contract creation calls. diff --git a/test/SolidityEndToEndTest.cpp b/test/SolidityEndToEndTest.cpp index f305e81e3..87a8eb79f 100644 --- a/test/SolidityEndToEndTest.cpp +++ b/test/SolidityEndToEndTest.cpp @@ -2946,6 +2946,70 @@ BOOST_AUTO_TEST_CASE(array_copy_storage_storage_struct) BOOST_CHECK(m_state.storage(m_contractAddress).empty()); } +BOOST_AUTO_TEST_CASE(pass_dynamic_arguments_to_the_base) +{ + char const* sourceCode = R"( + contract Base { + function Base(uint i) + { + m_i = i; + } + uint public m_i; + } + contract Derived is Base(2) { + function Derived(uint i) Base(i) + {} + } + contract Final is Derived(4) { + })"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("m_i()") == encodeArgs(4)); +} + +BOOST_AUTO_TEST_CASE(pass_dynamic_arguments_to_the_base_base) +{ + char const* sourceCode = R"( + contract Base { + function Base(uint j) + { + m_i = j; + } + uint public m_i; + } + contract Base1 is Base(3) { + function Base1(uint k) Base(k*k) {} + } + contract Derived is Base(3), Base1(2) { + function Derived(uint i) Base(i) Base1(i) + {} + } + contract Final is Derived(4) { + })"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("m_i()") == encodeArgs(4)); +} + +BOOST_AUTO_TEST_CASE(pass_dynamic_arguments_to_the_base_base_with_gap) +{ + char const* sourceCode = R"( + contract Base { + function Base(uint i) + { + m_i = i; + } + uint public m_i; + } + contract Base1 is Base(3) {} + contract Derived is Base(2), Base1 { + function Derived(uint i) Base(i) {} + } + contract Final is Derived(4) { + })"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("m_i()") == encodeArgs(4)); +} + + BOOST_AUTO_TEST_SUITE_END() } From a545b8a7a46c20940f436226ea67c6f23cffa4cd Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Mon, 2 Mar 2015 14:21:12 +0100 Subject: [PATCH 15/16] removed unused member added some comments for ModifierInvocation::checkTypeRequirements cleanup --- libsolidity/AST.cpp | 8 ++++---- libsolidity/AST.h | 2 +- libsolidity/CompilerContext.cpp | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 00660c5da..aba355768 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -353,7 +353,7 @@ void ModifierDefinition::checkTypeRequirements() m_body->checkTypeRequirements(); } -void ModifierInvocation::checkTypeRequirements(std::vector> const& _bases) +void ModifierInvocation::checkTypeRequirements(vector> const& _bases) { m_modifierName->checkTypeRequirements(); for (ASTPointer const& argument: m_arguments) @@ -365,12 +365,12 @@ void ModifierInvocation::checkTypeRequirements(std::vector(declaration)) parameters = &modifier->getParameters(); else + // check parameters for Base constructors for (auto const& base: _bases) if (declaration == base->getName()->getReferencedDeclaration()) { - m_referencedConstructor = dynamic_cast(*declaration).getConstructor(); - if (m_referencedConstructor) - parameters = &m_referencedConstructor->getParameters(); + if (auto referencedConstructor = dynamic_cast(*declaration).getConstructor()) + parameters = &referencedConstructor->getParameters(); else parameters = &emptyParameterList; break; diff --git a/libsolidity/AST.h b/libsolidity/AST.h index 967c6792d..c91c433ed 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -525,12 +525,12 @@ public: ASTPointer const& getName() const { return m_modifierName; } std::vector> const& getArguments() const { return m_arguments; } + /// @param _bases is the list of base contracts for base constructor calls. For modifiers an empty vector should be passed. void checkTypeRequirements(std::vector> const& _bases); private: ASTPointer m_modifierName; std::vector> m_arguments; - FunctionDefinition const* m_referencedConstructor = nullptr; }; /** diff --git a/libsolidity/CompilerContext.cpp b/libsolidity/CompilerContext.cpp index 7184dc37d..61c25052c 100644 --- a/libsolidity/CompilerContext.cpp +++ b/libsolidity/CompilerContext.cpp @@ -126,7 +126,7 @@ eth::AssemblyItem CompilerContext::getSuperFunctionEntryLabel(string const& _nam FunctionDefinition const* CompilerContext::getNextConstructor(ContractDefinition const& _contract) const { vector::const_iterator it = getSuperContract(_contract); - for (; it != m_inheritanceHierarchy.end(); it = getSuperContract(**it)) + for (; it != m_inheritanceHierarchy.end(); ++it) if ((*it)->getConstructor()) return (*it)->getConstructor(); From 211c37acb82ac368b53c93762045b295ea4bd452 Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 2 Mar 2015 15:12:54 +0100 Subject: [PATCH 16/16] Removed unused variables. --- libsolidity/LValue.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libsolidity/LValue.cpp b/libsolidity/LValue.cpp index 78154f427..452ca1c73 100644 --- a/libsolidity/LValue.cpp +++ b/libsolidity/LValue.cpp @@ -240,14 +240,14 @@ StorageArrayLength::StorageArrayLength(CompilerContext& _compilerContext, const solAssert(m_arrayType.isDynamicallySized(), ""); } -void StorageArrayLength::retrieveValue(SourceLocation const& _location, bool _remove) const +void StorageArrayLength::retrieveValue(SourceLocation const&, bool _remove) const { if (!_remove) m_context << eth::Instruction::DUP1; m_context << eth::Instruction::SLOAD; } -void StorageArrayLength::storeValue(Type const& _sourceType, SourceLocation const& _location, bool _move) const +void StorageArrayLength::storeValue(Type const&, SourceLocation const&, bool _move) const { if (_move) m_context << eth::Instruction::SWAP1; @@ -256,7 +256,7 @@ void StorageArrayLength::storeValue(Type const& _sourceType, SourceLocation cons ArrayUtils(m_context).resizeDynamicArray(m_arrayType); } -void StorageArrayLength::setToZero(SourceLocation const& _location, bool _removeReference) const +void StorageArrayLength::setToZero(SourceLocation const&, bool _removeReference) const { if (!_removeReference) m_context << eth::Instruction::DUP1;