cmd/asm: fix another infinite loop in register list parser

The code parsing register lists involves an inner loop on
each range defined by the lo,hi bounds. The condition on
this loop (for lo<=hi) is fragile, because the bounds
are unsigned 16 bits numbers.

In some corner cases, the calculated upper bound is 2^16-1
leading to an infinite loop.

Parsing operand `[):[o-FP` results in:
- an infinite loop for non ARM architectures
- the generation of almost 2^16 errors for the ARM architecture
  (which are then ignored)

This CL improves the code in 3 ways:
- bail out early when parsing non R prefixed registers
- make sure the register index is never negative
- make sure the number of iterations is limited by the
  maximum size of the range (as a defensive measure).

Fixes #12469

Change-Id: Ib1e7e36fb8ad5a3a52c50fc6219d3cfe2b39cc34
Reviewed-on: https://go-review.googlesource.com/14314
Reviewed-by: Rob Pike <r@golang.org>

diff --git a/src/cmd/asm/internal/asm/operand_test.go b/src/cmd/asm/internal/asm/operand_test.go
index 01335ed38b429967f28a19682bd178c34f39ec56..d196574a958575fffb783d9f7a4fdd88b27470c3 100644 (file)
--- a/src/cmd/asm/internal/asm/operand_test.go
+++ b/src/cmd/asm/internal/asm/operand_test.go
@@ -183,6 +183,7 @@ var amd64OperandTests = []operandTest{
        {"y+56(FP)", "y+56(FP)"},
        {"·AddUint32(SB)", "\"\".AddUint32(SB)"},
        {"·callReflect(SB)", "\"\".callReflect(SB)"},
+       {"[):[o-FP", ""}, // Issue 12469 - asm hung parsing the o-FP range on non ARM platforms.
 }
 
 var x86OperandTests = []operandTest{
@@ -240,6 +241,7 @@ var x86OperandTests = []operandTest{
        {"x+4(FP)", "x+4(FP)"},
        {"·AddUint32(SB)", "\"\".AddUint32(SB)"},
        {"·reflectcall(SB)", "\"\".reflectcall(SB)"},
+       {"[):[o-FP", ""}, // Issue 12469 - asm hung parsing the o-FP range on non ARM platforms.
 }
 
 var armOperandTests = []operandTest{
@@ -288,7 +290,9 @@ var armOperandTests = []operandTest{
        {"runtime·_sfloat2(SB)", "runtime._sfloat2(SB)"},
        {"·AddUint32(SB)", "\"\".AddUint32(SB)"},
        {"(R1, R3)", "(R1, R3)"},
-       {"[R0,R1,g,R15", ""}, // Issue 11764 - previously asm just hung parsing ']' missing register lists
+       {"[R0,R1,g,R15", ""}, // Issue 11764 - asm hung parsing ']' missing register lists.
+       {"[):[o-FP", ""},     // Issue 12469 - there was no infinite loop for ARM; these are just sanity checks.
+       {"[):[R0-FP", ""},
 }
 
 var ppc64OperandTests = []operandTest{
@@ -378,6 +382,7 @@ var ppc64OperandTests = []operandTest{
        {"runtime·abort(SB)", "runtime.abort(SB)"},
        {"·AddUint32(SB)", "\"\".AddUint32(SB)"},
        {"·trunc(SB)", "\"\".trunc(SB)"},
+       {"[):[o-FP", ""}, // Issue 12469 - asm hung parsing the o-FP range on non ARM platforms.
 }
 
 var arm64OperandTests = []operandTest{
@@ -427,4 +432,5 @@ var arm64OperandTests = []operandTest{
        {"ZR", "ZR"},
        {"(ZR)", "(ZR)"},
        {"(R29, RSP)", "(R29, RSP)"},
+       {"[):[o-FP", ""}, // Issue 12469 - asm hung parsing the o-FP range on non ARM platforms.
 }

diff --git a/src/cmd/asm/internal/asm/parse.go b/src/cmd/asm/internal/asm/parse.go
index a04f68f8c1b47a9aa60223fd85857822a3fa0fe1..7d81d4b7d1f231b2db45835b39d1b8e69bda920c 100644 (file)
--- a/src/cmd/asm/internal/asm/parse.go
+++ b/src/cmd/asm/internal/asm/parse.go
@@ -702,6 +702,7 @@ func (p *Parser) registerIndirect(a *obj.Addr, prefix rune) {
 // The opening bracket has been consumed.
 func (p *Parser) registerList(a *obj.Addr) {
        // One range per loop.
+       const maxReg = 16
        var bits uint16
 ListLoop:
        for {
@@ -713,6 +714,7 @@ ListLoop:
                        p.errorf("missing ']' in register list")
                        return
                }
+               // Parse the upper and lower bounds.
                lo := p.registerNumber(tok.String())
                hi := lo
                if p.peek() == '-' {
@@ -722,7 +724,8 @@ ListLoop:
                if hi < lo {
                        lo, hi = hi, lo
                }
-               for lo <= hi {
+               // Check there are no duplicates in the register list.
+               for i := 0; lo <= hi && i < maxReg; i++ {
                        if bits&(1<<lo) != 0 {
                                p.errorf("register R%d already in list", lo)
                        }
@@ -744,12 +747,19 @@ func (p *Parser) registerNumber(name string) uint16 {
        }
        if name[0] != 'R' {
                p.errorf("expected g or R0 through R15; found %s", name)
+               return 0
        }
        r, ok := p.registerReference(name)
        if !ok {
                return 0
        }
-       return uint16(r - p.arch.Register["R0"])
+       reg := r - p.arch.Register["R0"]
+       if reg < 0 {
+               // Could happen for an architecture having other registers prefixed by R
+               p.errorf("expected g or R0 through R15; found %s", name)
+               return 0
+       }
+       return uint16(reg)
 }
 
 // Note: There are two changes in the expression handling here