From ed0384b75922cd4834bc1a77dd5caf81c03d6dbd Mon Sep 17 00:00:00 2001 From: Christian Date: Sat, 14 Feb 2015 01:22:44 +0100 Subject: [PATCH] No write access to parameters of external functions. --- libsolidity/AST.cpp | 28 ++++++++++++++++-------- libsolidity/AST.h | 16 ++++++-------- libsolidity/ASTJsonConverter.cpp | 17 ++++++--------- libsolidity/Compiler.cpp | 23 ++++++++++++++------ test/SolidityEndToEndTest.cpp | 15 +++++++++++++ test/SolidityNameAndTypeResolution.cpp | 30 ++++++++++++++++++++++++++ 6 files changed, 95 insertions(+), 34 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 0dbad433f..0fafd2d12 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -288,12 +288,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 +597,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 +609,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 c40e24d1d..af45934f7 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -132,7 +132,6 @@ private: class Declaration: public ASTNode { public: - enum class LValueType { None, Local, Storage }; /// Visibility ordered from restricted to unrestricted. enum class Visibility { Default, Private, Protected, Public, External }; @@ -156,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; } @@ -448,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; } @@ -887,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. @@ -903,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..4dbcbbfe6 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -255,12 +255,22 @@ bool Compiler::visit(FunctionDefinition const& _function) // 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); - for (ASTPointer const& variable: _function.getParameters()) + if (_function.getVisibility() != Declaration::Visibility::External) { - m_context.addVariable(*variable, parametersSize); - parametersSize -= variable->getType()->getSizeOnStack(); + unsigned parametersSize = CompilerUtils::getSizeOnStack(_function.getParameters()); + m_context.adjustStackOffset(parametersSize); + for (ASTPointer const& variable: _function.getParameters()) + { + m_context.addVariable(*variable, parametersSize); + parametersSize -= variable->getType()->getSizeOnStack(); + } + } + else + { + unsigned calldataPos = CompilerUtils::dataStartOffset; + // calldatapos is _always_ dynamic. + for (ASTPointer const& variable: _function.getParameters()) + m_context.addCalldataVariable(*variable, calldataPos); } for (ASTPointer const& variable: _function.getReturnParameters()) m_context.addAndInitializeVariable(*variable); @@ -277,7 +287,8 @@ bool Compiler::visit(FunctionDefinition const& _function) // Note that the fact that the return arguments are of increasing index is vital for this // algorithm to work. - unsigned const c_argumentsSize = CompilerUtils::getSizeOnStack(_function.getParameters()); + unsigned const c_argumentsSize = (_function.getVisibility() == Declaration::Visibility::External + ? 0 : CompilerUtils::getSizeOnStack(_function.getParameters())); unsigned const c_returnValuesSize = CompilerUtils::getSizeOnStack(_function.getReturnParameters()); unsigned const c_localVariablesSize = CompilerUtils::getSizeOnStack(_function.getLocalVariables()); diff --git a/test/SolidityEndToEndTest.cpp b/test/SolidityEndToEndTest.cpp index 0325c4c6a..9899af29f 100644 --- a/test/SolidityEndToEndTest.cpp +++ b/test/SolidityEndToEndTest.cpp @@ -2534,6 +2534,21 @@ 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, 3+7)); +} + BOOST_AUTO_TEST_SUITE_END() } diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index f30de96ce..6b337ac74 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -1133,6 +1133,36 @@ BOOST_AUTO_TEST_CASE(external_base_visibility) 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() }