From aef24d8f7db4fb895055e4543af958d7dc2eb8cc Mon Sep 17 00:00:00 2001 From: eric fang Date: Mon, 21 Jun 2021 03:04:42 +0000 Subject: [PATCH] cmd/internal/obj/arm64: fix the encoding error when operating with ZR Some arm64 instructions accept ZR as its destination register, such as MOVD, AND, ADD etc. although it doesn't seem to make much sense, but we should make sure the encoding is correct. However there exists some encoding mistakes in the current assembler, they are: 1, 'MOVD $1, ZR' is incorrectly encoded as 'MOVD $1, ZR' + '0x00000000'. 2, 'AND $1, R2, ZR' is incorrectly encoded as 'MOVD $1, R27' + 'AND R27, R2, ZR' + '0x00000000'. 3, 'AND $1, ZR' is incorrectly encoded as 'AND $1, ZR, RSP'. Obviously the first two encoding errors can cause SIGILL, and the third one will rewrite RSP. At the same time, I found some weird encodings but they don't cause errors. 4, 'MOVD $0x0001000100010001, ZR' is encoded as 'MOVW $1, ZR' + 'MOVKW $(1<<16), ZR'. 5, 'AND $0x0001000100010001, R2, ZR' is encoded as 'MOVD $1, R27' + 'MOVK $(1<<16), R27' + 'MOVK $(1<<32), R27'. Some of these issues also apply to 32-bit versions of these instructions. These problems are not very complicated, and are basically caused by the improper adaptation of the class of the constant to the entry in the optab. But the relationship between these constant classes is a bit complicated, so I don't know how to deal with issue 4 and 5, because they won't cause errors, so this CL didn't deal with them. This CL fixed the first three issues. Issue 1: before: 'MOVD $1, ZR' => 'MOVD $1, ZR' + '0x00000000'. after: 'MOVD $1, ZR' => 'MOVD $1, ZR'. Issue 2: before: 'AND $1, R2, ZR' => 'MOVD $1, R27' + 'AND R27, R2, ZR' + '0x00000000'. after: 'AND $1, R2, ZR' => 'ORR $1, ZR, R27' + 'AND R27, R2, ZR'. Issue 3: before: 'AND $1, ZR' => 'AND $1, ZR, RSP'. after: 'AND $1, ZR' => 'ORR $1, ZR, R27' + 'AND R27, ZR, ZR'. Change-Id: I3c889079229f847b863ad56c88966be12d947202 Reviewed-on: https://go-review.googlesource.com/c/go/+/329750 Reviewed-by: eric fang Reviewed-by: Cherry Mui Trust: eric fang Run-TryBot: eric fang TryBot-Result: Go Bot --- src/cmd/asm/internal/asm/testdata/arm64.s | 6 ++++-- .../asm/internal/asm/testdata/arm64error.s | 2 +- src/cmd/internal/obj/arm64/asm7.go | 21 ++++++++++++++----- src/cmd/internal/obj/arm64/obj7.go | 4 +++- 4 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/cmd/asm/internal/asm/testdata/arm64.s b/src/cmd/asm/internal/asm/testdata/arm64.s index d8a20edfc1..7b40ed24b4 100644 --- a/src/cmd/asm/internal/asm/testdata/arm64.s +++ b/src/cmd/asm/internal/asm/testdata/arm64.s @@ -334,6 +334,8 @@ TEXT foo(SB), DUPOK|NOSPLIT, $-8 EONW $0x6006000060060, R5 // EONW $1689262177517664, R5 // 1b0c8052db00a072a5003b4a ORNW $0x6006000060060, R5 // ORNW $1689262177517664, R5 // 1b0c8052db00a072a5003b2a BICSW $0x6006000060060, R5 // BICSW $1689262177517664, R5 // 1b0c8052db00a072a5003b6a + AND $1, ZR // fb0340b2ff031b8a + ANDW $1, ZR // fb030032ff031b0a // TODO: this could have better encoding ANDW $-1, R10 // 1b0080124a011b0a AND $8, R0, RSP // 1f007d92 @@ -369,9 +371,9 @@ TEXT foo(SB), DUPOK|NOSPLIT, $-8 MOVD $-1, R1 // 01008092 MOVD $0x210000, R0 // MOVD $2162688, R0 // 2004a0d2 MOVD $0xffffffffffffaaaa, R1 // MOVD $-21846, R1 // a1aa8a92 - MOVW $1, ZR + MOVW $1, ZR // 3f008052 MOVW $1, R1 - MOVD $1, ZR + MOVD $1, ZR // 3f0080d2 MOVD $1, R1 MOVK $1, R1 MOVD $0x1000100010001000, RSP // MOVD $1152939097061330944, RSP // ff8304b2 diff --git a/src/cmd/asm/internal/asm/testdata/arm64error.s b/src/cmd/asm/internal/asm/testdata/arm64error.s index cf57179e43..145074347f 100644 --- a/src/cmd/asm/internal/asm/testdata/arm64error.s +++ b/src/cmd/asm/internal/asm/testdata/arm64error.s @@ -3,7 +3,7 @@ // license that can be found in the LICENSE file. TEXT errors(SB),$0 - AND $1, RSP // ERROR "illegal combination" + AND $1, RSP // ERROR "illegal source register" ANDS $1, R0, RSP // ERROR "illegal combination" ADDSW R7->32, R14, R13 // ERROR "shift amount out of range 0 to 31" ADD R1.UXTB<<5, R2, R3 // ERROR "shift amount out of range 0 to 4" diff --git a/src/cmd/internal/obj/arm64/asm7.go b/src/cmd/internal/obj/arm64/asm7.go index d99afa3d27..02687ab162 100644 --- a/src/cmd/internal/obj/arm64/asm7.go +++ b/src/cmd/internal/obj/arm64/asm7.go @@ -361,12 +361,12 @@ var optab = []Optab{ {AANDS, C_REG, C_NONE, C_NONE, C_REG, 1, 4, 0, 0, 0}, {ATST, C_REG, C_REG, C_NONE, C_NONE, 1, 4, 0, 0, 0}, {AAND, C_MBCON, C_REG, C_NONE, C_RSP, 53, 4, 0, 0, 0}, - {AAND, C_MBCON, C_NONE, C_NONE, C_REG, 53, 4, 0, 0, 0}, + {AAND, C_MBCON, C_NONE, C_NONE, C_RSP, 53, 4, 0, 0, 0}, {AANDS, C_MBCON, C_REG, C_NONE, C_REG, 53, 4, 0, 0, 0}, {AANDS, C_MBCON, C_NONE, C_NONE, C_REG, 53, 4, 0, 0, 0}, {ATST, C_MBCON, C_REG, C_NONE, C_NONE, 53, 4, 0, 0, 0}, {AAND, C_BITCON, C_REG, C_NONE, C_RSP, 53, 4, 0, 0, 0}, - {AAND, C_BITCON, C_NONE, C_NONE, C_REG, 53, 4, 0, 0, 0}, + {AAND, C_BITCON, C_NONE, C_NONE, C_RSP, 53, 4, 0, 0, 0}, {AANDS, C_BITCON, C_REG, C_NONE, C_REG, 53, 4, 0, 0, 0}, {AANDS, C_BITCON, C_NONE, C_NONE, C_REG, 53, 4, 0, 0, 0}, {ATST, C_BITCON, C_REG, C_NONE, C_NONE, 53, 4, 0, 0, 0}, @@ -404,6 +404,8 @@ var optab = []Optab{ /* TODO: MVN C_SHIFT */ /* MOVs that become MOVK/MOVN/MOVZ/ADD/SUB/OR */ + {AMOVW, C_MBCON, C_NONE, C_NONE, C_REG, 32, 4, 0, 0, 0}, + {AMOVD, C_MBCON, C_NONE, C_NONE, C_REG, 32, 4, 0, 0, 0}, {AMOVW, C_MOVCON, C_NONE, C_NONE, C_REG, 32, 4, 0, 0, 0}, {AMOVD, C_MOVCON, C_NONE, C_NONE, C_REG, 32, 4, 0, 0, 0}, {AMOVW, C_BITCON, C_NONE, C_NONE, C_RSP, 32, 4, 0, 0, 0}, @@ -2089,13 +2091,18 @@ func cmp(a int, b int) bool { return true } + case C_MBCON: + if b == C_ABCON0 { + return true + } + case C_BITCON: if b == C_ABCON0 || b == C_ABCON || b == C_MBCON { return true } case C_MOVCON: - if b == C_MBCON || b == C_ZCON || b == C_ADDCON0 || b == C_AMCON { + if b == C_MBCON || b == C_ZCON || b == C_ADDCON0 || b == C_ABCON0 || b == C_AMCON { return true } @@ -4198,6 +4205,10 @@ func (c *ctxt7) asmout(p *obj.Prog, o *Optab, out []uint32) { if r == 0 { r = rt } + if r == REG_RSP { + c.ctxt.Diag("illegal source register: %v", p) + break + } mode := 64 v := uint64(p.From.Offset) switch p.As { @@ -7039,8 +7050,8 @@ func (c *ctxt7) omovlit(as obj.As, p *obj.Prog, a *obj.Addr, dr int) uint32 { // load a constant (MOVCON or BITCON) in a into rt func (c *ctxt7) omovconst(as obj.As, p *obj.Prog, a *obj.Addr, rt int) (o1 uint32) { - if cls := oclass(a); cls == C_BITCON || cls == C_ABCON || cls == C_ABCON0 { - // or $bitcon, REGZERO, rt + if cls := oclass(a); (cls == C_BITCON || cls == C_ABCON || cls == C_ABCON0) && rt != REGZERO { + // or $bitcon, REGZERO, rt. rt can't be ZR. mode := 64 var as1 obj.As switch as { diff --git a/src/cmd/internal/obj/arm64/obj7.go b/src/cmd/internal/obj/arm64/obj7.go index 31b7c43245..a043d0972c 100644 --- a/src/cmd/internal/obj/arm64/obj7.go +++ b/src/cmd/internal/obj/arm64/obj7.go @@ -305,7 +305,9 @@ func progedit(ctxt *obj.Link, p *obj.Prog, newprog obj.ProgAlloc) { // for both 32-bit and 64-bit. 32-bit ops will // zero the high 32-bit of the destination register // anyway. - if (isANDWop(p.As) || isADDWop(p.As) || p.As == AMOVW) && p.From.Type == obj.TYPE_CONST { + // For MOVW, the destination register can't be ZR, + // so don't bother rewriting it in this situation. + if (isANDWop(p.As) || isADDWop(p.As) || p.As == AMOVW && p.To.Reg != REGZERO) && p.From.Type == obj.TYPE_CONST { v := p.From.Offset & 0xffffffff p.From.Offset = v | v<<32 }