[go] cmd/asm: add ARM64 assembler check for incorrect input

fannie zhang (Gerrit)

unread,

Mar 12, 2018, 9:34:01 PM3/12/18

to Ian Lance Taylor, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

fannie zhang has uploaded this change for review.

cmd/asm: add ARM64 assembler check for incorrect input

Current ARM64 assembler has no check for the invalid value of both
shift amount and post-index immediate offset of LD1/ST1. This patch
adds the check.

This patch also fixes the printing error of register number equals
to 31, which should be printed as ZR instead of R31. Test cases
are also added.

Change-Id: I476235f3ab3a3fc91fe89c5a3149a4d4529c05c7
---
M src/cmd/asm/internal/asm/testdata/arm64.s
M src/cmd/asm/internal/asm/testdata/arm64error.s
M src/cmd/internal/obj/arm64/asm7.go
M src/cmd/internal/obj/arm64/list7.go
M src/cmd/internal/obj/util.go
5 files changed, 147 insertions(+), 72 deletions(-)

diff --git a/src/cmd/asm/internal/asm/testdata/arm64.s b/src/cmd/asm/internal/asm/testdata/arm64.s
index 06435b4..f229f61 100644
--- a/src/cmd/asm/internal/asm/testdata/arm64.s
+++ b/src/cmd/asm/internal/asm/testdata/arm64.s
@@ -31,6 +31,9 @@
 	AND	R1@>33, R2, R3
 	ADD	R1.UXTB, R2, R3                 // 4300218b
 	ADD	R1.UXTB<<4, R2, R3              // 4310218b
+	ADD	R2, RSP, RSP                    // ff63228b
+	ADD	R2.SXTX<<1, RSP, RSP            // ffe7228b
+	ADD	ZR.SXTX<<1, R2, R3              // 43e43f8b
 	ADDW	R2.SXTW, R10, R12               // 4cc1220b
 	ADD	R18.UXTX, R14, R17              // d161328b
 	ADDSW	R18.UXTW, R14, R17              // d141322b
@@ -39,6 +42,7 @@
 	SUBW	R1.UXTX<<1, R3, R2              // 6264214b
 	SUBS	R3.UXTX, R8, R9                 // 096123eb
 	SUBSW	R17.UXTH, R15, R21              // f521316b
+	SUBW	ZR<<14, R19, R13                // 6d3a1f4b
 	CMP	R2.SXTH, R13                    // bfa122eb
 	CMN	R1.SXTX<<2, R10                 // 5fe921ab
 	CMPW	R2.UXTH<<3, R11                 // 7f2d226b
diff --git a/src/cmd/asm/internal/asm/testdata/arm64error.s b/src/cmd/asm/internal/asm/testdata/arm64error.s
index b77dabd..a59c12d 100644
--- a/src/cmd/asm/internal/asm/testdata/arm64error.s
+++ b/src/cmd/asm/internal/asm/testdata/arm64error.s
@@ -3,51 +3,57 @@
 // license that can be found in the LICENSE file.
 
 TEXT errors(SB),$0
