diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 0dbad433f..c6d8f5c58 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -274,6 +274,15 @@ TypePointer FunctionDefinition::getType(ContractDefinition const*) const void FunctionDefinition::checkTypeRequirements() { + // change all byte arrays parameters to point to calldata + if (getVisibility() == Visibility::External) + for (ASTPointer const& var: getParameters()) + { + auto const& type = var->getType(); + solAssert(!!type, ""); + if (auto const* byteArrayType = dynamic_cast(type.get())) + var->setType(byteArrayType->copyForLocation(ByteArrayType::Location::CallData)); + } for (ASTPointer const& var: getParameters() + getReturnParameters()) if (!var->getType()->canLiveOutsideStorage()) BOOST_THROW_EXCEPTION(var->createTypeError("Type is required to live outside storage.")); @@ -288,12 +297,23 @@ string FunctionDefinition::getCanonicalSignature() const return FunctionType(*this).getCanonicalSignature(getName()); } -Declaration::LValueType VariableDeclaration::getLValueType() const +bool VariableDeclaration::isLValue() const { - if (dynamic_cast(getScope()) || dynamic_cast(getScope())) - return Declaration::LValueType::Local; - else - return Declaration::LValueType::Storage; + if (auto const* function = dynamic_cast(getScope())) + if (function->getVisibility() == Declaration::Visibility::External && isFunctionParameter()) + return false; + return true; +} + +bool VariableDeclaration::isFunctionParameter() const +{ + auto const* function = dynamic_cast(getScope()); + if (!function) + return false; + for (auto const& variable: function->getParameters()) + if (variable.get() == this) + return true; + return false; } TypePointer ModifierDefinition::getType(ContractDefinition const*) const @@ -586,8 +606,7 @@ void MemberAccess::checkTypeRequirements() if (!m_type) BOOST_THROW_EXCEPTION(createTypeError("Member \"" + *m_memberName + "\" not found or not " "visible in " + type.toString())); - //@todo later, this will not always be STORAGE - m_lvalue = type.getCategory() == Type::Category::Struct ? Declaration::LValueType::Storage : Declaration::LValueType::None; + m_isLValue = (type.getCategory() == Type::Category::Struct); } void IndexAccess::checkTypeRequirements() @@ -599,14 +618,14 @@ void IndexAccess::checkTypeRequirements() MappingType const& type = dynamic_cast(*m_base->getType()); m_index->expectType(*type.getKeyType()); m_type = type.getValueType(); - m_lvalue = Declaration::LValueType::Storage; + m_isLValue = true; } void Identifier::checkTypeRequirements() { solAssert(m_referencedDeclaration, "Identifier not resolved."); - m_lvalue = m_referencedDeclaration->getLValueType(); + m_isLValue = m_referencedDeclaration->isLValue(); m_type = m_referencedDeclaration->getType(m_currentContract); if (!m_type) BOOST_THROW_EXCEPTION(createTypeError("Declaration referenced before type could be determined.")); diff --git a/libsolidity/AST.h b/libsolidity/AST.h index 51d8031a3..af45934f7 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -132,8 +132,8 @@ private: class Declaration: public ASTNode { public: - enum class LValueType { None, Local, Storage }; - enum class Visibility { Default, Public, Protected, Private }; + /// Visibility ordered from restricted to unrestricted. + enum class Visibility { Default, Private, Protected, Public, External }; Declaration(Location const& _location, ASTPointer const& _name, Visibility _visibility = Visibility::Default): @@ -142,7 +142,9 @@ public: /// @returns the declared name. ASTString const& getName() const { return *m_name; } Visibility getVisibility() const { return m_visibility == Visibility::Default ? getDefaultVisibility() : m_visibility; } - bool isPublic() const { return getVisibility() == Visibility::Public; } + bool isPublic() const { return getVisibility() >= Visibility::Public; } + bool isVisibleInContract() const { return getVisibility() != Visibility::External; } + bool isVisibleInDerivedContracts() const { return isVisibleInContract() && getVisibility() >= Visibility::Protected; } /// @returns the scope this declaration resides in. Can be nullptr if it is the global scope. /// Available only after name and type resolution step. @@ -153,8 +155,7 @@ public: /// The current contract has to be given since this context can change the type, especially of /// contract types. virtual TypePointer getType(ContractDefinition const* m_currentContract = nullptr) const = 0; - /// @returns the lvalue type of expressions referencing this declaration - virtual LValueType getLValueType() const { return LValueType::None; } + virtual bool isLValue() const { return false; } protected: virtual Visibility getDefaultVisibility() const { return Visibility::Public; } @@ -445,8 +446,9 @@ public: TypePointer getType(ContractDefinition const* = nullptr) const { return m_type; } void setType(std::shared_ptr const& _type) { m_type = _type; } - virtual LValueType getLValueType() const override; + virtual bool isLValue() const override; bool isLocalVariable() const { return !!dynamic_cast(getScope()); } + bool isFunctionParameter() const; bool isStateVariable() const { return m_isStateVariable; } bool isIndexed() const { return m_isIndexed; } @@ -884,8 +886,7 @@ public: virtual void checkTypeRequirements() = 0; std::shared_ptr const& getType() const { return m_type; } - bool isLValue() const { return m_lvalue != Declaration::LValueType::None; } - bool isLocalLValue() const { return m_lvalue == Declaration::LValueType::Local; } + bool isLValue() const { return m_isLValue; } /// Helper function, infer the type via @ref checkTypeRequirements and then check that it /// is implicitly convertible to @a _expectedType. If not, throw exception. @@ -900,9 +901,9 @@ public: protected: //! Inferred type of the expression, only filled after a call to checkTypeRequirements(). std::shared_ptr m_type; - //! If this expression is an lvalue (i.e. something that can be assigned to) and is stored - //! locally or in storage. This is set during calls to @a checkTypeRequirements() - Declaration::LValueType m_lvalue = Declaration::LValueType::None; + //! If this expression is an lvalue (i.e. something that can be assigned to). + //! This is set during calls to @a checkTypeRequirements() + bool m_isLValue = false; //! Whether the outer expression requested the address (true) or the value (false) of this expression. bool m_lvalueRequested = false; }; diff --git a/libsolidity/ASTJsonConverter.cpp b/libsolidity/ASTJsonConverter.cpp index a216a59ac..04feafe2f 100644 --- a/libsolidity/ASTJsonConverter.cpp +++ b/libsolidity/ASTJsonConverter.cpp @@ -118,11 +118,7 @@ bool ASTJsonConverter::visit(FunctionDefinition const& _node) bool ASTJsonConverter::visit(VariableDeclaration const& _node) { - bool isLocalVariable = (_node.getLValueType() == VariableDeclaration::LValueType::Local); - addJsonNode("VariableDeclaration", - { make_pair("name", _node.getName()), - make_pair("local", boost::lexical_cast(isLocalVariable))}, - true); + addJsonNode("VariableDeclaration", { make_pair("name", _node.getName()) }, true); return true; } @@ -216,11 +212,12 @@ bool ASTJsonConverter::visit(ExpressionStatement const&) bool ASTJsonConverter::visit(Expression const& _node) { - addJsonNode("Expression", - { make_pair("type", getType(_node)), - make_pair("lvalue", boost::lexical_cast(_node.isLValue())), - make_pair("local_lvalue", boost::lexical_cast(_node.isLocalLValue())) }, - true); + addJsonNode( + "Expression", + { make_pair("type", getType(_node)), + make_pair("lvalue", boost::lexical_cast(_node.isLValue())) }, + true + ); return true; } diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index dad79bb06..14acc0113 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -147,7 +147,7 @@ void Compiler::appendFunctionSelector(ContractDefinition const& _contract) // retrieve the function signature hash from the calldata if (!interfaceFunctions.empty()) - CompilerUtils(m_context).loadFromMemory(0, 4, false, true); + CompilerUtils(m_context).loadFromMemory(0, IntegerType(CompilerUtils::dataStartOffset * 8), true); // stack now is: 1 0 for (auto const& it: interfaceFunctions) @@ -178,23 +178,46 @@ void Compiler::appendFunctionSelector(ContractDefinition const& _contract) } } -unsigned Compiler::appendCalldataUnpacker(TypePointers const& _typeParameters, bool _fromMemory) +void Compiler::appendCalldataUnpacker(TypePointers const& _typeParameters, bool _fromMemory) { // We do not check the calldata size, everything is zero-padded. - unsigned dataOffset = CompilerUtils::dataStartOffset; // the 4 bytes of the function hash signature - //@todo this can be done more efficiently, saving some CALLDATALOAD calls + unsigned offset(CompilerUtils::dataStartOffset); + bool const c_padToWords = true; + + unsigned dynamicParameterCount = 0; for (TypePointer const& type: _typeParameters) - { - unsigned const c_numBytes = type->getCalldataEncodedSize(); - if (c_numBytes > 32) - BOOST_THROW_EXCEPTION(CompilerError() - << errinfo_comment("Type " + type->toString() + " not yet supported.")); - bool const c_leftAligned = type->getCategory() == Type::Category::String; - bool const c_padToWords = true; - dataOffset += CompilerUtils(m_context).loadFromMemory(dataOffset, c_numBytes, c_leftAligned, - !_fromMemory, c_padToWords); - } - return dataOffset; + if (type->isDynamicallySized()) + dynamicParameterCount++; + offset += dynamicParameterCount * 32; + unsigned currentDynamicParameter = 0; + for (TypePointer const& type: _typeParameters) + if (type->isDynamicallySized()) + { + // value on stack: [calldata_offset] (only if we are already in dynamic mode) + if (currentDynamicParameter == 0) + // switch from static to dynamic + m_context << u256(offset); + // retrieve length + CompilerUtils(m_context).loadFromMemory( + CompilerUtils::dataStartOffset + currentDynamicParameter * 32, + IntegerType(256), !_fromMemory, c_padToWords); + // stack: offset length + // add 32-byte padding to copy of length + m_context << u256(32) << eth::Instruction::DUP1 << u256(31) + << eth::Instruction::DUP4 << eth::Instruction::ADD + << eth::Instruction::DIV << eth::Instruction::MUL; + // stack: offset length padded_length + m_context << eth::Instruction::DUP3 << eth::Instruction::ADD; + currentDynamicParameter++; + // stack: offset length next_calldata_offset + } + else if (currentDynamicParameter == 0) + // we can still use static load + offset += CompilerUtils(m_context).loadFromMemory(offset, *type, !_fromMemory, c_padToWords); + else + CompilerUtils(m_context).loadFromMemoryDynamic(*type, !_fromMemory, c_padToWords); + if (dynamicParameterCount > 0) + m_context << eth::Instruction::POP; } void Compiler::appendReturnValuePacker(TypePointers const& _typeParameters) diff --git a/libsolidity/Compiler.h b/libsolidity/Compiler.h index b3eae5b17..5d5b6d47b 100644 --- a/libsolidity/Compiler.h +++ b/libsolidity/Compiler.h @@ -52,8 +52,8 @@ private: void appendConstructorCall(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. @returns the size of the data in bytes. - unsigned appendCalldataUnpacker(TypePointers const& _typeParameters, bool _fromMemory = false); + /// From memory if @a _fromMemory is true, otherwise from call data. + void appendCalldataUnpacker(TypePointers const& _typeParameters, bool _fromMemory = false); void appendReturnValuePacker(TypePointers const& _typeParameters); void registerStateVariables(ContractDefinition const& _contract); diff --git a/libsolidity/CompilerUtils.cpp b/libsolidity/CompilerUtils.cpp index dda1736d6..047bc6d62 100644 --- a/libsolidity/CompilerUtils.cpp +++ b/libsolidity/CompilerUtils.cpp @@ -33,35 +33,26 @@ namespace solidity const unsigned int CompilerUtils::dataStartOffset = 4; -unsigned CompilerUtils::loadFromMemory(unsigned _offset, unsigned _bytes, bool _leftAligned, - bool _fromCalldata, bool _padToWordBoundaries) +unsigned CompilerUtils::loadFromMemory(unsigned _offset, Type const& _type, + bool _fromCalldata, bool _padToWordBoundaries) { - if (_bytes == 0) - { - m_context << u256(0); - return 0; - } - eth::Instruction load = _fromCalldata ? eth::Instruction::CALLDATALOAD : eth::Instruction::MLOAD; - solAssert(_bytes <= 32, "Memory load of more than 32 bytes requested."); - if (_bytes == 32 || _padToWordBoundaries) - { - m_context << u256(_offset) << load; - return 32; - } - else - { - // load data and add leading or trailing zeros by dividing/multiplying depending on alignment - u256 shiftFactor = u256(1) << ((32 - _bytes) * 8); - m_context << shiftFactor; - if (_leftAligned) - m_context << eth::Instruction::DUP1; - m_context << u256(_offset) << load << eth::Instruction::DIV; - if (_leftAligned) - m_context << eth::Instruction::MUL; - return _bytes; - } + solAssert(_type.getCategory() != Type::Category::ByteArray, "Unable to statically load dynamic type."); + m_context << u256(_offset); + return loadFromMemoryHelper(_type, _fromCalldata, _padToWordBoundaries); } +void CompilerUtils::loadFromMemoryDynamic(Type const& _type, bool _fromCalldata, bool _padToWordBoundaries) +{ + solAssert(_type.getCategory() != Type::Category::ByteArray, "Byte arrays not yet implemented."); + m_context << eth::Instruction::DUP1; + unsigned numBytes = loadFromMemoryHelper(_type, _fromCalldata, _padToWordBoundaries); + // update memory counter + for (unsigned i = 0; i < _type.getSizeOnStack(); ++i) + m_context << eth::swapInstruction(1 + i); + m_context << u256(numBytes) << eth::Instruction::ADD; +} + + unsigned CompilerUtils::storeInMemory(unsigned _offset, Type const& _type, bool _padToWordBoundaries) { solAssert(_type.getCategory() != Type::Category::ByteArray, "Unable to statically store dynamic type."); @@ -79,9 +70,12 @@ void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBound if (type.getLocation() == ByteArrayType::Location::CallData) { - m_context << eth::Instruction::CALLDATASIZE << u256(0) << eth::Instruction::DUP3 - << eth::Instruction::CALLDATACOPY - << eth::Instruction::CALLDATASIZE << eth::Instruction::ADD; + // stack: target source_offset source_len + m_context << eth::Instruction::DUP1 << eth::Instruction::DUP3 << eth::Instruction::DUP5 + // stack: target source_offset source_len source_len source_offset target + << eth::Instruction::CALLDATACOPY + << eth::Instruction::DUP3 << eth::Instruction::ADD + << eth::Instruction::SWAP2 << eth::Instruction::POP << eth::Instruction::POP; } else { @@ -120,6 +114,7 @@ void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBound unsigned numBytes = prepareMemoryStore(_type, _padToWordBoundaries); if (numBytes > 0) { + solAssert(_type.getSizeOnStack() == 1, "Memory store of types with stack size != 1 not implemented."); m_context << eth::Instruction::DUP2 << eth::Instruction::MSTORE; m_context << u256(numBytes) << eth::Instruction::ADD; } @@ -179,29 +174,32 @@ void CompilerUtils::copyByteArrayToStorage(ByteArrayType const& _targetType, { case ByteArrayType::Location::CallData: { - // @todo this does not take length into account. It also assumes that after "CALLDATALENGTH" we only have zeros. + // 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: target_ref target_length_words + // 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: target_ref target_data_end target_data_ref + // stack here: source_offset source_len target_ref target_data_end target_data_ref // store length (in bytes) - m_context << eth::Instruction::CALLDATASIZE; - m_context << eth::Instruction::DUP1 << eth::Instruction::DUP5 << eth::Instruction::SSTORE; + 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 << u256(0); - // stack now: target_ref target_data_end target_data_ref calldata_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 @@ -212,16 +210,18 @@ void CompilerUtils::copyByteArrayToStorage(ByteArrayType const& _targetType, // increment calldata_offset by 32 << eth::Instruction::SWAP1 << u256(32) << eth::Instruction::ADD // check for loop condition - << eth::Instruction::DUP1 << eth::Instruction::CALLDATASIZE << eth::Instruction::GT; + << 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: target_ref target_data_end target_data_ref + // 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; + m_context << eth::Instruction::POP << eth::Instruction::SWAP2 + << eth::Instruction::POP << eth::Instruction::POP; break; } case ByteArrayType::Location::Storage: @@ -289,6 +289,30 @@ void CompilerUtils::copyByteArrayToStorage(ByteArrayType const& _targetType, } } +unsigned CompilerUtils::loadFromMemoryHelper(Type const& _type, bool _fromCalldata, bool _padToWordBoundaries) +{ + unsigned _encodedSize = _type.getCalldataEncodedSize(); + unsigned numBytes = _padToWordBoundaries ? getPaddedSize(_encodedSize) : _encodedSize; + bool leftAligned = _type.getCategory() == Type::Category::String; + if (numBytes == 0) + m_context << eth::Instruction::POP << u256(0); + else + { + solAssert(numBytes <= 32, "Static memory load of more than 32 bytes requested."); + m_context << (_fromCalldata ? eth::Instruction::CALLDATALOAD : eth::Instruction::MLOAD); + if (numBytes != 32) + { + // add leading or trailing zeros by dividing/multiplying depending on alignment + u256 shiftFactor = u256(1) << ((32 - numBytes) * 8); + m_context << shiftFactor << eth::Instruction::SWAP1 << eth::Instruction::DIV; + if (leftAligned) + m_context << shiftFactor << eth::Instruction::MUL; + } + } + + return numBytes; +} + void CompilerUtils::clearByteArray(ByteArrayType const& _type) const { solAssert(_type.getLocation() == ByteArrayType::Location::Storage, ""); diff --git a/libsolidity/CompilerUtils.h b/libsolidity/CompilerUtils.h index fe28ceadf..5369d3bf2 100644 --- a/libsolidity/CompilerUtils.h +++ b/libsolidity/CompilerUtils.h @@ -37,14 +37,16 @@ public: /// Loads data from memory to the stack. /// @param _offset offset in memory (or calldata) - /// @param _bytes number of bytes to load - /// @param _leftAligned if true, store left aligned on stack (otherwise right aligned) + /// @param _type data type to load /// @param _fromCalldata if true, load from calldata, not from memory /// @param _padToWordBoundaries if true, assume the data is padded to word (32 byte) boundaries - /// @returns the number of bytes consumed in memory (can be different from _bytes if - /// _padToWordBoundaries is true) - unsigned loadFromMemory(unsigned _offset, unsigned _bytes = 32, bool _leftAligned = false, - bool _fromCalldata = false, bool _padToWordBoundaries = false); + /// @returns the number of bytes consumed in memory. + unsigned loadFromMemory(unsigned _offset, Type const& _type = IntegerType(256), + bool _fromCalldata = false, bool _padToWordBoundaries = false); + /// Dynamic version of @see loadFromMemory, expects the memory offset on the stack. + /// Stack pre: memory_offset + /// Stack post: value... (memory_offset+length) + void loadFromMemoryDynamic(Type const& _type, bool _fromCalldata = false, bool _padToWordBoundaries = true); /// Stores data from stack in memory. /// @param _offset offset in memory /// @param _type type of the data on the stack @@ -93,6 +95,8 @@ public: private: /// Prepares the given type for storing in memory by shifting it if necessary. 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 diff --git a/libsolidity/DeclarationContainer.cpp b/libsolidity/DeclarationContainer.cpp index 2e810a4cf..2594d4281 100644 --- a/libsolidity/DeclarationContainer.cpp +++ b/libsolidity/DeclarationContainer.cpp @@ -28,14 +28,19 @@ namespace dev namespace solidity { -bool DeclarationContainer::registerDeclaration(Declaration const& _declaration, bool _update) +bool DeclarationContainer::registerDeclaration(Declaration const& _declaration, bool _invisible, bool _update) { - if (_declaration.getName().empty()) + ASTString const& name(_declaration.getName()); + if (name.empty()) return true; - if (!_update && m_declarations.find(_declaration.getName()) != m_declarations.end()) + if (!_update && (m_declarations.count(name) || m_invisibleDeclarations.count(name))) return false; - m_declarations[_declaration.getName()] = &_declaration; + + if (_invisible) + m_invisibleDeclarations.insert(name); + else + m_declarations[name] = &_declaration; return true; } diff --git a/libsolidity/DeclarationContainer.h b/libsolidity/DeclarationContainer.h index 1216fcef2..f70881f5b 100644 --- a/libsolidity/DeclarationContainer.h +++ b/libsolidity/DeclarationContainer.h @@ -23,6 +23,7 @@ #pragma once #include +#include #include #include @@ -43,8 +44,10 @@ public: DeclarationContainer const* _enclosingContainer = nullptr): m_enclosingDeclaration(_enclosingDeclaration), m_enclosingContainer(_enclosingContainer) {} /// Registers the declaration in the scope unless its name is already declared or the name is empty. + /// @param _invisible if true, registers the declaration, reports name clashes but does not return it in @a resolveName + /// @param _update if true, replaces a potential declaration that is already present /// @returns false if the name was already declared. - bool registerDeclaration(Declaration const& _declaration, bool _update = false); + bool registerDeclaration(Declaration const& _declaration, bool _invisible = false, bool _update = false); Declaration const* resolveName(ASTString const& _name, bool _recursive = false) const; Declaration const* getEnclosingDeclaration() const { return m_enclosingDeclaration; } std::map const& getDeclarations() const { return m_declarations; } @@ -53,6 +56,7 @@ private: Declaration const* m_enclosingDeclaration; DeclarationContainer const* m_enclosingContainer; std::map m_declarations; + std::set m_invisibleDeclarations; }; } diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index 0f0e94f21..3bf1c8c93 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -475,9 +475,7 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess) else if (member == "gasprice") m_context << eth::Instruction::GASPRICE; else if (member == "data") - { - // nothing to store on the stack - } + m_context << u256(0) << eth::Instruction::CALLDATASIZE; else BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Unknown magic member.")); break; @@ -510,6 +508,7 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess) m_context << m_context.getFunctionEntryLabel(*function).pushTag(); return; } + solAssert(false, "Function not found in member access."); } else if (auto enumType = dynamic_cast(type.getActualType().get())) m_context << enumType->getMemberValue(_memberAccess.getMemberName()); @@ -518,7 +517,19 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess) case Type::Category::ByteArray: { solAssert(member == "length", "Illegal bytearray member."); - m_context << eth::Instruction::SLOAD; + auto const& type = dynamic_cast(*_memberAccess.getExpression().getType()); + switch (type.getLocation()) + { + case ByteArrayType::Location::CallData: + m_context << eth::Instruction::SWAP1 << eth::Instruction::POP; + break; + case ByteArrayType::Location::Storage: + m_context << eth::Instruction::SLOAD; + break; + default: + solAssert(false, "Unsupported byte array location."); + break; + } break; } default: @@ -885,11 +896,8 @@ void ExpressionCompiler::appendExternalFunctionCall(FunctionType const& _functio m_context << eth::Instruction::POP; m_context << eth::Instruction::POP; // pop contract address - if (retSize > 0) - { - bool const c_leftAligned = firstType->getCategory() == Type::Category::String; - CompilerUtils(m_context).loadFromMemory(0, retSize, c_leftAligned, false, true); - } + if (firstType) + CompilerUtils(m_context).loadFromMemory(0, *firstType, false, true); } void ExpressionCompiler::appendArgumentsCopyToMemory(vector> const& _arguments, diff --git a/libsolidity/NameAndTypeResolver.cpp b/libsolidity/NameAndTypeResolver.cpp index dbe5693a8..ea70b65b4 100644 --- a/libsolidity/NameAndTypeResolver.cpp +++ b/libsolidity/NameAndTypeResolver.cpp @@ -86,7 +86,7 @@ void NameAndTypeResolver::checkTypeRequirements(ContractDefinition& _contract) void NameAndTypeResolver::updateDeclaration(Declaration const& _declaration) { - m_scopes[nullptr].registerDeclaration(_declaration, true); + m_scopes[nullptr].registerDeclaration(_declaration, false, true); solAssert(_declaration.getScope() == nullptr, "Updated declaration outside global scope."); } @@ -110,8 +110,9 @@ void NameAndTypeResolver::importInheritedScope(ContractDefinition const& _base) for (auto const& nameAndDeclaration: iterator->second.getDeclarations()) { Declaration const* declaration = nameAndDeclaration.second; - // Import if it was declared in the base and is not the constructor - if (declaration->getScope() == &_base && declaration->getName() != _base.getName()) + // Import if it was declared in the base, is not the constructor and is visible in derived classes + if (declaration->getScope() == &_base && declaration->getName() != _base.getName() && + declaration->isVisibleInDerivedContracts()) m_currentScope->registerDeclaration(*declaration); } } @@ -308,7 +309,7 @@ void DeclarationRegistrationHelper::closeCurrentScope() void DeclarationRegistrationHelper::registerDeclaration(Declaration& _declaration, bool _opensScope) { - if (!m_scopes[m_currentScope].registerDeclaration(_declaration)) + if (!m_scopes[m_currentScope].registerDeclaration(_declaration, !_declaration.isVisibleInContract())) BOOST_THROW_EXCEPTION(DeclarationError() << errinfo_sourceLocation(_declaration.getLocation()) << errinfo_comment("Identifier already declared.")); //@todo the exception should also contain the location of the first declaration diff --git a/libsolidity/Parser.cpp b/libsolidity/Parser.cpp index c96593f64..cf57ca50e 100644 --- a/libsolidity/Parser.cpp +++ b/libsolidity/Parser.cpp @@ -190,6 +190,8 @@ Declaration::Visibility Parser::parseVisibilitySpecifier(Token::Value _token) visibility = Declaration::Visibility::Protected; else if (_token == Token::Private) visibility = Declaration::Visibility::Private; + else if (_token == Token::External) + visibility = Declaration::Visibility::External; else solAssert(false, "Invalid visibility specifier."); m_scanner->next(); @@ -306,7 +308,7 @@ ASTPointer Parser::parseVariableDeclaration(VarDeclParserOp ASTPointer identifier; Token::Value token = m_scanner->getCurrentToken(); Declaration::Visibility visibility(Declaration::Visibility::Default); - if (_options.isStateVariable && Token::isVisibilitySpecifier(token)) + if (_options.isStateVariable && Token::isVariableVisibilitySpecifier(token)) visibility = parseVisibilitySpecifier(token); if (_options.allowIndexed && token == Token::Indexed) { diff --git a/libsolidity/Token.h b/libsolidity/Token.h index 3e599a6ee..7bf8a7ef0 100644 --- a/libsolidity/Token.h +++ b/libsolidity/Token.h @@ -150,6 +150,7 @@ namespace solidity K(Do, "do", 0) \ K(Else, "else", 0) \ K(Event, "event", 0) \ + K(External, "external", 0) \ K(Is, "is", 0) \ K(Indexed, "indexed", 0) \ K(For, "for", 0) \ @@ -378,7 +379,8 @@ public: static bool isUnaryOp(Value op) { return (Not <= op && op <= Delete) || op == Add || op == Sub; } static bool isCountOp(Value op) { return op == Inc || op == Dec; } static bool isShiftOp(Value op) { return (SHL <= op) && (op <= SHR); } - static bool isVisibilitySpecifier(Value op) { return op == Public || op == Private || op == Protected; } + static bool isVisibilitySpecifier(Value op) { return isVariableVisibilitySpecifier(op) || op == External; } + static bool isVariableVisibilitySpecifier(Value op) { return op == Public || op == Private || op == Protected; } static bool isEtherSubdenomination(Value op) { return op == SubWei || op == SubSzabo || op == SubFinney || op == Token::SubEther; } // Returns a string corresponding to the JS token string diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index b41834d84..d2f0e9bfd 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -540,12 +540,19 @@ bool ByteArrayType::operator==(Type const& _other) const unsigned ByteArrayType::getSizeOnStack() const { if (m_location == Location::CallData) - return 0; + // offset, length (stack top) + return 2; else + // offset return 1; } -const MemberList ByteArrayType::s_byteArrayMemberList = MemberList({{"length", make_shared(256)}}); +shared_ptr ByteArrayType::copyForLocation(ByteArrayType::Location _location) const +{ + return make_shared(_location); +} + +const MemberList ByteArrayType::s_byteArrayMemberList = MemberList({{"length", make_shared(256)}}); bool ContractType::operator==(Type const& _other) const { @@ -572,7 +579,8 @@ MemberList const& ContractType::getMembers() const { for (ContractDefinition const* base: m_contract.getLinearizedBaseContracts()) for (ASTPointer const& function: base->getDefinedFunctions()) - if (!function->isConstructor() && !function->getName().empty()) + if (!function->isConstructor() && !function->getName().empty() && + function->isVisibleInDerivedContracts()) members.insert(make_pair(function->getName(), make_shared(*function, true))); } else @@ -957,10 +965,10 @@ MemberList const& TypeType::getMembers() const ContractDefinition const& contract = dynamic_cast(*m_actualType).getContractDefinition(); 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 private + // 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()) + if (!f->isConstructor() && !f->getName().empty() && f->isVisibleInDerivedContracts()) members[f->getName()] = make_shared(*f); } else if (m_actualType->getCategory() == Category::Enum) diff --git a/libsolidity/Types.h b/libsolidity/Types.h index 3b4eee57f..b66857f0c 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -122,6 +122,8 @@ public: /// is not a simple big-endian encoding or the type cannot be stored in calldata. /// Note that irrespective of this size, each calldata element is padded to a multiple of 32 bytes. virtual unsigned getCalldataEncodedSize() const { return 0; } + /// @returns true if the type is dynamically encoded in calldata + virtual bool isDynamicallySized() const { return false; } /// @returns number of bytes required to hold this value in storage. /// For dynamically "allocated" types, it returns the size of the statically allocated head, virtual u256 getStorageSize() const { return 1; } @@ -289,12 +291,17 @@ public: virtual bool isImplicitlyConvertibleTo(Type const& _convertTo) const override; virtual TypePointer unaryOperatorResult(Token::Value _operator) const override; virtual bool operator==(const Type& _other) const override; + virtual bool isDynamicallySized() const { return true; } virtual unsigned getSizeOnStack() const override; virtual std::string toString() const override { return "bytes"; } virtual MemberList const& getMembers() const override { return s_byteArrayMemberList; } Location getLocation() const { return m_location; } + /// @returns a copy of this type with location changed to @a _location + /// @todo this might move as far up as Type later + std::shared_ptr copyForLocation(Location _location) const; + private: Location m_location; static const MemberList s_byteArrayMemberList; diff --git a/test/SolidityCompiler.cpp b/test/SolidityCompiler.cpp index 17d9a7c07..1369b038f 100644 --- a/test/SolidityCompiler.cpp +++ b/test/SolidityCompiler.cpp @@ -96,7 +96,7 @@ BOOST_AUTO_TEST_CASE(smoke_test) "}\n"; bytes code = compileContract(sourceCode); - unsigned boilerplateSize = 69; + unsigned boilerplateSize = 70; bytes expectation({byte(Instruction::JUMPDEST), byte(Instruction::PUSH1), 0x0, // initialize local variable x byte(Instruction::PUSH1), 0x2, @@ -114,8 +114,8 @@ BOOST_AUTO_TEST_CASE(ifStatement) " function f() { bool x; if (x) 77; else if (!x) 78; else 79; }" "}\n"; bytes code = compileContract(sourceCode); - unsigned shift = 56; - unsigned boilerplateSize = 69; + unsigned shift = 57; + unsigned boilerplateSize = 70; bytes expectation({byte(Instruction::JUMPDEST), byte(Instruction::PUSH1), 0x0, byte(Instruction::DUP1), @@ -155,8 +155,8 @@ BOOST_AUTO_TEST_CASE(loops) " function f() { while(true){1;break;2;continue;3;return;4;} }" "}\n"; bytes code = compileContract(sourceCode); - unsigned shift = 56; - unsigned boilerplateSize = 69; + unsigned shift = 57; + unsigned boilerplateSize = 70; bytes expectation({byte(Instruction::JUMPDEST), byte(Instruction::JUMPDEST), byte(Instruction::PUSH1), 0x1, diff --git a/test/SolidityEndToEndTest.cpp b/test/SolidityEndToEndTest.cpp index 0325c4c6a..103b11269 100644 --- a/test/SolidityEndToEndTest.cpp +++ b/test/SolidityEndToEndTest.cpp @@ -2283,9 +2283,9 @@ BOOST_AUTO_TEST_CASE(bytes_from_calldata_to_memory) } )"; compileAndRun(sourceCode); - bytes calldata = bytes(61, 0x22) + bytes(12, 0x12); - sendMessage(calldata, false); - BOOST_CHECK(m_output == encodeArgs(dev::sha3(bytes{'a', 'b', 'c'} + calldata))); + bytes calldata1 = bytes(61, 0x22) + bytes(12, 0x12); + sendMessage(calldata1, false); + BOOST_CHECK(m_output == encodeArgs(dev::sha3(bytes{'a', 'b', 'c'} + calldata1))); } BOOST_AUTO_TEST_CASE(call_forward_bytes) @@ -2534,6 +2534,49 @@ BOOST_AUTO_TEST_CASE(constructing_enums_from_ints) BOOST_CHECK(callContractFunction("test()") == encodeArgs(1)); } +BOOST_AUTO_TEST_CASE(external_function) +{ + char const* sourceCode = R"( + contract c { + function f(uint a) returns (uint) { return a; } + function test(uint a, uint b) external returns (uint r_a, uint r_b) { + r_a = f(a + 7); + r_b = b; + } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("test(uint256,uint256)", 2, 3) == encodeArgs(2+7, 3)); +} + +BOOST_AUTO_TEST_CASE(bytes_in_arguments) +{ + char const* sourceCode = R"( + contract c { + uint result; + function f(uint a, uint b) { result += a + b; } + function g(uint a) { result *= a; } + function test(uint a, bytes data1, bytes data2, uint b) external returns (uint r_a, uint r, uint r_b, uint l) { + r_a = a; + this.call(data1); + this.call(data2); + r = result; + r_b = b; + l = data1.length; + } + } + )"; + compileAndRun(sourceCode); + string innercalldata1 = asString(FixedHash<4>(dev::sha3("f(uint256,uint256)")).asBytes() + encodeArgs(8, 9)); + bytes calldata1 = encodeArgs(u256(innercalldata1.length()), 12, innercalldata1, 13); + string innercalldata2 = asString(FixedHash<4>(dev::sha3("g(uint256)")).asBytes() + encodeArgs(3)); + bytes calldata = encodeArgs( + u256(innercalldata1.length()), u256(innercalldata2.length()), + 12, innercalldata1, innercalldata2, 13); + BOOST_CHECK(callContractFunction("test(uint256,bytes,bytes,uint256)", calldata) + == encodeArgs(12, (8 + 9) * 3, 13, u256(innercalldata1.length()))); +} + BOOST_AUTO_TEST_SUITE_END() } diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index d6e4ed516..6b337ac74 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -1083,6 +1083,86 @@ BOOST_AUTO_TEST_CASE(enum_duplicate_values) BOOST_CHECK_THROW(parseTextAndResolveNames(text), DeclarationError); } +BOOST_AUTO_TEST_CASE(private_visibility) +{ + char const* sourceCode = R"( + contract base { + function f() private {} + } + contract derived is base { + function g() { f(); } + } + )"; + BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), DeclarationError); +} + +BOOST_AUTO_TEST_CASE(private_visibility_via_explicit_base_access) +{ + char const* sourceCode = R"( + contract base { + function f() private {} + } + contract derived is base { + function g() { base.f(); } + } + )"; + BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError); +} + +BOOST_AUTO_TEST_CASE(external_visibility) +{ + char const* sourceCode = R"( + contract c { + function f() external {} + function g() { f(); } + } + )"; + BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), DeclarationError); +} + +BOOST_AUTO_TEST_CASE(external_base_visibility) +{ + char const* sourceCode = R"( + contract base { + function f() external {} + } + contract derived is base { + function g() { base.f(); } + } + )"; + BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError); +} + +BOOST_AUTO_TEST_CASE(external_argument_assign) +{ + char const* sourceCode = R"( + contract c { + function f(uint a) external { a = 1; } + } + )"; + BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError); +} + +BOOST_AUTO_TEST_CASE(external_argument_increment) +{ + char const* sourceCode = R"( + contract c { + function f(uint a) external { a++; } + } + )"; + BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError); +} + +BOOST_AUTO_TEST_CASE(external_argument_delete) +{ + char const* sourceCode = R"( + contract c { + function f(uint a) external { delete a; } + } + )"; + BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError); +} + BOOST_AUTO_TEST_SUITE_END() } diff --git a/test/SolidityParser.cpp b/test/SolidityParser.cpp index af82f612a..5f9064e0c 100644 --- a/test/SolidityParser.cpp +++ b/test/SolidityParser.cpp @@ -735,6 +735,24 @@ BOOST_AUTO_TEST_CASE(malformed_enum_declaration) BOOST_CHECK_THROW(parseText(text), ParserError); } +BOOST_AUTO_TEST_CASE(external_function) +{ + char const* text = R"( + contract c { + function x() external {} + })"; + BOOST_CHECK_NO_THROW(parseTextExplainError(text)); +} + +BOOST_AUTO_TEST_CASE(external_variable) +{ + char const* text = R"( + contract c { + uint external x; + })"; + BOOST_CHECK_THROW(parseText(text), ParserError); +} + BOOST_AUTO_TEST_SUITE_END() }