From bddca4d37600e31bd51457bd588ef6ae7a44e00c Mon Sep 17 00:00:00 2001 From: John Jannotti Date: Sat, 14 Aug 2021 19:47:55 -0400 Subject: [PATCH 1/2] Ensure disassemble/reassemble cycle works in testProg. --- data/transactions/logic/assembler.go | 26 ++++++++++---- data/transactions/logic/assembler_test.go | 37 ++++++++++--------- data/transactions/logic/debugger.go | 2 +- data/transactions/logic/eval_test.go | 43 ++++++++--------------- 4 files changed, 56 insertions(+), 52 deletions(-) diff --git a/data/transactions/logic/assembler.go b/data/transactions/logic/assembler.go index 6b8f5793b9..361e6cca00 100644 --- a/data/transactions/logic/assembler.go +++ b/data/transactions/logic/assembler.go @@ -1938,6 +1938,7 @@ type disassembleState struct { numericTargets bool labelCount int pendingLabels map[int]string + rerun bool nextpc int err error @@ -1951,6 +1952,9 @@ func (dis *disassembleState) putLabel(label string, target int) { dis.pendingLabels = make(map[int]string) } dis.pendingLabels[target] = label + if target <= dis.pc { + dis.rerun = true + } } func (dis *disassembleState) outputLabelIfNeeded() (err error) { @@ -2412,11 +2416,13 @@ type disInfo struct { hasStatefulOps bool } -// disassembleInstrumented is like Disassemble, but additionally returns where -// each program counter value maps in the disassembly -func disassembleInstrumented(program []byte) (text string, ds disInfo, err error) { +// disassembleInstrumented is like Disassemble, but additionally +// returns where each program counter value maps in the +// disassembly. If the labels names are known, they may be passed in. +// When doing so, labels for all jump targets must be provided. +func disassembleInstrumented(program []byte, labels map[int]string) (text string, ds disInfo, err error) { out := strings.Builder{} - dis := disassembleState{program: program, out: &out} + dis := disassembleState{program: program, out: &out, pendingLabels: labels} version, vlen := binary.Uvarint(program) if vlen <= 0 { fmt.Fprintf(dis.out, "// invalid version\n") @@ -2468,18 +2474,26 @@ func disassembleInstrumented(program []byte) (text string, ds disInfo, err error } text = out.String() + + if dis.rerun { + if labels != nil { + err = errors.New("rerun even though we had labels") + return + } + return disassembleInstrumented(program, dis.pendingLabels) + } return } // Disassemble produces a text form of program bytes. // AssembleString(Disassemble()) should result in the same program bytes. func Disassemble(program []byte) (text string, err error) { - text, _, err = disassembleInstrumented(program) + text, _, err = disassembleInstrumented(program, nil) return } // HasStatefulOps checks if the program has stateful opcodes func HasStatefulOps(program []byte) (bool, error) { - _, ds, err := disassembleInstrumented(program) + _, ds, err := disassembleInstrumented(program, nil) return ds.hasStatefulOps, err } diff --git a/data/transactions/logic/assembler_test.go b/data/transactions/logic/assembler_test.go index a8cd151f79..6ebd199f16 100644 --- a/data/transactions/logic/assembler_test.go +++ b/data/transactions/logic/assembler_test.go @@ -384,19 +384,11 @@ func TestAssembleAlias(t *testing.T) { t.Parallel() source1 := `txn Accounts 0 // alias to txna pop -gtxn 0 ApplicationArgs 0 // alias to gtxn +gtxn 0 ApplicationArgs 0 // alias to gtxna pop ` - ops1, err := AssembleStringWithVersion(source1, AssemblerMaxVersion) - require.NoError(t, err) - - source2 := `txna Accounts 0 -pop -gtxna 0 ApplicationArgs 0 -pop -` - ops2, err := AssembleStringWithVersion(source2, AssemblerMaxVersion) - require.NoError(t, err) + ops1 := testProg(t, source1, AssemblerMaxVersion) + ops2 := testProg(t, strings.Replace(source1, "txn", "txna", -1), AssemblerMaxVersion) require.Equal(t, ops1.Program, ops2.Program) } @@ -431,6 +423,20 @@ func testProg(t testing.TB, source string, ver uint64, expected ...expect) *OpSt require.NoError(t, err) require.NotNil(t, ops) require.NotNil(t, ops.Program) + // It should always be possible to Disassemble + dis, err := Disassemble(ops.Program) + require.NoError(t, err) + // And, while the disassembly may not match input + // exactly, the assembly of the disassembly should + // give the same bytecode + ops2, err := AssembleStringWithVersion(dis, ver) + if len(ops2.Errors) > 0 || err != nil || ops2 == nil || ops2.Program == nil { + t.Log(program) + t.Log(dis) + } + require.Empty(t, ops2.Errors) + require.NoError(t, err) + require.Equal(t, ops.Program, ops2.Program) } else { require.Error(t, err) errors := ops.Errors @@ -592,8 +598,7 @@ func TestAssembleInt(t *testing.T) { } text := "int 0xcafebabe" - ops, err := AssembleStringWithVersion(text, v) - require.NoError(t, err) + ops := testProg(t, text, v) s := hex.EncodeToString(ops.Program) require.Equal(t, mutateProgVersion(v, expected), s) }) @@ -643,8 +648,7 @@ func TestAssembleBytes(t *testing.T) { } for _, vi := range variations { - ops, err := AssembleStringWithVersion(vi, v) - require.NoError(t, err) + ops := testProg(t, vi, v) s := hex.EncodeToString(ops.Program) require.Equal(t, mutateProgVersion(v, expected), s) } @@ -1184,8 +1188,7 @@ intc 0 intc 0 bnz done done:` - ops, err := AssembleStringWithVersion(source, AssemblerMaxVersion) - require.NoError(t, err) + ops := testProg(t, source, AssemblerMaxVersion) require.Equal(t, 9, len(ops.Program)) expectedProgBytes := []byte("\x01\x20\x01\x01\x22\x22\x40\x00\x00") expectedProgBytes[0] = byte(AssemblerMaxVersion) diff --git a/data/transactions/logic/debugger.go b/data/transactions/logic/debugger.go index 021a53faf1..b4947b83d0 100644 --- a/data/transactions/logic/debugger.go +++ b/data/transactions/logic/debugger.go @@ -87,7 +87,7 @@ func GetProgramID(program []byte) string { } func makeDebugState(cx *evalContext) DebugState { - disasm, dsInfo, err := disassembleInstrumented(cx.program) + disasm, dsInfo, err := disassembleInstrumented(cx.program, nil) if err != nil { // Report disassembly error as program text disasm = err.Error() diff --git a/data/transactions/logic/eval_test.go b/data/transactions/logic/eval_test.go index f7e3d5d84b..c6330032c2 100644 --- a/data/transactions/logic/eval_test.go +++ b/data/transactions/logic/eval_test.go @@ -1034,7 +1034,7 @@ func TestGlobal(t *testing.T) { addr, err := basics.UnmarshalChecksumAddress(testAddr) require.NoError(t, err) ledger.creatorAddr = addr - for v := uint64(0); v <= AssemblerMaxVersion; v++ { + for v := uint64(1); v <= AssemblerMaxVersion; v++ { _, ok := tests[v] require.True(t, ok) t.Run(fmt.Sprintf("v=%d", v), func(t *testing.T) { @@ -1055,9 +1055,6 @@ func TestGlobal(t *testing.T) { txgroup := make([]transactions.SignedTxn, 1) txgroup[0] = txn sb := strings.Builder{} - block := bookkeeping.Block{} - block.BlockHeader.Round = 999999 - block.BlockHeader.TimeStamp = 2069 proto := config.ConsensusParams{ MinTxnFee: 123, MinBalance: 1000000, @@ -2684,10 +2681,9 @@ func TestStackUnderflow(t *testing.T) { t.Parallel() for v := uint64(1); v <= AssemblerMaxVersion; v++ { t.Run(fmt.Sprintf("v=%d", v), func(t *testing.T) { - ops, err := AssembleStringWithVersion(`int 1`, v) + ops := testProg(t, `int 1`, v) ops.Program = append(ops.Program, 0x08) // + - require.NoError(t, err) - err = Check(ops.Program, defaultEvalParams(nil, nil)) + err := Check(ops.Program, defaultEvalParams(nil, nil)) require.NoError(t, err) sb := strings.Builder{} pass, err := Eval(ops.Program, defaultEvalParams(&sb, nil)) @@ -2707,10 +2703,9 @@ func TestWrongStackTypeRuntime(t *testing.T) { t.Parallel() for v := uint64(1); v <= AssemblerMaxVersion; v++ { t.Run(fmt.Sprintf("v=%d", v), func(t *testing.T) { - ops, err := AssembleStringWithVersion(`int 1`, v) - require.NoError(t, err) + ops := testProg(t, `int 1`, v) ops.Program = append(ops.Program, 0x01, 0x15) // sha256, len - err = Check(ops.Program, defaultEvalParams(nil, nil)) + err := Check(ops.Program, defaultEvalParams(nil, nil)) require.NoError(t, err) sb := strings.Builder{} pass, err := Eval(ops.Program, defaultEvalParams(&sb, nil)) @@ -2730,11 +2725,9 @@ func TestEqMismatch(t *testing.T) { t.Parallel() for v := uint64(1); v <= AssemblerMaxVersion; v++ { t.Run(fmt.Sprintf("v=%d", v), func(t *testing.T) { - ops, err := AssembleStringWithVersion(`byte 0x1234 -int 1`, v) - require.NoError(t, err) + ops := testProg(t, `byte 0x1234; int 1`, v) ops.Program = append(ops.Program, 0x12) // == - err = Check(ops.Program, defaultEvalParams(nil, nil)) + err := Check(ops.Program, defaultEvalParams(nil, nil)) require.NoError(t, err) // TODO: Check should know the type stack was wrong sb := strings.Builder{} pass, err := Eval(ops.Program, defaultEvalParams(&sb, nil)) @@ -2754,11 +2747,9 @@ func TestNeqMismatch(t *testing.T) { t.Parallel() for v := uint64(1); v <= AssemblerMaxVersion; v++ { t.Run(fmt.Sprintf("v=%d", v), func(t *testing.T) { - ops, err := AssembleStringWithVersion(`byte 0x1234 -int 1`, v) - require.NoError(t, err) + ops := testProg(t, `byte 0x1234; int 1`, v) ops.Program = append(ops.Program, 0x13) // != - err = Check(ops.Program, defaultEvalParams(nil, nil)) + err := Check(ops.Program, defaultEvalParams(nil, nil)) require.NoError(t, err) // TODO: Check should know the type stack was wrong sb := strings.Builder{} pass, err := Eval(ops.Program, defaultEvalParams(&sb, nil)) @@ -2778,11 +2769,9 @@ func TestWrongStackTypeRuntime2(t *testing.T) { t.Parallel() for v := uint64(1); v <= AssemblerMaxVersion; v++ { t.Run(fmt.Sprintf("v=%d", v), func(t *testing.T) { - ops, err := AssembleStringWithVersion(`byte 0x1234 -int 1`, v) - require.NoError(t, err) + ops := testProg(t, `byte 0x1234; int 1`, v) ops.Program = append(ops.Program, 0x08) // + - err = Check(ops.Program, defaultEvalParams(nil, nil)) + err := Check(ops.Program, defaultEvalParams(nil, nil)) require.NoError(t, err) sb := strings.Builder{} pass, _ := Eval(ops.Program, defaultEvalParams(&sb, nil)) @@ -2802,15 +2791,14 @@ func TestIllegalOp(t *testing.T) { t.Parallel() for v := uint64(1); v <= AssemblerMaxVersion; v++ { t.Run(fmt.Sprintf("v=%d", v), func(t *testing.T) { - ops, err := AssembleStringWithVersion(`int 1`, v) - require.NoError(t, err) + ops := testProg(t, `int 1`, v) for opcode, spec := range opsByOpcode[v] { if spec.op == nil { ops.Program = append(ops.Program, byte(opcode)) break } } - err = Check(ops.Program, defaultEvalParams(nil, nil)) + err := Check(ops.Program, defaultEvalParams(nil, nil)) require.Error(t, err) sb := strings.Builder{} pass, err := Eval(ops.Program, defaultEvalParams(&sb, nil)) @@ -2830,15 +2818,14 @@ func TestShortProgram(t *testing.T) { t.Parallel() for v := uint64(1); v <= AssemblerMaxVersion; v++ { t.Run(fmt.Sprintf("v=%d", v), func(t *testing.T) { - ops, err := AssembleStringWithVersion(`int 1 + ops := testProg(t, `int 1 bnz done done: int 1 `, v) - require.NoError(t, err) // cut two last bytes - intc_1 and last byte of bnz ops.Program = ops.Program[:len(ops.Program)-2] - err = Check(ops.Program, defaultEvalParams(nil, nil)) + err := Check(ops.Program, defaultEvalParams(nil, nil)) require.Error(t, err) sb := strings.Builder{} pass, err := Eval(ops.Program, defaultEvalParams(&sb, nil)) From 001848bcb160bbb7cf464e7aca4466729e69ff2c Mon Sep 17 00:00:00 2001 From: John Jannotti Date: Tue, 17 Aug 2021 13:07:39 -0400 Subject: [PATCH 2/2] Explain `rerun` --- data/transactions/logic/assembler.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/data/transactions/logic/assembler.go b/data/transactions/logic/assembler.go index 361e6cca00..b5315466fa 100644 --- a/data/transactions/logic/assembler.go +++ b/data/transactions/logic/assembler.go @@ -1938,7 +1938,15 @@ type disassembleState struct { numericTargets bool labelCount int pendingLabels map[int]string - rerun bool + + // If we find a (back) jump to a label we did not generate + // (because we didn't know about it yet), rerun is set to + // true, and we make a second attempt to assemble once the + // first attempt is done. The second attempt retains all the + // labels found in the first pass. In effect, the first + // attempt to assemble becomes a first-pass in a two-pass + // assembly process that simply collects jump target labels. + rerun bool nextpc int err error