aleth Test aleth-interpreter with evm-test

In https://github.com/ethereum/evmone/pull/85 there is the evm-test tool that can test the aleth-interpreter DLL.

I have run it already and noticed some failures.

One is fixed by #5651.

Many of failures are due to a fact the unit test checks are more restrictive than EVMC requires. E.g. EVMC do not require to return specific error code, it is acceptable to return generic EVMC_FAILURE instead of specific errors. To make such unit test more generic I propose to

probe the EVM implementation to see what error code it returns,
expect the same error code to be returned in the same cases.

I also noticed some tests run a long time and fail.

Jul 05 '19 14:07 chfast

@gumb0 if you have some free time, can you take a look on this?

Jul 05 '19 14:07 chfast

I'm investigating evm.memory_access test failure

EVM Test 0.2.0-dev

Testing ../../aleth/build/libaleth-interpreter/libaleth-interpreter.so

Note: Google Test filter = evm.memory_access
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from evm
[ RUN      ] evm.memory_access
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1145: Failure
Expected equality of these values:
  result.status_code
    Which is: 0
  EVMC_OUT_OF_GAS
    Which is: 3
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1146: Failure
Expected equality of these values:
  result.gas_left
    Which is: 9223336851274727387
  0
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1145: Failure
Expected equality of these values:
  result.status_code
    Which is: 0
  EVMC_OUT_OF_GAS
    Which is: 3
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1146: Failure
Expected equality of these values:
  result.gas_left
    Which is: 9223336851677380595
  0
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1145: Failure
Expected equality of these values:
  result.status_code
    Which is: 0
  EVMC_OUT_OF_GAS
    Which is: 3
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1146: Failure
Expected equality of these values:
  result.gas_left
    Which is: 9223336851677380595
  0
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1145: Failure
Expected equality of these values:
  result.status_code
    Which is: 0
  EVMC_OUT_OF_GAS
    Which is: 3
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1146: Failure
Expected equality of these values:
  result.gas_left
    Which is: 9223336851677379895
  0
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1145: Failure
Expected equality of these values:
  result.status_code
    Which is: 9
  EVMC_OUT_OF_GAS
    Which is: 3
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1145: Failure
Expected equality of these values:
  result.status_code
    Which is: 0
  EVMC_OUT_OF_GAS
    Which is: 3
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1146: Failure
Expected equality of these values:
  result.gas_left
    Which is: 9223336817720295042
  0
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1145: Failure
Expected equality of these values:
  result.status_code
    Which is: 0
  EVMC_OUT_OF_GAS
    Which is: 3
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1146: Failure
Expected equality of these values:
  result.gas_left
    Which is: 9223336817720294664
  0
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1145: Failure
Expected equality of these values:
  result.status_code
    Which is: 0
  EVMC_OUT_OF_GAS
    Which is: 3
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1146: Failure
Expected equality of these values:
  result.gas_left
    Which is: 9223336817720294286
  0
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1145: Failure
Expected equality of these values:
  result.status_code
    Which is: 0
  EVMC_OUT_OF_GAS
    Which is: 3
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1146: Failure
Expected equality of these values:
  result.gas_left
    Which is: 9223336817720293908
  0
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1145: Failure
Expected equality of these values:
  result.status_code
    Which is: 0
  EVMC_OUT_OF_GAS
    Which is: 3
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:1146: Failure
Expected equality of these values:
  result.gas_left
    Which is: 9223336817720293530
  0
terminate called after throwing an instance of 'std::bad_alloc'
  what():  std::bad_alloc
fish: “bin/evm-test ../../aleth/build/…” terminated by signal SIGABRT (Abort)

Jul 10 '19 14:07 gumb0

