Skip to content
This repository has been archived by the owner on Oct 28, 2021. It is now read-only.

EIP211: RETURNDATA #4062

Merged
merged 10 commits into from
May 18, 2017
Merged

EIP211: RETURNDATA #4062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3fae652
VM: Add return data buffer
chfast Apr 25, 2017
ec797dd
Extend EVM Schedule with "return data" flag
chfast May 5, 2017
ce59f0c
EVM: remove disassembly feature
chfast May 5, 2017
f07808e
Introduce RETURNDATA* opcodes
chfast May 5, 2017
798322d
Implement RETURNDATASIZE
chfast May 5, 2017
d30ddfb
Implement RETURNDATACOPY
chfast May 5, 2017
3ee15d0
Fix RETURNDATACOPY and RETURNDATASIZE prices
chfast May 9, 2017
05e438e
Refactor code handling return buffer
chfast May 10, 2017
861a9ea
Add support for REVERT in CREATE
chfast May 16, 2017
e18d839
Change RETURNDATA opcode numbers
chfast May 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions libethereum/ExtVM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ void ExtVM::setStore(u256 _n, u256 _v)
m_s.setStorage(myAddress, _n, _v);
}

h160 ExtVM::create(u256 _endowment, u256& io_gas, bytesConstRef _code, OnOpFunc const& _onOp)
std::pair<h160, owning_bytes_ref> ExtVM::create(u256 _endowment, u256& io_gas, bytesConstRef _code, OnOpFunc const& _onOp)
{
Executive e{m_s, envInfo(), m_sealEngine, depth + 1};
if (!e.create(myAddress, _endowment, gasPrice, io_gas, _code, origin))
Expand All @@ -124,7 +124,7 @@ h160 ExtVM::create(u256 _endowment, u256& io_gas, bytesConstRef _code, OnOpFunc
e.accrueSubState(sub);
}
io_gas = e.gas();
return e.newAddress();
return {e.newAddress(), e.takeOutput()};
}

void ExtVM::suicide(Address _a)
Expand Down
2 changes: 1 addition & 1 deletion libethereum/ExtVM.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class ExtVM: public ExtVMFace
virtual size_t codeSizeAt(Address _a) override final;

/// Create a new contract.
virtual h160 create(u256 _endowment, u256& io_gas, bytesConstRef _code, OnOpFunc const& _onOp = {}) override final;
virtual std::pair<h160, owning_bytes_ref> create(u256 _endowment, u256& io_gas, bytesConstRef _code, OnOpFunc const& _onOp = {}) override final;

/// Create a new message call. Leave _myAddressOverride as the default to use the present address as caller.
/// @returns success flag and output data, if any.
Expand Down
2 changes: 1 addition & 1 deletion libevm/ExtVMFace.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ class ExtVMFace
virtual void suicide(Address) { sub.suicides.insert(myAddress); }

/// Create a new (contract) account.
virtual h160 create(u256, u256&, bytesConstRef, OnOpFunc const&) { return h160(); }
virtual std::pair<h160, owning_bytes_ref> create(u256, u256&, bytesConstRef, OnOpFunc const&) = 0;

/// Make a new message call.
/// @returns success flag and output data, if any.
Expand Down
7 changes: 6 additions & 1 deletion libevm/JitVM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,17 @@ int64_t call(
u256 gas = _msg->gas;
// ExtVM::create takes the sender address from .myAddress.
assert(fromEvmC(_msg->sender) == env.myAddress);
auto addr = env.create(value, gas, input, {});

// TODO: EVMJIT does not support RETURNDATA at the moment, so
// the output is ignored here.
h160 addr;
std::tie(addr, std::ignore) = env.create(value, gas, input, {});
auto gasLeft = static_cast<int64_t>(gas);
if (addr)
std::copy(addr.begin(), addr.end(), _outputData);
else
gasLeft |= EVM_CALL_FAILURE;

return gasLeft;
}

Expand Down
26 changes: 26 additions & 0 deletions libevm/VM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,18 @@ void VM::interpretCases()
}
NEXT

