From 823ec33484165c2ff0247e23607b10b545d29057 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Mon, 29 Nov 2021 14:46:24 +0100 Subject: [PATCH] core/vm: simplify error handling in interpreter loop (#23952) * core/vm: break loop on any error * core/vm: move ErrExecutionReverted to opRevert() * core/vm: use "stop token" to stop the loop * core/vm: unconditionally pc++ in the loop * core/vm: set return data in instruction impls --- core/vm/errors.go | 4 ++++ core/vm/instructions.go | 23 +++++++++++++++-------- core/vm/interpreter.go | 25 +++++++++---------------- core/vm/jump_table.go | 19 +------------------ 4 files changed, 29 insertions(+), 42 deletions(-) diff --git a/core/vm/errors.go b/core/vm/errors.go index c813aa36af..236e22568b 100644 --- a/core/vm/errors.go +++ b/core/vm/errors.go @@ -34,6 +34,10 @@ var ( ErrWriteProtection = errors.New("write protection") ErrReturnDataOutOfBounds = errors.New("return data out of bounds") ErrGasUintOverflow = errors.New("gas 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 92b3b9ae88..987fdcb177 100644 --- a/core/vm/instructions.go +++ b/core/vm/instructions.go @@ -526,7 +526,7 @@ func opJump(pc *uint64, interpreter *EVMInterpreter, callContext *callCtx) ([]by if !callContext.contract.validJumpdest(&pos) { return nil, ErrInvalidJump } - *pc = pos.Uint64() + *pc = pos.Uint64() - 1 // pc will be increased by the interpreter loop return nil, nil } @@ -536,9 +536,7 @@ func opJumpi(pc *uint64, interpreter *EVMInterpreter, callContext *callCtx) ([]b if !callContext.contract.validJumpdest(&pos) { return nil, ErrInvalidJump } - *pc = pos.Uint64() - } else { - *pc++ + *pc = pos.Uint64() - 1 // pc will be increased by the interpreter loop } return nil, nil } @@ -592,8 +590,10 @@ func opCreate(pc *uint64, interpreter *EVMInterpreter, callContext *callCtx) ([] callContext.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 } @@ -623,8 +623,10 @@ func opCreate2(pc *uint64, interpreter *EVMInterpreter, callContext *callCtx) ([ callContext.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 } @@ -656,6 +658,7 @@ func opCall(pc *uint64, interpreter *EVMInterpreter, callContext *callCtx) ([]by } callContext.contract.Gas += returnGas + interpreter.returnData = ret return ret, nil } @@ -687,6 +690,7 @@ func opCallCode(pc *uint64, interpreter *EVMInterpreter, callContext *callCtx) ( } callContext.contract.Gas += returnGas + interpreter.returnData = ret return ret, nil } @@ -715,6 +719,7 @@ func opDelegateCall(pc *uint64, interpreter *EVMInterpreter, callContext *callCt } callContext.contract.Gas += returnGas + interpreter.returnData = ret return ret, nil } @@ -743,6 +748,7 @@ func opStaticCall(pc *uint64, interpreter *EVMInterpreter, callContext *callCtx) } callContext.contract.Gas += returnGas + interpreter.returnData = ret return ret, nil } @@ -750,18 +756,19 @@ func opReturn(pc *uint64, interpreter *EVMInterpreter, callContext *callCtx) ([] offset, size := callContext.stack.pop(), callContext.stack.pop() ret := callContext.memory.GetPtr(int64(offset.Uint64()), int64(size.Uint64())) - return ret, nil + return ret, errStopToken } func opRevert(pc *uint64, interpreter *EVMInterpreter, callContext *callCtx) ([]byte, error) { offset, size := callContext.stack.pop(), callContext.stack.pop() ret := callContext.memory.GetPtr(int64(offset.Uint64()), int64(size.Uint64())) - return ret, nil + interpreter.returnData = ret + return ret, ErrExecutionReverted } func opStop(pc *uint64, interpreter *EVMInterpreter, callContext *callCtx) ([]byte, error) { - return nil, nil + return nil, errStopToken } func opSuicide(pc *uint64, interpreter *EVMInterpreter, callContext *callCtx) ([]byte, error) { @@ -769,7 +776,7 @@ func opSuicide(pc *uint64, interpreter *EVMInterpreter, callContext *callCtx) ([ balance := interpreter.evm.StateDB.GetBalance(callContext.contract.Address()) interpreter.evm.StateDB.AddBalance(common.Address(beneficiary.Bytes20()), balance) interpreter.evm.StateDB.Suicide(callContext.contract.Address()) - 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 2571429b29..5d094c4eea 100644 --- a/core/vm/interpreter.go +++ b/core/vm/interpreter.go @@ -273,24 +273,17 @@ 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: - log.Debug("ErrExecutionReverted", "pc", pc, "reverts", operation.reverts, "err", err) - 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 } // CanRun tells if the contract, passed as an argument, can be diff --git a/core/vm/jump_table.go b/core/vm/jump_table.go index 7580544ba2..f56dc89d56 100644 --- a/core/vm/jump_table.go +++ b/core/vm/jump_table.go @@ -41,11 +41,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 ( @@ -109,7 +105,6 @@ func newConstantinopleInstructionSet() JumpTable { maxStack: maxStack(4, 1), memorySize: memoryCreate2, writes: true, - returns: true, } return instructionSet } @@ -125,7 +120,6 @@ func newByzantiumInstructionSet() JumpTable { minStack: minStack(6, 1), maxStack: maxStack(6, 1), memorySize: memoryStaticCall, - returns: true, } instructionSet[RETURNDATASIZE] = &operation{ execute: opReturnDataSize, @@ -147,8 +141,6 @@ func newByzantiumInstructionSet() JumpTable { minStack: minStack(2, 0), maxStack: maxStack(2, 0), memorySize: memoryRevert, - reverts: true, - returns: true, } return instructionSet } @@ -185,7 +177,6 @@ func newHomesteadInstructionSet() JumpTable { minStack: minStack(6, 1), maxStack: maxStack(6, 1), memorySize: memoryDelegateCall, - returns: true, } return instructionSet } @@ -199,7 +190,6 @@ func newFrontierInstructionSet() JumpTable { constantGas: 0, minStack: minStack(0, 0), maxStack: maxStack(0, 0), - halts: true, }, ADD: { execute: opAdd, @@ -509,14 +499,12 @@ func newFrontierInstructionSet() JumpTable { constantGas: GasMidStep, minStack: minStack(1, 0), maxStack: maxStack(1, 0), - jumps: true, }, JUMPI: { execute: opJumpi, constantGas: GasSlowStep, minStack: minStack(2, 0), maxStack: maxStack(2, 0), - jumps: true, }, PC: { execute: opPc, @@ -974,7 +962,6 @@ func newFrontierInstructionSet() JumpTable { maxStack: maxStack(3, 1), memorySize: memoryCreate, writes: true, - returns: true, }, CALL: { execute: opCall, @@ -983,7 +970,6 @@ func newFrontierInstructionSet() JumpTable { minStack: minStack(7, 1), maxStack: maxStack(7, 1), memorySize: memoryCall, - returns: true, }, CALLCODE: { execute: opCallCode, @@ -992,7 +978,6 @@ func newFrontierInstructionSet() JumpTable { minStack: minStack(7, 1), maxStack: maxStack(7, 1), memorySize: memoryCall, - returns: true, }, RETURN: { execute: opReturn, @@ -1000,14 +985,12 @@ func newFrontierInstructionSet() JumpTable { minStack: minStack(2, 0), maxStack: maxStack(2, 0), memorySize: memoryReturn, - halts: true, }, SELFDESTRUCT: { execute: opSuicide, dynamicGas: gasSelfdestruct, minStack: minStack(1, 0), maxStack: maxStack(1, 0), - halts: true, writes: true, }, }