@chfast It looks like aleth-interpreter here successfully allocates 4 Gb of memory, then runs sha3 on it, and it finishes successfully (I didn't step into ethash). There's enough gas for it, because the test gives it std::numeric_limits<int64_t>::max() gas.

evmone though fails early with OOG because it expects memory requirements not to exceed 4 Gb - 1 https://github.com/ethereum/evmone/blob/f25d9c7a05d5a88bd17bd5db9402bb3568c61ddc/lib/evmone/instructions.cpp#L24. And this is what the test expects, too.

In the end I think interpreter for me throws bad_alloc when it needs to allocate 4Gb at offset 4Gb.

Jul 10 '19 16:07 gumb0

I know there's proposal https://eips.ethereum.org/EIPS/eip-1985 So probably we should introduce the same 2**32 - 1 limit in interpreter at some point (when EIP is adopted?)

Jul 10 '19 16:07 gumb0

Ok, thanks very much for checking.

These were evmone unit tests so they were testing evmone implementation limits. I think it would be good to modify the test by limiting the gas limit to something that will not allow allocating 4GB of memory. Maybe also add 2G offset + 2G size case.

About EIP-1985, we actually discussed that yesterday whenever this should also set a limit for memory size. It do not do it at the moment. We should move this discussion to https://ethereum-magicians.org/t/eip-1985-sane-limits-for-certain-evm-parameters/3224.

Jul 11 '19 06:07 chfast

With lower gas limit interpreter still fails memory_access test with RETURNDATACOPY opcode, because there's no return data it returns EVMC_INVALID_MEMORY_ACCESS, while evmone returns EVMC_OUT_OF_GAS.

Does it make sense to change the order of checks in interpreter here - first check gas for memory, then check return data size? Then it will return OOG first https://github.com/ethereum/aleth/blob/089e7ddaa741825fdf62974ad66b077c715b8f25/libaleth-interpreter/VM.cpp#L1016-L1022

It seems that not enough return data is more important, so maybe this could be changed in evmone?

Jul 11 '19 17:07 gumb0

evm.invalid failure should be fixed in https://github.com/ethereum/aleth/pull/5666

Jul 11 '19 18:07 gumb0

Another failure is

[ RUN      ] evm.undefined_instructions
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:933: Failure
Expected equality of these values:
  res.status_code
    Which is: 7
  EVMC_UNDEFINED_INSTRUCTION
    Which is: 5
1b
/home/andrei/dev/evmone/test/unittests/evm_test.cpp:933: Failure
Expected equality of these values:
  res.status_code
    Which is: 7
  EVMC_UNDEFINED_INSTRUCTION
    Which is: 5
1c
...

Here the problem is with newer opcodes (bit shifts) not defined yet in old revisions, interpreter checks the stack requirements first, before figuring out that it's not defined for the given revision https://github.com/ethereum/aleth/blob/089e7ddaa741825fdf62974ad66b077c715b8f25/libaleth-interpreter/VM.cpp#L244 Stack is empty in test, so it fails with EVMC_STACK_UNDERFLOW instead of EVMC_UNDEFINED_INSTRUCTION

Jul 11 '19 18:07 gumb0

It seems that not enough return data is more important, so maybe this could be changed in evmone?

The order of check is not specified and I expected such issues. In general we will have to modify tests to allow different errors. But in this case evmone can be changed.

Jul 12 '19 05:07 chfast

Here the problem is with newer opcodes (bit shifts) not defined yet in old revisions, interpreter checks the stack requirements first, before figuring out that it's not defined for the given revision

Here you can add some stack items before the instruction in code. Please check the bytecode.hpp helpers. Should be something like 7 * push(0) + opcode.

Jul 12 '19 05:07 chfast

I've changed undefined_instructions test, but still it seems weird that aleth-interpreter checks stack requirements according to the latest revision instead of current one. https://github.com/ethereum/evmone/pull/91

Jul 15 '19 11:07 gumb0

It seems that not enough return data is more important, so maybe this could be changed in evmone?

The order of check is not specified and I expected such issues. In general we will have to modify tests to allow different errors. But in this case evmone can be changed.

I checked evmone's implementation. It is easier to check memory first (OOG) because then we know that the "size" is not insane value and returndata buffer checking is easier. I will modify the test accordingly.

Jul 15 '19 12:07 chfast

The test already expects EVMC_OUT_OF_GAS, so no change needed?

Jul 15 '19 13:07 gumb0

The test already expects EVMC_OUT_OF_GAS, so no change needed?

No change needed.

Jul 15 '19 13:07 chfast

For instructions 0xac, 0xad, 0xae interpreter still returns EVM_INVALID_INSTRUCTION instead of EVMC_UNDEFINED_INSTRUCTION.

These are special codes used by optimizer (Instruction::PUSHC, Instruction::JUMPC, Instruction::JUMPCI), and it replaces all their occurrences in input code with INVALID

Jul 15 '19 17:07 gumb0

All green now!

Jul 23 '19 10:07 gumb0