Skip to content

Commit

Permalink
[release-branch.go1.17] cmd/compile: ensure constant shift amounts ar…
Browse files Browse the repository at this point in the history 
…e in range for arm

Ensure constant shift amounts are in the range [0-31]. When shift amounts
are out of range, bad things happen. Shift amounts out of range occur
when lowering 64-bit shifts (we take an in-range shift s in [0-63] and
calculate s-32 and 32-s, both of which might be out of [0-31]).

The constant shift operations themselves still work, but their shift
amounts get copied unmolested to operations like ORshiftLL which use only
the low 5 bits. That changes an operation like <<100 which unconditionally
produces 0, to <<4, which doesn't.

Fixes #48479

Change-Id: I87363ef2b4ceaf3b2e316426064626efdfbb8ee3
Reviewed-on: https://go-review.googlesource.com/c/go/+/350969
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
(cherry picked from commit eff27e8)
Reviewed-on: https://go-review.googlesource.com/c/go/+/351069
Reviewed-by: Austin Clements <austin@google.com>
  • Loading branch information
@randall77 @dmitshur
randall77 authored and dmitshur committed Oct 27, 2021
1 parent 644555d commit 3a03ddf
Show file tree
Hide file tree
Showing 5 changed files with 318 additions and 68 deletions.
114 changes: 57 additions & 57 deletions src/cmd/compile/internal/ssa/gen/ARM.rules
View file
Expand Up @@ -497,9 +497,9 @@
(XOR x (MOVWconst [c])) => (XORconst [c] x)
(BIC x (MOVWconst [c])) => (BICconst [c] x)

(SLL x (MOVWconst [c])) => (SLLconst x [c&31]) // Note: I don't think we ever generate bad constant shifts (i.e. c>=32)
(SRL x (MOVWconst [c])) => (SRLconst x [c&31])
(SRA x (MOVWconst [c])) => (SRAconst x [c&31])
(SLL x (MOVWconst [c])) && 0 <= c && c < 32 => (SLLconst x [c])
(SRL x (MOVWconst [c])) && 0 <= c && c < 32 => (SRLconst x [c])
(SRA x (MOVWconst [c])) && 0 <= c && c < 32 => (SRAconst x [c])

(CMP x (MOVWconst [c])) => (CMPconst [c] x)
(CMP (MOVWconst [c]) x) => (InvertFlags (CMPconst [c] x))
Expand Down Expand Up @@ -1072,60 +1072,60 @@
(CMNshiftRL x (MOVWconst [c]) [d]) => (CMNconst x [int32(uint32(c)>>uint64(d))])
(CMNshiftRA x (MOVWconst [c]) [d]) => (CMNconst x [c>>uint64(d)])