CASE(RETURNDATASIZE)
{
if (!m_schedule->haveReturnData)
throwBadInstruction();

ON_OP();
updateIOGas();

m_SPP[0] = m_returnData.size();
}
NEXT

CASE(CODESIZE)
{
ON_OP();
Expand Down Expand Up @@ -715,6 +727,20 @@ void VM::interpretCases()
}
NEXT

CASE(RETURNDATACOPY)
{
if (!m_schedule->haveReturnData)
throwBadInstruction();

m_copyMemSize = toInt63(m_SP[2]);
updateMem(memNeed(m_SP[0], m_SP[2]));
ON_OP();
updateIOGas();

copyDataToMemory(&m_returnData, m_SP);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not strictly connected to this PR, but probably it would be better to have the offset + size as explicit parameters in copyDataToMemory.

}
NEXT

CASE(CODECOPY)
{
m_copyMemSize = toInt63(m_SP[2]);
Expand Down
3 changes: 3 additions & 0 deletions libevm/VM.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ class VM: public VMFace
// space for code
bytes m_code;

/// RETURNDATA buffer for memory returned from direct subcalls.
bytes m_returnData;

// space for data stack, grows towards smaller addresses from the end
u256 m_stack[1024];
u256 *m_stackEnd = &m_stack[1024];
Expand Down
34 changes: 29 additions & 5 deletions libevm/VMCalls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,21 @@ void VM::caseCreate()
uint64_t initOff = (uint64_t)m_SP[1];
uint64_t initSize = (uint64_t)m_SP[2];

// Clear the return data buffer. This will not free the memory.
m_returnData.clear();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the contract initialization executes REVERT, m_returnData has to be set.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the last sentence on this line: https://github.com/ethereum/EIPs/pull/211/files#diff-fd8c393953fe66162d8bb518d87bfb57R32

(That's something I learned while doing YP.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented.

if (m_ext->balance(m_ext->myAddress) >= endowment && m_ext->depth < 1024)
{
*m_io_gas_p = m_io_gas;
u256 createGas = *m_io_gas_p;
if (!m_schedule->staticCallDepthLimit())
createGas -= createGas / 64;
u256 gas = createGas;
m_SPP[0] = (u160)m_ext->create(endowment, gas, bytesConstRef(m_mem.data() + initOff, initSize), m_onOp);
h160 addr;
owning_bytes_ref output;
std::tie(addr, output) = m_ext->create(endowment, gas, bytesConstRef(m_mem.data() + initOff, initSize), m_onOp);
m_SPP[0] = (u160)addr;
m_returnData = output.toBytes();
*m_io_gas_p -= (createGas - gas);
m_io_gas = uint64_t(*m_io_gas_p);
}
Expand All @@ -139,14 +146,31 @@ void VM::caseCreate()
void VM::caseCall()
{
m_bounce = &VM::interpretCases;

// TODO: Please check if that does not actually increases the stack size.
// That was the case before.
unique_ptr<CallParameters> callParams(new CallParameters());

// Clear the return data buffer. This will not free the memory.
m_returnData.clear();

bytesRef output;
if (caseCallSetup(callParams.get(), output))
{
std::pair<bool, owning_bytes_ref> callResult = m_ext->call(*callParams);
callResult.second.copyTo(output);

m_SPP[0] = callResult.first ? 1 : 0;
bool success = false;
owning_bytes_ref outputRef;
std::tie(success, outputRef) = m_ext->call(*callParams);
outputRef.copyTo(output);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I mistaken that output is only written to but not read from? (This didn't change in this commit, but I started wondering now.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, it's the caller's memory!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Magic. The output is actually a memory reference initialized in caseCallSetup(). This is exactly CALL output buffer, might be of length 0 depending on CALL arguments. Here we copy the data there according to what the CALL requested.


// Here we have 2 options:
// 1. Keep the whole returned memory buffer (owning_bytes_ref):
// higher memory footprint, no memory copy.
// 2. Copy only the return data from the returned memory buffer:
// minimal memory footprint, additional memory copy.
// Option 2 used:
m_returnData = outputRef.toBytes();

m_SPP[0] = success ? 1 : 0;
}
else
m_SPP[0] = 0;
Expand Down
1 change: 1 addition & 0 deletions libevm/VMOpt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ void VM::reportStackUse()
std::array<InstructionMetric, 256> VM::c_metrics;
void VM::initMetrics()
{
// FIXME: This is not thread safe.
static bool done=false;
if (!done)
{
Expand Down
2 changes: 2 additions & 0 deletions libevmcore/EVMSchedule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ struct EVMSchedule
bool eip150Mode = false;
bool eip158Mode = false;
bool haveRevert = false;
bool haveReturnData = false;
std::array<unsigned, 8> tierStepGas;
unsigned expGas = 10;
unsigned expByteGas = 10;
Expand Down Expand Up @@ -109,6 +110,7 @@ static const EVMSchedule MetropolisSchedule = []
{
EVMSchedule schedule = EIP158Schedule;
schedule.haveRevert = true;
schedule.haveReturnData = true;
return schedule;
}();

Expand Down
169 changes: 2 additions & 167 deletions libevmcore/Instruction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,156 +29,6 @@ using namespace std;
using namespace dev;
using namespace dev::eth;

const std::map<std::string, Instruction> dev::eth::c_instructions =
{
{ "STOP", Instruction::STOP },
{ "ADD", Instruction::ADD },
{ "SUB", Instruction::SUB },
{ "MUL", Instruction::MUL },
{ "DIV", Instruction::DIV },
{ "SDIV", Instruction::SDIV },
{ "MOD", Instruction::MOD },
{ "SMOD", Instruction::SMOD },
{ "EXP", Instruction::EXP },
{ "BNOT", Instruction::NOT },
{ "LT", Instruction::LT },
{ "GT", Instruction::GT },
{ "SLT", Instruction::SLT },
{ "SGT", Instruction::SGT },
{ "EQ", Instruction::EQ },
{ "NOT", Instruction::ISZERO },
{ "AND", Instruction::AND },
{ "OR", Instruction::OR },
{ "XOR", Instruction::XOR },
{ "BYTE", Instruction::BYTE },
{ "ADDMOD", Instruction::ADDMOD },
{ "MULMOD", Instruction::MULMOD },
{ "SIGNEXTEND", Instruction::SIGNEXTEND },
{ "SHA3", Instruction::SHA3 },
{ "ADDRESS", Instruction::ADDRESS },
{ "BALANCE", Instruction::BALANCE },
{ "ORIGIN", Instruction::ORIGIN },
{ "CALLER", Instruction::CALLER },
{ "CALLVALUE", Instruction::CALLVALUE },
{ "CALLDATALOAD", Instruction::CALLDATALOAD },
{ "CALLDATASIZE", Instruction::CALLDATASIZE },
{ "CALLDATACOPY", Instruction::CALLDATACOPY },
{ "CODESIZE", Instruction::CODESIZE },
{ "CODECOPY", Instruction::CODECOPY },
{ "GASPRICE", Instruction::GASPRICE },
{ "EXTCODESIZE", Instruction::EXTCODESIZE },
{ "EXTCODECOPY", Instruction::EXTCODECOPY },
{ "BLOCKHASH", Instruction::BLOCKHASH },
{ "COINBASE", Instruction::COINBASE },
{ "TIMESTAMP", Instruction::TIMESTAMP },
{ "NUMBER", Instruction::NUMBER },
{ "DIFFICULTY", Instruction::DIFFICULTY },
{ "GASLIMIT", Instruction::GASLIMIT },
{ "JUMPTO", Instruction::JUMPTO },
{ "JUMPIF", Instruction::JUMPTO },
{ "JUMPSUB", Instruction::JUMPSUB },
{ "JUMPV", Instruction::JUMPV },
{ "JUMPSUBV", Instruction::JUMPSUBV },
{ "RETURNSUB", Instruction::RETURNSUB },
{ "POP", Instruction::POP },
{ "MLOAD", Instruction::MLOAD },
{ "MSTORE", Instruction::MSTORE },
{ "MSTORE8", Instruction::MSTORE8 },
{ "SLOAD", Instruction::SLOAD },
{ "SSTORE", Instruction::SSTORE },
{ "JUMP", Instruction::JUMP },
{ "JUMPI", Instruction::JUMPI },
{ "PC", Instruction::PC },
{ "MSIZE", Instruction::MSIZE },
{ "GAS", Instruction::GAS },
{ "BEGIN", Instruction::JUMPDEST },
{ "JUMPDEST", Instruction::JUMPDEST },
{ "BEGINDATA", Instruction::BEGINDATA },
{ "BEGINSUB", Instruction::BEGINSUB },
{ "PUSH1", Instruction::PUSH1 },
{ "PUSH2", Instruction::PUSH2 },
{ "PUSH3", Instruction::PUSH3 },
{ "PUSH4", Instruction::PUSH4 },
{ "PUSH5", Instruction::PUSH5 },
{ "PUSH6", Instruction::PUSH6 },
{ "PUSH7", Instruction::PUSH7 },
{ "PUSH8", Instruction::PUSH8 },
{ "PUSH9", Instruction::PUSH9 },
{ "PUSH10", Instruction::PUSH10 },
{ "PUSH11", Instruction::PUSH11 },
{ "PUSH12", Instruction::PUSH12 },
{ "PUSH13", Instruction::PUSH13 },
{ "PUSH14", Instruction::PUSH14 },
{ "PUSH15", Instruction::PUSH15 },
{ "PUSH16", Instruction::PUSH16 },
{ "PUSH17", Instruction::PUSH17 },
{ "PUSH18", Instruction::PUSH18 },
{ "PUSH19", Instruction::PUSH19 },
{ "PUSH20", Instruction::PUSH20 },
{ "PUSH21", Instruction::PUSH21 },
{ "PUSH22", Instruction::PUSH22 },
{ "PUSH23", Instruction::PUSH23 },
{ "PUSH24", Instruction::PUSH24 },
{ "PUSH25", Instruction::PUSH25 },
{ "PUSH26", Instruction::PUSH26 },
{ "PUSH27", Instruction::PUSH27 },
{ "PUSH28", Instruction::PUSH28 },
{ "PUSH29", Instruction::PUSH29 },
{ "PUSH30", Instruction::PUSH30 },
{ "PUSH31", Instruction::PUSH31 },
{ "PUSH32", Instruction::PUSH32 },
{ "DUP1", Instruction::DUP1 },
{ "DUP2", Instruction::DUP2 },
{ "DUP3", Instruction::DUP3 },
{ "DUP4", Instruction::DUP4 },
{ "DUP5", Instruction::DUP5 },
{ "DUP6", Instruction::DUP6 },
{ "DUP7", Instruction::DUP7 },
{ "DUP8", Instruction::DUP8 },
{ "DUP9", Instruction::DUP9 },
{ "DUP10", Instruction::DUP10 },
{ "DUP11", Instruction::DUP11 },
{ "DUP12", Instruction::DUP12 },
{ "DUP13", Instruction::DUP13 },
{ "DUP14", Instruction::DUP14 },
{ "DUP15", Instruction::DUP15 },
{ "DUP16", Instruction::DUP16 },
{ "SWAP1", Instruction::SWAP1 },
{ "SWAP2", Instruction::SWAP2 },
{ "SWAP3", Instruction::SWAP3 },
{ "SWAP4", Instruction::SWAP4 },
{ "SWAP5", Instruction::SWAP5 },
{ "SWAP6", Instruction::SWAP6 },
{ "SWAP7", Instruction::SWAP7 },
{ "SWAP8", Instruction::SWAP8 },
{ "SWAP9", Instruction::SWAP9 },
{ "SWAP10", Instruction::SWAP10 },
{ "SWAP11", Instruction::SWAP11 },
{ "SWAP12", Instruction::SWAP12 },
{ "SWAP13", Instruction::SWAP13 },
{ "SWAP14", Instruction::SWAP14 },
{ "SWAP15", Instruction::SWAP15 },
{ "SWAP16", Instruction::SWAP16 },
{ "LOG0", Instruction::LOG0 },
{ "LOG1", Instruction::LOG1 },
{ "LOG2", Instruction::LOG2 },
{ "LOG3", Instruction::LOG3 },
{ "LOG4", Instruction::LOG4 },
{ "CREATE", Instruction::CREATE },
{ "CALL", Instruction::CALL },
{ "CALLCODE", Instruction::CALLCODE },
{ "RETURN", Instruction::RETURN },
{ "DELEGATECALL", Instruction::DELEGATECALL },
{ "REVERT", Instruction::REVERT },
{ "INVALID", Instruction::INVALID },
{ "SUICIDE", Instruction::SUICIDE },

// these are generated by the interpreter - should never be in user code
{ "PUSHC", Instruction::PUSHC },
{ "JUMPC", Instruction::JUMPC },
{ "JUMPCI", Instruction::JUMPCI }
};

static const std::map<Instruction, InstructionInfo> c_instructionInfo =
{ // Add, Args, Ret, SideEffects, GasPriceTier
{ Instruction::STOP, { "STOP", 0, 0, 0, true, Tier::Zero } },
Expand Down Expand Up @@ -218,6 +68,8 @@ static const std::map<Instruction, InstructionInfo> c_instructionInfo =
{ Instruction::GASPRICE, { "GASPRICE", 0, 0, 1, false, Tier::Base } },
{ Instruction::EXTCODESIZE, { "EXTCODESIZE", 0, 1, 1, false, Tier::Special } },
{ Instruction::EXTCODECOPY, { "EXTCODECOPY", 0, 4, 0, true, Tier::Special } },
{ Instruction::RETURNDATASIZE,{"RETURNDATASIZE", 0, 0, 1, false, Tier::Base } },
{ Instruction::RETURNDATACOPY,{"RETURNDATACOPY", 0, 3, 0, true, Tier::VeryLow } },
{ Instruction::BLOCKHASH, { "BLOCKHASH", 0, 1, 1, false, Tier::Ext } },
{ Instruction::COINBASE, { "COINBASE", 0, 0, 1, false, Tier::Base } },
{ Instruction::TIMESTAMP, { "TIMESTAMP", 0, 0, 1, false, Tier::Base } },
Expand Down Expand Up @@ -350,23 +202,6 @@ void dev::eth::eachInstruction(
}
}

string dev::eth::disassemble(bytes const& _mem)
{
stringstream ret;
eachInstruction(_mem, [&](Instruction _instr, u256 const& _data) {
if (!isValidInstruction(_instr))
ret << "0x" << hex << int(_instr) << " ";
else
{
InstructionInfo info = instructionInfo(_instr);
ret << info.name << " ";
if (info.additional)
ret << "0x" << hex << _data << " ";
}
});
return ret.str();
}

InstructionInfo dev::eth::instructionInfo(Instruction _inst)
{
auto it = c_instructionInfo.find(_inst);
Expand Down
5 changes: 2 additions & 3 deletions libevmcore/Instruction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ enum class Instruction: uint8_t
GASPRICE, ///< get price of gas in current environment
EXTCODESIZE, ///< get external code size (from another contract)
EXTCODECOPY, ///< copy external code (from another contract)
RETURNDATASIZE = 0x3d, ///< size of data returned from previous call
RETURNDATACOPY = 0x3e, ///< copy data returned from previous call to memory

BLOCKHASH = 0x40, ///< get hash of most recent complete block
COINBASE, ///< get the block's coinbase address
Expand Down Expand Up @@ -274,9 +276,6 @@ extern const std::map<std::string, Instruction> c_instructions;
/// Iterate through EVM code and call a function on each instruction.
void eachInstruction(bytes const& _mem, std::function<void(Instruction,u256 const&)> const& _onInstruction);

/// Convert from EVM code to simple EVM assembly language.
std::string disassemble(bytes const& _mem);

}
}

Loading