-	MOVD.P	300(R2), R3                 // ERROR "offset out of range [-255,254]"
-	MOVD.P	R3, 344(R2)                 // ERROR "offset out of range [-255,254]"
-	VLD1	(R8)(R13), [V2.B16]         // ERROR "illegal combination"
-	VLD1	8(R9), [V2.B16]             // ERROR "illegal combination"
-	VST1	[V1.B16], (R8)(R13)         // ERROR "illegal combination"
-	VST1	[V1.B16], 9(R2)             // ERROR "illegal combination"
-	VLD1	8(R8)(R13), [V2.B16]        // ERROR "illegal combination"
-	ADD	R1.UXTB<<5, R2, R3          // ERROR "shift amount out of range 0 to 4"
-	ADDS	R1.UXTX<<7, R2, R3          // ERROR "shift amount out of range 0 to 4"
-	VMOV	V8.D[2], V12.D[1]           // ERROR "register element index out of range 0 to 1"
-	VMOV	V8.S[4], V12.S[1]           // ERROR "register element index out of range 0 to 3"
-	VMOV	V8.H[8], V12.H[1]           // ERROR "register element index out of range 0 to 7"
-	VMOV	V8.B[16], V12.B[1]          // ERROR "register element index out of range 0 to 15"
-	VMOV	V8.D[0], V12.S[1]           // ERROR "operand mismatch"
-	VMOV	V8.D[0], V12.H[1]           // ERROR "operand mismatch"
-	VMOV	V8.D[0], V12.B[1]           // ERROR "operand mismatch"
-	VMOV	V8.S[0], V12.H[1]           // ERROR "operand mismatch"
-	VMOV	V8.S[0], V12.B[1]           // ERROR "operand mismatch"
-	VMOV	V8.H[0], V12.B[1]           // ERROR "operand mismatch"
-	VMOV	V8.B[16], R3                // ERROR "register element index out of range 0 to 15"
-	VMOV	V8.H[9], R3                 // ERROR "register element index out of range 0 to 7"
-	VMOV	V8.S[4], R3                 // ERROR "register element index out of range 0 to 3"
-	VMOV	V8.D[2], R3                 // ERROR "register element index out of range 0 to 1"
-	VDUP	V8.B[16], R3.B16            // ERROR "register element index out of range 0 to 15"
-	VDUP	V8.B[17], R3.B8             // ERROR "register element index out of range 0 to 15"
-	VDUP	V8.H[9], R3.H4              // ERROR "register element index out of range 0 to 7"
-	VDUP	V8.H[9], R3.H8              // ERROR "register element index out of range 0 to 7"
-	VDUP	V8.S[4], R3.S2              // ERROR "register element index out of range 0 to 3"
-	VDUP	V8.S[4], R3.S4              // ERROR "register element index out of range 0 to 3"
-	VDUP	V8.D[2], R3.D2              // ERROR "register element index out of range 0 to 1"
-	VFMLA	V1.D2, V12.D2, V3.S2        // ERROR "operand mismatch"
-	VFMLA	V1.S2, V12.S2, V3.D2        // ERROR "operand mismatch"
-	VFMLA	V1.S4, V12.S2, V3.D2        // ERROR "operand mismatch"
-	VFMLA	V1.H4, V12.H4, V3.D2        // ERROR "operand mismatch"
-	VFMLS	V1.S2, V12.S2, V3.S4        // ERROR "operand mismatch"
-	VFMLS	V1.S2, V12.D2, V3.S4        // ERROR "operand mismatch"
-	VFMLS	V1.S2, V12.S4, V3.D2        // ERROR "operand mismatch"
-	VFMLA	V1.B8, V12.B8, V3.B8        // ERROR "invalid arrangement"
-	VFMLA	V1.B16, V12.B16, V3.B16     // ERROR "invalid arrangement"
-	VFMLA	V1.H4, V12.H4, V3.H4        // ERROR "invalid arrangement"
-	VFMLA	V1.H8, V12.H8, V3.H8        // ERROR "invalid arrangement"
-	VFMLA	V1.H4, V12.H4, V3.H4        // ERROR "invalid arrangement"
-	VFMLS	V1.B8, V12.B8, V3.B8        // ERROR "invalid arrangement"
-	VFMLS	V1.B16, V12.B16, V3.B16     // ERROR "invalid arrangement"
-	VFMLS	V1.H4, V12.H4, V3.H4        // ERROR "invalid arrangement"
-	VFMLS	V1.H8, V12.H8, V3.H8        // ERROR "invalid arrangement"
-	VFMLS	V1.H4, V12.H4, V3.H4        // ERROR "invalid arrangement"
+	MOVD.P	300(R2), R3                                      // ERROR "offset out of range [-255,254]"
+	MOVD.P	R3, 344(R2)                                      // ERROR "offset out of range [-255,254]"
+	VLD1	(R8)(R13), [V2.B16]                              // ERROR "illegal combination"
+	VLD1	8(R9), [V2.B16]                                  // ERROR "illegal combination"
+	VST1	[V1.B16], (R8)(R13)                              // ERROR "illegal combination"
+	VST1	[V1.B16], 9(R2)                                  // ERROR "illegal combination"
+	VLD1	8(R8)(R13), [V2.B16]                             // ERROR "illegal combination"
+	ADD	R1.UXTB<<5, R2, R3                               // ERROR "shift amount out of range 0 to 4"
+	ADDS	R1.UXTX<<7, R2, R3                               // ERROR "shift amount out of range 0 to 4"
+	VMOV	V8.D[2], V12.D[1]                                // ERROR "register element index out of range 0 to 1"
+	VMOV	V8.S[4], V12.S[1]                                // ERROR "register element index out of range 0 to 3"
+	VMOV	V8.H[8], V12.H[1]                                // ERROR "register element index out of range 0 to 7"
+	VMOV	V8.B[16], V12.B[1]                               // ERROR "register element index out of range 0 to 15"
+	VMOV	V8.D[0], V12.S[1]                                // ERROR "operand mismatch"
+	VMOV	V8.D[0], V12.H[1]                                // ERROR "operand mismatch"
+	VMOV	V8.D[0], V12.B[1]                                // ERROR "operand mismatch"
+	VMOV	V8.S[0], V12.H[1]                                // ERROR "operand mismatch"
+	VMOV	V8.S[0], V12.B[1]                                // ERROR "operand mismatch"
+	VMOV	V8.H[0], V12.B[1]                                // ERROR "operand mismatch"
+	VMOV	V8.B[16], R3                                     // ERROR "register element index out of range 0 to 15"
+	VMOV	V8.H[9], R3                                      // ERROR "register element index out of range 0 to 7"
+	VMOV	V8.S[4], R3                                      // ERROR "register element index out of range 0 to 3"
+	VMOV	V8.D[2], R3                                      // ERROR "register element index out of range 0 to 1"
+	VDUP	V8.B[16], R3.B16                                 // ERROR "register element index out of range 0 to 15"
+	VDUP	V8.B[17], R3.B8                                  // ERROR "register element index out of range 0 to 15"
+	VDUP	V8.H[9], R3.H4                                   // ERROR "register element index out of range 0 to 7"
+	VDUP	V8.H[9], R3.H8                                   // ERROR "register element index out of range 0 to 7"
+	VDUP	V8.S[4], R3.S2                                   // ERROR "register element index out of range 0 to 3"
+	VDUP	V8.S[4], R3.S4                                   // ERROR "register element index out of range 0 to 3"
+	VDUP	V8.D[2], R3.D2                                   // ERROR "register element index out of range 0 to 1"
+	VFMLA	V1.D2, V12.D2, V3.S2                             // ERROR "operand mismatch"
+	VFMLA	V1.S2, V12.S2, V3.D2                             // ERROR "operand mismatch"
+	VFMLA	V1.S4, V12.S2, V3.D2                             // ERROR "operand mismatch"
+	VFMLA	V1.H4, V12.H4, V3.D2                             // ERROR "operand mismatch"
+	VFMLS	V1.S2, V12.S2, V3.S4                             // ERROR "operand mismatch"
+	VFMLS	V1.S2, V12.D2, V3.S4                             // ERROR "operand mismatch"
+	VFMLS	V1.S2, V12.S4, V3.D2                             // ERROR "operand mismatch"
+	VFMLA	V1.B8, V12.B8, V3.B8                             // ERROR "invalid arrangement"
+	VFMLA	V1.B16, V12.B16, V3.B16                          // ERROR "invalid arrangement"
+	VFMLA	V1.H4, V12.H4, V3.H4                             // ERROR "invalid arrangement"
+	VFMLA	V1.H8, V12.H8, V3.H8                             // ERROR "invalid arrangement"
+	VFMLA	V1.H4, V12.H4, V3.H4                             // ERROR "invalid arrangement"
+	VFMLS	V1.B8, V12.B8, V3.B8                             // ERROR "invalid arrangement"
+	VFMLS	V1.B16, V12.B16, V3.B16                          // ERROR "invalid arrangement"
+	VFMLS	V1.H4, V12.H4, V3.H4                             // ERROR "invalid arrangement"
+	VFMLS	V1.H8, V12.H8, V3.H8                             // ERROR "invalid arrangement"
+	VFMLS	V1.H4, V12.H4, V3.H4                             // ERROR "invalid arrangement"
+	VST1.P	[V4.S4,V5.S4], 48(R1)                            // ERROR "invalid post-increment amount"
+	VST1.P	[V4.S4], 8(R1)                                   // ERROR "invalid post-increment amount"
+	VLD1.P	32(R1), [V8.S4, V9.S4, V10.S4]                   // ERROR "invalid post-increment amount"
+	VLD1.P	48(R1), [V7.S4, V8.S4, V9.S4, V10.S4]            // ERROR "invalid post-increment amount"
+	ADDSW	R7->32, R14, R13                                 // ERROR "shift amount out of range 0 to 31"
+	BICW	R7@>33, R5, R16                                  // ERROR "shift amount out of range 0 to 31"
 	RET
