Commit Graph

411 Commits

Author SHA1 Message Date
Jamie Lokier a3c8a5c3f3
EVMC: Small stacks when using EVMC, closes #575 (segfaults)
This patch reduces stack space used with EVM in ENABLE_EVMC=1 mode, from 13 MB
worst case to 550 kB, a 24x reduction.

This completes fixing the "stack problem" and closes #575 (`EVM: Different
segmentation faults when running the test suite with EVMC`).

It also closes #256 (`recursive EVM call trigger unrecoverable stack overflow`).

After this patch, it is possible to re-enable the CI targets which had to be
disabled due to #575.

This change is also a required precursor for switching over to "nearly EVMC" as
the clean and focused Nimbus-internal API between EVM and sync/database
processes, and is also key to the use of Chronos `async` in those processes
when calling the EVM.

(The motivation is the internal interface has to be substantially changed
_anyway_ for the parallel sync and database processes, and EVMC turns out to be
well-designed and well-suited for this.  It provides good separation between
modules, and suits our needs better than our other current interface.  Might as
well use a good one designed by someone else.  EVMC is 98% done in Nimbus
thanks to great work done before by @jangko, and we can use Nimbus-specific
extensions where we need flexibility, including for performance.  Being aligned
with the ecosystem is a useful bonus feature.)

All tests below were run on Ubuntu 20.04 LTS server, x86-64.  This matches one
of the targets that has been disabled for a while in CI in EVMC mode due to
stack overflow crashing the tests, so it's a good choice.

Measurements before
===================

Testing commit `e76e0144 2021-04-22 11:29:42 +0700 add submodules: graphql and
toml-serialization`.

    $ rm -f build/all_tests && make ENABLE_EVMC=1 test
    $ ulimit -S -s 16384 # Requires larger stack than default to avoid crash.
    $ ./build/all_tests 9 | tee tlog
    [Suite] persist block json tests
    ...
    Stack range 38416 depthHigh 3
    ...
    Stack range 13074720 depthHigh 1024
    [OK] tests/fixtures/PersistBlockTests/block1431916.json

These tests use 13.07 MB of stack to run, and so crash with the default stack
limit on Ubuntu Server 20.04 (8MB).  Exactly 12768 bytes per EVM call stack
frame.

    $ rm -f build/all_tests && make ENABLE_EVMC=1 test
    $ ulimit -S -s 16384 # Requires larger stack than default.
    $ ./build/all_tests 7 | tee tlog
    [Suite] new generalstate json tests
        ...
    Stack range 14384 depthHigh 2
        ...
    Stack range 3495456 depthHigh 457
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest639.json
    ...
    Stack range 3709600 depthHigh 485
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest458.json
        ...
    Stack range 7831600 depthHigh 1024
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stCreate2/Create2OnDepth1024.json

These tests use 7.83MB of stack to run.  About 7648 bytes per EVM call stack
frame.  It _only just_ avoids crashing with the default Ubuntu Server stack
limit of 8 MB.  However, it still crashes on Windows x86-64, which is why the
Windows CI EVMC target is currently disabled.

On Linux where this passes, this is so borderline that it affects work and
testing of the complex storage code, because that's called from the EVM.

Also, this greatly exceeds the default thread stack size.

Measurements after
==================

    $ rm -f build/all_tests && make ENABLE_EVMC=1 test
    $ ulimit -S -s 600 # Because we can!  600k stack.
    $ ./build/all_tests 9 | tee tlog
    [Suite] persist block json tests
    ...
    Stack range 1936 depthHigh 3
    ...
        Stack range 556272 depthHigh 1022
        Stack range 556512 depthHigh 1023
        Stack range 556816 depthHigh 1023
        Stack range 557056 depthHigh 1024
        Stack range 557360 depthHigh 1024
        [OK] tests/fixtures/PersistBlockTests/block1431916.json

    $ rm -f build/all_tests && make ENABLE_EVMC=1 test
    $ ulimit -S -s 600 # Because we can!  600k stack.
    $ ./build/all_tests 7 | tee tlog
    [Suite] new generalstate json tests
        ...
    Stack range 1392 depthHigh 2
        ...
    Stack range 248912 depthHigh 457
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest639.json
    ...
    Stack range 264144 depthHigh 485
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest458.json
        ...
        Stack range 557360 depthHigh 1024
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stStaticCall/static_CallRecursiveBombPreCall.json

For both tests, a satisfying *544 bytes* per EVM call stack frame, and EVM
takes less than 600 kB total.  With other overheads, both tests run in 600 kB
stack total at maximum EVM depth.

We must add some headroom on this for database activity called from the EVM,
and different compile targets.  But it means the EVM itself is no longer a
stack burden.

This is much smaller than the default thread stack size on Linux (2MB), with
plenty of margin.  (Just fyi, it isn't smaller than a _small_ thread stack on
Linux from a long time ago (128kB), and some small embedded C targets.)

This size is well suited to running EVMs in threads.

Further reduction
=================

This patch solves the stack problem.  Windows and Linux 64-bit EVMC CI targets
can be re-enabled, and there is no longer a problem with stack usage.

We can reduce further to ~340 bytes per frame and 350 kB total, while still
complying with EVMC.  But as this involves changing how errors are handled to
comply fully with EVMC, and removing `dispose` calls, it's not worth doing now
while there are other EVMC changes in progress that will have the same effect.

A Nimbus-specific extension will allow us to avoid recursion with EVMC anyway,
bringing bytes per frame to zero.  We need the extension anyway, to support
Chronos `async` which parallel transaction processing is built around.

Interop with non-Nimbus over EVMC won't let us avoid recursion, but then we
can't control the stack frame size either.  To prevent stack overflow in
interop I anticipate using (this method in Aleth)
[6e96ce34e3/libethereum/ExtVM.cpp (L61)].

Smoke test other versions of GCC and Clang/LLVM
===============================================

As all builds including Windows use GCC or Apple's Clang/LLVM, this is just to
verify we're in the right ballpark on all targets.  I've only checked `x86_64`
though, not 32-bit, and not ARM.

It's interesting to see GCC 10 uses less stack.  This is because it optimises
`struct` returns better, sometimes skipping an intermediate copy.  Here it
benefits the EVMC API, but I found GCC 10 also improves the larger stack usage
of the rest of `nimbus-eth1` as well.

Apple clang 12.0.0 (clang-1200.0.26.2) on MacOS 10.15:

- 544 bytes per EVM call stack frame

GCC 10.3.0 (Ubuntu 10.3.0-1ubuntu1) on Ubuntu 21.04:

- 464 bytes per EVM call stack frame

GCC 10.2.0 (Ubuntu 10.2.0-5ubuntu1~20.04) on Ubuntu 20.04 LTS:

- 464 bytes per EVM call stack frame

GCC 11.0.1 20210417 (experimental; Ubuntu 11-20210417-1ubuntu1) on Ubuntu 21.04:

- 8 bytes per EVM call stack frame

GCC 9.3.0 (Ubuntu 9.3.0-17ubuntu1~20.04) on Ubuntu 20.04 LTS:

- 544 bytes per EVM call stack frame

GCC 8.4.0 (Ubuntu 8.4.0-3ubuntu2) on Ubuntu 20.04 LTS:

- 544 bytes per EVM call stack frame

GCC 7.5.0 (Ubuntu 7.5.0-6ubuntu2) on Ubuntu 20.04 LTS:

- 544 bytes per EVM call stack frame

GCC 9.2.1 20191008 (Ubuntu 9.2.1-9ubuntu2) on Ubuntu 19.10:

- 528 bytes per EVM call stack frame

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-04-27 05:53:32 +01:00
Jamie Lokier 085661c24f
EVM: Eliminate recursion entirely
This patch eliminates recursion entirely from the EVM when ENABLE_EVMC=0.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-04-20 16:12:45 +01:00
Jamie Lokier 8211db1ea8
EVM: Small patch that reduces EVM stack usage to almost nothing
There's been a lot of talk about the Nimbus EVM "stack problem".  I think we
assumed changing it would require big changes to the interpreter code, touching
a lot of functions.

It turned out to be a low hanging fruit.

This patch solves the stack problem, but hardly touches anything.  The change
in EVM stack memory is from 13 MB worst case to just 48 kB, a 250x reduction.

I've been doing work on the database/storage/trie code.  While looking at the
API between the EVM and the database/storage/trie, this stack patch stood out
and made itself obvious.  As it's tiny, rather than more talk, here it is.

Note: This patch is intentionally small, non-invasive, and hopefully easy to
understand, so that it doesn't conflict with other work done on the EVM, and
can easily be grafted into any other EVM structure.

Motivation
==========

- We run out of space and crash on some targets, unless the stack limit is
  raised above its default.  Surprise segmentation faults are unhelpful.

- Some CI targets have been disabled for months due to this.

- Because usage borders on the system limits, when working on
  database/storage/trie/sync code (called from the EVM), segmentation faults
  occur and are misleading.  They cause lost time due to thinking there's a
  crash bug in the code being worked on, when there's nothing wrong with it.

- Sometimes unrelated, trivial code changes elsewhere trigger CI test failures.
  It looks like abrupt termination.  A simple, recent patch was crashing in
  `make test` even though it was a trivial refactor.  Turns out it pushed the
  stack over the edge.

- A large stack has to be scanned by the Nim garbage collector sometimes.
  Larger stack means slower GC and memory allocation.

- The structure of this small patch suggests how to weave async into the EVM
  with almost no changes to the EVM, and no async transformation overhead.

- The patch seemed obvious when working on the API between EVM and storage.

Measurements before
===================

All these tests were run on Ubuntu 20.04 server, x86-64.  This is one of the
targets that has been disabled for a while in CI in EVMC mode due to crashing,
and excessive stack usage is the cause.

Testing commit 0c34a8e3 `2021-04-08 17:46:00 +0200 CI: use MSYS2 on Windows`.

    $ rm -f build/all_tests && make ENABLE_EVMC=1 test
    $ ulimit -S -s 16384 # Requires larger stack than default to avoid crash.
    $ ./build/all_tests 9 | tee tlog
    [Suite] persist block json tests
    ...
	Stack range 38496 depthHigh 3
    ...
    Stack range 13140272 depthHigh 1024
    [OK] tests/fixtures/PersistBlockTests/block1431916.json

These tests use 13.14 MB of stack to run, and so crash with the default stack
limit on Ubuntu Server 20.04 (8MB).  Exactly 12832 bytes per EVM call stack
frame.  It's interesting to see some stack frames take a bit more.

    $ rm -f build/all_tests && make ENABLE_EVMC=1 test
    $ ulimit -S -s 16384 # Requires larger stack than default.
    $ ./build/all_tests 7 | tee tlog
    [Suite] new generalstate json tests
	...
	Stack range 15488 depthHigh 2
	...
	Stack range 3539312 depthHigh 457
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest639.json
    ...
	Stack range 3756144 depthHigh 485
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest458.json
	...
	Stack range 7929968 depthHigh 1024
     [OK] tests/fixtures/eth_tests/GeneralStateTests/stCreate2/Create2OnDepth1024.json

These tests use 7.92MB of stack to run.  About 7264 bytes per EVM call stack
frame.  It _only just_ avoids crashing with the default Ubuntu Server stack
limit of 8 MB.  However, it still crashes on Windows x86-64, which is why the
CI target is currently disabled.

On Linux where this passes, this is so borderline that it affects work and
testing of storage and sync code, because that's called from the EVM.  Which
was a motivation for dealing with the stack instead of letting this linger.

Also, this stack greatly exceeds the default thread stack size.

    $ rm -f build/all_tests && make ENABLE_EVMC=0 test
    $ ulimit -S -s 16384 # Requires larger stack than default to avoid crash.
    $ ./build/all_tests 9 | tee tlog
    [Suite] persist block json tests
    ...
    Stack range 33216 depthHigh 3
    ...
    Stack range 11338032 depthHigh 1024
    [OK] tests/fixtures/PersistBlockTests/block1431916.json

These tests use 11.33 MB stack to run, and so crash with a default stack limit
of 8MB.  Exactly 11072 bytes per EVM call stack frame.  It's interesting to see
some stack frames take a bit more.

    $ rm -f build/all_tests && make ENABLE_EVMC=0 test
    $ ulimit -S -s 16384 # Requires larger stack than default.
    $ ./build/all_tests 7 | tee tlog
    [Suite] new generalstate json tests
	...
    Stack range 10224 depthHigh 2
	...
    Stack range 2471760 depthHigh 457
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest639.json
    ...
    Stack range 2623184 depthHigh 485
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest458.json
	...
    Stack range 5537824 depthHigh 1024
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stCreate2/Create2OnDepth1024.json

These tests use 5.54 MB of stack to run, and avoid crashing on with a default
stack limit of 8 MB.  About 5408 bytes per EVM call stack frame.

However, this is uncomfortably close to the limit, as the stack frame size is
sensitive to changes in the code.

Also, this stack greatly exceeds the default thread stack size.

Measurements after
==================

(This patch doesn't address EVMC mode, which is not our default.  EVMC stack
usage remains about the same.  EVMC mode is addressed in another tiny patch.)

    $ rm -f build/all_tests && make ENABLE_EVMC=0 test
    $ ulimit -S -s 80 # Because we can!  80k stack.
    $ ./build/all_tests 9 | tee tlog
    [Suite] persist block json tests
    ...
    Stack range 496 depthHigh 3
    ...
    Stack range 49504 depthHigh 1024
    [OK] tests/fixtures/PersistBlockTests/block1431916.json

    $ rm -f build/all_tests && make ENABLE_EVMC=0 test
    $ ulimit -S -s 72 # Because we can!  72k stack.
    $ ./build/all_tests 7 | tee tlog
    [Suite] new generalstate json tests
	...
    Stack range 448 depthHigh 2
	...
    Stack range 22288 depthHigh 457
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest639.json
    ...
    Stack range 23632 depthHigh 485
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest458.json
	...
    Stack range 49504 depthHigh 1024
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stCreate2/Create2OnDepth1024.json

For both tests, a satisfying *48 bytes* per EVM call stack frame, and EVM takes
not much more than 48 kB.  With other overheads, both tests run in 80 kB stack
total at maximum EVM depth.

We must add some headroom on this for database activity called from the EVM,
and different compile targets.  But it means the EVM itself is no longer a
stack burden.

This is much smaller than the default thread stack size on Linux (2MB), with
plenty of margin.  It's even smaller than Linux from a long time ago (128kB),
and some small embedded C targets.  (Just fyi, though, some JVM environments
allocated just 32 kB to thread stacks.)

This size is also well suited to running EVMs in threads, if that's useful.

Subtle exception handling and `dispose`
=======================================

It is important that each `snapshot` has a corresponding `dispose` in the event
of an exception being raised.  This code does do that, but in a subtle way.

The pair of functions `execCallOrCreate` and `execCallOrCreateAux` are
equivalent to the following code, where you can see `dispose` more clearly:

    proc execCallOrCreate*(c: Computation) =
      defer: c.dispose()
      if c.beforeExec():
        return
      c.executeOpcodes()
      while not c.continuation.isNil:
        c.child.execCallOrCreate()
        c.child = nil
        (c.continuation)()
        c.executeOpcodes()
      c.afterExec()

That works fine, but only reduces the stack used to 300-700 kB instead of 48 kB.

To get lower we split the above into separate `execCallOrCreate` and
`execCallOrCreateAux`.  Only the outermost has `defer`, and instead of handling
one level, it walks the entire `c.parent` chain calling `dispose` if needed.
The inner one avoids `defer`, which greatly reduces the size of its stackframe.

`c` is a `var` parameter, at each level of recursion.  So the outermost proc
sees the temporary changes made by all inner calls.  This is why `c` is updated
and the `c.parent` chain is maintained at each step.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-04-13 23:35:26 +01:00
Jordan Hrycaj dfc93a74ad
moved validateTransaction() to executor
why:
  not part of VM (see andri's requested change at #573)
2021-04-07 15:13:28 +01:00
Jordan Hrycaj 827b8c9c81
reset explicit import paths for local modules
why:
  it was convenient to have relocatable source modules when writing the
  vm interface wrappers. this patch moves it back to the standard.

also:
  there are no deep links into the vm folder anymore which leaves some
  room for manoeuvring inside
2021-04-01 12:53:22 +01:00
Jordan Hrycaj 00ba7a2718
merge vm_forks and vm_opcode_values => vm_type2
why:
  all types, but they cannot be merged int vm_types because of a circular
  dependency.
2021-03-31 17:53:15 +01:00
Jordan Hrycaj 9e365734e6
renamed nvm_ prefixed modules to its original names
why:
  the nvm_ prefix was used inside the vm folder to hide them temporarily
  from the outside world while writing export wrappers. now all
  functionality is accessed via vm_*, rather than vm/* imports.

todo:
  at a later stage the import headers of the vm modules need to get fixed
  to meet style guide standards (as jacek kindly pointed out.)
2021-03-31 17:19:54 +01:00
Jordan Hrycaj 474bd9e910
expanded nvm_interpreter
details:
  explicit symbol exports rather than wholesale module names
2021-03-31 16:49:11 +01:00
Jordan Hrycaj 7c28d5d362
provide vm_utils_numeric as import/export wrapper
details:
  moved original vm/interpreter/utils/utils_numeric.nim => vm/interpreter/utils/utils_numeric.nim
2021-03-31 16:49:07 +01:00
Jordan Hrycaj 99568c9b46
provide vm_opcode_values as import/export wrapper
details:
  moved original vm/interpreter/opcode_values.nim => vm/interpreter/nvm_opcode_values.nim
2021-03-31 16:49:03 +01:00
Jordan Hrycaj cf63b9b03f
provide vm_memory as import/export wrapper
details:
  moved original vm/memory.nim => vm/nvm_memory.nim
2021-03-31 16:48:44 +01:00
Jordan Hrycaj 7b5d00307c
provide vm_precompiles as import/export wrapper
details:
  moved original vm/precompiles.nim => vm/nvm_precompiles.nim
2021-03-31 16:47:15 +01:00
Jordan Hrycaj 5ce7ca6b32
provide vm_interpreter as import/export wrapper
details:
  moved original vm/interpreter.nim => vm/nvm_interpreter.nim
2021-03-31 16:47:08 +01:00
Jordan Hrycaj eee24de450
provide vm_message as import/export wrapper
details:
  moved original vm/message.nim => vm/nvm_message.nim
2021-03-31 16:47:02 +01:00
Jordan Hrycaj cf3a356d76
provide vm_computation as import/export wrapper
details:
  moved original vm/computation.nim => vm/nvm_computation.nim
2021-03-31 16:38:10 +01:00
Jordan Hrycaj 689458a346
provide vm_gas_costs as import/export wrapper
details:
  moved original vm/interpreter/vm_gas_costs.nim => vm/interpreter/nvm_gas_costs.nim
2021-03-31 16:03:51 +01:00
Jordan Hrycaj 3a3e4d5707
provide vm_forks as import/export wrapper
details:
  moved original vm/interpreter/vm_forks.nim => vm/interpreter/nvm_forks.nim
2021-03-31 16:03:34 +01:00
Jordan Hrycaj 907465300f
isolate vm_state_transactions as import/export wrapper
details:
  moved original vm_state_transactions.nim => vm/nvm_state_transactions.nim
2021-03-31 10:05:47 +01:00
Jordan Hrycaj eda1290c25
isolate vm_state as import/export wrapper
details:
  moved original vm_state.nim => vm/nvm_state.nim
2021-03-31 09:58:26 +01:00
Jordan Hrycaj ed59f602d5
isolate vm_types as import/export wrapper
details:
  moved original vm_types.nim => vm/nvm_types.nim
2021-03-31 09:48:50 +01:00
Jordan Hrycaj a3db0f41d8
remove relative paths ./ and ../ from import section
why:
  relative paths make sources inherently non-relocatable

details:
  import base is set to the nimbus directoy, so importing ./stack
  from file interpreter.nim becomes vm/stack etc.

caveat:
   a file named nimbus/strformat.nim would clash with strformat (but
   not with std/strformat)
2021-03-30 17:20:43 +01:00
jangko 8486c1834c fixes #558, use distinct uint for ChainId and NetworkId to prevent confusion 2021-02-14 11:01:27 +07:00
jangko b6ad47f3a4 fixes evmc bug and add github action job to test evmc 2021-01-20 11:50:07 +07:00
jangko 8c5c967715 bump submodules 2021-01-20 11:50:07 +07:00
jangko ad284e3d25 fixes EIP2929 SLOAD 2021-01-14 23:22:28 +07:00
jangko f6c44ffcc0 fixes EIP2929 CALL opCode 2021-01-14 23:22:28 +07:00
jangko 9709525916
fix byzantium bug related to revertPrecompileTouched 2021-01-13 08:08:56 +07:00
jangko f906d177f4
add comments about disabled EIPs 2021-01-11 15:33:30 +07:00
jangko 01dec1d359
fixes EIP2929 opcodes impl 2021-01-11 14:57:40 +07:00
jangko 3db535aa39
EIP2929 implementation 2021-01-11 14:56:42 +07:00
jangko ab314c1e04
temporary disable EIP2046 and EIP2565 2021-01-11 14:53:51 +07:00
jangko c84e34cac6
simplify blscurve utils impl 2020-12-02 18:14:34 +07:00
jangko 397119468a
simplify bncurve getPoint 2020-12-02 16:17:52 +07:00
jangko fec9d26873
more eip2537 cleanup 2020-12-02 15:15:58 +07:00
jangko 90415d537f
add map_to_curve_g1 implementation for miracl 2020-11-30 21:19:46 +07:00
jangko 3f79588a74
EIP2537 part 3 2020-11-29 08:01:17 +07:00
jangko 0799b4534c
EIP2537 part 2 2020-11-28 23:13:10 +07:00
jangko c1b7ae5b02
EIP2537 part 1 2020-11-27 21:42:17 +07:00
andri lim 360b74327e
Merge pull request #544 from status-im/precompiles_test_rework
Precompiles test rework
2020-11-26 10:34:26 +07:00
jangko 971e00e580
precompile contracts test rework 2020-11-25 20:42:15 +07:00
jangko 56bc1205e5
returnStack: use seq[int] instead of Uint256 Stack 2020-11-25 19:09:16 +07:00
jangko a263e6b1a6
implement EIP2315 tests 2020-11-25 18:23:02 +07:00
jangko a38882a9a0
implement EIP 2315 opcodes 2020-11-25 17:09:10 +07:00
jangko 5a78b8a5a7
stubbing berlin opcodes 2020-11-25 16:43:34 +07:00
jangko e2cd9b20fa
add returnStack to Computation 2020-11-25 16:26:24 +07:00
jangko 5bb6418bcb
implement EIP2565 2020-11-24 16:19:02 +07:00
jangko 97f73fd03d
implement EIP 2046 2020-11-19 14:23:07 +07:00
jangko c68aa47464
fixes fork comparison related code 2020-11-19 11:59:53 +07:00
jangko 648cc77f80
add berlin hf enum 2020-11-19 10:58:37 +07:00
andri lim 676ce3aedb
Merge pull request #528 from narimiran/parse-enum-2
write `Fork` enum in a "modern way"
2020-09-04 15:09:01 +07:00