From 0c4ece01664dc7a45c88f302158a417416d5423b Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Fri, 16 Aug 2024 10:21:13 +0200 Subject: [PATCH] core/vm: use methods over direct access to stack internals --- core/vm/evm.go | 3 + core/vm/gas_table.go | 4 +- core/vm/instructions_test.go | 38 ++++---- core/vm/interpreter.go | 8 +- core/vm/runtime/runtime_test.go | 60 ++++++++++++ core/vm/stack.go | 166 ++++++++++++++++++++------------ 6 files changed, 193 insertions(+), 86 deletions(-) diff --git a/core/vm/evm.go b/core/vm/evm.go index b45a434545..c3f11b31b9 100644 --- a/core/vm/evm.go +++ b/core/vm/evm.go @@ -125,6 +125,8 @@ type EVM struct { // jumpDests is the aggregated result of JUMPDEST analysis made through // the life cycle of EVM. jumpDests map[common.Hash]bitvec + + arena *stackArena } // NewEVM constructs an EVM instance with the supplied block context, state @@ -139,6 +141,7 @@ func NewEVM(blockCtx BlockContext, statedb StateDB, chainConfig *params.ChainCon chainConfig: chainConfig, chainRules: chainConfig.Rules(blockCtx.BlockNumber, blockCtx.Random != nil, blockCtx.Time), jumpDests: make(map[common.Hash]bitvec), + arena: newArena(), } evm.precompiles = activePrecompiledContracts(evm.chainRules) evm.interpreter = NewEVMInterpreter(evm) diff --git a/core/vm/gas_table.go b/core/vm/gas_table.go index f711aa4a18..7e12f670c4 100644 --- a/core/vm/gas_table.go +++ b/core/vm/gas_table.go @@ -345,7 +345,7 @@ func gasCreate2Eip3860(evm *EVM, contract *Contract, stack *Stack, mem *Memory, } func gasExpFrontier(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memorySize uint64) (uint64, error) { - expByteLen := uint64((stack.data[stack.len()-2].BitLen() + 7) / 8) + expByteLen := uint64((stack.Back(1).BitLen() + 7) / 8) var ( gas = expByteLen * params.ExpByteFrontier // no overflow check required. Max is 256 * ExpByte gas @@ -358,7 +358,7 @@ func gasExpFrontier(evm *EVM, contract *Contract, stack *Stack, mem *Memory, mem } func gasExpEIP158(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memorySize uint64) (uint64, error) { - expByteLen := uint64((stack.data[stack.len()-2].BitLen() + 7) / 8) + expByteLen := uint64((stack.Back(1).BitLen() + 7) / 8) var ( gas = expByteLen * params.ExpByteEIP158 // no overflow check required. Max is 256 * ExpByte gas diff --git a/core/vm/instructions_test.go b/core/vm/instructions_test.go index 0902d17c54..cce88eebb5 100644 --- a/core/vm/instructions_test.go +++ b/core/vm/instructions_test.go @@ -97,7 +97,7 @@ func init() { func testTwoOperandOp(t *testing.T, tests []TwoOperandTestcase, opFn executionFunc, name string) { var ( evm = NewEVM(BlockContext{}, nil, params.TestChainConfig, Config{}) - stack = newstack() + stack = newStackForTesting() pc = uint64(0) ) @@ -108,8 +108,8 @@ func testTwoOperandOp(t *testing.T, tests []TwoOperandTestcase, opFn executionFu stack.push(x) stack.push(y) opFn(&pc, evm.interpreter, &ScopeContext{nil, stack, nil}) - if len(stack.data) != 1 { - t.Errorf("Expected one item on stack after %v, got %d: ", name, len(stack.data)) + if stack.len() != 1 { + t.Errorf("Expected one item on stack after %v, got %d: ", name, stack.len()) } actual := stack.pop() @@ -195,7 +195,7 @@ func TestSAR(t *testing.T) { func TestAddMod(t *testing.T) { var ( evm = NewEVM(BlockContext{}, nil, params.TestChainConfig, Config{}) - stack = newstack() + stack = newStackForTesting() pc = uint64(0) ) tests := []struct { @@ -238,7 +238,7 @@ func TestWriteExpectedValues(t *testing.T) { getResult := func(args []*twoOperandParams, opFn executionFunc) []TwoOperandTestcase { var ( evm = NewEVM(BlockContext{}, nil, params.TestChainConfig, Config{}) - stack = newstack() + stack = newStackForTesting() pc = uint64(0) ) result := make([]TwoOperandTestcase, len(args)) @@ -282,7 +282,7 @@ func TestJsonTestcases(t *testing.T) { func opBenchmark(bench *testing.B, op executionFunc, args ...string) { var ( evm = NewEVM(BlockContext{}, nil, params.TestChainConfig, Config{}) - stack = newstack() + stack = newStackForTesting() scope = &ScopeContext{nil, stack, nil} ) // convert args @@ -520,7 +520,7 @@ func BenchmarkOpIsZero(b *testing.B) { func TestOpMstore(t *testing.T) { var ( evm = NewEVM(BlockContext{}, nil, params.TestChainConfig, Config{}) - stack = newstack() + stack = newStackForTesting() mem = NewMemory() ) mem.Resize(64) @@ -543,7 +543,7 @@ func TestOpMstore(t *testing.T) { func BenchmarkOpMstore(bench *testing.B) { var ( evm = NewEVM(BlockContext{}, nil, params.TestChainConfig, Config{}) - stack = newstack() + stack = newStackForTesting() mem = NewMemory() ) mem.Resize(64) @@ -563,7 +563,7 @@ func TestOpTstore(t *testing.T) { var ( statedb, _ = state.New(types.EmptyRootHash, state.NewDatabaseForTesting()) evm = NewEVM(BlockContext{}, statedb, params.TestChainConfig, Config{}) - stack = newstack() + stack = newStackForTesting() mem = NewMemory() caller = common.Address{} to = common.Address{1} @@ -602,7 +602,7 @@ func TestOpTstore(t *testing.T) { func BenchmarkOpKeccak256(bench *testing.B) { var ( evm = NewEVM(BlockContext{}, nil, params.TestChainConfig, Config{}) - stack = newstack() + stack = newStackForTesting() mem = NewMemory() ) mem.Resize(32) @@ -675,7 +675,7 @@ func TestCreate2Addresses(t *testing.T) { codeHash := crypto.Keccak256(code) address := crypto.CreateAddress2(origin, salt, codeHash) /* - stack := newstack() + stack := newStackForTesting() // salt, but we don't need that for this test stack.push(big.NewInt(int64(len(code)))) //size stack.push(big.NewInt(0)) // memstart @@ -704,12 +704,12 @@ func TestRandom(t *testing.T) { } { var ( evm = NewEVM(BlockContext{Random: &tt.random}, nil, params.TestChainConfig, Config{}) - stack = newstack() + stack = newStackForTesting() pc = uint64(0) ) opRandom(&pc, evm.interpreter, &ScopeContext{nil, stack, nil}) - if len(stack.data) != 1 { - t.Errorf("Expected one item on stack after %v, got %d: ", tt.name, len(stack.data)) + if have, want := stack.len(), 1; have != want { + t.Errorf("test '%v': want %d item(s) on stack, have %d: ", tt.name, have, want) } actual := stack.pop() expected, overflow := uint256.FromBig(new(big.Int).SetBytes(tt.random.Bytes())) @@ -744,14 +744,14 @@ func TestBlobHash(t *testing.T) { } { var ( evm = NewEVM(BlockContext{}, nil, params.TestChainConfig, Config{}) - stack = newstack() + stack = newStackForTesting() pc = uint64(0) ) evm.SetTxContext(TxContext{BlobHashes: tt.hashes}) stack.push(uint256.NewInt(tt.idx)) opBlobHash(&pc, evm.interpreter, &ScopeContext{nil, stack, nil}) - if len(stack.data) != 1 { - t.Errorf("Expected one item on stack after %v, got %d: ", tt.name, len(stack.data)) + if have, want := stack.len(), 1; have != want { + t.Errorf("test '%v': want %d item(s) on stack, have %d: ", tt.name, have, want) } actual := stack.pop() expected, overflow := uint256.FromBig(new(big.Int).SetBytes(tt.expect.Bytes())) @@ -847,7 +847,7 @@ func TestOpMCopy(t *testing.T) { } { var ( evm = NewEVM(BlockContext{}, nil, params.TestChainConfig, Config{}) - stack = newstack() + stack = newStackForTesting() pc = uint64(0) ) data := common.FromHex(strings.ReplaceAll(tc.pre, " ", "")) @@ -910,7 +910,7 @@ func TestPush(t *testing.T) { scope := &ScopeContext{ Memory: nil, - Stack: newstack(), + Stack: newStackForTesting(), Contract: &Contract{ Code: code, }, diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index d0e5967e6e..79d76ff1fd 100644 --- a/core/vm/interpreter.go +++ b/core/vm/interpreter.go @@ -181,9 +181,9 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) ( } var ( - op OpCode // current opcode - mem = NewMemory() // bound memory - stack = newstack() // local stack + op OpCode // current opcode + mem = NewMemory() // bound memory + stack = in.evm.arena.stack() // local stack callContext = &ScopeContext{ Memory: mem, Stack: stack, @@ -205,7 +205,7 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) ( // so that it gets executed _after_: the OnOpcode needs the stacks before // they are returned to the pools defer func() { - returnStack(stack) + stack.release() mem.Free() }() contract.Input = input diff --git a/core/vm/runtime/runtime_test.go b/core/vm/runtime/runtime_test.go index d75a5b0459..61f42e24a4 100644 --- a/core/vm/runtime/runtime_test.go +++ b/core/vm/runtime/runtime_test.go @@ -937,3 +937,63 @@ func TestDelegatedAccountAccessCost(t *testing.T) { } } } + +func TestManyLargeStacks(t *testing.T) { + // This piece of code will push 512 items to the stack, and then call itself + // recursively. + code := make([]byte, 10) + for i := range code { + code[i] = byte(vm.PUSH0) + } + code = append(code, []byte{ + byte(vm.ADDRESS), // address to call + byte(vm.GAS), + byte(vm.CALL), + }...) + + main := common.HexToAddress("0xbb") + statedb, _ := state.New(types.EmptyRootHash, state.NewDatabase(rawdb.NewMemoryDatabase()), nil) + statedb.SetCode(main, code) + + //tracer := logger.NewJSONLogger(nil, os.Stdout) + var tracer *tracing.Hooks + _, _, err := Call(main, nil, &Config{ + GasLimit: 10_000_000, + State: statedb, + EVMConfig: vm.Config{ + Tracer: tracer, + }}) + if err != nil { + t.Fatal("didn't expect error", err) + } +} + +func BenchmarkLargeDeepStacks(b *testing.B) { + // This piece of code will push 512 items to the stack, and then call itself + // recursively. + code := make([]byte, 512) + for i := range code { + code[i] = byte(vm.PUSH0) + } + code = append(code, []byte{ + byte(vm.ADDRESS), // address to call + byte(vm.GAS), + byte(vm.CALL), + }...) + benchmarkNonModifyingCode(10_000_000, code, "deep-large-stacks-10M", "", b) +} + +func BenchmarkShortDeepStacks(b *testing.B) { + // This piece of code will push a few items to the stack, and then call itself + // recursively. + code := make([]byte, 8) + for i := range code { + code[i] = byte(vm.PUSH0) + } + code = append(code, []byte{ + byte(vm.ADDRESS), // address to call + byte(vm.GAS), + byte(vm.CALL), + }...) + benchmarkNonModifyingCode(10_000_000, code, "deep-short-stacks-10M", "", b) +} diff --git a/core/vm/stack.go b/core/vm/stack.go index 879dc9aa6d..11b305d20f 100644 --- a/core/vm/stack.go +++ b/core/vm/stack.go @@ -17,111 +17,155 @@ package vm import ( - "sync" + "slices" "github.com/holiman/uint256" ) -var stackPool = sync.Pool{ - New: func() interface{} { - return &Stack{data: make([]uint256.Int, 0, 16)} - }, +// stackArena is an arena which actual evm stacks use for data storage +type stackArena struct { + data []uint256.Int + top int // first free slot +} + +func (sa *stackArena) push(value *uint256.Int) { + if len(sa.data) <= sa.top { + // we need to grow the arena + sa.data = slices.Grow(sa.data, 512) + sa.data = sa.data[:cap(sa.data)] + } + sa.data[sa.top] = *value + sa.top++ +} + +func (sa *stackArena) pop() { + sa.top-- +} + +func newArena() *stackArena { + return &stackArena{ + data: make([]uint256.Int, 1024), + } +} + +// stack returns an instance of a stack which uses the underlying arena. The instance +// must be released by invoking the (*Stack).release() method +func (sa *stackArena) stack() *Stack { + return &Stack{ + bottom: sa.top, + size: 0, + inner: sa, + } +} + +// newStackForTesting is meant to be used solely for testing. It creates a stack +// backed by a newly allocated arena. +func newStackForTesting() *Stack { + arena := &stackArena{ + data: make([]uint256.Int, 256), + } + return arena.stack() } // Stack is an object for basic stack operations. Items popped to the stack are // expected to be changed and modified. stack does not take care of adding newly // initialized objects. type Stack struct { - data []uint256.Int + bottom int // bottom is the index of the first element of this stack + size int // size is the number of elements in this stack + inner *stackArena } -func newstack() *Stack { - return stackPool.Get().(*Stack) -} - -func returnStack(s *Stack) { - s.data = s.data[:0] - stackPool.Put(s) +// release un-claims the area of the arena which was claimed by the stack. +func (s *Stack) release() { + // When the stack is returned, need to notify the arena that the new 'top' is + // the returned stack's bottom. + s.inner.top = s.bottom } // Data returns the underlying uint256.Int array. -func (st *Stack) Data() []uint256.Int { - return st.data +func (s *Stack) Data() []uint256.Int { + return s.inner.data[s.bottom : s.bottom+s.size] } -func (st *Stack) push(d *uint256.Int) { +func (s *Stack) push(d *uint256.Int) { // NOTE push limit (1024) is checked in baseCheck - st.data = append(st.data, *d) + s.inner.push(d) + s.size++ } -func (st *Stack) pop() (ret uint256.Int) { - ret = st.data[len(st.data)-1] - st.data = st.data[:len(st.data)-1] - return +func (s *Stack) pop() uint256.Int { + ret := s.inner.data[s.bottom+s.size-1] + s.inner.pop() + s.size-- + return ret } -func (st *Stack) len() int { - return len(st.data) +func (s *Stack) len() int { + return s.size } -func (st *Stack) swap1() { - st.data[st.len()-2], st.data[st.len()-1] = st.data[st.len()-1], st.data[st.len()-2] +func (s *Stack) swap1() { + s.inner.data[s.bottom+s.size-2], s.inner.data[s.bottom+s.size-1] = s.inner.data[s.bottom+s.size-1], s.inner.data[s.bottom+s.size-2] } -func (st *Stack) swap2() { - st.data[st.len()-3], st.data[st.len()-1] = st.data[st.len()-1], st.data[st.len()-3] +func (s *Stack) swap2() { + s.inner.data[s.bottom+s.size-3], s.inner.data[s.bottom+s.size-1] = s.inner.data[s.bottom+s.size-1], s.inner.data[s.bottom+s.size-3] } -func (st *Stack) swap3() { - st.data[st.len()-4], st.data[st.len()-1] = st.data[st.len()-1], st.data[st.len()-4] +func (s *Stack) swap3() { + s.inner.data[s.bottom+s.size-4], s.inner.data[s.bottom+s.size-1] = s.inner.data[s.bottom+s.size-1], s.inner.data[s.bottom+s.size-4] } -func (st *Stack) swap4() { - st.data[st.len()-5], st.data[st.len()-1] = st.data[st.len()-1], st.data[st.len()-5] +func (s *Stack) swap4() { + s.inner.data[s.bottom+s.size-5], s.inner.data[s.bottom+s.size-1] = s.inner.data[s.bottom+s.size-1], s.inner.data[s.bottom+s.size-5] } -func (st *Stack) swap5() { - st.data[st.len()-6], st.data[st.len()-1] = st.data[st.len()-1], st.data[st.len()-6] +func (s *Stack) swap5() { + s.inner.data[s.bottom+s.size-6], s.inner.data[s.bottom+s.size-1] = s.inner.data[s.bottom+s.size-1], s.inner.data[s.bottom+s.size-6] } -func (st *Stack) swap6() { - st.data[st.len()-7], st.data[st.len()-1] = st.data[st.len()-1], st.data[st.len()-7] +func (s *Stack) swap6() { + s.inner.data[s.bottom+s.size-7], s.inner.data[s.bottom+s.size-1] = s.inner.data[s.bottom+s.size-1], s.inner.data[s.bottom+s.size-7] } -func (st *Stack) swap7() { - st.data[st.len()-8], st.data[st.len()-1] = st.data[st.len()-1], st.data[st.len()-8] +func (s *Stack) swap7() { + s.inner.data[s.bottom+s.size-8], s.inner.data[s.bottom+s.size-1] = s.inner.data[s.bottom+s.size-1], s.inner.data[s.bottom+s.size-8] } -func (st *Stack) swap8() { - st.data[st.len()-9], st.data[st.len()-1] = st.data[st.len()-1], st.data[st.len()-9] +func (s *Stack) swap8() { + s.inner.data[s.bottom+s.size-9], s.inner.data[s.bottom+s.size-1] = s.inner.data[s.bottom+s.size-1], s.inner.data[s.bottom+s.size-9] } -func (st *Stack) swap9() { - st.data[st.len()-10], st.data[st.len()-1] = st.data[st.len()-1], st.data[st.len()-10] +func (s *Stack) swap9() { + s.inner.data[s.bottom+s.size-10], s.inner.data[s.bottom+s.size-1] = s.inner.data[s.bottom+s.size-1], s.inner.data[s.bottom+s.size-10] } -func (st *Stack) swap10() { - st.data[st.len()-11], st.data[st.len()-1] = st.data[st.len()-1], st.data[st.len()-11] +func (s *Stack) swap10() { + s.inner.data[s.bottom+s.size-11], s.inner.data[s.bottom+s.size-1] = s.inner.data[s.bottom+s.size-1], s.inner.data[s.bottom+s.size-11] } -func (st *Stack) swap11() { - st.data[st.len()-12], st.data[st.len()-1] = st.data[st.len()-1], st.data[st.len()-12] +func (s *Stack) swap11() { + s.inner.data[s.bottom+s.size-12], s.inner.data[s.bottom+s.size-1] = s.inner.data[s.bottom+s.size-1], s.inner.data[s.bottom+s.size-12] } -func (st *Stack) swap12() { - st.data[st.len()-13], st.data[st.len()-1] = st.data[st.len()-1], st.data[st.len()-13] +func (s *Stack) swap12() { + s.inner.data[s.bottom+s.size-13], s.inner.data[s.bottom+s.size-1] = s.inner.data[s.bottom+s.size-1], s.inner.data[s.bottom+s.size-13] } -func (st *Stack) swap13() { - st.data[st.len()-14], st.data[st.len()-1] = st.data[st.len()-1], st.data[st.len()-14] +func (s *Stack) swap13() { + s.inner.data[s.bottom+s.size-14], s.inner.data[s.bottom+s.size-1] = s.inner.data[s.bottom+s.size-1], s.inner.data[s.bottom+s.size-14] } -func (st *Stack) swap14() { - st.data[st.len()-15], st.data[st.len()-1] = st.data[st.len()-1], st.data[st.len()-15] +func (s *Stack) swap14() { + s.inner.data[s.bottom+s.size-15], s.inner.data[s.bottom+s.size-1] = s.inner.data[s.bottom+s.size-1], s.inner.data[s.bottom+s.size-15] } -func (st *Stack) swap15() { - st.data[st.len()-16], st.data[st.len()-1] = st.data[st.len()-1], st.data[st.len()-16] +func (s *Stack) swap15() { + s.inner.data[s.bottom+s.size-16], s.inner.data[s.bottom+s.size-1] = s.inner.data[s.bottom+s.size-1], s.inner.data[s.bottom+s.size-16] } -func (st *Stack) swap16() { - st.data[st.len()-17], st.data[st.len()-1] = st.data[st.len()-1], st.data[st.len()-17] +func (s *Stack) swap16() { + s.inner.data[s.bottom+s.size-17], s.inner.data[s.bottom+s.size-1] = s.inner.data[s.bottom+s.size-1], s.inner.data[s.bottom+s.size-17] } -func (st *Stack) dup(n int) { - st.push(&st.data[st.len()-n]) +func (s *Stack) dup(n int) { + // TODO: check size of inner + s.inner.data[s.bottom+s.size] = s.inner.data[s.bottom+s.size-n] + s.size++ + s.inner.top++ } -func (st *Stack) peek() *uint256.Int { - return &st.data[st.len()-1] +func (s *Stack) peek() *uint256.Int { + return &s.inner.data[s.bottom+s.size-1] } // Back returns the n'th item in stack -func (st *Stack) Back(n int) *uint256.Int { - return &st.data[st.len()-n-1] +func (s *Stack) Back(n int) *uint256.Int { + return &s.inner.data[s.bottom+s.size-n-1] }