From 5582435a9522af1705357fd3997b4dfe569cd0a4 Mon Sep 17 00:00:00 2001 From: Andrew Ashikhmin <34320705+yperbasis@users.noreply.github.com> Date: Wed, 21 Dec 2022 11:57:53 +0100 Subject: [PATCH] core/vm: Simplify error handling in interpreter loop (#6394) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-pick https://github.com/ethereum/go-ethereum/pull/23952 Co-authored-by: Paweł Bylica --- core/vm/absint_cfg_proof_check.go | 6 +----- core/vm/absint_cfg_proof_gen.go | 2 +- core/vm/errors.go | 4 ++++ core/vm/instructions.go | 23 +++++++++++++++-------- core/vm/interpreter.go | 24 +++++++++--------------- core/vm/jump_table.go | 19 +------------------ 6 files changed, 31 insertions(+), 47 deletions(-) diff --git a/core/vm/absint_cfg_proof_check.go b/core/vm/absint_cfg_proof_check.go index ac16f618d25..e1b78befd0d 100644 --- a/core/vm/absint_cfg_proof_check.go +++ b/core/vm/absint_cfg_proof_check.go @@ -9,8 +9,6 @@ import ( ) type CfgOpSem struct { - reverts bool - halts bool isPush bool isDup bool isSwap bool @@ -32,8 +30,6 @@ func NewCfgAbsSem() *CfgAbsSem { continue } opsem := CfgOpSem{} - opsem.reverts = op.reverts - opsem.halts = op.halts opsem.isPush = op.isPush opsem.isDup = op.isDup opsem.isSwap = op.isSwap @@ -87,7 +83,7 @@ func resolveCheck(sem *CfgAbsSem, code []byte, st0 *astate, pc0 int) (map[int]bo succs := make(map[int]bool) jumps := make(map[int]bool) - if opsem == nil || opsem.halts || opsem.reverts { + if opsem == nil { return succs, jumps, nil } diff --git a/core/vm/absint_cfg_proof_gen.go b/core/vm/absint_cfg_proof_gen.go index 216bed304c3..938517ac9b4 100644 --- a/core/vm/absint_cfg_proof_gen.go +++ b/core/vm/absint_cfg_proof_gen.go @@ -87,7 +87,7 @@ func toProgram(code []byte) *Program { op := OpCode(code[pc]) stmt.opcode = op stmt.operation = jt[op] - stmt.ends = stmt.operation == nil || stmt.operation.halts || stmt.operation.reverts + stmt.ends = stmt.operation == nil //fmt.Printf("%v %v %v", pc, stmt.opcode, stmt.operation.valid) if op.IsPush() { diff --git a/core/vm/errors.go b/core/vm/errors.go index 63cac917e51..253974d2344 100644 --- a/core/vm/errors.go +++ b/core/vm/errors.go @@ -42,6 +42,10 @@ var ( ErrReturnStackExceeded = errors.New("return stack limit reached") ErrInvalidCode = errors.New("invalid code") ErrNonceUintOverflow = errors.New("nonce uint64 overflow") + + // errStopToken is an internal token indicating interpreter loop termination, + // never returned to outside callers. + errStopToken = errors.New("stop token") ) // ErrStackUnderflow wraps an evm error when the items on the stack less diff --git a/core/vm/instructions.go b/core/vm/instructions.go index 30a5d1adfb0..5788363ae39 100644 --- a/core/vm/instructions.go +++ b/core/vm/instructions.go @@ -582,7 +582,7 @@ func opJump(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byt } return nil, ErrInvalidJump } - *pc = pos.Uint64() + *pc = pos.Uint64() - 1 // pc will be increased by the interpreter loop return nil, nil } @@ -605,9 +605,7 @@ func opJumpi(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]by } return nil, ErrInvalidJump } - *pc = pos.Uint64() - } else { - *pc++ + *pc = pos.Uint64() - 1 // pc will be increased by the interpreter loop } return nil, nil } @@ -663,8 +661,10 @@ func opCreate(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]b scope.Contract.Gas += returnGas if suberr == ErrExecutionReverted { + interpreter.returnData = res // set REVERT data to return data buffer return res, nil } + interpreter.returnData = nil // clear dirty return data buffer return nil, nil } @@ -695,8 +695,10 @@ func opCreate2(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([] scope.Contract.Gas += returnGas if suberr == ErrExecutionReverted { + interpreter.returnData = res // set REVERT data to return data buffer return res, nil } + interpreter.returnData = nil // clear dirty return data buffer return nil, nil } @@ -731,6 +733,7 @@ func opCall(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byt scope.Contract.Gas += returnGas + interpreter.returnData = ret return ret, nil } @@ -764,6 +767,7 @@ func opCallCode(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([ scope.Contract.Gas += returnGas + interpreter.returnData = ret return ret, nil } @@ -793,6 +797,7 @@ func opDelegateCall(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext scope.Contract.Gas += returnGas + interpreter.returnData = ret return ret, nil } @@ -822,19 +827,21 @@ func opStaticCall(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) scope.Contract.Gas += returnGas + interpreter.returnData = ret return ret, nil } func opReturn(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) { offset, size := scope.Stack.Pop(), scope.Stack.Pop() ret := scope.Memory.GetPtr(offset.Uint64(), size.Uint64()) - return ret, nil + return ret, errStopToken } func opRevert(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) { offset, size := scope.Stack.Pop(), scope.Stack.Pop() ret := scope.Memory.GetPtr(offset.Uint64(), size.Uint64()) - return ret, nil + interpreter.returnData = ret + return ret, ErrExecutionReverted } func opUndefined(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) { @@ -842,7 +849,7 @@ func opUndefined(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ( } func opStop(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) { - return nil, nil + return nil, errStopToken } func opSuicide(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) { @@ -855,7 +862,7 @@ func opSuicide(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([] } interpreter.evm.IntraBlockState().AddBalance(beneficiaryAddr, balance) interpreter.evm.IntraBlockState().Suicide(callerAddr) - return nil, nil + return nil, errStopToken } // following functions are used by the instruction jump table diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index fbb7efbfc1e..f9b5481d9c5 100644 --- a/core/vm/interpreter.go +++ b/core/vm/interpreter.go @@ -295,24 +295,18 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) ( } // execute the operation res, err = operation.execute(pc, in, callContext) - // if the operation clears the return data (e.g. it has returning data) - // set the last return to the result of the operation. - if operation.returns { - in.returnData = res - } - switch { - case err != nil: - return nil, err - case operation.reverts: - return res, ErrExecutionReverted - case operation.halts: - return res, nil - case !operation.jumps: - _pc++ + if err != nil { + break } + _pc++ } - return nil, nil + + if err == errStopToken { + err = nil // clear stop token error + } + + return res, err } func (vm *VM) setReadonly(outerReadonly bool) func() { diff --git a/core/vm/jump_table.go b/core/vm/jump_table.go index 713bf947117..000117a082e 100644 --- a/core/vm/jump_table.go +++ b/core/vm/jump_table.go @@ -50,11 +50,7 @@ type operation struct { // memorySize returns the memory size required for the operation memorySize memorySizeFunc - halts bool // indicates whether the operation should halt further execution - jumps bool // indicates whether the program counter should not increment - writes bool // determines whether this a state modifying operation - reverts bool // determines whether the operation reverts state (implicitly halts) - returns bool // determines whether the operations sets the return data content + writes bool // determines whether this a state modifying operation } var ( @@ -188,7 +184,6 @@ func newConstantinopleInstructionSet() JumpTable { numPush: 1, memorySize: memoryCreate2, writes: true, - returns: true, } validate(&instructionSet) return instructionSet @@ -207,7 +202,6 @@ func newByzantiumInstructionSet() JumpTable { numPop: 6, numPush: 1, memorySize: memoryStaticCall, - returns: true, } instructionSet[RETURNDATASIZE] = &operation{ execute: opReturnDataSize, @@ -235,8 +229,6 @@ func newByzantiumInstructionSet() JumpTable { numPop: 2, numPush: 0, memorySize: memoryRevert, - reverts: true, - returns: true, } validate(&instructionSet) return instructionSet @@ -278,7 +270,6 @@ func newHomesteadInstructionSet() JumpTable { numPop: 6, numPush: 1, memorySize: memoryDelegateCall, - returns: true, } validate(&instructionSet) return instructionSet @@ -295,7 +286,6 @@ func newFrontierInstructionSet() JumpTable { maxStack: maxStack(0, 0), numPop: 0, numPush: 0, - halts: true, }, ADD: { execute: opAdd, @@ -704,7 +694,6 @@ func newFrontierInstructionSet() JumpTable { maxStack: maxStack(1, 0), numPop: 1, numPush: 0, - jumps: true, }, JUMPI: { execute: opJumpi, @@ -713,7 +702,6 @@ func newFrontierInstructionSet() JumpTable { maxStack: maxStack(2, 0), numPop: 2, numPush: 0, - jumps: true, }, PC: { execute: opPc, @@ -1447,7 +1435,6 @@ func newFrontierInstructionSet() JumpTable { numPush: 1, memorySize: memoryCreate, writes: true, - returns: true, }, CALL: { execute: opCall, @@ -1458,7 +1445,6 @@ func newFrontierInstructionSet() JumpTable { numPop: 7, numPush: 1, memorySize: memoryCall, - returns: true, }, CALLCODE: { execute: opCallCode, @@ -1469,7 +1455,6 @@ func newFrontierInstructionSet() JumpTable { numPop: 7, numPush: 1, memorySize: memoryCall, - returns: true, }, RETURN: { execute: opReturn, @@ -1479,7 +1464,6 @@ func newFrontierInstructionSet() JumpTable { numPop: 2, numPush: 0, memorySize: memoryReturn, - halts: true, }, SELFDESTRUCT: { execute: opSuicide, @@ -1488,7 +1472,6 @@ func newFrontierInstructionSet() JumpTable { maxStack: maxStack(1, 0), numPop: 1, numPush: 0, - halts: true, writes: true, }, }