From f8f50e14d24a85da2775662698db54a73905bd44 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 8 Mar 2018 11:17:31 +0100 Subject: Test that internal functions only used by constructor are not included in runtime context. --- test/libsolidity/SolidityCompiler.cpp | 60 +++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 test/libsolidity/SolidityCompiler.cpp diff --git a/test/libsolidity/SolidityCompiler.cpp b/test/libsolidity/SolidityCompiler.cpp new file mode 100644 index 00000000..e87ab603 --- /dev/null +++ b/test/libsolidity/SolidityCompiler.cpp @@ -0,0 +1,60 @@ +/* + This file is part of solidity. + + solidity 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. + + solidity 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 solidity. If not, see . + */ +/** + * Unit tests for the compiler itself. + */ + +#include + +#include + +using namespace std; + +namespace dev +{ +namespace solidity +{ +namespace test +{ + +BOOST_FIXTURE_TEST_SUITE(Compiler, AnalysisFramework) + +BOOST_AUTO_TEST_CASE(does_not_include_creation_time_only_internal_functions) +{ + char const* sourceCode = R"( + contract C { + uint x; + function C() { f(); } + function f() internal { for (uint i = 0; i < 10; ++i) x += 3 + i; } + } + )"; + BOOST_REQUIRE(success(sourceCode)); + m_compiler.setOptimiserSettings(dev::test::Options::get().optimize); + BOOST_REQUIRE_MESSAGE(m_compiler.compile(), "Compiling contract failed"); + bytes const& creationBytecode = m_compiler.object("C").bytecode; + bytes const& runtimeBytecode = m_compiler.runtimeObject("C").bytecode; + BOOST_CHECK(creationBytecode.size() >= 120); + BOOST_CHECK(creationBytecode.size() <= 150); + BOOST_CHECK(runtimeBytecode.size() >= 50); + BOOST_CHECK(runtimeBytecode.size() <= 70); +} + +BOOST_AUTO_TEST_SUITE_END() + +} +} +} -- cgit From 0a67d616db73c912fd16186f97be6ff2d9447975 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 8 Mar 2018 11:17:08 +0100 Subject: Use shortcut for internal function calls to avoid runtime reference. --- libsolidity/codegen/ExpressionCompiler.cpp | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index 37069c3e..f4ca9dff 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -518,7 +518,25 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) arguments[i]->accept(*this); utils().convertType(*arguments[i]->annotation().type, *function.parameterTypes()[i]); } - _functionCall.expression().accept(*this); + + { + bool shortcutTaken = false; + if (auto identifier = dynamic_cast(&_functionCall.expression())) + if (auto functionDef = dynamic_cast(identifier->annotation().referencedDeclaration)) + { + // Do not directly visit the identifier, because this way, we can avoid + // the runtime entry label to be created at the creation time context. + CompilerContext::LocationSetter locationSetter2(m_context, *identifier); + m_context << m_context.functionEntryLabel(m_context.resolveVirtualFunction(*functionDef)).pushTag(); + if (m_context.runtimeContext()) + utils().leftShiftNumberOnStack(32); + shortcutTaken = true; + } + + if (!shortcutTaken) + _functionCall.expression().accept(*this); + } + unsigned parameterSize = CompilerUtils::sizeOnStack(function.parameterTypes()); if (function.bound()) { @@ -1359,6 +1377,10 @@ void ExpressionCompiler::endVisit(Identifier const& _identifier) } } else if (FunctionDefinition const* functionDef = dynamic_cast(declaration)) + // If the identifier is called right away, this code is executed in visit(FunctionCall...), because + // we want to avoid having a reference to the runtime function entry point in the + // constructor context, since this would force the compiler to include unreferenced + // internal functions in the runtime contex. utils().pushCombinedFunctionEntryLabel(m_context.resolveVirtualFunction(*functionDef)); else if (auto variable = dynamic_cast(declaration)) appendVariable(*variable, static_cast(_identifier)); -- cgit From 0a58e57ceb2af6b7369742152ea3c4332e7585fb Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 8 Mar 2018 11:18:47 +0100 Subject: Changelog entry. --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index c5577b86..aa1554f5 100644 --- a/Changelog.md +++ b/Changelog.md @@ -5,6 +5,7 @@ Features: Bugfixes: * Code Generator: Allow ``block.blockhash`` without being called. + * Code Generator: Do not include internal functions in the runtime bytecode which are only referenced in the constructor. * Code Generator: Properly skip unneeded storage array cleanup when not reducing length. * Code Generator: Bugfix in modifier lookup in libraries. * Commandline interface: Support ``--evm-version constantinople`` properly. -- cgit From fab527c4146f08971938c63074c8c92e6ff241bc Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Tue, 27 Mar 2018 03:39:37 +0100 Subject: Add runtimeOnly option to pushCombinedFunctionEntryLabel --- libsolidity/codegen/CompilerUtils.cpp | 9 +++++---- libsolidity/codegen/CompilerUtils.h | 3 ++- libsolidity/codegen/ExpressionCompiler.cpp | 4 +--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp index 1bd1103b..68f0b3a1 100644 --- a/libsolidity/codegen/CompilerUtils.cpp +++ b/libsolidity/codegen/CompilerUtils.cpp @@ -585,7 +585,7 @@ void CompilerUtils::combineExternalFunctionType(bool _leftAligned) leftShiftNumberOnStack(64); } -void CompilerUtils::pushCombinedFunctionEntryLabel(Declaration const& _function) +void CompilerUtils::pushCombinedFunctionEntryLabel(Declaration const& _function, bool _runtimeOnly) { m_context << m_context.functionEntryLabel(_function).pushTag(); // If there is a runtime context, we have to merge both labels into the same @@ -593,9 +593,10 @@ void CompilerUtils::pushCombinedFunctionEntryLabel(Declaration const& _function) if (CompilerContext* rtc = m_context.runtimeContext()) { leftShiftNumberOnStack(32); - m_context << - rtc->functionEntryLabel(_function).toSubAssemblyTag(m_context.runtimeSub()) << - Instruction::OR; + if (_runtimeOnly) + m_context << + rtc->functionEntryLabel(_function).toSubAssemblyTag(m_context.runtimeSub()) << + Instruction::OR; } } diff --git a/libsolidity/codegen/CompilerUtils.h b/libsolidity/codegen/CompilerUtils.h index 76670d47..389673ef 100644 --- a/libsolidity/codegen/CompilerUtils.h +++ b/libsolidity/codegen/CompilerUtils.h @@ -188,7 +188,8 @@ public: /// Appends code that combines the construction-time (if available) and runtime function /// entry label of the given function into a single stack slot. /// Note: This might cause the compilation queue of the runtime context to be extended. - void pushCombinedFunctionEntryLabel(Declaration const& _function); + /// If @a _runtimeOnly, the entry label will include the runtime assembly tag. + void pushCombinedFunctionEntryLabel(Declaration const& _function, bool _runtimeOnly = true); /// Appends code for an implicit or explicit type conversion. This includes erasing higher /// order bits (@see appendHighBitCleanup) when widening integer but also copy to memory diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index f4ca9dff..9e2d30d5 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -527,9 +527,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) // Do not directly visit the identifier, because this way, we can avoid // the runtime entry label to be created at the creation time context. CompilerContext::LocationSetter locationSetter2(m_context, *identifier); - m_context << m_context.functionEntryLabel(m_context.resolveVirtualFunction(*functionDef)).pushTag(); - if (m_context.runtimeContext()) - utils().leftShiftNumberOnStack(32); + utils().pushCombinedFunctionEntryLabel(m_context.resolveVirtualFunction(*functionDef), false); shortcutTaken = true; } -- cgit