core/vm: remove unnecessary check, fix benchmarks, use immediates in analysis

Using the immediates-map in jumpdest analysis leads to less error prone code (only one place to update it)
and it improves the worst case from
```
BenchmarkJumpdestOpEOFAnalysis/EOFCREATE-24          724   1471215 ns/op 835.23 MB/s
```
to
```
BenchmarkJumpdestOpEOFAnalysis/EOFCREATE-24         1710    693609 ns/op 1771.60 MB/s
```
which is a quite significant 2x improvement and brings them in line with the average case

cmd/eofdump/testdata/results.initcode.txt

@@ -1906,7 +1906,7 @@ ERR: unreachable code: arg 253, last 1, pos 13
 ERR: invalid magic: want ef00 in sub container 0
 ERR: missing data header: found section fe instead in sub container 0
 ERR: invalid type content, max stack height exceeds limit for section 0: have 65007 in sub container 0
-ERR: section already referenced, arg :0
+ERR: stack underflow (0 <=> 4): at pos 0
 ERR: subcontainer not referenced at all
 ERR: truncated immediate: op PUSH20, pos 6
 ERR: invalid jump destination: out-of-bounds offset: offset 12336, dest 12342, pos 4

cmd/eofdump/testdata/results.regular.txt

@@ -1906,14 +1906,14 @@ ERR: unreachable code: arg 253, last 1, pos 13
 ERR: invalid container size: have 24594, want 24591
 ERR: missing data header: found section fe instead in sub container 0
 ERR: invalid type content, max stack height exceeds limit for section 0: have 65007 in sub container 0
-ERR: section already referenced, arg :0
+ERR: stack underflow (0 <=> 4): at pos 0
 ERR: subcontainer not referenced at all
 ERR: truncated immediate: op PUSH20, pos 6
 ERR: invalid jump destination: out-of-bounds offset: offset 12336, dest 12342, pos 4
 ERR: invalid backward jump want 4 as current min got 5 at pos 12,
 ERR: invalid backward jump want 0 as current min got 1 at pos 12,
 ERR: unreachable code: arg 48, last 1, pos 13
-ERR: section already referenced, arg :0
+ERR: invalid code termination: end with ADDRESS, pos 17
 ERR: unreachable code: arg 48, last 2, pos 8
 ERR: invalid code termination: end with ADDRESS, pos 1
 ERR: invalid section argument: arg 12336, last 1, pos 14

core/asm/asm_test.go

