From 069b150e42d12f3f3736dd4af2d82881db244b66 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 13 Mar 2018 16:50:44 +0100 Subject: Bugfix in virtual lookup for modifiers in libraries. --- libsolidity/codegen/CompilerContext.cpp | 14 +++++++++++--- libsolidity/codegen/CompilerContext.h | 2 +- libsolidity/codegen/ContractCompiler.cpp | 5 ++++- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp index 0bf88267..47333046 100644 --- a/libsolidity/codegen/CompilerContext.cpp +++ b/libsolidity/codegen/CompilerContext.cpp @@ -193,14 +193,22 @@ Declaration const* CompilerContext::nextFunctionToCompile() const return m_functionCompilationQueue.nextFunctionToCompile(); } -ModifierDefinition const& CompilerContext::functionModifier(string const& _name) const +ModifierDefinition const& CompilerContext::resolveVirtualFunctionModifier( + ModifierDefinition const& _modifier +) const { + // Libraries do not allow inheritance and their functions can be inlined, so we should not + // search the inheritance hierarchy (which will be the wrong one in case the function + // is inlined). + if (auto scope = dynamic_cast(_modifier.scope())) + if (scope->isLibrary()) + return _modifier; solAssert(!m_inheritanceHierarchy.empty(), "No inheritance hierarchy set."); for (ContractDefinition const* contract: m_inheritanceHierarchy) for (ModifierDefinition const* modifier: contract->functionModifiers()) - if (modifier->name() == _name) + if (modifier->name() == _modifier.name()) return *modifier; - solAssert(false, "Function modifier " + _name + " not found."); + solAssert(false, "Function modifier " + _modifier.name() + " not found in inheritance hierarchy."); } unsigned CompilerContext::baseStackOffsetOfVariable(Declaration const& _declaration) const diff --git a/libsolidity/codegen/CompilerContext.h b/libsolidity/codegen/CompilerContext.h index cf626683..7b663277 100644 --- a/libsolidity/codegen/CompilerContext.h +++ b/libsolidity/codegen/CompilerContext.h @@ -130,7 +130,7 @@ public: void appendMissingLowLevelFunctions(); ABIFunctions& abiFunctions() { return m_abiFunctions; } - ModifierDefinition const& functionModifier(std::string const& _name) const; + ModifierDefinition const& resolveVirtualFunctionModifier(ModifierDefinition const& _modifier) const; /// Returns the distance of the given local variable from the bottom of the stack (of the current function). unsigned baseStackOffsetOfVariable(Declaration const& _declaration) const; /// If supplied by a value returned by @ref baseStackOffsetOfVariable(variable), returns diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 5a9498f0..95d6c8b5 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -1002,7 +1002,10 @@ void ContractCompiler::appendModifierOrFunctionCode() appendModifierOrFunctionCode(); else { - ModifierDefinition const& modifier = m_context.functionModifier(modifierInvocation->name()->name()); + ModifierDefinition const& nonVirtualModifier = dynamic_cast( + *modifierInvocation->name()->annotation().referencedDeclaration + ); + ModifierDefinition const& modifier = m_context.resolveVirtualFunctionModifier(nonVirtualModifier); CompilerContext::LocationSetter locationSetter(m_context, modifier); solAssert(modifier.parameters().size() == modifierInvocation->arguments().size(), ""); for (unsigned i = 0; i < modifier.parameters().size(); ++i) -- cgit From 58af150c3dce26206825122d048e4e0732fcb50f Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 13 Mar 2018 16:50:27 +0100 Subject: Changelog entry. --- Changelog.md | 1 + .../syntaxTests/virtualLookup/modifiers_in_libraries.sol | 14 ++++++++++++++ 2 files changed, 15 insertions(+) create mode 100644 test/libsolidity/syntaxTests/virtualLookup/modifiers_in_libraries.sol diff --git a/Changelog.md b/Changelog.md index 648af66c..2e950f1d 100644 --- a/Changelog.md +++ b/Changelog.md @@ -4,6 +4,7 @@ Features: Bugfixes: * Code Generator: Properly skip unneeded storgae array cleanup when not reducing length. + * Code Generator: Bugfix in modifier lookup in libraries. * Commandline interface: Support ``--evm-version constantinople`` properly. * Standard JSON: Support ``constantinople`` as ``evmVersion`` properly. diff --git a/test/libsolidity/syntaxTests/virtualLookup/modifiers_in_libraries.sol b/test/libsolidity/syntaxTests/virtualLookup/modifiers_in_libraries.sol new file mode 100644 index 00000000..b033fd0c --- /dev/null +++ b/test/libsolidity/syntaxTests/virtualLookup/modifiers_in_libraries.sol @@ -0,0 +1,14 @@ +library WithModifier { + modifier mod() { require(msg.value > 10 ether); _; } + function withMod(uint self) mod() internal view { require(self > 0); } +} + +contract Test { + using WithModifier for *; + + function f(uint _value) public payable { + _value.withMod(); + WithModifier.withMod(_value); + } +} +// ---- -- cgit From 51f9e350b152483968391757155189ed0880c755 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 13 Mar 2018 16:50:38 +0100 Subject: Tests. --- test/libsolidity/SolidityEndToEndTest.cpp | 52 +++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 33cd1419..282136f7 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -2879,6 +2879,58 @@ BOOST_AUTO_TEST_CASE(function_modifier_multiple_times_local_vars) ABI_CHECK(callContractFunction("a()"), encodeArgs(0)); } +BOOST_AUTO_TEST_CASE(function_modifier_library) +{ + char const* sourceCode = R"( + library L { + struct S { uint v; } + modifier mod(S storage s) { s.v++; _; } + function libFun(S storage s) mod(s) internal { s.v += 0x100; } + } + + contract Test { + using L for *; + L.S s; + + function f() public returns (uint) { + s.libFun(); + L.libFun(s); + return s.v; + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("f()"), encodeArgs(0x202)); +} + +BOOST_AUTO_TEST_CASE(function_modifier_library_inheritance) +{ + // Tests that virtual lookup for modifiers in libraries does not consider + // the current inheritance hierarchy. + + char const* sourceCode = R"( + library L { + struct S { uint v; } + modifier mod(S storage s) { s.v++; _; } + function libFun(S storage s) mod(s) internal { s.v += 0x100; } + } + + contract Test { + using L for *; + L.S s; + modifier mod(L.S storage) { revert(); _; } + + function f() public returns (uint) { + s.libFun(); + L.libFun(s); + return s.v; + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("f()"), encodeArgs(0x202)); +} + BOOST_AUTO_TEST_CASE(crazy_elementary_typenames_on_stack) { char const* sourceCode = R"( -- cgit