diff --git a/src/cmd/internal/obj/arm64/asm7.go b/src/cmd/internal/obj/arm64/asm7.go
index 72c0948..f48281b 100644
--- a/src/cmd/internal/obj/arm64/asm7.go
+++ b/src/cmd/internal/obj/arm64/asm7.go
@@ -2221,6 +2221,51 @@
 	}
 }
 
+/* checkoffset checks whether the immediate offset is valid or not*/
+func (c *ctxt7) checkoffset(p *obj.Prog, as obj.As) {
+	var offset, list int64
+	switch as {
+	case AVLD1:
+		offset = p.From.Offset
+		list = p.To.Offset
+	case AVST1:
+		offset = p.To.Offset
+		list = p.From.Offset
+	default:
+		c.ctxt.Diag("invalid operation on op %v", p.As)
+		break
+	}
+	opcode := (list >> 12) & 15
+	q := (list >> 30) & 1
+	if offset != 0 {
+		switch opcode {
+		case 0x7:
+			// one register
+			if !(q == 0 && offset == 8) && !(q == 1 && offset == 16) {
+				c.ctxt.Diag("invalid post-increment amount: %v", p)
+			}
+		case 0xa:
+			// two registers
+			if !(q == 0 && offset == 16) && !(q == 1 && offset == 32) {
+				c.ctxt.Diag("invalid post-increment amount: %v", p)
+			}
+		case 0x6:
+			// three registers
+			if !(q == 0 && offset == 24) && !(q == 1 && offset == 48) {
+				c.ctxt.Diag("invalid post-increment amount: %v", p)
+			}
+		case 0x2:
+			// four registers
+			if !(q == 0 && offset == 32) && !(q == 1 && offset == 64) {
+				c.ctxt.Diag("invalid post-increment amount: %v", p)
+			}
+		default:
+			c.ctxt.Diag("invalid register numbers in ARM64 register list: %v", p)
+			break
+		}
+	}
+}
+
 func (c *ctxt7) asmout(p *obj.Prog, o *Optab, out []uint32) {
 	o1 := uint32(0)
 	o2 := uint32(0)
@@ -2272,6 +2317,11 @@
 	case 3: /* op R<<n[,R],R (shifted register) */
 		o1 = c.oprrr(p, p.As)
 
+		amount := (p.From.Offset >> 10) & 63
+		sf := o1 & (1 << 31)
+		if sf == 0 && amount >= 32 {
+			c.ctxt.Diag("shift amount out of range 0 to 31: %v", p)
+		}
 		o1 |= uint32(p.From.Offset) /* includes reg, op, etc */
 		rt := int(p.To.Reg)
 		if p.To.Type == obj.TYPE_NONE {
@@ -3628,6 +3678,7 @@
 			o1 |= 1 << 23
 			if p.From.Index == 0 {
 				// immediate offset variant
+				c.checkoffset(p, p.As)
 				o1 |= 0x1f << 16
 			} else {
 				// register offset variant
@@ -3716,6 +3767,7 @@
 			o1 |= 1 << 23
 			if p.To.Index == 0 {
 				// immediate offset variant
+				c.checkoffset(p, p.As)
 				o1 |= 0x1f << 16
 			} else {
 				// register offset variant
diff --git a/src/cmd/internal/obj/arm64/list7.go b/src/cmd/internal/obj/arm64/list7.go
index cf92120..69e7b70 100644
--- a/src/cmd/internal/obj/arm64/list7.go
+++ b/src/cmd/internal/obj/arm64/list7.go
@@ -92,6 +92,7 @@
 }
 
 func rconv(r int) string {
+	ext := (r >> 5) & 7
 	if r == REGG {
 		return "g"
 	}
@@ -171,52 +172,52 @@
 	case r == REG_PSTL3STRM:
 		return "PSTL3STRM"
 	case REG_UXTB <= r && r < REG_UXTH:
-		if (r>>5)&7 != 0 {
-			return fmt.Sprintf("R%d.UXTB<<%d", r&31, (r>>5)&7)
+		if ext != 0 {
+			return fmt.Sprintf("%s.UXTB<<%d", regOrZr(r), ext)
 		} else {
-			return fmt.Sprintf("R%d.UXTB", r&31)
+			return fmt.Sprintf("%s.UXTB", regOrZr(r))
 		}
 	case REG_UXTH <= r && r < REG_UXTW:
-		if (r>>5)&7 != 0 {
-			return fmt.Sprintf("R%d.UXTH<<%d", r&31, (r>>5)&7)
+		if ext != 0 {
+			return fmt.Sprintf("%s.UXTH<<%d", regOrZr(r), ext)
 		} else {
-			return fmt.Sprintf("R%d.UXTH", r&31)
+			return fmt.Sprintf("%s.UXTH", regOrZr(r))
 		}
 	case REG_UXTW <= r && r < REG_UXTX:
-		if (r>>5)&7 != 0 {
-			return fmt.Sprintf("R%d.UXTW<<%d", r&31, (r>>5)&7)
+		if ext != 0 {
+			return fmt.Sprintf("%s.UXTW<<%d", regOrZr(r), ext)
 		} else {
-			return fmt.Sprintf("R%d.UXTW", r&31)
+			return fmt.Sprintf("%s.UXTW", regOrZr(r))
 		}
 	case REG_UXTX <= r && r < REG_SXTB:
-		if (r>>5)&7 != 0 {
-			return fmt.Sprintf("R%d.UXTX<<%d", r&31, (r>>5)&7)
+		if ext != 0 {
+			return fmt.Sprintf("%s.UXTX<<%d", regOrZr(r), ext)
 		} else {
-			return fmt.Sprintf("R%d.UXTX", r&31)
+			return fmt.Sprintf("%s.UXTX", regOrZr(r))
 		}
 	case REG_SXTB <= r && r < REG_SXTH:
-		if (r>>5)&7 != 0 {
-			return fmt.Sprintf("R%d.SXTB<<%d", r&31, (r>>5)&7)
+		if ext != 0 {
+			return fmt.Sprintf("%s.SXTB<<%d", regOrZr(r), ext)
 		} else {
-			return fmt.Sprintf("R%d.SXTB", r&31)
+			return fmt.Sprintf("%s.SXTB", regOrZr(r))
 		}
 	case REG_SXTH <= r && r < REG_SXTW:
-		if (r>>5)&7 != 0 {
-			return fmt.Sprintf("R%d.SXTH<<%d", r&31, (r>>5)&7)
+		if ext != 0 {
+			return fmt.Sprintf("%s.SXTH<<%d", regOrZr(r), ext)
 		} else {
-			return fmt.Sprintf("R%d.SXTH", r&31)
+			return fmt.Sprintf("%s.SXTH", regOrZr(r))
 		}
 	case REG_SXTW <= r && r < REG_SXTX:
-		if (r>>5)&7 != 0 {
-			return fmt.Sprintf("R%d.SXTW<<%d", r&31, (r>>5)&7)
+		if ext != 0 {
+			return fmt.Sprintf("%s.SXTW<<%d", regOrZr(r), ext)
 		} else {
-			return fmt.Sprintf("R%d.SXTW", r&31)
+			return fmt.Sprintf("%s.SXTW", regOrZr(r))
 		}
 	case REG_SXTX <= r && r < REG_SPECIAL:
-		if (r>>5)&7 != 0 {
-			return fmt.Sprintf("R%d.SXTX<<%d", r&31, (r>>5)&7)
+		if ext != 0 {
+			return fmt.Sprintf("%s.SXTX<<%d", regOrZr(r), ext)
 		} else {
-			return fmt.Sprintf("R%d.SXTX", r&31)
+			return fmt.Sprintf("%s.SXTX", regOrZr(r))
 		}
 	case REG_ARNG <= r && r < REG_ELEM:
 		return fmt.Sprintf("V%d.%s", r&31, arrange((r>>5)&15))
@@ -289,3 +290,11 @@
 	str += "]"
 	return str
 }
+
+func regOrZr(r int) string {
+	reg := "ZR"
+	if r&31 != 31 {
+		reg = fmt.Sprintf("R%d", r&31)
+	}
+	return reg
+}
diff --git a/src/cmd/internal/obj/util.go b/src/cmd/internal/obj/util.go
index 245e9e9..34c9dd1 100644
--- a/src/cmd/internal/obj/util.go
+++ b/src/cmd/internal/obj/util.go
@@ -263,7 +263,11 @@
 			}
 		case "arm64":
 			op := ops[((v>>22)&3)<<1:]
-			str = fmt.Sprintf("R%d%c%c%d", (v>>16)&31, op[0], op[1], (v>>10)&63)
+			reg := "ZR"
+			if (v>>16)&31 != 31 {
+				reg = fmt.Sprintf("R%d", (v>>16)&31)
+			}
+			str = fmt.Sprintf("%s%c%c%d", reg, op[0], op[1], (v>>10)&63)
 		default:
 			panic("TYPE_SHIFT is not supported on " + objabi.GOARCH)
 		}

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

fannie zhang (Gerrit)

unread,

Mar 12, 2018, 9:36:25 PM3/12/18

to goph...@pubsubhelper.golang.org, Cherry Zhang, Gobot Gobot, Brad Fitzpatrick, Chad Rosier, Geoff Berry, Wei Xiao, golang-co...@googlegroups.com

This CL adds assembler check for incorrect input. Could you please help to review? Thank you.

View Change

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

fannie zhang (Gerrit)

unread,

Mar 14, 2018, 11:20:13 PM3/14/18

to goph...@pubsubhelper.golang.org, Cherry Zhang, Gobot Gobot, Brad Fitzpatrick, Chad Rosier, Geoff Berry, Wei Xiao, golang-co...@googlegroups.com

ping Cherry Zhang

View Change

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

Cherry Zhang (Gerrit)

unread,

Mar 15, 2018, 7:00:53 AM3/15/18

to fannie zhang, goph...@pubsubhelper.golang.org, Gobot Gobot, Brad Fitzpatrick, Chad Rosier, Geoff Berry, Wei Xiao, golang-co...@googlegroups.com

View Change

2 comments:

File src/cmd/internal/obj/arm64/list7.go:
- Patch Set #1, Line 294: regOrZr
  What about "regname"?
File src/cmd/internal/obj/util.go:
- Patch Set #1, Line 266:
```
			reg := "ZR"
```

- ```
			if (v>>16)&31 != 31 {
```

				reg = fmt.Sprintf("R%d", (v>>16)&31)
			}

- Can we use Rconv here? Something like,
  r := (v>>16)&31
  str = fmt.Sprintf(..., Rconv(r + RBaseARM64), ...)
  It would be good if we can keep this logic in a single place.

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

fannie zhang (Gerrit)

unread,

Mar 16, 2018, 3:24:52 AM3/16/18

to Cherry Zhang, Gobot Gobot, Geoff Berry, Brad Fitzpatrick, Chad Rosier, goph...@pubsubhelper.golang.org, Wei Xiao, golang-co...@googlegroups.com

fannie zhang uploaded patch set #2 to this change.

View Change

cmd/asm: add ARM64 assembler check for incorrect input

Current ARM64 assembler has no check for the invalid value of both
shift amount and post-index immediate offset of LD1/ST1. This patch
adds the check.

This patch also fixes the printing error of register number equals
to 31, which should be printed as ZR instead of R31. Test cases
are also added.

Change-Id: I476235f3ab3a3fc91fe89c5a3149a4d4529c05c7
---
M src/cmd/asm/internal/asm/testdata/arm64.s
M src/cmd/asm/internal/asm/testdata/arm64error.s
M src/cmd/internal/obj/arm64/asm7.go
M src/cmd/internal/obj/arm64/list7.go
M src/cmd/internal/obj/util.go

5 files changed, 146 insertions(+), 75 deletions(-)

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

fannie zhang (Gerrit)

unread,

Mar 16, 2018, 3:26:01 AM3/16/18

to goph...@pubsubhelper.golang.org, Cherry Zhang, Gobot Gobot, Brad Fitzpatrick, Chad Rosier, Geoff Berry, Wei Xiao, golang-co...@googlegroups.com

View Change

2 comments:

File src/cmd/internal/obj/arm64/list7.go:

- Patch Set #1, Line 294:

- What about "regname"?
  Done
File src/cmd/internal/obj/util.go:

- Patch Set #1, Line 266:

			r := (v >> 16) & 31
			str = fmt.Sprintf("%s%c%c%d", Rconv(int(r+RBaseARM64)), op[0], op[1], (v>>10)&63)
		default:
			p

Can we use Rconv here? Something like, […]

Good idea. Thank you.

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

fannie zhang (Gerrit)

unread,

Mar 16, 2018, 3:52:41 AM3/16/18

to goph...@pubsubhelper.golang.org, Ben Shi, Cherry Zhang, Gobot Gobot, Brad Fitzpatrick, Chad Rosier, Geoff Berry, Wei Xiao, golang-co...@googlegroups.com

fannie zhang removed Ben Shi from this change.

View Change

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

Cherry Zhang (Gerrit)

unread,

Mar 18, 2018, 12:18:52 PM3/18/18

to fannie zhang, goph...@pubsubhelper.golang.org, Gobot Gobot, Brad Fitzpatrick, Chad Rosier, Geoff Berry, Wei Xiao, golang-co...@googlegroups.com

View Change

10 comments:

File src/cmd/asm/internal/asm/testdata/arm64error.s:
- Patch Set #2, Line 59: ADDSW R7->32, R14, R13 // ERROR "shift amount out of range 0 to 31"
  Put these along with the non-vector instruction tests.
File src/cmd/internal/obj/arm64/asm7.go:
- Patch Set #2, Line 2230: or not
  "or not" is not needed.
  Add "... for VLD1.P and VST1.P".
- Patch Set #2, Line 2242: break
  break is not needed.
- Patch Set #2, Line 2246: if offset != 0 {
  Maybe reverse this condition:
```
if offset == 0 {
  return
}
...
```
- Patch Set #2, Line 2251: amount
  amount -> offset
- Patch Set #2, Line 2248:
```
		case 0x7:
			// one register
```


			if !(q == 0 && offset == 8) && !(q == 1 && offset == 16) {

				c.ctxt.Diag("invalid post-increment amount: %v", p)
			}

- ```
		case 0xa:
			// two registers
```


			if !(q == 0 && offset == 16) && !(q == 1 && offset == 32) {

				c.ctxt.Diag("invalid post-increment amount: %v", p)
			}

- ```
		case 0x6:
			// three registers
```


			if !(q == 0 && offset == 24) && !(q == 1 && offset == 48) {

				c.ctxt.Diag("invalid post-increment amount: %v", p)
			}

- ```
		case 0x2:
			// four registers
```


			if !(q == 0 && offset == 32) && !(q == 1 && offset == 64) {

				c.ctxt.Diag("invalid post-increment amount: %v", p)
			}

- There are a lot duplicates. I suggest to decide the number of registers first and do a single check, like
```
var n int
switch opcode {
case 0x7:
  n = 1 // one register
case 0xa:
  n = 2 // two registers
...
}
if !(q == 0 && offset == n * 8) && !(q == 1 && offset == n * 16) {
 ...
}
```
- Patch Set #2, Line 2270: break
  break is not needed.
- Patch Set #2, Line 2327: sf
  What does sf stand for? If there is no better reason, maybe change it to is64bit.
File src/cmd/internal/obj/arm64/list7.go:
- Patch Set #2, Line 297:
```
	reg := "ZR"
	if r&31 != 31 {
```

- ```
		reg = fmt.Sprintf("R%d", r&31)
	}
```

	return reg

Nit: I think it would be clearer to reverse the condition:

if r&31 == 31 {
  return "ZR"
}
return fmt.Sprintf("R%d", r&31)

File src/cmd/internal/obj/util.go:
- Patch Set #2, Line 267: int
  Is this case necessary? r is already of type int, right?

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

fannie zhang (Gerrit)

unread,

Mar 19, 2018, 6:39:26 AM3/19/18

to Cherry Zhang, Gobot Gobot, Geoff Berry, Brad Fitzpatrick, Chad Rosier, goph...@pubsubhelper.golang.org, Wei Xiao, golang-co...@googlegroups.com

fannie zhang uploaded patch set #3 to this change.

View Change

cmd/asm: add ARM64 assembler check for incorrect input

Current ARM64 assembler has no check for the invalid value of both
shift amount and post-index immediate offset of LD1/ST1. This patch
adds the check.

This patch also fixes the printing error of register number equals
to 31, which should be printed as ZR instead of R31. Test cases
are also added.

Change-Id: I476235f3ab3a3fc91fe89c5a3149a4d4529c05c7
---
M src/cmd/asm/internal/asm/testdata/arm64.s
M src/cmd/asm/internal/asm/testdata/arm64error.s
M src/cmd/internal/obj/arm64/asm7.go
M src/cmd/internal/obj/arm64/list7.go
M src/cmd/internal/obj/util.go

5 files changed, 135 insertions(+), 75 deletions(-)

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

fannie zhang (Gerrit)

unread,

Mar 19, 2018, 6:39:49 AM3/19/18

to goph...@pubsubhelper.golang.org, Cherry Zhang, Gobot Gobot, Brad Fitzpatrick, Chad Rosier, Geoff Berry, Wei Xiao, golang-co...@googlegroups.com

View Change

10 comments:

File src/cmd/asm/internal/asm/testdata/arm64error.s:
- Patch Set #2, Line 59: VLD1.P 32(R1), [V8.S4, V9.S4, V10.S4] // ERROR "invalid post-increment offset"

- Put these along with the non-vector instruction tests.

- Done
File src/cmd/internal/obj/arm64/asm7.go:
- Patch Set #2, Line 2230: for VL
  "or not" is not needed. […]
  Done

- Patch Set #2, Line 2242:

- break is not needed.
  Done
- Patch Set #2, Line 2246: return
  Maybe reverse this condition: […]
  Done

- Patch Set #2, Line 2251:

- amount -> offset
  Done

- Patch Set #2, Line 2248:

- ```
	switch opcode {
```


	case 0x7:
		n = 1 // one register
	case 0xa:
		n = 2 // two registers

	case 0x6:
		n = 3 // three registers
	case 0x2:
		n = 4 // four registers
	default:


		c.ctxt.Diag("invalid register numbers in ARM64 register list: %v", p)
	}

	if !(q == 0 && offset == n*8) && !(q == 1 && offset == n*16) {
		c.ctxt.Diag("invalid post-increment offset: %v", p)


	}
}

func (c *ctxt7) asmout(p *obj.Prog, o *Optab, out []uint32) {
	o1 := uint32(0)
	o2

- There are a lot duplicates. […]
  Done
- Patch Set #2, Line 2270: := u
  break is not needed.
  Done

- Patch Set #2, Line 2327: if

- What does sf stand for? If there is no better reason, maybe change it to is64bit.
  Done
File src/cmd/internal/obj/arm64/list7.go:

- Patch Set #2, Line 297:

- ```
	if r&31 == 31 {
```


		return "ZR"
	}
	return fmt.Sprintf("R%d", r&31)
}

- Nit: I think it would be clearer to reverse the condition: […]
  Done
File src/cmd/internal/obj/util.go:
- Patch Set #2, Line 267: r+R

- Is this case necessary? r is already of type int, right?

- Yes, you are right.Thank you.

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

Cherry Zhang (Gerrit)

unread,

Mar 19, 2018, 7:33:39 PM3/19/18

to fannie zhang, goph...@pubsubhelper.golang.org, Gobot Gobot, Brad Fitzpatrick, Chad Rosier, Geoff Berry, Wei Xiao, golang-co...@googlegroups.com

Patch set 3:Run-TryBot +1Code-Review +2

View Change

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

Gobot Gobot (Gerrit)

unread,

Mar 19, 2018, 7:33:48 PM3/19/18

to fannie zhang, goph...@pubsubhelper.golang.org, Cherry Zhang, Brad Fitzpatrick, Chad Rosier, Geoff Berry, Wei Xiao, golang-co...@googlegroups.com

TryBots beginning. Status page: https://farmer.golang.org/try?commit=d968c14d

View Change

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

Gobot Gobot (Gerrit)

unread,

Mar 19, 2018, 7:37:29 PM3/19/18

to fannie zhang, goph...@pubsubhelper.golang.org, Cherry Zhang, Brad Fitzpatrick, Chad Rosier, Geoff Berry, Wei Xiao, golang-co...@googlegroups.com

Build is still in progress...
This change failed on linux-amd64:
See https://storage.googleapis.com/go-build-log/d968c14d/linux-amd64_56c4a3e3.log

Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.

View Change

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

Gobot Gobot (Gerrit)

unread,

Mar 19, 2018, 7:43:38 PM3/19/18

to fannie zhang, goph...@pubsubhelper.golang.org, Cherry Zhang, Brad Fitzpatrick, Chad Rosier, Geoff Berry, Wei Xiao, golang-co...@googlegroups.com

1 of 17 TryBots failed:
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/d968c14d/linux-amd64_56c4a3e3.log

Consult https://build.golang.org/ to see whether they are new failures.

Patch set 3:TryBot-Result -1

View Change

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

Cherry Zhang (Gerrit)

unread,

Mar 19, 2018, 7:45:54 PM3/19/18

to fannie zhang, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Gobot Gobot, Brad Fitzpatrick, Chad Rosier, Geoff Berry, Wei Xiao, golang-co...@googlegroups.com

Cherry Zhang merged this change.

View Change

Approvals:
  Cherry Zhang: Looks good to me, approved; Run TryBots

Objections:
  Gobot Gobot: TryBots failed

cmd/asm: add ARM64 assembler check for incorrect input

Current ARM64 assembler has no check for the invalid value of both
shift amount and post-index immediate offset of LD1/ST1. This patch
adds the check.

This patch also fixes the printing error of register number equals
to 31, which should be printed as ZR instead of R31. Test cases
are also added.

Change-Id: I476235f3ab3a3fc91fe89c5a3149a4d4529c05c7

Reviewed-on: https://go-review.googlesource.com/100255
Reviewed-by: Cherry Zhang <cher...@google.com>
Run-TryBot: Cherry Zhang <cher...@google.com>


---
M src/cmd/asm/internal/asm/testdata/arm64.s
M src/cmd/asm/internal/asm/testdata/arm64error.s
M src/cmd/internal/obj/arm64/asm7.go
M src/cmd/internal/obj/arm64/list7.go
M src/cmd/internal/obj/util.go

5 files changed, 135 insertions(+), 75 deletions(-)

diff --git a/src/cmd/asm/internal/asm/testdata/arm64.s b/src/cmd/asm/internal/asm/testdata/arm64.s
index c97b64d..c53023e 100644


--- a/src/cmd/asm/internal/asm/testdata/arm64.s
+++ b/src/cmd/asm/internal/asm/testdata/arm64.s
@@ -31,6 +31,9 @@
 	AND	R1@>33, R2, R3
 	ADD	R1.UXTB, R2, R3                 // 4300218b
 	ADD	R1.UXTB<<4, R2, R3              // 4310218b
+	ADD	R2, RSP, RSP                    // ff63228b
+	ADD	R2.SXTX<<1, RSP, RSP            // ffe7228b
+	ADD	ZR.SXTX<<1, R2, R3              // 43e43f8b
 	ADDW	R2.SXTW, R10, R12               // 4cc1220b
 	ADD	R18.UXTX, R14, R17              // d161328b
 	ADDSW	R18.UXTW, R14, R17              // d141322b
@@ -39,6 +42,7 @@
 	SUBW	R1.UXTX<<1, R3, R2              // 6264214b
 	SUBS	R3.UXTX, R8, R9                 // 096123eb
 	SUBSW	R17.UXTH, R15, R21              // f521316b
+	SUBW	ZR<<14, R19, R13                // 6d3a1f4b
 	CMP	R2.SXTH, R13                    // bfa122eb
 	CMN	R1.SXTX<<2, R10                 // 5fe921ab
 	CMPW	R2.UXTH<<3, R11                 // 7f2d226b
diff --git a/src/cmd/asm/internal/asm/testdata/arm64error.s b/src/cmd/asm/internal/asm/testdata/arm64error.s

index 93c3acd..dcdb4fe 100644
--- a/src/cmd/asm/internal/asm/testdata/arm64error.s
+++ b/src/cmd/asm/internal/asm/testdata/arm64error.s
@@ -3,54 +3,59 @@

-
-	AND	$1, RSP                     // ERROR "illegal combination"
-	ANDS	$1, R0, RSP                 // ERROR "illegal combination"
+	AND	$1, RSP                                          // ERROR "illegal combination"
+	ANDS	$1, R0, RSP                                      // ERROR "illegal combination"


+	MOVD.P	300(R2), R3                                      // ERROR "offset out of range [-255,254]"
+	MOVD.P	R3, 344(R2)                                      // ERROR "offset out of range [-255,254]"

+	ADDSW	R7->32, R14, R13                                 // ERROR "shift amount out of range 0 to 31"
+	BICW	R7@>33, R5, R16                                  // ERROR "shift amount out of range 0 to 31"

+	VST1.P	[V4.S4,V5.S4], 48(R1)                            // ERROR "invalid post-increment offset"
+	VST1.P	[V4.S4], 8(R1)                                   // ERROR "invalid post-increment offset"
+	VLD1.P	32(R1), [V8.S4, V9.S4, V10.S4]                   // ERROR "invalid post-increment offset"
+	VLD1.P	48(R1), [V7.S4, V8.S4, V9.S4, V10.S4]            // ERROR "invalid post-increment offset"


 	RET
diff --git a/src/cmd/internal/obj/arm64/asm7.go b/src/cmd/internal/obj/arm64/asm7.go

index 3b7ad24..7b940dd 100644
--- a/src/cmd/internal/obj/arm64/asm7.go
+++ b/src/cmd/internal/obj/arm64/asm7.go
@@ -2227,6 +2227,41 @@
 	}
 }
 
+/* checkoffset checks whether the immediate offset is valid for VLD1.P and VST1.P*/


+func (c *ctxt7) checkoffset(p *obj.Prog, as obj.As) {

+	var offset, list, n int64


+	switch as {
+	case AVLD1:
+		offset = p.From.Offset
+		list = p.To.Offset
+	case AVST1:
+		offset = p.To.Offset
+		list = p.From.Offset
+	default:
+		c.ctxt.Diag("invalid operation on op %v", p.As)
+	}

+	opcode := (list >> 12) & 15
+	q := (list >> 30) & 1

+	if offset == 0 {
+		return
+	}


+	switch opcode {
+	case 0x7:

+		n = 1 // one register
+	case 0xa:
+		n = 2 // two registers
+	case 0x6:
+		n = 3 // three registers
+	case 0x2:
+		n = 4 // four registers


+	default:
+		c.ctxt.Diag("invalid register numbers in ARM64 register list: %v", p)
+	}

+	if !(q == 0 && offset == n*8) && !(q == 1 && offset == n*16) {
+		c.ctxt.Diag("invalid post-increment offset: %v", p)
+	}
+}


+
 func (c *ctxt7) asmout(p *obj.Prog, o *Optab, out []uint32) {
 	o1 := uint32(0)
 	o2 := uint32(0)

@@ -2278,6 +2313,11 @@


 	case 3: /* op R<<n[,R],R (shifted register) */
 		o1 = c.oprrr(p, p.As)
 
+		amount := (p.From.Offset >> 10) & 63

+		is64bit := o1 & (1 << 31)
+		if is64bit == 0 && amount >= 32 {


+			c.ctxt.Diag("shift amount out of range 0 to 31: %v", p)
+		}
 		o1 |= uint32(p.From.Offset) /* includes reg, op, etc */
 		rt := int(p.To.Reg)
 		if p.To.Type == obj.TYPE_NONE {

@@ -3634,6 +3674,7 @@


 			o1 |= 1 << 23
 			if p.From.Index == 0 {
 				// immediate offset variant
+				c.checkoffset(p, p.As)
 				o1 |= 0x1f << 16
 			} else {
 				// register offset variant

@@ -3722,6 +3763,7 @@


 			o1 |= 1 << 23
 			if p.To.Index == 0 {
 				// immediate offset variant
+				c.checkoffset(p, p.As)
 				o1 |= 0x1f << 16
 			} else {
 				// register offset variant
diff --git a/src/cmd/internal/obj/arm64/list7.go b/src/cmd/internal/obj/arm64/list7.go

index 37c61d2..266e2ba 100644


--- a/src/cmd/internal/obj/arm64/list7.go
+++ b/src/cmd/internal/obj/arm64/list7.go
@@ -92,6 +92,7 @@
 }
 
 func rconv(r int) string {
+	ext := (r >> 5) & 7
 	if r == REGG {
 		return "g"
 	}

@@ -173,52 +174,52 @@


 	case r == REG_PSTL3STRM:
 		return "PSTL3STRM"
 	case REG_UXTB <= r && r < REG_UXTH:
-		if (r>>5)&7 != 0 {
-			return fmt.Sprintf("R%d.UXTB<<%d", r&31, (r>>5)&7)
+		if ext != 0 {

+			return fmt.Sprintf("%s.UXTB<<%d", regname(r), ext)


 		} else {
-			return fmt.Sprintf("R%d.UXTB", r&31)

+			return fmt.Sprintf("%s.UXTB", regname(r))


 		}
 	case REG_UXTH <= r && r < REG_UXTW:
-		if (r>>5)&7 != 0 {
-			return fmt.Sprintf("R%d.UXTH<<%d", r&31, (r>>5)&7)
+		if ext != 0 {

+			return fmt.Sprintf("%s.UXTH<<%d", regname(r), ext)


 		} else {
-			return fmt.Sprintf("R%d.UXTH", r&31)

+			return fmt.Sprintf("%s.UXTH", regname(r))


 		}
 	case REG_UXTW <= r && r < REG_UXTX:
-		if (r>>5)&7 != 0 {
-			return fmt.Sprintf("R%d.UXTW<<%d", r&31, (r>>5)&7)
+		if ext != 0 {

+			return fmt.Sprintf("%s.UXTW<<%d", regname(r), ext)


 		} else {
-			return fmt.Sprintf("R%d.UXTW", r&31)

+			return fmt.Sprintf("%s.UXTW", regname(r))


 		}
 	case REG_UXTX <= r && r < REG_SXTB:
-		if (r>>5)&7 != 0 {
-			return fmt.Sprintf("R%d.UXTX<<%d", r&31, (r>>5)&7)
+		if ext != 0 {

+			return fmt.Sprintf("%s.UXTX<<%d", regname(r), ext)


 		} else {
-			return fmt.Sprintf("R%d.UXTX", r&31)

+			return fmt.Sprintf("%s.UXTX", regname(r))


 		}
 	case REG_SXTB <= r && r < REG_SXTH:
-		if (r>>5)&7 != 0 {
-			return fmt.Sprintf("R%d.SXTB<<%d", r&31, (r>>5)&7)
+		if ext != 0 {

+			return fmt.Sprintf("%s.SXTB<<%d", regname(r), ext)


 		} else {
-			return fmt.Sprintf("R%d.SXTB", r&31)

+			return fmt.Sprintf("%s.SXTB", regname(r))


 		}
 	case REG_SXTH <= r && r < REG_SXTW:
-		if (r>>5)&7 != 0 {
-			return fmt.Sprintf("R%d.SXTH<<%d", r&31, (r>>5)&7)
+		if ext != 0 {

+			return fmt.Sprintf("%s.SXTH<<%d", regname(r), ext)


 		} else {
-			return fmt.Sprintf("R%d.SXTH", r&31)

+			return fmt.Sprintf("%s.SXTH", regname(r))


 		}
 	case REG_SXTW <= r && r < REG_SXTX:
-		if (r>>5)&7 != 0 {
-			return fmt.Sprintf("R%d.SXTW<<%d", r&31, (r>>5)&7)
+		if ext != 0 {

+			return fmt.Sprintf("%s.SXTW<<%d", regname(r), ext)


 		} else {
-			return fmt.Sprintf("R%d.SXTW", r&31)

+			return fmt.Sprintf("%s.SXTW", regname(r))


 		}
 	case REG_SXTX <= r && r < REG_SPECIAL:
-		if (r>>5)&7 != 0 {
-			return fmt.Sprintf("R%d.SXTX<<%d", r&31, (r>>5)&7)
+		if ext != 0 {

+			return fmt.Sprintf("%s.SXTX<<%d", regname(r), ext)


 		} else {
-			return fmt.Sprintf("R%d.SXTX", r&31)

+			return fmt.Sprintf("%s.SXTX", regname(r))


 		}
 	case REG_ARNG <= r && r < REG_ELEM:
 		return fmt.Sprintf("V%d.%s", r&31, arrange((r>>5)&15))

@@ -291,3 +292,10 @@


 	str += "]"
 	return str
 }
+

+func regname(r int) string {
+	if r&31 == 31 {
+		return "ZR"
+	}
+	return fmt.Sprintf("R%d", r&31)
+}
diff --git a/src/cmd/internal/obj/util.go b/src/cmd/internal/obj/util.go
index 245e9e9..9b25231 100644
--- a/src/cmd/internal/obj/util.go
+++ b/src/cmd/internal/obj/util.go
@@ -263,7 +263,8 @@


 			}
 		case "arm64":
 			op := ops[((v>>22)&3)<<1:]
-			str = fmt.Sprintf("R%d%c%c%d", (v>>16)&31, op[0], op[1], (v>>10)&63)

+			r := (v >> 16) & 31
+			str = fmt.Sprintf("%s%c%c%d", Rconv(r+RBaseARM64), op[0], op[1], (v>>10)&63)


 		default:
 			panic("TYPE_SHIFT is not supported on " + objabi.GOARCH)
 		}

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

Rob Pike (Gerrit)

unread,

Mar 19, 2018, 7:50:30 PM3/19/18

to fannie zhang, Cherry Zhang, goph...@pubsubhelper.golang.org, Rob Pike, Gobot Gobot, Brad Fitzpatrick, Chad Rosier, Geoff Berry, Wei Xiao, golang-co...@googlegroups.com

View Change

2 comments:

File src/cmd/internal/obj/arm64/asm7.go:
- Patch Set #4, Line 2261: c.ctxt.Diag("invalid post-increment offset: %v", p)
  These error messages are not great. They dump the Prog but don't indicate which field is wrong or what its correct value might be.
- Patch Set #4, Line 2319: c.ctxt.Diag("shift amount out of range 0 to 31: %v", p)
  Ditto. One would expect, given the text of the message, to see an integer printed, not a struct.

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

Cherry Zhang (Gerrit)

unread,

Mar 19, 2018, 7:57:11 PM3/19/18

to fannie zhang, goph...@pubsubhelper.golang.org, Rob Pike, Gobot Gobot, Brad Fitzpatrick, Chad Rosier, Geoff Berry, Wei Xiao, golang-co...@googlegroups.com

View Change

1 comment:

File src/cmd/internal/obj/arm64/asm7.go:
- Patch Set #4, Line 2319: c.ctxt.Diag("shift amount out of range 0 to 31: %v", p)

- Ditto. One would expect, given the text of the message, to see an integer printed, not a struct.

- The Prog is printed in text form, for example,
  shift amount out of range 0 to 31: 00000 (a.s:5) ADDW R0<<52, R1, R5
  Do you think this is ok? Or should we print
  shift amount out of range 0 to 31: 52

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

Rob Pike (Gerrit)

unread,

Mar 19, 2018, 8:33:58 PM3/19/18

to fannie zhang, Cherry Zhang, goph...@pubsubhelper.golang.org, Rob Pike, Gobot Gobot, Brad Fitzpatrick, Chad Rosier, Geoff Berry, Wei Xiao, golang-co...@googlegroups.com

You're right, it's fine as is. I had forgotten that Progs had a pretty printer.
Apologies for the distraction.

View Change

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

Cherry Zhang (Gerrit)

unread,

Mar 19, 2018, 11:11:49 PM3/19/18

to fannie zhang, goph...@pubsubhelper.golang.org, Rob Pike, Gobot Gobot, Brad Fitzpatrick, Chad Rosier, Geoff Berry, Wei Xiao, golang-co...@googlegroups.com

Patch Set 4:

You're right, it's fine as is. I had forgotten that Progs had a pretty printer.
Apologies for the distraction.

No problem. Thank you.

View Change

To view, visit change 100255. To unsubscribe, or for help writing mail filters, visit settings.

Reply all

Reply to author

Forward