(ADDshiftLLreg x y (MOVWconst [c])) => (ADDshiftLL x y [c])
(ADDshiftRLreg x y (MOVWconst [c])) => (ADDshiftRL x y [c])
(ADDshiftRAreg x y (MOVWconst [c])) => (ADDshiftRA x y [c])
(ADCshiftLLreg x y (MOVWconst [c]) flags) => (ADCshiftLL x y [c] flags)
(ADCshiftRLreg x y (MOVWconst [c]) flags) => (ADCshiftRL x y [c] flags)
(ADCshiftRAreg x y (MOVWconst [c]) flags) => (ADCshiftRA x y [c] flags)
(ADDSshiftLLreg x y (MOVWconst [c])) => (ADDSshiftLL x y [c])
(ADDSshiftRLreg x y (MOVWconst [c])) => (ADDSshiftRL x y [c])
(ADDSshiftRAreg x y (MOVWconst [c])) => (ADDSshiftRA x y [c])
(SUBshiftLLreg x y (MOVWconst [c])) => (SUBshiftLL x y [c])
(SUBshiftRLreg x y (MOVWconst [c])) => (SUBshiftRL x y [c])
(SUBshiftRAreg x y (MOVWconst [c])) => (SUBshiftRA x y [c])
(SBCshiftLLreg x y (MOVWconst [c]) flags) => (SBCshiftLL x y [c] flags)
(SBCshiftRLreg x y (MOVWconst [c]) flags) => (SBCshiftRL x y [c] flags)
(SBCshiftRAreg x y (MOVWconst [c]) flags) => (SBCshiftRA x y [c] flags)
(SUBSshiftLLreg x y (MOVWconst [c])) => (SUBSshiftLL x y [c])
(SUBSshiftRLreg x y (MOVWconst [c])) => (SUBSshiftRL x y [c])
(SUBSshiftRAreg x y (MOVWconst [c])) => (SUBSshiftRA x y [c])
(RSBshiftLLreg x y (MOVWconst [c])) => (RSBshiftLL x y [c])
(RSBshiftRLreg x y (MOVWconst [c])) => (RSBshiftRL x y [c])
(RSBshiftRAreg x y (MOVWconst [c])) => (RSBshiftRA x y [c])
(RSCshiftLLreg x y (MOVWconst [c]) flags) => (RSCshiftLL x y [c] flags)
(RSCshiftRLreg x y (MOVWconst [c]) flags) => (RSCshiftRL x y [c] flags)
(RSCshiftRAreg x y (MOVWconst [c]) flags) => (RSCshiftRA x y [c] flags)
(RSBSshiftLLreg x y (MOVWconst [c])) => (RSBSshiftLL x y [c])
(RSBSshiftRLreg x y (MOVWconst [c])) => (RSBSshiftRL x y [c])
(RSBSshiftRAreg x y (MOVWconst [c])) => (RSBSshiftRA x y [c])
(ANDshiftLLreg x y (MOVWconst [c])) => (ANDshiftLL x y [c])
(ANDshiftRLreg x y (MOVWconst [c])) => (ANDshiftRL x y [c])
(ANDshiftRAreg x y (MOVWconst [c])) => (ANDshiftRA x y [c])
(ORshiftLLreg x y (MOVWconst [c])) => (ORshiftLL x y [c])
(ORshiftRLreg x y (MOVWconst [c])) => (ORshiftRL x y [c])
(ORshiftRAreg x y (MOVWconst [c])) => (ORshiftRA x y [c])
(XORshiftLLreg x y (MOVWconst [c])) => (XORshiftLL x y [c])
(XORshiftRLreg x y (MOVWconst [c])) => (XORshiftRL x y [c])
(XORshiftRAreg x y (MOVWconst [c])) => (XORshiftRA x y [c])
(BICshiftLLreg x y (MOVWconst [c])) => (BICshiftLL x y [c])
(BICshiftRLreg x y (MOVWconst [c])) => (BICshiftRL x y [c])
(BICshiftRAreg x y (MOVWconst [c])) => (BICshiftRA x y [c])
(MVNshiftLLreg x (MOVWconst [c])) => (MVNshiftLL x [c])
(MVNshiftRLreg x (MOVWconst [c])) => (MVNshiftRL x [c])
(MVNshiftRAreg x (MOVWconst [c])) => (MVNshiftRA x [c])
(CMPshiftLLreg x y (MOVWconst [c])) => (CMPshiftLL x y [c])
(CMPshiftRLreg x y (MOVWconst [c])) => (CMPshiftRL x y [c])
(CMPshiftRAreg x y (MOVWconst [c])) => (CMPshiftRA x y [c])
(TSTshiftLLreg x y (MOVWconst [c])) => (TSTshiftLL x y [c])
(TSTshiftRLreg x y (MOVWconst [c])) => (TSTshiftRL x y [c])
(TSTshiftRAreg x y (MOVWconst [c])) => (TSTshiftRA x y [c])
(TEQshiftLLreg x y (MOVWconst [c])) => (TEQshiftLL x y [c])
(TEQshiftRLreg x y (MOVWconst [c])) => (TEQshiftRL x y [c])
(TEQshiftRAreg x y (MOVWconst [c])) => (TEQshiftRA x y [c])
(CMNshiftLLreg x y (MOVWconst [c])) => (CMNshiftLL x y [c])
(CMNshiftRLreg x y (MOVWconst [c])) => (CMNshiftRL x y [c])
(CMNshiftRAreg x y (MOVWconst [c])) => (CMNshiftRA x y [c])
(ADDshiftLLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (ADDshiftLL x y [c])
(ADDshiftRLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (ADDshiftRL x y [c])
(ADDshiftRAreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (ADDshiftRA x y [c])
(ADCshiftLLreg x y (MOVWconst [c]) flags) && 0 <= c && c < 32 => (ADCshiftLL x y [c] flags)
(ADCshiftRLreg x y (MOVWconst [c]) flags) && 0 <= c && c < 32 => (ADCshiftRL x y [c] flags)
(ADCshiftRAreg x y (MOVWconst [c]) flags) && 0 <= c && c < 32 => (ADCshiftRA x y [c] flags)
(ADDSshiftLLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (ADDSshiftLL x y [c])
(ADDSshiftRLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (ADDSshiftRL x y [c])
(ADDSshiftRAreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (ADDSshiftRA x y [c])
(SUBshiftLLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (SUBshiftLL x y [c])
(SUBshiftRLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (SUBshiftRL x y [c])
(SUBshiftRAreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (SUBshiftRA x y [c])
(SBCshiftLLreg x y (MOVWconst [c]) flags) && 0 <= c && c < 32 => (SBCshiftLL x y [c] flags)
(SBCshiftRLreg x y (MOVWconst [c]) flags) && 0 <= c && c < 32 => (SBCshiftRL x y [c] flags)
(SBCshiftRAreg x y (MOVWconst [c]) flags) && 0 <= c && c < 32 => (SBCshiftRA x y [c] flags)
(SUBSshiftLLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (SUBSshiftLL x y [c])
(SUBSshiftRLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (SUBSshiftRL x y [c])
(SUBSshiftRAreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (SUBSshiftRA x y [c])
(RSBshiftLLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (RSBshiftLL x y [c])
(RSBshiftRLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (RSBshiftRL x y [c])
(RSBshiftRAreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (RSBshiftRA x y [c])
(RSCshiftLLreg x y (MOVWconst [c]) flags) && 0 <= c && c < 32 => (RSCshiftLL x y [c] flags)
(RSCshiftRLreg x y (MOVWconst [c]) flags) && 0 <= c && c < 32 => (RSCshiftRL x y [c] flags)
(RSCshiftRAreg x y (MOVWconst [c]) flags) && 0 <= c && c < 32 => (RSCshiftRA x y [c] flags)
(RSBSshiftLLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (RSBSshiftLL x y [c])
(RSBSshiftRLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (RSBSshiftRL x y [c])
(RSBSshiftRAreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (RSBSshiftRA x y [c])
(ANDshiftLLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (ANDshiftLL x y [c])
(ANDshiftRLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (ANDshiftRL x y [c])
(ANDshiftRAreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (ANDshiftRA x y [c])
(ORshiftLLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (ORshiftLL x y [c])
(ORshiftRLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (ORshiftRL x y [c])
(ORshiftRAreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (ORshiftRA x y [c])
(XORshiftLLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (XORshiftLL x y [c])
(XORshiftRLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (XORshiftRL x y [c])
(XORshiftRAreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (XORshiftRA x y [c])
(BICshiftLLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (BICshiftLL x y [c])
(BICshiftRLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (BICshiftRL x y [c])
(BICshiftRAreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (BICshiftRA x y [c])
(MVNshiftLLreg x (MOVWconst [c])) && 0 <= c && c < 32 => (MVNshiftLL x [c])
(MVNshiftRLreg x (MOVWconst [c])) && 0 <= c && c < 32 => (MVNshiftRL x [c])
(MVNshiftRAreg x (MOVWconst [c])) && 0 <= c && c < 32 => (MVNshiftRA x [c])
(CMPshiftLLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (CMPshiftLL x y [c])
(CMPshiftRLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (CMPshiftRL x y [c])
(CMPshiftRAreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (CMPshiftRA x y [c])
(TSTshiftLLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (TSTshiftLL x y [c])
(TSTshiftRLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (TSTshiftRL x y [c])
(TSTshiftRAreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (TSTshiftRA x y [c])
(TEQshiftLLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (TEQshiftLL x y [c])
(TEQshiftRLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (TEQshiftRL x y [c])
(TEQshiftRAreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (TEQshiftRA x y [c])
(CMNshiftLLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (CMNshiftLL x y [c])
(CMNshiftRLreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (CMNshiftRL x y [c])
(CMNshiftRAreg x y (MOVWconst [c])) && 0 <= c && c < 32 => (CMNshiftRA x y [c])

// Generate rotates
(ADDshiftLL [c] (SRLconst x [32-c]) x) => (SRRconst [32-c] x)
Expand Down
9 changes: 5 additions & 4 deletions src/cmd/compile/internal/ssa/gen/ARMOps.go
View file
Expand Up @@ -228,14 +228,15 @@ func init() {

// shifts
{name: "SLL", argLength: 2, reg: gp21, asm: "SLL"}, // arg0 << arg1, shift amount is mod 256
{name: "SLLconst", argLength: 1, reg: gp11, asm: "SLL", aux: "Int32"}, // arg0 << auxInt
{name: "SLLconst", argLength: 1, reg: gp11, asm: "SLL", aux: "Int32"}, // arg0 << auxInt, 0 <= auxInt < 32
{name: "SRL", argLength: 2, reg: gp21, asm: "SRL"}, // arg0 >> arg1, unsigned, shift amount is mod 256
{name: "SRLconst", argLength: 1, reg: gp11, asm: "SRL", aux: "Int32"}, // arg0 >> auxInt, unsigned
{name: "SRLconst", argLength: 1, reg: gp11, asm: "SRL", aux: "Int32"}, // arg0 >> auxInt, unsigned, 0 <= auxInt < 32
{name: "SRA", argLength: 2, reg: gp21, asm: "SRA"}, // arg0 >> arg1, signed, shift amount is mod 256
{name: "SRAconst", argLength: 1, reg: gp11, asm: "SRA", aux: "Int32"}, // arg0 >> auxInt, signed
{name: "SRAconst", argLength: 1, reg: gp11, asm: "SRA", aux: "Int32"}, // arg0 >> auxInt, signed, 0 <= auxInt < 32
{name: "SRR", argLength: 2, reg: gp21}, // arg0 right rotate by arg1 bits
{name: "SRRconst", argLength: 1, reg: gp11, aux: "Int32"}, // arg0 right rotate by auxInt bits
{name: "SRRconst", argLength: 1, reg: gp11, aux: "Int32"}, // arg0 right rotate by auxInt bits, 0 <= auxInt < 32

// auxInt for all of these satisfy 0 <= auxInt < 32
{name: "ADDshiftLL", argLength: 2, reg: gp21, asm: "ADD", aux: "Int32"}, // arg0 + arg1<<auxInt
{name: "ADDshiftRL", argLength: 2, reg: gp21, asm: "ADD", aux: "Int32"}, // arg0 + arg1>>auxInt, unsigned shift
{name: "ADDshiftRA", argLength: 2, reg: gp21, asm: "ADD", aux: "Int32"}, // arg0 + arg1>>auxInt, signed shift
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/ssa/gen/genericOps.go
View file
Expand Up @@ -106,7 +106,7 @@ var genericOps = []opData{

// For shifts, AxB means the shifted value has A bits and the shift amount has B bits.
// Shift amounts are considered unsigned.
// If arg1 is known to be less than the number of bits in arg0,
// If arg1 is known to be nonnegative and less than the number of bits in arg0,
// then auxInt may be set to 1.
// This enables better code generation on some platforms.
{name: "Lsh8x8", argLength: 2, aux: "Bool"}, // arg0 << arg1
Expand Down

0 comments on commit 3a03ddf

Please sign in to comment.