From 67bad856b10af3f0f337dbd1ea31860d5c2ee77f Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 1 Oct 2018 20:55:58 +0200 Subject: [PATCH] core/vm: avoid storing jumpdest analysis for initcode --- core/vm/analysis.go | 24 ++------------------ core/vm/analysis_test.go | 25 ++++++++++++++++++-- core/vm/contract.go | 49 ++++++++++++++++++++++++++++++++-------- core/vm/evm.go | 2 +- core/vm/instructions.go | 4 ++-- 5 files changed, 68 insertions(+), 36 deletions(-) diff --git a/core/vm/analysis.go b/core/vm/analysis.go index f9c4298d39..d0b367530c 100644 --- a/core/vm/analysis.go +++ b/core/vm/analysis.go @@ -17,33 +17,13 @@ package vm import ( - "math/big" - "github.com/ethereum/go-ethereum/common" ) -// destinations stores one map per contract (keyed by hash of code). -// The maps contain an entry for each location of a JUMPDEST -// instruction. +// destinations stores one bitmap per contract (keyed by hash of code). +// The bitmaps mark code and data-sections for a piece of contract code type destinations map[common.Hash]bitvec -// has checks whether code has a JUMPDEST at dest. -func (d destinations) has(codehash common.Hash, code []byte, dest *big.Int) bool { - // PC cannot go beyond len(code) and certainly can't be bigger than 63bits. - // Don't bother checking for JUMPDEST in that case. - udest := dest.Uint64() - if dest.BitLen() >= 63 || udest >= uint64(len(code)) { - return false - } - - m, analysed := d[codehash] - if !analysed { - m = codeBitmap(code) - d[codehash] = m - } - return OpCode(code[udest]) == JUMPDEST && m.codeSegment(udest) -} - // bitvec is a bit vector which maps bytes in a program. // An unset bit means the byte is an opcode, a set bit means // it's data (i.e. argument of PUSHxx). diff --git a/core/vm/analysis_test.go b/core/vm/analysis_test.go index a64f90ed9c..b7f04a74fc 100644 --- a/core/vm/analysis_test.go +++ b/core/vm/analysis_test.go @@ -16,7 +16,10 @@ package vm -import "testing" +import ( + "github.com/ethereum/go-ethereum/crypto" + "testing" +) func TestJumpDestAnalysis(t *testing.T) { tests := []struct { @@ -49,5 +52,23 @@ func TestJumpDestAnalysis(t *testing.T) { t.Fatalf("expected %x, got %02x", test.exp, ret[test.which]) } } - +} + +func BenchmarkJumpdestAnalysis_1200k(bench *testing.B) { + // 1.4 ms + code := make([]byte, 1200000) + bench.ResetTimer() + for i := 0; i < bench.N; i++ { + codeBitmap(code) + } + bench.StopTimer() +} +func BenchmarkJumpdestHashing_1200k(bench *testing.B) { + // 4 ms + code := make([]byte, 1200000) + bench.ResetTimer() + for i := 0; i < bench.N; i++ { + crypto.Keccak256Hash(code) + } + bench.StopTimer() } diff --git a/core/vm/contract.go b/core/vm/contract.go index 26bca68951..0b87a6543f 100644 --- a/core/vm/contract.go +++ b/core/vm/contract.go @@ -49,10 +49,10 @@ type Contract struct { caller ContractRef self ContractRef - jumpdests destinations // result of JUMPDEST analysis. + jumpdests destinations // Aggregated result of JUMPDEST analysis. Code []byte - CodeHash common.Hash + CodeHash *common.Hash CodeAddr *common.Address Input []byte @@ -84,6 +84,37 @@ func NewContract(caller ContractRef, object ContractRef, value *big.Int, gas uin return c } +func (c *Contract) validJumpdest(dest *big.Int) bool { + udest := dest.Uint64() + // PC cannot go beyond len(code) and certainly can't be bigger than 63bits. + // Don't bother checking for JUMPDEST in that case. + if dest.BitLen() >= 63 || udest >= uint64(len(c.Code)) { + return false + } + // Only JUMPDESTs allowed for destinations + if OpCode(c.Code[udest]) != JUMPDEST { + return false + } + var analysis bitvec + // Do we have a contract hash already? + if c.CodeHash != nil { + var exist bool + // Does parent context have the analysis? + analysis, exist = c.jumpdests[*c.CodeHash] + if !exist { + // Do the analysis + analysis = codeBitmap(c.Code) + // Save in parent context + c.jumpdests[*c.CodeHash] = analysis + } + return analysis.codeSegment(udest) + } + //Don't have the hash, most likely a piece of initcode not already in state trie + analysis = codeBitmap(c.Code) + // Don't bother saving this + return analysis.codeSegment(udest) +} + // AsDelegate sets the contract to be a delegate call and returns the current // contract (for chaining calls) func (c *Contract) AsDelegate() *Contract { @@ -138,16 +169,16 @@ func (c *Contract) Value() *big.Int { return c.value } -// SetCode sets the code to the contract -func (c *Contract) SetCode(hash common.Hash, code []byte) { - c.Code = code - c.CodeHash = hash -} - // SetCallCode sets the code of the contract and address of the backing data // object func (c *Contract) SetCallCode(addr *common.Address, hash common.Hash, code []byte) { c.Code = code - c.CodeHash = hash + c.CodeHash = &hash + c.CodeAddr = addr +} + +func (c *Contract) SetCodeOptionalHash(addr *common.Address, codeAndHash codeAndHash) { + c.Code = codeAndHash.code + c.CodeHash = codeAndHash.hash c.CodeAddr = addr } diff --git a/core/vm/evm.go b/core/vm/evm.go index e604d79822..132bc4318b 100644 --- a/core/vm/evm.go +++ b/core/vm/evm.go @@ -395,7 +395,7 @@ func (evm *EVM) create(caller ContractRef, codeAndHash codeAndHash, gas uint64, // EVM. The contract is a scoped environment for this execution context // only. contract := NewContract(caller, AccountRef(address), value, gas) - contract.SetCallCode(&address, codeAndHash.Hash(), codeAndHash.code) + contract.SetCodeOptionalHash(&address, codeAndHash) if evm.vmConfig.NoRecursion && evm.depth > 0 { return nil, address, gas, nil diff --git a/core/vm/instructions.go b/core/vm/instructions.go index 4d1bd4a342..9623fb8dee 100644 --- a/core/vm/instructions.go +++ b/core/vm/instructions.go @@ -620,7 +620,7 @@ func opSstore(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memor func opJump(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory *Memory, stack *Stack) ([]byte, error) { pos := stack.pop() - if !contract.jumpdests.has(contract.CodeHash, contract.Code, pos) { + if !contract.validJumpdest(pos) { nop := contract.GetOp(pos.Uint64()) return nil, fmt.Errorf("invalid jump destination (%v) %v", nop, pos) } @@ -633,7 +633,7 @@ func opJump(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory func opJumpi(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory *Memory, stack *Stack) ([]byte, error) { pos, cond := stack.pop(), stack.pop() if cond.Sign() != 0 { - if !contract.jumpdests.has(contract.CodeHash, contract.Code, pos) { + if !contract.validJumpdest(pos) { nop := contract.GetOp(pos.Uint64()) return nil, fmt.Errorf("invalid jump destination (%v) %v", nop, pos) }