From 9d1f0dec2889f433b8e59748428ccda6534ce950 Mon Sep 17 00:00:00 2001 From: John Jannotti Date: Wed, 14 Sep 2022 21:21:16 -0400 Subject: [PATCH 1/7] Allow fall-through in switch Moved opcode number to be by other "strange jumps" Added simple tests for normal cases, and for fallthrough Added some overflow protection from handcrafted bytecode by limiting switch labels to 2^16. --- data/transactions/logic/assembler.go | 7 ++-- data/transactions/logic/assembler_test.go | 32 ++++++++++----- data/transactions/logic/eval.go | 26 ++++++------ data/transactions/logic/eval_test.go | 50 +++++++++++++++++++---- data/transactions/logic/opcodes.go | 7 +--- 5 files changed, 85 insertions(+), 37 deletions(-) diff --git a/data/transactions/logic/assembler.go b/data/transactions/logic/assembler.go index 9b32925c2b..53cc64053b 100644 --- a/data/transactions/logic/assembler.go +++ b/data/transactions/logic/assembler.go @@ -27,6 +27,7 @@ import ( "errors" "fmt" "io" + "math" "sort" "strconv" "strings" @@ -2565,15 +2566,15 @@ func parseSwitch(program []byte, pos int) (targets []int, nextpc int, err error) return } pos += bytesUsed - if numOffsets > uint64(len(program)) { - err = errTooManyItems + if numOffsets > math.MaxUint16 { + err = errors.New("switch with too many labels") return } end := pos + int(2*numOffsets) // end of op: offset is applied to this position for i := 0; i < int(numOffsets); i++ { offset := decodeBranchOffset(program, pos) - target := int(offset) + int(end) + target := end + offset targets = append(targets, target) pos += 2 } diff --git a/data/transactions/logic/assembler_test.go b/data/transactions/logic/assembler_test.go index cfbec0e15b..a4bc01738c 100644 --- a/data/transactions/logic/assembler_test.go +++ b/data/transactions/logic/assembler_test.go @@ -410,7 +410,7 @@ const randomnessCompiled = "81ffff03d101d000" const v7Compiled = v6Compiled + "5e005f018120af060180070123456789abcd49490501988003012345494984" + randomnessCompiled + "800243218001775c0280018881015d" -const v8Compiled = v7Compiled + pairingCompiled + "8101e002fff800008101" +const v8Compiled = v7Compiled + pairingCompiled + "81018a02fff800008101" var nonsense = map[uint64]string{ 1: v1Nonsense, @@ -517,16 +517,21 @@ type Expect struct { s string } -func testMatch(t testing.TB, actual, expected string) bool { +func testMatch(t testing.TB, actual, expected string) (ok bool) { + defer func() { + if !ok { + t.Logf("'%s' does not match '%s'", actual, expected) + } + }() t.Helper() if strings.HasPrefix(expected, "...") && strings.HasSuffix(expected, "...") { - return assert.Contains(t, actual, expected[3:len(expected)-3]) + return strings.Contains(actual, expected[3:len(expected)-3]) } else if strings.HasPrefix(expected, "...") { - return assert.Contains(t, actual+"^", expected[3:]+"^") + return strings.Contains(actual+"^", expected[3:]+"^") } else if strings.HasSuffix(expected, "...") { - return assert.Contains(t, "^"+actual, "^"+expected[:len(expected)-3]) + return strings.Contains("^"+actual, "^"+expected[:len(expected)-3]) } else { - return assert.Equal(t, expected, actual) + return expected == actual } } @@ -597,13 +602,13 @@ func testProg(t testing.TB, source string, ver uint64, expected ...Expect) *OpSt errors := ops.Errors for _, exp := range expected { if exp.l == 0 { - // line 0 means: "must match all" + // line 0 means: "must match some line" require.Len(t, expected, 1) - fail := false + fail := true for _, err := range errors { msg := err.Unwrap().Error() - if !testMatch(t, msg, exp.s) { - fail = true + if testMatch(t, msg, exp.s) { + fail = false } } if fail { @@ -2779,6 +2784,13 @@ func TestAssembleSwitch(t *testing.T) { ` testProg(t, source, 8, NewExpect(3, "reference to undefined label \"label2\"")) + // No labels is pretty degenerate, but ok, I suppose. It's just a no-op + testProg(t, ` +int 0 +switchi +int 1 +`, 8) + // confirm size of varuint list size source = ` pushint 1 diff --git a/data/transactions/logic/eval.go b/data/transactions/logic/eval.go index 5b8ab61920..fa808ab012 100644 --- a/data/transactions/logic/eval.go +++ b/data/transactions/logic/eval.go @@ -1992,21 +1992,24 @@ func branchTarget(cx *EvalContext) (int, error) { func switchTarget(cx *EvalContext, branchIdx uint64) (int, uint64, error) { numOffsets, bytesUsed := binary.Uvarint(cx.program[cx.pc+1:]) - if numOffsets <= 0 { + if bytesUsed <= 0 || numOffsets > math.MaxUint16 { return 0, 0, fmt.Errorf("could not decode switch label count at pc=%d", cx.pc+1) } + + end := cx.pc + 1 + bytesUsed // end of opcode + number of offsets, beginning of offset list + eoi := end + 2*int(numOffsets) // end of instruction if branchIdx >= numOffsets { - return 0, 0, fmt.Errorf("provided branch index %d exceeds max offset index %d", branchIdx, numOffsets-1) + return eoi, numOffsets, nil } - end := cx.pc + 1 + bytesUsed // end of opcode + number of offsets, beginning of offset list + // 2*branchIdx will be no where near overflow b/c branchIdx < numOffsets < math.MaxUint16 pos := end + int(2*branchIdx) // position of referenced offset: each offset is 2 bytes - if pos >= len(cx.program)-1 { - return 0, 0, fmt.Errorf("invalid byte code: expected offset value but reached end of program") + if pos >= len(cx.program)-1 { // Need two bytes to decode an offset + return 0, 0, fmt.Errorf("offset position %d is beyond program", pos) } offset := decodeBranchOffset(cx.program, pos) - target := end + 2*int(numOffsets) + int(offset) // offset is applied to the end of this opcode + target := eoi + offset // offset is applied to the end of this opcode // branching to exactly the end of the program (target == len(cx.program)), the next pc after the last instruction, // is okay and ends normally @@ -2035,21 +2038,20 @@ func checkBranch(cx *EvalContext) error { // checks any switch that is {op} {varuint offset index} [{int16 offset}...] func checkSwitch(cx *EvalContext) error { - // first call to get the number of offsets, 0 is a safe choice because there must exist at least one label - _, numOffsets, err := switchTarget(cx, 0) + // call with too big of an index. the end-of-instruction PC is returned because + // that's where an out of range branchIdx goes + eoi, numOffsets, err := switchTarget(cx, math.MaxUint64) if err != nil { return err } - _, bytesUsed := binary.Uvarint(cx.program[cx.pc+1:]) // decoding the value will work because switchTarget() above already checked - opSize := 1 + bytesUsed + 2*int(numOffsets) for branchIdx := uint64(0); branchIdx < numOffsets; branchIdx++ { target, _, err := switchTarget(cx, branchIdx) if err != nil { return err } - if target < cx.pc+opSize { + if target < eoi { // If a branch goes backwards, we should have already noted that an instruction began at that location. if _, ok := cx.instructionStarts[target]; !ok { return fmt.Errorf("back branch target %d is not an aligned instruction", target) @@ -2059,7 +2061,7 @@ func checkSwitch(cx *EvalContext) error { } // this opcode's size is dynamic so nextpc must be set here - cx.nextpc = cx.pc + opSize + cx.nextpc = eoi return nil } diff --git a/data/transactions/logic/eval_test.go b/data/transactions/logic/eval_test.go index 06ec2bb775..ac41772e00 100644 --- a/data/transactions/logic/eval_test.go +++ b/data/transactions/logic/eval_test.go @@ -5530,8 +5530,44 @@ func TestTypeComplaints(t *testing.T) { func TestSwitchInt(t *testing.T) { partitiontest.PartitionTest(t) - t.Parallel() + + // take the 0th label + testAccepts(t, ` +int 0 +switchi zero one +err +zero: int 1; return +one: int 0; +`, 8) + + // take the 1th label + testRejects(t, ` +int 1 +switchi zero one +err +zero: int 1; return +one: int 0; +`, 8) + + // same, but jumping to end of program + testAccepts(t, ` +int 1; dup +switchi zero one +zero: err +one: +`, 8) + + // no match + testAccepts(t, ` +int 2 +switchi zero one +int 1; return // falls through to here +zero: int 0; return +one: int 0; return +`, 8) + + // jump forward and backward testAccepts(t, ` int 0 start: @@ -5549,12 +5585,12 @@ assert int 1 `, 8) - // test code fails when target index is out of bounds - testPanics(t, ` -int 2 -switchi start end -start: -end: + // 0 labels are allowed, but weird! + testAccepts(t, ` +int 0 +switchi int 1 `, 8) + + testPanics(t, notrack("switchi; int 1"), 8) } diff --git a/data/transactions/logic/opcodes.go b/data/transactions/logic/opcodes.go index 1cc731a66a..50802b307f 100644 --- a/data/transactions/logic/opcodes.go +++ b/data/transactions/logic/opcodes.go @@ -544,7 +544,8 @@ var OpSpecs = []OpSpec{ // "Function oriented" {0x88, "callsub", opCallSub, proto(":"), 4, detBranch()}, {0x89, "retsub", opRetSub, proto(":"), 4, detDefault()}, - // Leave a little room for indirect function calls, or similar + {0x8a, "switchi", opSwitchInt, proto("i:"), 8, detSwitch()}, + // 0x8b will likely be a switch on pairs of values/targets // More math {0x90, "shl", opShiftLeft, proto("ii:i"), 4, detDefault()}, @@ -563,7 +564,6 @@ var OpSpecs = []OpSpec{ {0x99, "bn256_add", opBn256Add, proto("bb:b"), pairingVersion, costly(70)}, {0x9a, "bn256_scalar_mul", opBn256ScalarMul, proto("bb:b"), pairingVersion, costly(970)}, {0x9b, "bn256_pairing", opBn256Pairing, proto("bb:i"), pairingVersion, costly(8700)}, - // leave room here for eip-2537 style opcodes // Byteslice math. {0xa0, "b+", opBytesPlus, proto("bb:b"), 4, costly(10)}, @@ -606,9 +606,6 @@ var OpSpecs = []OpSpec{ // randomness support {0xd0, "vrf_verify", opVrfVerify, proto("bbb:bi"), randomnessVersion, field("s", &VrfStandards).costs(5700)}, {0xd1, "block", opBlock, proto("i:a"), randomnessVersion, field("f", &BlockFields)}, - - // switch on value - {0xe0, "switchi", opSwitchInt, proto("i:"), 8, detSwitch()}, } type sortByOpcode []OpSpec From c0fa16ca2bb2aa67e43507653a1c5ef8e4b9afa3 Mon Sep 17 00:00:00 2001 From: michaeldiamant Date: Thu, 15 Sep 2022 10:37:56 -0400 Subject: [PATCH 2/7] Replace hard-coded version with AssemblerMaxVersion in assembler_test.go --- data/transactions/logic/assembler_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/data/transactions/logic/assembler_test.go b/data/transactions/logic/assembler_test.go index a4bc01738c..c68fe7be77 100644 --- a/data/transactions/logic/assembler_test.go +++ b/data/transactions/logic/assembler_test.go @@ -2782,14 +2782,14 @@ func TestAssembleSwitch(t *testing.T) { switchi label1 label2 label1: ` - testProg(t, source, 8, NewExpect(3, "reference to undefined label \"label2\"")) + testProg(t, source, AssemblerMaxVersion, NewExpect(3, "reference to undefined label \"label2\"")) // No labels is pretty degenerate, but ok, I suppose. It's just a no-op testProg(t, ` int 0 switchi int 1 -`, 8) +`, AssemblerMaxVersion) // confirm size of varuint list size source = ` @@ -2798,7 +2798,7 @@ int 1 label1: label2: ` - ops, err := AssembleStringWithVersion(source, 8) + ops, err := AssembleStringWithVersion(source, AssemblerMaxVersion) require.NoError(t, err) val, bytesUsed := binary.Uvarint(ops.Program[4:]) require.Equal(t, uint64(2), val) @@ -2819,7 +2819,7 @@ int 1 switchi %s %s `, strings.Join(labelReferences, " "), strings.Join(labels, "\n")) - ops, err = AssembleStringWithVersion(source, 8) + ops, err = AssembleStringWithVersion(source, AssemblerMaxVersion) require.NoError(t, err) val, bytesUsed = binary.Uvarint(ops.Program[4:]) require.Equal(t, uint64(1<<9), val) @@ -2831,5 +2831,5 @@ int 1 switchi label1 label1 label1: ` - testProg(t, source, 8) + testProg(t, source, AssemblerMaxVersion) } From 76b87f51082335a137823a80621a51c2f6a0d62b Mon Sep 17 00:00:00 2001 From: michaeldiamant Date: Thu, 15 Sep 2022 10:44:48 -0400 Subject: [PATCH 3/7] Update docs for switchi fall-through behavior --- data/transactions/logic/doc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data/transactions/logic/doc.go b/data/transactions/logic/doc.go index 848fb9f26f..379d746b4c 100644 --- a/data/transactions/logic/doc.go +++ b/data/transactions/logic/doc.go @@ -327,7 +327,7 @@ var opDocExtras = map[string]string{ "itxn_submit": "`itxn_submit` resets the current transaction so that it can not be resubmitted. A new `itxn_begin` is required to prepare another inner transaction.", "base64_decode": "*Warning*: Usage should be restricted to very rare use cases. In almost all cases, smart contracts should directly handle non-encoded byte-strings. This opcode should only be used in cases where base64 is the only available option, e.g. interoperability with a third-party that only signs base64 strings.\n\n Decodes A using the base64 encoding E. Specify the encoding with an immediate arg either as URL and Filename Safe (`URLEncoding`) or Standard (`StdEncoding`). See [RFC 4648 sections 4 and 5](https://rfc-editor.org/rfc/rfc4648.html#section-4). It is assumed that the encoding ends with the exact number of `=` padding characters as required by the RFC. When padding occurs, any unused pad bits in the encoding must be set to zero or the decoding will fail. The special cases of `\\n` and `\\r` are allowed but completely ignored. An error will result when attempting to decode a string with a character that is not in the encoding alphabet or not one of `=`, `\\r`, or `\\n`.", "json_ref": "*Warning*: Usage should be restricted to very rare use cases, as JSON decoding is expensive and quite limited. In addition, JSON objects are large and not optimized for size.\n\nAlmost all smart contracts should use simpler and smaller methods (such as the [ABI](https://arc.algorand.foundation/ARCs/arc-0004). This opcode should only be used in cases where JSON is only available option, e.g. when a third-party only signs JSON.", - "switchi": "The `switchi` instruction opcode 0xe0 is followed by `n`, the number of targets, each of which are encoded as 2 byte values indicating the position of the target label relative to the end of the `switchi` instruction (i.e. the offset). The last element on the stack represents the index of the target to branch to. If the index is greater than or equal to n, the evaluation will fail. Otherwise, the program will branch to `pc + 1 + sizeof(n) + 2 * n + target[index]`. Branch targets must be aligned instructions. (e.g. Branching to the second byte of a 2 byte op will be rejected.)", + "switchi": "The `switchi` instruction opcode is followed by `n`, the number of targets, each of which are encoded as 2 byte values indicating the position of the target label relative to the end of the `switchi` instruction (i.e. the offset). The last element on the stack represents the index of the target to branch to. If the index is greater than or equal to n, then evaluation falls through to the next instruction. Otherwise, the program will branch to `pc + 1 + sizeof(n) + 2 * n + target[index]`. Branch targets must be aligned instructions. (e.g. Branching to the second byte of a 2 byte op will be rejected.)", } // OpDocExtra returns extra documentation text about an op From d78965e04d54f368e79fb82ce88b041c066e959d Mon Sep 17 00:00:00 2001 From: michaeldiamant Date: Thu, 15 Sep 2022 11:14:33 -0400 Subject: [PATCH 4/7] Add switchi assembly test case --- data/transactions/logic/assembler_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/data/transactions/logic/assembler_test.go b/data/transactions/logic/assembler_test.go index c68fe7be77..54ad11a006 100644 --- a/data/transactions/logic/assembler_test.go +++ b/data/transactions/logic/assembler_test.go @@ -2784,6 +2784,13 @@ func TestAssembleSwitch(t *testing.T) { ` testProg(t, source, AssemblerMaxVersion, NewExpect(3, "reference to undefined label \"label2\"")) + // fail when target index != uint64 + testProg(t, ` + byte "fail" + switchi label1 + labe11: + `, AssemblerMaxVersion, Expect{3, "switchi label1 arg 0 wanted type uint64..."}) + // No labels is pretty degenerate, but ok, I suppose. It's just a no-op testProg(t, ` int 0 From c066ff29baec7d5347891e7264388111d15fbd80 Mon Sep 17 00:00:00 2001 From: John Jannotti Date: Thu, 15 Sep 2022 14:44:48 -0400 Subject: [PATCH 5/7] Changing encoding to use a single byte for label count --- data/transactions/logic/assembler.go | 9 +-- data/transactions/logic/assembler_test.go | 86 ++++++++++------------- data/transactions/logic/doc.go | 5 +- data/transactions/logic/eval.go | 45 +++++------- data/transactions/logic/eval_test.go | 14 ++++ 5 files changed, 76 insertions(+), 83 deletions(-) diff --git a/data/transactions/logic/assembler.go b/data/transactions/logic/assembler.go index 53cc64053b..e30663e105 100644 --- a/data/transactions/logic/assembler.go +++ b/data/transactions/logic/assembler.go @@ -916,11 +916,12 @@ func asmBranch(ops *OpStream, spec *OpSpec, args []string) error { } func asmSwitch(ops *OpStream, spec *OpSpec, args []string) error { - numOffsets := uint64(len(args)) + if len(args) > 256 { + return ops.errorf("%s cannot take more than 256 labels", spec.Name) + } + numOffsets := len(args) ops.pending.WriteByte(spec.Opcode) - var scratch [binary.MaxVarintLen64]byte - vlen := binary.PutUvarint(scratch[:], uint64(len(args))) - ops.pending.Write(scratch[:vlen]) + ops.pending.WriteByte(byte(numOffsets)) opEndPos := ops.pending.Len() + 2*int(numOffsets) for _, arg := range args { ops.referToLabel(ops.pending.Len(), arg, opEndPos) diff --git a/data/transactions/logic/assembler_test.go b/data/transactions/logic/assembler_test.go index 54ad11a006..843fd70944 100644 --- a/data/transactions/logic/assembler_test.go +++ b/data/transactions/logic/assembler_test.go @@ -18,7 +18,6 @@ package logic import ( "bytes" - "encoding/binary" "encoding/hex" "fmt" "strings" @@ -395,7 +394,7 @@ pushint 1 replace3 ` -const v8Nonsense = v7Nonsense + pairingNonsense + ` +const switchNonsense = ` switch_label0: pushint 1 switchi switch_label0 switch_label1 @@ -403,6 +402,8 @@ switch_label1: pushint 1 ` +const v8Nonsense = v7Nonsense + pairingNonsense + switchNonsense + const v6Compiled = "2004010002b7a60c26050242420c68656c6c6f20776f726c6421070123456789abcd208dae2087fbba51304eb02b91f656948397a7946390e8cb70fc9ea4d95f92251d047465737400320032013202320380021234292929292b0431003101310231043105310731083109310a310b310c310d310e310f3111311231133114311533000033000133000233000433000533000733000833000933000a33000b33000c33000d33000e33000f3300113300123300133300143300152d2e01022581f8acd19181cf959a1281f8acd19181cf951a81f8acd19181cf1581f8acd191810f082209240a220b230c240d250e230f23102311231223132314181b1c28171615400003290349483403350222231d4a484848482b50512a632223524100034200004322602261222704634848222862482864286548482228246628226723286828692322700048482371004848361c0037001a0031183119311b311d311e311f312023221e312131223123312431253126312731283129312a312b312c312d312e312f447825225314225427042455220824564c4d4b0222382124391c0081e80780046a6f686e2281d00f23241f880003420001892224902291922494249593a0a1a2a3a4a5a6a7a8a9aaabacadae24af3a00003b003c003d816472064e014f012a57000823810858235b235a2359b03139330039b1b200b322c01a23c1001a2323c21a23c3233e233f8120af06002a494905002a49490700b53a03b6b7043cb8033a0c2349c42a9631007300810881088120978101c53a8101c6003a" const randomnessCompiled = "81ffff03d101d000" @@ -1971,8 +1972,7 @@ intc_0 // 1 bnz label1 label1: `, v) - ops, err := AssembleStringWithVersion(source, v) - require.NoError(t, err) + ops := testProg(t, source, v) dis, err := Disassemble(ops.Program) require.NoError(t, err) require.Equal(t, source, dis) @@ -2085,8 +2085,7 @@ func TestHasStatefulOps(t *testing.T) { t.Parallel() source := "int 1" - ops, err := AssembleStringWithVersion(source, AssemblerMaxVersion) - require.NoError(t, err) + ops := testProg(t, source, AssemblerMaxVersion) has, err := HasStatefulOps(ops.Program) require.NoError(t, err) require.False(t, has) @@ -2096,8 +2095,7 @@ int 1 app_opted_in err ` - ops, err = AssembleStringWithVersion(source, AssemblerMaxVersion) - require.NoError(t, err) + ops = testProg(t, source, AssemblerMaxVersion) has, err = HasStatefulOps(ops.Program) require.NoError(t, err) require.True(t, has) @@ -2274,46 +2272,38 @@ func TestAssemblePragmaVersion(t *testing.T) { text := `#pragma version 1 int 1 ` - ops, err := AssembleStringWithVersion(text, 1) - require.NoError(t, err) - ops1, err := AssembleStringWithVersion("int 1", 1) - require.NoError(t, err) + ops := testProg(t, text, 1) + ops1 := testProg(t, "int 1", 1) require.Equal(t, ops1.Program, ops.Program) testProg(t, text, 0, Expect{1, "version mismatch..."}) testProg(t, text, 2, Expect{1, "version mismatch..."}) testProg(t, text, assemblerNoVersion) - ops, err = AssembleStringWithVersion(text, assemblerNoVersion) - require.NoError(t, err) + ops = testProg(t, text, assemblerNoVersion) require.Equal(t, ops1.Program, ops.Program) text = `#pragma version 2 int 1 ` - ops, err = AssembleStringWithVersion(text, 2) - require.NoError(t, err) - ops2, err := AssembleStringWithVersion("int 1", 2) - require.NoError(t, err) + ops = testProg(t, text, 2) + ops2 := testProg(t, "int 1", 2) require.Equal(t, ops2.Program, ops.Program) testProg(t, text, 0, Expect{1, "version mismatch..."}) testProg(t, text, 1, Expect{1, "version mismatch..."}) - ops, err = AssembleStringWithVersion(text, assemblerNoVersion) - require.NoError(t, err) + ops = testProg(t, text, assemblerNoVersion) require.Equal(t, ops2.Program, ops.Program) // check if no version it defaults to v1 text = `byte "test" len ` - ops, err = AssembleStringWithVersion(text, assemblerNoVersion) - require.NoError(t, err) - ops1, err = AssembleStringWithVersion(text, 1) + ops = testProg(t, text, assemblerNoVersion) + ops1 = testProg(t, text, 1) require.Equal(t, ops1.Program, ops.Program) - require.NoError(t, err) - ops2, err = AssembleString(text) + ops2, err := AssembleString(text) require.NoError(t, err) require.Equal(t, ops2.Program, ops.Program) @@ -2341,9 +2331,8 @@ func TestErrShortBytecblock(t *testing.T) { t.Parallel() text := `intcblock 0x1234567812345678 0x1234567812345671 0x1234567812345672 0x1234567812345673 4 5 6 7 8` - ops, err := AssembleStringWithVersion(text, 1) - require.NoError(t, err) - _, _, err = parseIntcblock(ops.Program, 1) + ops := testProg(t, text, 1) + _, _, err := parseIntcblock(ops.Program, 1) require.Equal(t, err, errShortIntcblock) var cx EvalContext @@ -2385,8 +2374,7 @@ func TestMethodWarning(t *testing.T) { for _, test := range tests { for v := uint64(1); v <= AssemblerMaxVersion; v++ { src := fmt.Sprintf("method \"%s\"\nint 1", test.method) - ops, err := AssembleStringWithVersion(src, v) - require.NoError(t, err) + ops := testProg(t, src, v) if test.pass { require.Len(t, ops.Warnings, 0) @@ -2689,7 +2677,7 @@ func TestMergeProtos(t *testing.T) { func TestGetSpec(t *testing.T) { partitiontest.PartitionTest(t) t.Parallel() - ops, _ := AssembleStringWithVersion("int 1", AssemblerMaxVersion) + ops := testProg(t, "int 1", AssemblerMaxVersion) ops.versionedPseudoOps["dummyPseudo"] = make(map[int]OpSpec) ops.versionedPseudoOps["dummyPseudo"][1] = OpSpec{Name: "b:", Version: AssemblerMaxVersion, Proto: proto("b:")} ops.versionedPseudoOps["dummyPseudo"][2] = OpSpec{Name: ":", Version: AssemblerMaxVersion} @@ -2798,39 +2786,37 @@ switchi int 1 `, AssemblerMaxVersion) - // confirm size of varuint list size + // confirm arg limit source = ` pushint 1 switchi label1 label2 label1: label2: ` - ops, err := AssembleStringWithVersion(source, AssemblerMaxVersion) - require.NoError(t, err) - val, bytesUsed := binary.Uvarint(ops.Program[4:]) - require.Equal(t, uint64(2), val) - require.Equal(t, 1, bytesUsed) - - var labelReferences []string - for i := 0; i < (1 << 9); i++ { - labelReferences = append(labelReferences, fmt.Sprintf("label%d", i)) - } + ops := testProg(t, source, AssemblerMaxVersion) + require.Len(t, ops.Program, 9) // ver (1) + pushint (2) + opcode (1) + length (1) + labels (2*2) var labels []string - for i := 0; i < (1 << 9); i++ { - labels = append(labels, fmt.Sprintf("label%d:", i)) + for i := 0; i < 256; i++ { + labels = append(labels, fmt.Sprintf("label%d", i)) } + // test that 256 labels is ok source = fmt.Sprintf(` pushint 1 switchi %s %s - `, strings.Join(labelReferences, " "), strings.Join(labels, "\n")) - ops, err = AssembleStringWithVersion(source, AssemblerMaxVersion) - require.NoError(t, err) - val, bytesUsed = binary.Uvarint(ops.Program[4:]) - require.Equal(t, uint64(1<<9), val) - require.Equal(t, 2, bytesUsed) + `, strings.Join(labels, " "), strings.Join(labels, ":\n")+":\n") + ops = testProg(t, source, AssemblerMaxVersion) + require.Len(t, ops.Program, 517) // ver (1) + pushint (2) + opcode (1) + length (1) + labels (2*256) + + // 257 is too many + source = fmt.Sprintf(` + pushint 1 + switchi %s extra + %s + `, strings.Join(labels, " "), strings.Join(labels, ":\n")+":\n") + ops = testProg(t, source, AssemblerMaxVersion, Expect{3, "switchi cannot take more than 256 labels"}) // allow duplicate label reference source = ` diff --git a/data/transactions/logic/doc.go b/data/transactions/logic/doc.go index 379d746b4c..cbb754f3ae 100644 --- a/data/transactions/logic/doc.go +++ b/data/transactions/logic/doc.go @@ -194,7 +194,7 @@ var opDocByName = map[string]string{ "vrf_verify": "Verify the proof B of message A against pubkey C. Returns vrf output and verification flag.", "block": "field F of block A. Fail unless A falls between txn.LastValid-1002 and txn.FirstValid (exclusive)", - "switchi": "branch to target at index A. Fail if index A is out of bounds.", + "switchi": "branch to the Ath label. Continue at following instruction if index A exceeds the number of labels.", } // OpDoc returns a description of the op @@ -264,7 +264,7 @@ var opcodeImmediateNotes = map[string]string{ "vrf_verify": "{uint8 parameters index}", "block": "{uint8 block field}", - "switchi": "{varuint length} [{int16 branch offset, big-endian}, ...]", + "switchi": "{uint8 branch count} [{int16 branch offset, big-endian}, ...]", } // OpImmediateNote returns a short string about immediate data which follows the op byte @@ -327,7 +327,6 @@ var opDocExtras = map[string]string{ "itxn_submit": "`itxn_submit` resets the current transaction so that it can not be resubmitted. A new `itxn_begin` is required to prepare another inner transaction.", "base64_decode": "*Warning*: Usage should be restricted to very rare use cases. In almost all cases, smart contracts should directly handle non-encoded byte-strings. This opcode should only be used in cases where base64 is the only available option, e.g. interoperability with a third-party that only signs base64 strings.\n\n Decodes A using the base64 encoding E. Specify the encoding with an immediate arg either as URL and Filename Safe (`URLEncoding`) or Standard (`StdEncoding`). See [RFC 4648 sections 4 and 5](https://rfc-editor.org/rfc/rfc4648.html#section-4). It is assumed that the encoding ends with the exact number of `=` padding characters as required by the RFC. When padding occurs, any unused pad bits in the encoding must be set to zero or the decoding will fail. The special cases of `\\n` and `\\r` are allowed but completely ignored. An error will result when attempting to decode a string with a character that is not in the encoding alphabet or not one of `=`, `\\r`, or `\\n`.", "json_ref": "*Warning*: Usage should be restricted to very rare use cases, as JSON decoding is expensive and quite limited. In addition, JSON objects are large and not optimized for size.\n\nAlmost all smart contracts should use simpler and smaller methods (such as the [ABI](https://arc.algorand.foundation/ARCs/arc-0004). This opcode should only be used in cases where JSON is only available option, e.g. when a third-party only signs JSON.", - "switchi": "The `switchi` instruction opcode is followed by `n`, the number of targets, each of which are encoded as 2 byte values indicating the position of the target label relative to the end of the `switchi` instruction (i.e. the offset). The last element on the stack represents the index of the target to branch to. If the index is greater than or equal to n, then evaluation falls through to the next instruction. Otherwise, the program will branch to `pc + 1 + sizeof(n) + 2 * n + target[index]`. Branch targets must be aligned instructions. (e.g. Branching to the second byte of a 2 byte op will be rejected.)", } // OpDocExtra returns extra documentation text about an op diff --git a/data/transactions/logic/eval.go b/data/transactions/logic/eval.go index fa808ab012..d2ef4405f1 100644 --- a/data/transactions/logic/eval.go +++ b/data/transactions/logic/eval.go @@ -1990,34 +1990,30 @@ func branchTarget(cx *EvalContext) (int, error) { return target, nil } -func switchTarget(cx *EvalContext, branchIdx uint64) (int, uint64, error) { - numOffsets, bytesUsed := binary.Uvarint(cx.program[cx.pc+1:]) - if bytesUsed <= 0 || numOffsets > math.MaxUint16 { - return 0, 0, fmt.Errorf("could not decode switch label count at pc=%d", cx.pc+1) - } +func switchTarget(cx *EvalContext, branchIdx uint64) (int, error) { + numOffsets := cx.program[cx.pc+1] - end := cx.pc + 1 + bytesUsed // end of opcode + number of offsets, beginning of offset list + end := cx.pc + 2 // end of opcode + number of offsets, beginning of offset list eoi := end + 2*int(numOffsets) // end of instruction - if branchIdx >= numOffsets { - return eoi, numOffsets, nil + + if eoi > len(cx.program) { // eoi will equal len(p) if switch is last instruction + return 0, fmt.Errorf("switch claims to extend beyond program") } - // 2*branchIdx will be no where near overflow b/c branchIdx < numOffsets < math.MaxUint16 - pos := end + int(2*branchIdx) // position of referenced offset: each offset is 2 bytes - if pos >= len(cx.program)-1 { // Need two bytes to decode an offset - return 0, 0, fmt.Errorf("offset position %d is beyond program", pos) + offset := 0 + if branchIdx < uint64(numOffsets) { + pos := end + int(2*branchIdx) // position of referenced offset: each offset is 2 bytes + offset = decodeBranchOffset(cx.program, pos) } - offset := decodeBranchOffset(cx.program, pos) - target := eoi + offset // offset is applied to the end of this opcode + target := eoi + offset // branching to exactly the end of the program (target == len(cx.program)), the next pc after the last instruction, // is okay and ends normally if target > len(cx.program) || target < 0 { - return 0, 0, fmt.Errorf("branch target %d outside of program", target) + return 0, fmt.Errorf("branch target %d outside of program", target) } - - return target, numOffsets, nil + return target, nil } // checks any branch that is {op} {int16 be offset} @@ -2036,17 +2032,13 @@ func checkBranch(cx *EvalContext) error { return nil } -// checks any switch that is {op} {varuint offset index} [{int16 offset}...] +// checks switch is encoded properly (and calculates nextpc) func checkSwitch(cx *EvalContext) error { - // call with too big of an index. the end-of-instruction PC is returned because - // that's where an out of range branchIdx goes - eoi, numOffsets, err := switchTarget(cx, math.MaxUint64) - if err != nil { - return err - } + numOffsets := uint64(cx.program[cx.pc+1]) + eoi := cx.pc + 2 + int(2*numOffsets) for branchIdx := uint64(0); branchIdx < numOffsets; branchIdx++ { - target, _, err := switchTarget(cx, branchIdx) + target, err := switchTarget(cx, branchIdx) if err != nil { return err } @@ -2107,8 +2099,9 @@ func opB(cx *EvalContext) error { func opSwitchInt(cx *EvalContext) error { last := len(cx.stack) - 1 branchIdx := cx.stack[last].Uint + cx.stack = cx.stack[:last] - target, _, err := switchTarget(cx, branchIdx) + target, err := switchTarget(cx, branchIdx) if err != nil { return err } diff --git a/data/transactions/logic/eval_test.go b/data/transactions/logic/eval_test.go index ac41772e00..26e8a8c718 100644 --- a/data/transactions/logic/eval_test.go +++ b/data/transactions/logic/eval_test.go @@ -5593,4 +5593,18 @@ int 1 `, 8) testPanics(t, notrack("switchi; int 1"), 8) + + // make the switch the final instruction + testAccepts(t, ` +int 1 +int 0 +switchi done1 done2; done1: ; done2: ; +`, 8) + + // make the switch the final instruction, and don't match + testAccepts(t, ` +int 1 +int 88 +switchi done1 done2; done1: ; done2: ; +`, 8) } From d705b0c1d12a53587190a1e763d064d8f647138e Mon Sep 17 00:00:00 2001 From: John Jannotti Date: Thu, 15 Sep 2022 16:56:06 -0400 Subject: [PATCH 6/7] Fix switch disassembly Made some changes to cast the label count (numOffsets) to `int` sooner. My bug was multiplying numOffsets by 2 while it was still a byte and _then_ casting to int. It had already wrapped. --- data/transactions/logic/assembler.go | 25 ++++++++--------------- data/transactions/logic/assembler_test.go | 10 ++++----- data/transactions/logic/eval.go | 14 ++++++------- 3 files changed, 20 insertions(+), 29 deletions(-) diff --git a/data/transactions/logic/assembler.go b/data/transactions/logic/assembler.go index e30663e105..47d5da1a15 100644 --- a/data/transactions/logic/assembler.go +++ b/data/transactions/logic/assembler.go @@ -916,13 +916,13 @@ func asmBranch(ops *OpStream, spec *OpSpec, args []string) error { } func asmSwitch(ops *OpStream, spec *OpSpec, args []string) error { - if len(args) > 256 { - return ops.errorf("%s cannot take more than 256 labels", spec.Name) - } numOffsets := len(args) + if numOffsets > math.MaxUint8 { + return ops.errorf("%s cannot take more than 255 labels", spec.Name) + } ops.pending.WriteByte(spec.Opcode) ops.pending.WriteByte(byte(numOffsets)) - opEndPos := ops.pending.Len() + 2*int(numOffsets) + opEndPos := ops.pending.Len() + 2*numOffsets for _, arg := range args { ops.referToLabel(ops.pending.Len(), arg, opEndPos) // zero bytes will get replaced with actual offset in resolveLabels() @@ -2561,19 +2561,10 @@ func checkByteConstBlock(cx *EvalContext) error { } func parseSwitch(program []byte, pos int) (targets []int, nextpc int, err error) { - numOffsets, bytesUsed := binary.Uvarint(program[pos:]) - if bytesUsed <= 0 { - err = fmt.Errorf("could not decode switch target list size at pc=%d", pos) - return - } - pos += bytesUsed - if numOffsets > math.MaxUint16 { - err = errors.New("switch with too many labels") - return - } - - end := pos + int(2*numOffsets) // end of op: offset is applied to this position - for i := 0; i < int(numOffsets); i++ { + numOffsets := int(program[pos]) + pos++ + end := pos + 2*numOffsets // end of op: offset is applied to this position + for i := 0; i < numOffsets; i++ { offset := decodeBranchOffset(program, pos) target := end + offset targets = append(targets, target) diff --git a/data/transactions/logic/assembler_test.go b/data/transactions/logic/assembler_test.go index 843fd70944..5f8e9baa8c 100644 --- a/data/transactions/logic/assembler_test.go +++ b/data/transactions/logic/assembler_test.go @@ -2797,26 +2797,26 @@ int 1 require.Len(t, ops.Program, 9) // ver (1) + pushint (2) + opcode (1) + length (1) + labels (2*2) var labels []string - for i := 0; i < 256; i++ { + for i := 0; i < 255; i++ { labels = append(labels, fmt.Sprintf("label%d", i)) } - // test that 256 labels is ok + // test that 255 labels is ok source = fmt.Sprintf(` pushint 1 switchi %s %s `, strings.Join(labels, " "), strings.Join(labels, ":\n")+":\n") ops = testProg(t, source, AssemblerMaxVersion) - require.Len(t, ops.Program, 517) // ver (1) + pushint (2) + opcode (1) + length (1) + labels (2*256) + require.Len(t, ops.Program, 515) // ver (1) + pushint (2) + opcode (1) + length (1) + labels (2*255) - // 257 is too many + // 256 is too many source = fmt.Sprintf(` pushint 1 switchi %s extra %s `, strings.Join(labels, " "), strings.Join(labels, ":\n")+":\n") - ops = testProg(t, source, AssemblerMaxVersion, Expect{3, "switchi cannot take more than 256 labels"}) + ops = testProg(t, source, AssemblerMaxVersion, Expect{3, "switchi cannot take more than 255 labels"}) // allow duplicate label reference source = ` diff --git a/data/transactions/logic/eval.go b/data/transactions/logic/eval.go index d2ef4405f1..f87b844240 100644 --- a/data/transactions/logic/eval.go +++ b/data/transactions/logic/eval.go @@ -1991,10 +1991,10 @@ func branchTarget(cx *EvalContext) (int, error) { } func switchTarget(cx *EvalContext, branchIdx uint64) (int, error) { - numOffsets := cx.program[cx.pc+1] + numOffsets := int(cx.program[cx.pc+1]) - end := cx.pc + 2 // end of opcode + number of offsets, beginning of offset list - eoi := end + 2*int(numOffsets) // end of instruction + end := cx.pc + 2 // end of opcode + number of offsets, beginning of offset list + eoi := end + 2*numOffsets // end of instruction if eoi > len(cx.program) { // eoi will equal len(p) if switch is last instruction return 0, fmt.Errorf("switch claims to extend beyond program") @@ -2034,11 +2034,11 @@ func checkBranch(cx *EvalContext) error { // checks switch is encoded properly (and calculates nextpc) func checkSwitch(cx *EvalContext) error { - numOffsets := uint64(cx.program[cx.pc+1]) - eoi := cx.pc + 2 + int(2*numOffsets) + numOffsets := int(cx.program[cx.pc+1]) + eoi := cx.pc + 2 + 2*numOffsets - for branchIdx := uint64(0); branchIdx < numOffsets; branchIdx++ { - target, err := switchTarget(cx, branchIdx) + for branchIdx := 0; branchIdx < numOffsets; branchIdx++ { + target, err := switchTarget(cx, uint64(branchIdx)) if err != nil { return err } From 928966ae526af6b4dc14d3c27db8f55fdf624e33 Mon Sep 17 00:00:00 2001 From: John Jannotti Date: Fri, 16 Sep 2022 11:32:43 -0400 Subject: [PATCH 7/7] docVersion and bump pairing ops --- cmd/opdoc/opdoc.go | 2 +- data/transactions/logic/README.md | 1 + data/transactions/logic/TEAL_opcodes.md | 7 +++++++ data/transactions/logic/assembler_test.go | 11 +++++++++-- data/transactions/logic/langspec.json | 13 ++++++++++++- data/transactions/logic/opcodes.go | 2 +- 6 files changed, 31 insertions(+), 5 deletions(-) diff --git a/cmd/opdoc/opdoc.go b/cmd/opdoc/opdoc.go index 94a394bbca..226d87a78b 100644 --- a/cmd/opdoc/opdoc.go +++ b/cmd/opdoc/opdoc.go @@ -28,7 +28,7 @@ import ( "github.com/algorand/go-algorand/protocol" ) -var docVersion = 7 +var docVersion = 8 func opGroupMarkdownTable(names []string, out io.Writer) { fmt.Fprint(out, `| Opcode | Description | diff --git a/data/transactions/logic/README.md b/data/transactions/logic/README.md index 6c68a8a7c9..521f9b57b7 100644 --- a/data/transactions/logic/README.md +++ b/data/transactions/logic/README.md @@ -594,6 +594,7 @@ Account fields used in the `acct_params_get` opcode. | `assert` | immediately fail unless A is a non-zero number | | `callsub target` | branch unconditionally to TARGET, saving the next instruction on the call stack | | `retsub` | pop the top instruction from the call stack and branch to it | +| `switchi target ...` | branch to the Ath label. Continue at following instruction if index A exceeds the number of labels. | ### State Access diff --git a/data/transactions/logic/TEAL_opcodes.md b/data/transactions/logic/TEAL_opcodes.md index 6c5d679108..45cba65d05 100644 --- a/data/transactions/logic/TEAL_opcodes.md +++ b/data/transactions/logic/TEAL_opcodes.md @@ -1053,6 +1053,13 @@ The call stack is separate from the data stack. Only `callsub` and `retsub` mani The call stack is separate from the data stack. Only `callsub` and `retsub` manipulate it. +## switchi target ... + +- Opcode: 0x8a {uint8 branch count} [{int16 branch offset, big-endian}, ...] +- Stack: ..., A: uint64 → ... +- branch to the Ath label. Continue at following instruction if index A exceeds the number of labels. +- Availability: v8 + ## shl - Opcode: 0x90 diff --git a/data/transactions/logic/assembler_test.go b/data/transactions/logic/assembler_test.go index 5f8e9baa8c..8a2928731b 100644 --- a/data/transactions/logic/assembler_test.go +++ b/data/transactions/logic/assembler_test.go @@ -402,7 +402,9 @@ switch_label1: pushint 1 ` -const v8Nonsense = v7Nonsense + pairingNonsense + switchNonsense +const v8Nonsense = v7Nonsense + switchNonsense + +const v9Nonsense = v8Nonsense + pairingNonsense const v6Compiled = "2004010002b7a60c26050242420c68656c6c6f20776f726c6421070123456789abcd208dae2087fbba51304eb02b91f656948397a7946390e8cb70fc9ea4d95f92251d047465737400320032013202320380021234292929292b0431003101310231043105310731083109310a310b310c310d310e310f3111311231133114311533000033000133000233000433000533000733000833000933000a33000b33000c33000d33000e33000f3300113300123300133300143300152d2e01022581f8acd19181cf959a1281f8acd19181cf951a81f8acd19181cf1581f8acd191810f082209240a220b230c240d250e230f23102311231223132314181b1c28171615400003290349483403350222231d4a484848482b50512a632223524100034200004322602261222704634848222862482864286548482228246628226723286828692322700048482371004848361c0037001a0031183119311b311d311e311f312023221e312131223123312431253126312731283129312a312b312c312d312e312f447825225314225427042455220824564c4d4b0222382124391c0081e80780046a6f686e2281d00f23241f880003420001892224902291922494249593a0a1a2a3a4a5a6a7a8a9aaabacadae24af3a00003b003c003d816472064e014f012a57000823810858235b235a2359b03139330039b1b200b322c01a23c1001a2323c21a23c3233e233f8120af06002a494905002a49490700b53a03b6b7043cb8033a0c2349c42a9631007300810881088120978101c53a8101c6003a" @@ -411,7 +413,11 @@ const randomnessCompiled = "81ffff03d101d000" const v7Compiled = v6Compiled + "5e005f018120af060180070123456789abcd49490501988003012345494984" + randomnessCompiled + "800243218001775c0280018881015d" -const v8Compiled = v7Compiled + pairingCompiled + "81018a02fff800008101" +const switchCompiled = "81018a02fff800008101" + +const v8Compiled = v7Compiled + switchCompiled + +const v9Compiled = v7Compiled + pairingCompiled var nonsense = map[uint64]string{ 1: v1Nonsense, @@ -422,6 +428,7 @@ var nonsense = map[uint64]string{ 6: v6Nonsense, 7: v7Nonsense, 8: v8Nonsense, + 9: v9Nonsense, } var compiled = map[uint64]string{ diff --git a/data/transactions/logic/langspec.json b/data/transactions/logic/langspec.json index ff1a681667..5db44dfefb 100644 --- a/data/transactions/logic/langspec.json +++ b/data/transactions/logic/langspec.json @@ -1,5 +1,5 @@ { - "EvalMaxVersion": 7, + "EvalMaxVersion": 8, "LogicSigVersion": 7, "Ops": [ { @@ -1576,6 +1576,17 @@ "Flow Control" ] }, + { + "Opcode": 138, + "Name": "switchi", + "Args": "U", + "Size": 0, + "Doc": "branch to the Ath label. Continue at following instruction if index A exceeds the number of labels.", + "ImmediateNote": "{uint8 branch count} [{int16 branch offset, big-endian}, ...]", + "Groups": [ + "Flow Control" + ] + }, { "Opcode": 144, "Name": "shl", diff --git a/data/transactions/logic/opcodes.go b/data/transactions/logic/opcodes.go index 50802b307f..ec88ea5ca5 100644 --- a/data/transactions/logic/opcodes.go +++ b/data/transactions/logic/opcodes.go @@ -68,7 +68,7 @@ const randomnessVersion = 7 // vrf_verify, block // EXPERIMENTAL. These should be revisited whenever a new LogicSigVersion is // moved from vFuture to a new consensus version. If they remain unready, bump // their version, and fixup TestAssemble() in assembler_test.go. -const pairingVersion = 8 // bn256 opcodes. will add bls12-381, and unify the available opcodes. +const pairingVersion = 9 // bn256 opcodes. will add bls12-381, and unify the available opcodes. type linearCost struct { baseCost int