From df42afec64684ef9c2fd0eef69dd1e2ebaf28980 Mon Sep 17 00:00:00 2001 From: wanderer Date: Mon, 17 Nov 2014 15:27:30 -0500 Subject: [PATCH 1/3] added filler.json --- test/recursiveCreateFiller.json | 35 +++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 test/recursiveCreateFiller.json diff --git a/test/recursiveCreateFiller.json b/test/recursiveCreateFiller.json new file mode 100644 index 000000000..0d18fb7a5 --- /dev/null +++ b/test/recursiveCreateFiller.json @@ -0,0 +1,35 @@ +{ + "recursiveCreate": { + "env": { + "previousHash": "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", + "currentNumber": "0", + "currentGasLimit": "10000000", + "currentDifficulty": "256", + "currentTimestamp": 1, + "currentCoinbase": "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba" + }, + "pre": { + "095e7baea6a6c7c4c2dfeb977efac326af552d87": { + "balance": "20000000", + "nonce": 0, + "code": "{(CODECOPY 0 0 32)(CREATE 0 0 32)}", + "storage": {} + }, + "a94f5374fce5edbc8e2a8697c15331677e6ebf0b": { + "balance": "1000000000000000000", + "nonce": 0, + "code": "", + "storage": {} + } + }, + "transaction": { + "nonce": "0", + "gasPrice": "1", + "gasLimit": "465224", + "to": "095e7baea6a6c7c4c2dfeb977efac326af552d87", + "value": "100000", + "secretKey": "45a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8", + "data": "" + } + } +} From 732e5c2b0fed646e0d7e7a03af2da8eb0238f7f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Wed, 19 Nov 2014 13:13:19 +0100 Subject: [PATCH 2/3] In VM tests, check only if an exception occurred if an exception expected (no post state and output checking) --- test/TestHelper.cpp | 1 + test/vm.cpp | 77 ++++++++++++++++++++++++++------------------- 2 files changed, 45 insertions(+), 33 deletions(-) diff --git a/test/TestHelper.cpp b/test/TestHelper.cpp index 1b13f9e82..114399a49 100644 --- a/test/TestHelper.cpp +++ b/test/TestHelper.cpp @@ -374,6 +374,7 @@ void executeTests(const string& _name, const string& _testPathAppendix, std::fun { BOOST_ERROR("Failed test with Exception: " << _e.what()); } + break; } } diff --git a/test/vm.cpp b/test/vm.cpp index cacbf94cc..bdbe8155d 100644 --- a/test/vm.cpp +++ b/test/vm.cpp @@ -300,6 +300,7 @@ void doVMTests(json_spirit::mValue& v, bool _fillin) VM vm(fev.gas); u256 gas; + bool vmExceptionOccured = false; try { output = vm.go(fev, fev.simpleTrace()).toVector(); @@ -308,7 +309,7 @@ void doVMTests(json_spirit::mValue& v, bool _fillin) catch (VMException const& _e) { cnote << "VM did throw an exception: " << diagnostic_information(_e); - gas = 0; + vmExceptionOccured = true; } catch (Exception const& _e) { @@ -342,48 +343,58 @@ void doVMTests(json_spirit::mValue& v, bool _fillin) { o["env"] = mValue(fev.exportEnv()); o["exec"] = mValue(fev.exportExec()); - o["post"] = mValue(fev.exportState()); - o["callcreates"] = fev.exportCallCreates(); - o["out"] = "0x" + toHex(output); - fev.push(o, "gas", gas); + if (!vmExceptionOccured) + { + o["post"] = mValue(fev.exportState()); + o["callcreates"] = fev.exportCallCreates(); + o["out"] = "0x" + toHex(output); + fev.push(o, "gas", gas); + } } else { - BOOST_REQUIRE(o.count("post") > 0); - BOOST_REQUIRE(o.count("callcreates") > 0); - BOOST_REQUIRE(o.count("out") > 0); - BOOST_REQUIRE(o.count("gas") > 0); + if (o.count("post") > 0) // No exceptions expected + { + BOOST_CHECK(!vmExceptionOccured); - dev::test::FakeExtVM test; - test.importState(o["post"].get_obj()); - test.importCallCreates(o["callcreates"].get_array()); + BOOST_REQUIRE(o.count("post") > 0); + BOOST_REQUIRE(o.count("callcreates") > 0); + BOOST_REQUIRE(o.count("out") > 0); + BOOST_REQUIRE(o.count("gas") > 0); - checkOutput(output, o); + dev::test::FakeExtVM test; + test.importState(o["post"].get_obj()); + test.importCallCreates(o["callcreates"].get_array()); - BOOST_CHECK_EQUAL(toInt(o["gas"]), gas); + checkOutput(output, o); - auto& expectedAddrs = test.addresses; - auto& resultAddrs = fev.addresses; - for (auto&& expectedPair : expectedAddrs) - { - auto& expectedAddr = expectedPair.first; - auto resultAddrIt = resultAddrs.find(expectedAddr); - if (resultAddrIt == resultAddrs.end()) - BOOST_ERROR("Missing expected address " << expectedAddr); - else - { - auto& expectedState = expectedPair.second; - auto& resultState = resultAddrIt->second; - BOOST_CHECK_MESSAGE(std::get<0>(expectedState) == std::get<0>(resultState), expectedAddr << ": incorrect balance " << std::get<0>(resultState) << ", expected " << std::get<0>(expectedState)); - BOOST_CHECK_MESSAGE(std::get<1>(expectedState) == std::get<1>(resultState), expectedAddr << ": incorrect txCount " << std::get<1>(resultState) << ", expected " << std::get<1>(expectedState)); - BOOST_CHECK_MESSAGE(std::get<3>(expectedState) == std::get<3>(resultState), expectedAddr << ": incorrect code"); + BOOST_CHECK_EQUAL(toInt(o["gas"]), gas); - checkStorage(std::get<2>(expectedState), std::get<2>(resultState), expectedAddr); + auto& expectedAddrs = test.addresses; + auto& resultAddrs = fev.addresses; + for (auto&& expectedPair : expectedAddrs) + { + auto& expectedAddr = expectedPair.first; + auto resultAddrIt = resultAddrs.find(expectedAddr); + if (resultAddrIt == resultAddrs.end()) + BOOST_ERROR("Missing expected address " << expectedAddr); + else + { + auto& expectedState = expectedPair.second; + auto& resultState = resultAddrIt->second; + BOOST_CHECK_MESSAGE(std::get<0>(expectedState) == std::get<0>(resultState), expectedAddr << ": incorrect balance " << std::get<0>(resultState) << ", expected " << std::get<0>(expectedState)); + BOOST_CHECK_MESSAGE(std::get<1>(expectedState) == std::get<1>(resultState), expectedAddr << ": incorrect txCount " << std::get<1>(resultState) << ", expected " << std::get<1>(expectedState)); + BOOST_CHECK_MESSAGE(std::get<3>(expectedState) == std::get<3>(resultState), expectedAddr << ": incorrect code"); + + checkStorage(std::get<2>(expectedState), std::get<2>(resultState), expectedAddr); + } } - } - checkAddresses, bytes> > >(test.addresses, fev.addresses); - BOOST_CHECK(test.callcreates == fev.callcreates); + checkAddresses, bytes> > >(test.addresses, fev.addresses); + BOOST_CHECK(test.callcreates == fev.callcreates); + } + else // Exception expected + BOOST_CHECK(vmExceptionOccured); } } } From c7972ba8509ffe7b21365555ce500ecbceef3197 Mon Sep 17 00:00:00 2001 From: Christian Date: Wed, 19 Nov 2014 10:24:22 +0100 Subject: [PATCH 3/3] Special handling for constructor. --- libsolidity/Compiler.cpp | 93 ++++++++++++++++++++++------------- libsolidity/Compiler.h | 13 +++-- test/solidityEndToEndTest.cpp | 22 +++++++++ 3 files changed, 90 insertions(+), 38 deletions(-) diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index eed886783..a3865bc30 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -43,42 +43,59 @@ void Compiler::compileContract(ContractDefinition& _contract) { m_context = CompilerContext(); // clear it just in case - //@todo constructor - for (ASTPointer const& function: _contract.getDefinedFunctions()) - m_context.addFunction(*function); - //@todo sort them? - for (ASTPointer const& variable: _contract.getStateVariables()) - m_context.addStateVariable(*variable); + if (function->getName() != _contract.getName()) // don't add the constructor here + m_context.addFunction(*function); + registerStateVariables(_contract); - appendFunctionSelector(_contract.getDefinedFunctions()); + appendFunctionSelector(_contract); for (ASTPointer const& function: _contract.getDefinedFunctions()) - function->accept(*this); + if (function->getName() != _contract.getName()) // don't add the constructor here + function->accept(*this); - packIntoContractCreator(); + packIntoContractCreator(_contract); } -void Compiler::packIntoContractCreator() +void Compiler::packIntoContractCreator(ContractDefinition const& _contract) { - CompilerContext creatorContext; - eth::AssemblyItem sub = creatorContext.addSubroutine(m_context.getAssembly()); + CompilerContext runtimeContext; + swap(m_context, runtimeContext); + + registerStateVariables(_contract); + + FunctionDefinition* constructor = nullptr; + for (ASTPointer const& function: _contract.getDefinedFunctions()) + if (function->getName() == _contract.getName()) + { + constructor = function.get(); + break; + } + if (constructor) + { + eth::AssemblyItem returnTag = m_context.pushNewTag(); + m_context.addFunction(*constructor); // note that it cannot be called due to syntactic reasons + //@todo copy constructor arguments from calldata to memory prior to this + //@todo calling other functions inside the constructor should either trigger a parse error + //or we should copy them here (register them above and call "accept") - detecting which + // functions are referenced / called needs to be done in a recursive way. + appendCalldataUnpacker(*constructor, true); + m_context.appendJumpTo(m_context.getFunctionEntryLabel(*constructor)); + constructor->accept(*this); + m_context << returnTag; + } + + eth::AssemblyItem sub = m_context.addSubroutine(runtimeContext.getAssembly()); // stack contains sub size - creatorContext << eth::Instruction::DUP1 << sub << u256(0) << eth::Instruction::CODECOPY; - creatorContext << u256(0) << eth::Instruction::RETURN; - swap(m_context, creatorContext); + m_context << eth::Instruction::DUP1 << sub << u256(0) << eth::Instruction::CODECOPY; + m_context << u256(0) << eth::Instruction::RETURN; } -void Compiler::appendFunctionSelector(vector> const& _functions) +void Compiler::appendFunctionSelector(ContractDefinition const& _contract) { - // sort all public functions and store them together with a tag for their argument decoding section - map> publicFunctions; - for (ASTPointer const& f: _functions) - if (f->isPublic()) - publicFunctions.insert(make_pair(f->getName(), make_pair(f.get(), m_context.newTag()))); + vector interfaceFunctions = _contract.getInterfaceFunctions(); + vector callDataUnpackerEntryPoints; - //@todo remove constructor - - if (publicFunctions.size() > 255) + if (interfaceFunctions.size() > 255) BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("More than 255 public functions for contract.")); // retrieve the first byte of the call data, which determines the called function @@ -90,21 +107,20 @@ void Compiler::appendFunctionSelector(vector> con << eth::dupInstruction(2); // stack here: 1 0 0, stack top will be counted up until it matches funid - for (pair> const& f: publicFunctions) + for (unsigned funid = 0; funid < interfaceFunctions.size(); ++funid) { - eth::AssemblyItem const& callDataUnpackerEntry = f.second.second; + callDataUnpackerEntryPoints.push_back(m_context.newTag()); m_context << eth::dupInstruction(2) << eth::dupInstruction(2) << eth::Instruction::EQ; - m_context.appendConditionalJumpTo(callDataUnpackerEntry); + m_context.appendConditionalJumpTo(callDataUnpackerEntryPoints.back()); m_context << eth::dupInstruction(4) << eth::Instruction::ADD; //@todo avoid the last ADD (or remove it in the optimizer) } m_context << eth::Instruction::STOP; // function not found - for (pair> const& f: publicFunctions) + for (unsigned funid = 0; funid < interfaceFunctions.size(); ++funid) { - FunctionDefinition const& function = *f.second.first; - eth::AssemblyItem const& callDataUnpackerEntry = f.second.second; - m_context << callDataUnpackerEntry; + FunctionDefinition const& function = *interfaceFunctions[funid]; + m_context << callDataUnpackerEntryPoints[funid]; eth::AssemblyItem returnTag = m_context.pushNewTag(); appendCalldataUnpacker(function); m_context.appendJumpTo(m_context.getFunctionEntryLabel(function)); @@ -113,10 +129,11 @@ void Compiler::appendFunctionSelector(vector> con } } -void Compiler::appendCalldataUnpacker(FunctionDefinition const& _function) +unsigned Compiler::appendCalldataUnpacker(FunctionDefinition const& _function, bool _fromMemory) { // We do not check the calldata size, everything is zero-padded. unsigned dataOffset = 1; + eth::Instruction load = _fromMemory ? eth::Instruction::MLOAD : eth::Instruction::CALLDATALOAD; //@todo this can be done more efficiently, saving some CALLDATALOAD calls for (ASTPointer const& var: _function.getParameters()) @@ -127,12 +144,13 @@ void Compiler::appendCalldataUnpacker(FunctionDefinition const& _function) << errinfo_sourceLocation(var->getLocation()) << errinfo_comment("Type " + var->getType()->toString() + " not yet supported.")); if (numBytes == 32) - m_context << u256(dataOffset) << eth::Instruction::CALLDATALOAD; + m_context << u256(dataOffset) << load; else m_context << (u256(1) << ((32 - numBytes) * 8)) << u256(dataOffset) - << eth::Instruction::CALLDATALOAD << eth::Instruction::DIV; + << load << eth::Instruction::DIV; dataOffset += numBytes; } + return dataOffset; } void Compiler::appendReturnValuePacker(FunctionDefinition const& _function) @@ -158,6 +176,13 @@ void Compiler::appendReturnValuePacker(FunctionDefinition const& _function) m_context << u256(dataOffset) << u256(0) << eth::Instruction::RETURN; } +void Compiler::registerStateVariables(ContractDefinition const& _contract) +{ + //@todo sort them? + for (ASTPointer const& variable: _contract.getStateVariables()) + m_context.addStateVariable(*variable); +} + bool Compiler::visit(FunctionDefinition& _function) { //@todo to simplify this, the calling convention could by changed such that diff --git a/libsolidity/Compiler.h b/libsolidity/Compiler.h index d931f5359..3887d3b5b 100644 --- a/libsolidity/Compiler.h +++ b/libsolidity/Compiler.h @@ -40,12 +40,17 @@ public: static bytes compile(ContractDefinition& _contract, bool _optimize); private: - /// Creates a new compiler context / assembly and packs the current code into the data part. - void packIntoContractCreator(); - void appendFunctionSelector(std::vector > const& _functions); - void appendCalldataUnpacker(FunctionDefinition const& _function); + /// Creates a new compiler context / assembly, packs the current code into the data part and + /// adds the constructor code. + void packIntoContractCreator(ContractDefinition const& _contract); + void appendFunctionSelector(ContractDefinition const& _contract); + /// Creates code that unpacks the arguments for the given function, from memory if + /// @a _fromMemory is true, otherwise from call data. @returns the size of the data in bytes. + unsigned appendCalldataUnpacker(FunctionDefinition const& _function, bool _fromMemory = false); void appendReturnValuePacker(FunctionDefinition const& _function); + void registerStateVariables(ContractDefinition const& _contract); + virtual bool visit(FunctionDefinition& _function) override; virtual bool visit(IfStatement& _ifStatement) override; virtual bool visit(WhileStatement& _whileStatement) override; diff --git a/test/solidityEndToEndTest.cpp b/test/solidityEndToEndTest.cpp index 617cbabc9..4e68103ac 100644 --- a/test/solidityEndToEndTest.cpp +++ b/test/solidityEndToEndTest.cpp @@ -700,6 +700,28 @@ BOOST_AUTO_TEST_CASE(structs) BOOST_CHECK(callContractFunction(0) == bytes({0x01})); } +BOOST_AUTO_TEST_CASE(constructor) +{ + char const* sourceCode = "contract test {\n" + " mapping(uint => uint) data;\n" + " function test() {\n" + " data[7] = 8;\n" + " }\n" + " function get(uint key) returns (uint value) {\n" + " return data[key];" + " }\n" + "}\n"; + compileAndRun(sourceCode); + map data; + data[7] = 8; + auto get = [&](u256 const& _x) -> u256 + { + return data[_x]; + }; + testSolidityAgainstCpp(0, get, u256(6)); + testSolidityAgainstCpp(0, get, u256(7)); +} + BOOST_AUTO_TEST_SUITE_END() }