core/vm: fill gaps in jump table with opUndefined #24031

chfast · 2021-12-01T18:02:44Z

Instead of checking if a slot in the jump table is nil, fill all unassigned slots with opUndefined() returning expected error ErrInvalidOpCode.

holiman

LGTM. Any benchmarks?

chfast · 2021-12-02T08:21:55Z

This is not much, ~1%.
This is currently broken because some traces in tests does not meet expectations. I will have to fix it first.

holiman · 2021-12-02T08:49:30Z

core/vm/jump_table.go

+	// Fill all unassigned slots with opUndefined.
+	for i, entry := range tbl {
+		if entry == nil {
+			tbl[i] = &operation{execute: opUndefined}


Heh, a fun side-effect here. Since you didn't define maxStack, it's left at 0, so this op will (mostly) never actually execute, it will always fail since the maxStack is reached (unless the stack is totally empty when this op is encountered). Setting maxStack to maxStack(0,0) would solve it

Oh yes, I wanted this but did not realized I need to set non-zero maxStack (in my implementation I have "stack increase").

chfast · 2021-12-02T09:57:54Z

name                                  old time/op  new time/op  delta                                                                                                                                                             
micro/memory_grow_mload/nogrow         231µs ± 1%   228µs ± 0%  -1.28%  (p=0.000 n=19+19)                                                                                                                                         
micro/memory_grow_mload/by1            242µs ± 1%   239µs ± 1%  -0.97%  (p=0.000 n=18+19)                                                                                                                                         
micro/memory_grow_mload/by16           305µs ± 1%   304µs ± 1%    ~     (p=0.108 n=20+20)                                                                                                                                         
micro/memory_grow_mload/by32           383µs ± 1%   382µs ± 1%  -0.35%  (p=0.023 n=20+20)                                                                                                                                         
micro/signextend/zero                  428µs ± 1%   407µs ± 1%  -4.82%  (p=0.000 n=20+20)                                                                                                                                         
micro/signextend/one                   443µs ± 0%   419µs ± 0%  -5.37%  (p=0.000 n=16+19)                                                                                                                                         
micro/loop_with_many_jumpdests/empty  1.71µs ± 2%  1.71µs ± 2%    ~     (p=0.634 n=20+20)                                                                                                                                         
micro/jump_around/empty                146µs ± 1%   145µs ± 1%  -1.04%  (p=0.000 n=19+20)                                                                                                                                         
micro/memory_grow_mstore/nogrow        466µs ± 1%   460µs ± 1%  -1.26%  (p=0.000 n=20+19)                                                                                                                                         
micro/memory_grow_mstore/by1           480µs ± 1%   474µs ± 0%  -1.38%  (p=0.000 n=20+20)                                                                                                                                         
micro/memory_grow_mstore/by16          563µs ± 0%   553µs ± 1%  -1.75%  (p=0.000 n=20+19)                                                                                                                                         
micro/memory_grow_mstore/by32          629µs ± 1%   625µs ± 0%  -0.59%  (p=0.000 n=20+19)                                                                                                                                         
micro/JUMPDEST_n0/empty               3.12ms ± 0%  3.00ms ± 0%  -3.80%  (p=0.000 n=17+18)                                                                                                                                         
main/sha1_divs/empty                   203µs ± 0%   202µs ± 1%  -0.69%  (p=0.000 n=20+19)
main/sha1_divs/1351                   4.11ms ± 0%  4.07ms ± 0%  -1.00%  (p=0.000 n=20+17)
main/sha1_divs/2737                   8.02ms ± 0%  7.93ms ± 0%  -1.21%  (p=0.000 n=20+16)
main/sha1_divs/5311                   15.7ms ± 0%  15.5ms ± 0%  -1.12%  (p=0.000 n=19+19)
main/weierstrudel/0                    475µs ± 0%   468µs ± 1%  -1.55%  (p=0.000 n=18+19)
main/weierstrudel/1                    985µs ± 0%   980µs ± 1%  -0.49%  (p=0.000 n=18+19)
main/weierstrudel/3                   1.55ms ± 1%  1.54ms ± 1%  -0.25%  (p=0.003 n=18+19)
main/weierstrudel/9                   3.21ms ± 0%  3.21ms ± 1%    ~     (p=0.141 n=18+20)
main/weierstrudel/14                  4.59ms ± 1%  4.57ms ± 0%  -0.49%  (p=0.000 n=19+19)
main/sha1_shifts/empty                 159µs ± 1%   157µs ± 0%  -0.91%  (p=0.000 n=20+20)
main/sha1_shifts/1351                 3.24ms ± 1%  3.23ms ± 0%  -0.34%  (p=0.002 n=20+17)
main/sha1_shifts/2737                 6.30ms ± 0%  6.30ms ± 0%    ~     (p=0.851 n=20+18)
main/sha1_shifts/5311                 12.4ms ± 0%  12.3ms ± 0%  -0.35%  (p=0.000 n=19+17)
main/blake2b_huff/empty                102µs ± 1%   102µs ± 0%    ~     (p=0.411 n=19+20)
main/blake2b_huff/2805nulls           1.96ms ± 0%  1.96ms ± 1%    ~     (p=0.374 n=19+18)
main/blake2b_huff/5610nulls           3.80ms ± 0%  3.80ms ± 0%    ~     (p=0.086 n=18+19)
main/blake2b_huff/8415nulls           5.57ms ± 0%  5.56ms ± 0%    ~     (p=0.552 n=19+17)
main/blake2b_shifts/2805nulls         15.1ms ± 1%  15.1ms ± 0%  -0.34%  (p=0.000 n=18+18)
main/blake2b_shifts/5610nulls         30.1ms ± 1%  30.0ms ± 0%  -0.51%  (p=0.000 n=18+19)
main/blake2b_shifts/8415nulls         44.9ms ± 1%  44.6ms ± 0%  -0.61%  (p=0.000 n=18+17)
[Geo mean]                            1.19ms       1.18ms       -1.02%

s1na · 2021-12-02T10:13:25Z

core/vm/interpreter.go

@@ -193,9 +193,6 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) (
 		// enough stack items available to perform the operation.
 		op = contract.GetOp(pc)
 		operation := in.cfg.JumpTable[op]
-		if operation == nil {


Sorry to be constantly nagging about tracing haha but this is gonna do a lot of CaptureState(opUndefined...)

opUndefined still aborts with ErrInvalidOpCode so there shouldn't really be a change there?

How this is a problem?

@s1na Well, the fix is it's basically the same as for the previous one: whenever there are two outputs for the same step of execution (same pc twice in a row), then ignore the first and only care about the second one.

opUndefined still aborts with ErrInvalidOpCode so there shouldn't really be a change there?

@axic the change is that with this PR, it will do a capturestate prior to execute, which it didn't do previously -- erroring before it reached execute.

Gonna do a lot of... was an exaggeration, since there aren't many txes which actually execute undefined opcodes, I think? but yeah not saying this is a show-stopper or anything. Mainly wanted to document the change.

Cherry-pick ethereum/go-ethereum#24031 Co-authored-by: Paweł Bylica <chfast@gmail.com>

chfast added the area:evm label Dec 1, 2021

chfast requested review from holiman, karalabe and rjl493456442 as code owners December 1, 2021 18:02

holiman approved these changes Dec 1, 2021

View reviewed changes

holiman reviewed Dec 2, 2021

View reviewed changes

core/vm: fill gaps in jump table with opUndefined

a7046a5

chfast force-pushed the evm_undefined branch from 78e61d6 to a7046a5 Compare December 2, 2021 09:24

s1na reviewed Dec 2, 2021

View reviewed changes

holiman merged commit 9331fe2 into ethereum:master Dec 3, 2021

holiman deleted the evm_undefined branch December 3, 2021 10:04

holiman added this to the 1.10.14 milestone Dec 3, 2021

han0110 mentioned this pull request Jan 20, 2022

Update geth-utils and bus-mapping to build more witness privacy-scaling-explorations/zkevm-circuits#292

Merged

JacekGlen pushed a commit to JacekGlen/go-ethereum that referenced this pull request May 26, 2022

core/vm: fill gaps in jump table with opUndefined (ethereum#24031)

77c5a02

yperbasis mentioned this pull request Dec 19, 2022

core/vm: fill gaps in jump table with opUndefined erigontech/erigon#6372

Merged

yperbasis added a commit to erigontech/erigon that referenced this pull request Dec 20, 2022

core/vm: fill gaps in jump table with opUndefined (#6372)

31d30df

Cherry-pick ethereum/go-ethereum#24031 Co-authored-by: Paweł Bylica <chfast@gmail.com>

gzliudan mentioned this pull request Feb 27, 2024

preparation for solidity v0.8.23 upgrade XinFinOrg/XDPoSChain#452

Merged

19 tasks

gzliudan pushed a commit to gzliudan/XDPoSChain that referenced this pull request Mar 1, 2024

core/vm: fill gaps in jump table with opUndefined (ethereum#24031)

7d3c783

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core/vm: fill gaps in jump table with opUndefined #24031

core/vm: fill gaps in jump table with opUndefined #24031

chfast commented Dec 1, 2021 •

edited

Loading

holiman left a comment

chfast commented Dec 2, 2021

holiman Dec 2, 2021

chfast Dec 2, 2021

chfast commented Dec 2, 2021

s1na Dec 2, 2021

axic Dec 2, 2021

chfast Dec 2, 2021

holiman Dec 2, 2021

holiman Dec 2, 2021

s1na Dec 2, 2021

core/vm: fill gaps in jump table with opUndefined #24031

core/vm: fill gaps in jump table with opUndefined #24031

Conversation

chfast commented Dec 1, 2021 • edited Loading

holiman left a comment

Choose a reason for hiding this comment

chfast commented Dec 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chfast commented Dec 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chfast commented Dec 1, 2021 •

edited

Loading