Test aleth-interpreter with evm-test #5653

chfast · 2019-07-05T14:02:21Z

In ethereum/evmone#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.

The text was updated successfully, but these errors were encountered:

chfast · 2019-07-05T14:54:16Z

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

gumb0 · 2019-07-10T14:28:48Z

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)

gumb0 · 2019-07-10T16:48:12Z

@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.

gumb0 · 2019-07-10T16:55:03Z

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?)

chfast · 2019-07-11T06:48:35Z

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.

gumb0 · 2019-07-11T17:52:29Z

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

aleth/libaleth-interpreter/VM.cpp

Lines 1016 to 1022 in 089e7dd

    
           bigint const endOfAccess = bigint(m_SP[1]) + bigint(m_SP[2]); 
        
           if (m_returnData.size() < endOfAccess) 
        
               throwBufferOverrun(endOfAccess); 
        
           m_copyMemSize = toInt63(m_SP[2]); 
        
           updateMem(memNeed(m_SP[0], m_SP[2])); 
        
           updateIOGas();

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

gumb0 · 2019-07-11T18:31:26Z

evm.invalid failure should be fixed in #5666

gumb0 · 2019-07-11T18:40:02Z

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

aleth/libaleth-interpreter/VM.cpp

Line 244 in 089e7dd

adjustStack(metric.num_stack_arguments, metric.num_stack_returned_items);

Stack is empty in test, so it fails with EVMC_STACK_UNDERFLOW instead of EVMC_UNDEFINED_INSTRUCTION

chfast · 2019-07-12T05:26:37Z

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.

chfast · 2019-07-12T05:28:17Z

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.

gumb0 · 2019-07-15T11:13:34Z

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.
ethereum/evmone#91

chfast · 2019-07-15T12:37:07Z

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.

gumb0 · 2019-07-15T13:02:48Z

The test already expects EVMC_OUT_OF_GAS, so no change needed?

chfast · 2019-07-15T13:04:39Z

The test already expects EVMC_OUT_OF_GAS, so no change needed?

No change needed.

gumb0 · 2019-07-15T17:29:30Z

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

gumb0 · 2019-07-23T10:36:02Z

All green now!

gumb0 self-assigned this Jul 8, 2019

This was referenced Jul 17, 2019

Replace optimizer-special opcodes with 0xaf instead of INVALID. #5680

Merged

Use metrics of the requested revision in aleth-interpreter #5679

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test aleth-interpreter with evm-test #5653

Test aleth-interpreter with evm-test #5653

chfast commented Jul 5, 2019

chfast commented Jul 5, 2019

gumb0 commented Jul 10, 2019

gumb0 commented Jul 10, 2019

gumb0 commented Jul 10, 2019 •

edited

Loading

chfast commented Jul 11, 2019

gumb0 commented Jul 11, 2019 •

edited

Loading

gumb0 commented Jul 11, 2019

gumb0 commented Jul 11, 2019

chfast commented Jul 12, 2019

chfast commented Jul 12, 2019

gumb0 commented Jul 15, 2019

chfast commented Jul 15, 2019

gumb0 commented Jul 15, 2019

chfast commented Jul 15, 2019

gumb0 commented Jul 15, 2019

gumb0 commented Jul 23, 2019

Test aleth-interpreter with evm-test #5653

Test aleth-interpreter with evm-test #5653

Comments

chfast commented Jul 5, 2019

chfast commented Jul 5, 2019

gumb0 commented Jul 10, 2019

gumb0 commented Jul 10, 2019

gumb0 commented Jul 10, 2019 • edited Loading

chfast commented Jul 11, 2019

gumb0 commented Jul 11, 2019 • edited Loading

gumb0 commented Jul 11, 2019

gumb0 commented Jul 11, 2019

chfast commented Jul 12, 2019

chfast commented Jul 12, 2019

gumb0 commented Jul 15, 2019

chfast commented Jul 15, 2019

gumb0 commented Jul 15, 2019

chfast commented Jul 15, 2019

gumb0 commented Jul 15, 2019

gumb0 commented Jul 23, 2019

gumb0 commented Jul 10, 2019 •

edited

Loading

gumb0 commented Jul 11, 2019 •

edited

Loading