@@ -34,6 +34,7 @@ func TestInstructionIterator(t *testing.T) {
 		{2, "5900", ""},                            // push0
 		{0, "", ""},                                // empty
 		{2, "d1aabb00", ""},                        // DATALOADN(aabb),STOP
+		{0, "d1aa", "incomplete instruction at 0"}, // DATALOADN(aa) invalid
 
 	} {
 		var (

core/vm/analysis_eof.go

@@ -35,14 +35,7 @@ func eofCodeBitmapInternal(code, bits bitvec) bitvec {
 		)
 		pc++
 
-		switch {
+		if op == RJUMPV {
-		case op < PUSH1:
-			continue
-		case op <= PUSH32:
-			numbits = uint16(op - PUSH1 + 1)
-		case op == RJUMP || op == RJUMPI || op == CALLF || op == JUMPF || op == DATALOADN:
-			numbits = 2
-		case op == RJUMPV:
 			// RJUMPV is unique as it has a variable sized operand.
 			// The total size is determined by the count byte which
 			// immediate proceeds RJUMPV. Truncation will be caught
@@ -60,11 +53,11 @@ func eofCodeBitmapInternal(code, bits bitvec) bitvec {
 				// as possible.
 				numbits = uint16(end - pc)
 			}
-		case op == DUPN || op == SWAPN || op == EXCHANGE || op == EOFCREATE || op == RETURNCONTRACT:
+		} else {
-			numbits = 1
+			numbits = uint16(Immediates(op))
-		default:
+			if numbits == 0 {
-			// Op had no immediate operand, continue.
+				continue
-			continue
+			}
 		}
 
 		if numbits >= 8 {

core/vm/analysis_legacy_test.go

@@ -105,3 +105,31 @@ func BenchmarkJumpdestOpAnalysis(bench *testing.B) {
 	op = STOP
 	bench.Run(op.String(), bencher)
 }
+
+func BenchmarkJumpdestOpEOFAnalysis(bench *testing.B) {
+	var op OpCode
+	bencher := func(b *testing.B) {
+		code := make([]byte, analysisCodeSize)
+		b.SetBytes(analysisCodeSize)
+		for i := range code {
+			code[i] = byte(op)
+		}
+		bits := make(bitvec, len(code)/8+1+4)
+		b.ResetTimer()
+		for i := 0; i < b.N; i++ {
+			clear(bits)
+			eofCodeBitmapInternal(code, bits)
+		}
+	}
+	for op = PUSH1; op <= PUSH32; op++ {
+		bench.Run(op.String(), bencher)
+	}
+	op = JUMPDEST
+	bench.Run(op.String(), bencher)
+	op = STOP
+	bench.Run(op.String(), bencher)
+	op = RJUMPV
+	bench.Run(op.String(), bencher)
+	op = EOFCREATE
+	bench.Run(op.String(), bencher)
+}

core/vm/eof.go

@@ -127,10 +127,15 @@ func (c *Container) MarshalBinary() []byte {
 
 // UnmarshalBinary decodes an EOF container.
 func (c *Container) UnmarshalBinary(b []byte, isInitcode bool) error {
-	return c.unmarshalSubContainer(b, isInitcode, true)
+	return c.unmarshalContainer(b, isInitcode, true)
 }
 
-func (c *Container) unmarshalSubContainer(b []byte, isInitcode bool, topLevel bool) error {
+// UnmarshalSubContainer decodes an EOF container that is inside another container.
+func (c *Container) UnmarshalSubContainer(b []byte, isInitcode bool) error {
+	return c.unmarshalContainer(b, isInitcode, false)
+}
+
+func (c *Container) unmarshalContainer(b []byte, isInitcode bool, topLevel bool) error {
 	if !hasEOFMagic(b) {
 		return fmt.Errorf("%w: want %x", ErrInvalidMagic, eofMagic)
 	}
@@ -275,7 +280,7 @@ func (c *Container) unmarshalSubContainer(b []byte, isInitcode bool, topLevel bo
 			}
 			subC := new(Container)
 			end := min(idx+size, len(b))
-			if err := subC.unmarshalSubContainer(b[idx:end], isInitcode, false); err != nil {
+			if err := subC.unmarshalContainer(b[idx:end], isInitcode, false); err != nil {
 				if topLevel {
 					return fmt.Errorf("%w in sub container %d", err, i)
 				}

core/vm/validate.go

@@ -182,9 +182,6 @@ func validateCode(code []byte, section int, container *Container, jt *JumpTable,
 			if visitedSubcontainers == nil {
 				visitedSubcontainers = make(map[int]int)
 			}
-			if _, ok := visitedSubcontainers[arg]; ok {
-				return nil, fmt.Errorf("section already referenced, arg :%d", arg)
-			}
 			// We need to store per subcontainer how it was referenced
 			if v, ok := visitedSubcontainers[arg]; ok && v != refByEOFCreate {
 				return nil, fmt.Errorf("section already referenced, arg :%d", arg)

core/vm/validate_test.go

@@ -258,6 +258,8 @@ func TestValidateCode(t *testing.T) {
 	}
 }
 
+// BenchmarkRJUMPI tries to benchmark the RJUMPI opcode validation
+// For this we do a bunch of RJUMPIs that jump backwards (in a potential infinite loop).
 func BenchmarkRJUMPI(b *testing.B) {
 	snippet := []byte{
 		byte(PUSH0),
@@ -275,13 +277,15 @@ func BenchmarkRJUMPI(b *testing.B) {
 	}
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		_, err := validateCode(code, 0, container, &pragueEOFInstructionSet, true)
+		_, err := validateCode(code, 0, container, &pragueEOFInstructionSet, false)
 		if err != nil {
 			b.Fatal(err)
 		}
 	}
 }
 
+// BenchmarkRJUMPV tries to benchmark the validation of the RJUMPV opcode
+// for this we set up as many RJUMPV opcodes with a full jumptable (containing 0s) as possible.
 func BenchmarkRJUMPV(b *testing.B) {
 	snippet := []byte{
 		byte(PUSH0),
@@ -305,13 +309,18 @@ func BenchmarkRJUMPV(b *testing.B) {
 	}
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		_, err := validateCode(code, 0, container, &pragueEOFInstructionSet, true)
+		_, err := validateCode(code, 0, container, &pragueEOFInstructionSet, false)
 		if err != nil {
 			b.Fatal(err)
 		}
 	}
 }
 
+// BenchmarkEOFValidation tries to benchmark the code validation for the CALLF/RETF operation.
+// For this we set up code that calls into 1024 code sections which can either
+// - just contain a RETF opcode
+// - or code to again call into 1024 code sections.
+// We can't have all code sections calling each other, otherwise we would exceed 48KB.
 func BenchmarkEOFValidation(b *testing.B) {
 	var container Container
 	var code []byte
@@ -320,22 +329,21 @@ func BenchmarkEOFValidation(b *testing.B) {
 		code = append(code, byte(CALLF))
 		code = binary.BigEndian.AppendUint16(code, uint16(i%(maxSections-1))+1)
 	}
-	code = append(code, byte(STOP))
 	// First container
-	container.codeSections = append(container.codeSections, code)
+	container.codeSections = append(container.codeSections, append(code, byte(STOP)))
 	container.types = append(container.types, &functionMetadata{inputs: 0, outputs: 0x80, maxStackHeight: 0})
 
 	inner := []byte{
-		byte(STOP),
+		byte(RETF),
 	}
 
 	for i := 0; i < 1023; i++ {
 		container.codeSections = append(container.codeSections, inner)
-		container.types = append(container.types, &functionMetadata{inputs: 0, outputs: 0x80, maxStackHeight: 0})
+		container.types = append(container.types, &functionMetadata{inputs: 0, outputs: 0, maxStackHeight: 0})
 	}
 
 	for i := 0; i < 12; i++ {
-		container.codeSections[i+1] = code
+		container.codeSections[i+1] = append(code, byte(RETF))
 	}
 
 	bin := container.MarshalBinary()
@@ -349,12 +357,16 @@ func BenchmarkEOFValidation(b *testing.B) {
 		if err := container2.UnmarshalBinary(bin, true); err != nil {
 			b.Fatal(err)
 		}
-		if err := container2.ValidateCode(&pragueEOFInstructionSet, true); err != nil {
+		if err := container2.ValidateCode(&pragueEOFInstructionSet, false); err != nil {
 			b.Fatal(err)
 		}
 	}
 }
 
+// BenchmarkEOFValidation tries to benchmark the code validation for the CALLF/RETF operation.
+// For this we set up code that calls into 1024 code sections which
+// - contain calls to some other code sections.
+// We can't have all code sections calling each other, otherwise we would exceed 48KB.
 func BenchmarkEOFValidation2(b *testing.B) {
 	var container Container
 	var code []byte
@@ -380,12 +392,13 @@ func BenchmarkEOFValidation2(b *testing.B) {
 		byte(CALLF), 0x03, 0xF6,
 		byte(CALLF), 0x03, 0xF7,
 		byte(CALLF), 0x03, 0xF8,
-		byte(JUMPF), 0x00, 0x00,
+		byte(CALLF), 0x03, 0xF,
+		byte(RETF),
 	}
 
 	for i := 0; i < 1023; i++ {
 		container.codeSections = append(container.codeSections, inner)
-		container.types = append(container.types, &functionMetadata{inputs: 0, outputs: 0x80, maxStackHeight: 0})
+		container.types = append(container.types, &functionMetadata{inputs: 0, outputs: 0, maxStackHeight: 0})
 	}
 
 	bin := container.MarshalBinary()
@@ -399,12 +412,17 @@ func BenchmarkEOFValidation2(b *testing.B) {
 		if err := container2.UnmarshalBinary(bin, true); err != nil {
 			b.Fatal(err)
 		}
-		if err := container2.ValidateCode(&pragueEOFInstructionSet, true); err != nil {
+		if err := container2.ValidateCode(&pragueEOFInstructionSet, false); err != nil {
 			b.Fatal(err)
 		}
 	}
 }
 
+// BenchmarkEOFValidation3 tries to benchmark the code validation for the CALLF/RETF and RJUMPI/V operations.
+// For this we set up code that calls into 1024 code sections which either
+// - contain an RJUMP opcode
+// - contain calls to other code sections
+// We can't have all code sections calling each other, otherwise we would exceed 48KB.
 func BenchmarkEOFValidation3(b *testing.B) {
 	var container Container
 	var code []byte
@@ -418,23 +436,25 @@ func BenchmarkEOFValidation3(b *testing.B) {
 		snippet = append(snippet, []byte{0x00, 0x00}...)
 	}
 	code = append(code, snippet...)
+	// First container, calls into all other containers
 	maxSections := 1024
 	for i := 0; i < maxSections; i++ {
 		code = append(code, byte(CALLF))
 		code = binary.BigEndian.AppendUint16(code, uint16(i%(maxSections-1))+1)
 	}
 	code = append(code, byte(STOP))
-	// First container
 	container.codeSections = append(container.codeSections, code)
 	container.types = append(container.types, &functionMetadata{inputs: 0, outputs: 0x80, maxStackHeight: 1})
 
+	// Other containers
 	for i := 0; i < 1023; i++ {
-		container.codeSections = append(container.codeSections, []byte{byte(RJUMP), 0x00, 0x00, byte(JUMPF), 0x00, 0x00})
+		container.codeSections = append(container.codeSections, []byte{byte(RJUMP), 0x00, 0x00, byte(RETF)})
-		container.types = append(container.types, &functionMetadata{inputs: 0, outputs: 0x80, maxStackHeight: 0})
+		container.types = append(container.types, &functionMetadata{inputs: 0, outputs: 0, maxStackHeight: 0})
 	}
-	for i := 0; i < 65; i++ {
+	// Other containers
-		container.codeSections[i+1] = append(snippet, byte(STOP))
+	for i := 0; i < 68; i++ {
-		container.types[i+1] = &functionMetadata{inputs: 0, outputs: 0x80, maxStackHeight: 1}
+		container.codeSections[i+1] = append(snippet, byte(RETF))
+		container.types[i+1] = &functionMetadata{inputs: 0, outputs: 0, maxStackHeight: 1}
 	}
 	bin := container.MarshalBinary()
 	if len(bin) > 48*1024 {
@@ -448,7 +468,7 @@ func BenchmarkEOFValidation3(b *testing.B) {
 			if err := container2.UnmarshalBinary(bin, true); err != nil {
 				b.Fatal(err)
 			}
-			if err := container2.ValidateCode(&pragueEOFInstructionSet, true); err != nil {
+			if err := container2.ValidateCode(&pragueEOFInstructionSet, false); err != nil {
 				b.Fatal(err)
 			}
 		}
@@ -474,7 +494,7 @@ func BenchmarkRJUMPI_2(b *testing.B) {
 	}
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		_, err := validateCode(code, 0, container, &pragueEOFInstructionSet, true)
+		_, err := validateCode(code, 0, container, &pragueEOFInstructionSet, false)
 		if err != nil {
 			b.Fatal(err)
 		}