cannon: Clean up program patching (#11944)

* cannon: Cut todo, reorganize syscalls

* cannon: Rename patchGo to patchGoGC, add comments, cut TODO

* cannon: Only allow gc patching for single-threaded Cannon

* cannon: Cut patch flag from load-elf command altogether

Makefile

@@ -148,7 +148,7 @@ cannon-prestate: op-program cannon ## Generates prestate using cannon and op-pro
 .PHONY: cannon-prestate
 
 cannon-prestate-mt: op-program cannon ## Generates prestate using cannon and op-program in the multithreaded cannon format
-	./cannon/bin/cannon load-elf --type cannon-mt --patch stack --path op-program/bin/op-program-client.elf --out op-program/bin/prestate-mt.bin.gz --meta op-program/bin/meta-mt.json
+	./cannon/bin/cannon load-elf --type cannon-mt --path op-program/bin/op-program-client.elf --out op-program/bin/prestate-mt.bin.gz --meta op-program/bin/meta-mt.json
 	./cannon/bin/cannon run --proof-at '=0' --stop-at '=1' --input op-program/bin/prestate-mt.bin.gz --meta op-program/bin/meta-mt.json --proof-fmt 'op-program/bin/%d-mt.json' --output ""
 	mv op-program/bin/0-mt.json op-program/bin/prestate-proof-mt.json
 .PHONY: cannon-prestate

cannon/cmd/load_elf.go

@@ -4,15 +4,15 @@ import (
 	"debug/elf"
 	"fmt"
 
+	"github.com/urfave/cli/v2"
+
 	"github.com/ethereum-optimism/optimism/cannon/mipsevm"
 	"github.com/ethereum-optimism/optimism/cannon/mipsevm/multithreaded"
+	"github.com/ethereum-optimism/optimism/cannon/mipsevm/program"
+	"github.com/ethereum-optimism/optimism/cannon/mipsevm/singlethreaded"
 	"github.com/ethereum-optimism/optimism/cannon/mipsevm/versions"
 	"github.com/ethereum-optimism/optimism/cannon/serialize"
 	"github.com/ethereum-optimism/optimism/op-service/ioutil"
-	"github.com/urfave/cli/v2"
-
-	"github.com/ethereum-optimism/optimism/cannon/mipsevm/program"
-	"github.com/ethereum-optimism/optimism/cannon/mipsevm/singlethreaded"
 	"github.com/ethereum-optimism/optimism/op-service/jsonutil"
 )
 
@@ -29,15 +29,9 @@ var (
 		TakesFile: true,
 		Required:  true,
 	}
-	LoadELFPatchFlag = &cli.StringSliceFlag{
-		Name:     "patch",
-		Usage:    "Type of patching to do",
-		Value:    cli.NewStringSlice("go", "stack"),
-		Required: false,
-	}
 	LoadELFOutFlag = &cli.PathFlag{
 		Name:     "out",
-		Usage:    "Output path to write JSON state to. State is dumped to stdout if set to -. Not written if empty.",
+		Usage:    "Output path to write state to. State is dumped to stdout if set to '-'. Not written if empty. Use file extension '.bin', '.bin.gz', or '.json' for binary, compressed binary, or JSON formats.",
 		Value:    "state.json",
 		Required: false,
 	}
@@ -67,14 +61,31 @@ func vmTypeFromString(ctx *cli.Context) (VMType, error) {
 }
 
 func LoadELF(ctx *cli.Context) error {
+	elfPath := ctx.Path(LoadELFPathFlag.Name)
+	elfProgram, err := elf.Open(elfPath)
+	if err != nil {
+		return fmt.Errorf("failed to open ELF file %q: %w", elfPath, err)
+	}
+	if elfProgram.Machine != elf.EM_MIPS {
+		return fmt.Errorf("ELF is not big-endian MIPS R3000, but got %q", elfProgram.Machine.String())
+	}
+
 	var createInitialState func(f *elf.File) (mipsevm.FPVMState, error)
 
+	var patcher = program.PatchStack
 	if vmType, err := vmTypeFromString(ctx); err != nil {
 		return err
 	} else if vmType == cannonVMType {
 		createInitialState = func(f *elf.File) (mipsevm.FPVMState, error) {
 			return program.LoadELF(f, singlethreaded.CreateInitialState)
 		}
+		patcher = func(state mipsevm.FPVMState) error {
+			err := program.PatchGoGC(elfProgram, state)
+			if err != nil {
+				return err
+			}
+			return program.PatchStack(state)
+		}
 	} else if vmType == mtVMType {
 		createInitialState = func(f *elf.File) (mipsevm.FPVMState, error) {
 			return program.LoadELF(f, multithreaded.CreateInitialState)
@@ -82,30 +93,14 @@ func LoadELF(ctx *cli.Context) error {
 	} else {
 		return fmt.Errorf("invalid VM type: %q", vmType)
 	}
-	elfPath := ctx.Path(LoadELFPathFlag.Name)
-	elfProgram, err := elf.Open(elfPath)
-	if err != nil {
-		return fmt.Errorf("failed to open ELF file %q: %w", elfPath, err)
-	}
-	if elfProgram.Machine != elf.EM_MIPS {
-		return fmt.Errorf("ELF is not big-endian MIPS R3000, but got %q", elfProgram.Machine.String())
-	}
+
 	state, err := createInitialState(elfProgram)
 	if err != nil {
 		return fmt.Errorf("failed to load ELF data into VM state: %w", err)
 	}
-	for _, typ := range ctx.StringSlice(LoadELFPatchFlag.Name) {
-		switch typ {
-		case "stack":
-			err = program.PatchStack(state)
-		case "go":
-			err = program.PatchGo(elfProgram, state)
-		default:
-			return fmt.Errorf("unrecognized form of patching: %q", typ)
-		}
-		if err != nil {
-			return fmt.Errorf("failed to apply patch %s: %w", typ, err)
-		}
+	err = patcher(state)
+	if err != nil {
+		return fmt.Errorf("failed to patch state: %w", err)
 	}
 	meta, err := program.MakeMetadata(elfProgram)
 	if err != nil {
@@ -125,13 +120,12 @@ func LoadELF(ctx *cli.Context) error {
 
 var LoadELFCommand = &cli.Command{
 	Name:        "load-elf",
-	Usage:       "Load ELF file into Cannon JSON state",
-	Description: "Load ELF file into Cannon JSON state, optionally patch out functions",
+	Usage:       "Load ELF file into Cannon state",
+	Description: "Load ELF file into Cannon state",
 	Action:      LoadELF,
 	Flags: []cli.Flag{
 		LoadELFVMTypeFlag,
 		LoadELFPathFlag,
-		LoadELFPatchFlag,
 		LoadELFOutFlag,
 		LoadELFMetaFlag,
 	},

cannon/mipsevm/exec/mips_syscalls.go

@@ -59,12 +59,7 @@ const (
 	SysLlseek        = 4140
 	SysMinCore       = 4217
 	SysTgkill        = 4266
-)
-
-// Profiling-related syscalls
-// Should be able to ignore if we patch out prometheus calls and disable memprofiling
-// TODO(cp-903) - Update patching for mt-cannon so that these can be ignored
-const (
+	// Profiling-related syscalls
 	SysSetITimer    = 4104
 	SysTimerCreate  = 4257
 	SysTimerSetTime = 4258

cannon/mipsevm/program/patch.go

@@ -11,9 +11,9 @@ import (
 	"github.com/ethereum-optimism/optimism/cannon/mipsevm/memory"
 )
 
-// TODO(cp-903) Consider breaking up go patching into performance and threading-related patches so we can
-// selectively apply the perf patching to MTCannon
-func PatchGo(f *elf.File, st mipsevm.FPVMState) error {
+// PatchGoGC patches out garbage-collection-related symbols to disable garbage collection
+// and improves performance by patching out floating-point-related symbols
+func PatchGoGC(f *elf.File, st mipsevm.FPVMState) error {
 	symbols, err := f.Symbols()
 	if err != nil {
 		return fmt.Errorf("failed to read symbols data, cannot patch program: %w", err)
@@ -53,6 +53,7 @@ func PatchGo(f *elf.File, st mipsevm.FPVMState) error {
 	return nil
 }
 
+// PatchStack sets up the program's initial stack frame and stack pointer
 func PatchStack(st mipsevm.FPVMState) error {
 	// setup stack pointer
 	sp := uint32(0x7f_ff_d0_00)

cannon/mipsevm/testutil/elf.go

@@ -9,7 +9,7 @@ import (
 	"github.com/ethereum-optimism/optimism/cannon/mipsevm/program"
 )
 
-func LoadELFProgram[T mipsevm.FPVMState](t require.TestingT, name string, initState program.CreateInitialFPVMState[T], doPatchGo bool) (T, *program.Metadata) {
+func LoadELFProgram[T mipsevm.FPVMState](t require.TestingT, name string, initState program.CreateInitialFPVMState[T], doPatchGoGC bool) (T, *program.Metadata) {
 	elfProgram, err := elf.Open(name)
 	require.NoError(t, err, "open ELF file")
 	meta, err := program.MakeMetadata(elfProgram)
@@ -18,8 +18,8 @@ func LoadELFProgram[T mipsevm.FPVMState](t require.TestingT, name string, initSt
 	state, err := program.LoadELF(elfProgram, initState)
 	require.NoError(t, err, "load ELF into state")
 
-	if doPatchGo {
-		err = program.PatchGo(elfProgram, state)
+	if doPatchGoGC {
+		err = program.PatchGoGC(elfProgram, state)
 		require.NoError(t, err, "apply Go runtime patches")
 	}