From 9bd9c628aad9d76b1a88c48861c14501d9b36754 Mon Sep 17 00:00:00 2001
From: Chris Fallin <cfallin@mozilla.com>
Date: Fri, 17 Jul 2020 14:51:33 -0700
Subject: [PATCH] Aarch64: mask shift-amounts incorporated into reg-reg-shift
 ALU insts.

We had previously fixed a bug in which constant shift amounts should be
masked to modulo the number of bits in the operand; however, we did not
fix the analogous case for shifts incorporated into the second register
argument of ALU instructions that support integrated shifts.  This
failure to mask resulted in illegal instructions being generated, e.g.
in https://bugzilla.mozilla.org/show_bug.cgi?id=1653502. This PR fixes
the issue by masking the amount, as the shift semantics require.
---
 cranelift/codegen/src/isa/aarch64/inst/args.rs    |  5 +++++
 cranelift/codegen/src/isa/aarch64/lower.rs        |  8 ++++++--
 .../filetests/vcode/aarch64/arithmetic.clif       | 15 +++++++++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/cranelift/codegen/src/isa/aarch64/inst/args.rs b/cranelift/codegen/src/isa/aarch64/inst/args.rs
index 43e8471ac7..43dcc816e5 100644
--- a/cranelift/codegen/src/isa/aarch64/inst/args.rs
+++ b/cranelift/codegen/src/isa/aarch64/inst/args.rs
@@ -52,6 +52,11 @@ impl ShiftOpShiftImm {
     pub fn value(self) -> u8 {
         self.0
     }
+
+    /// Mask down to a given number of bits.
+    pub fn mask(self, bits: u8) -> ShiftOpShiftImm {
+        ShiftOpShiftImm(self.0 & (bits - 1))
+    }
 }
 
 /// A shift operator with an amount, guaranteed to be within range.
diff --git a/cranelift/codegen/src/isa/aarch64/lower.rs b/cranelift/codegen/src/isa/aarch64/lower.rs
index 2f440f8415..39b4b1c0be 100644
--- a/cranelift/codegen/src/isa/aarch64/lower.rs
+++ b/cranelift/codegen/src/isa/aarch64/lower.rs
@@ -321,8 +321,12 @@ fn put_input_in_rs<C: LowerCtx<I = Inst>>(
 
             // Can we get the shift amount as an immediate?
             if let Some(shiftimm) = input_to_shiftimm(ctx, shift_amt) {
-                let reg = put_input_in_reg(ctx, shiftee, narrow_mode);
-                return ResultRS::RegShift(reg, ShiftOpAndAmt::new(ShiftOp::LSL, shiftimm));
+                let shiftee_bits = ty_bits(ctx.input_ty(insn, 0));
+                if shiftee_bits <= u8::MAX as usize {
+                    let shiftimm = shiftimm.mask(shiftee_bits as u8);
+                    let reg = put_input_in_reg(ctx, shiftee, narrow_mode);
+                    return ResultRS::RegShift(reg, ShiftOpAndAmt::new(ShiftOp::LSL, shiftimm));
+                }
             }
         }
     }
diff --git a/cranelift/filetests/filetests/vcode/aarch64/arithmetic.clif b/cranelift/filetests/filetests/vcode/aarch64/arithmetic.clif
index 9fe09c2166..0a33eeaa47 100644
--- a/cranelift/filetests/filetests/vcode/aarch64/arithmetic.clif
+++ b/cranelift/filetests/filetests/vcode/aarch64/arithmetic.clif
@@ -365,3 +365,18 @@ block0(v0: i64, v1: i64):
 ; nextln:  mov sp, fp
 ; nextln:  ldp fp, lr, [sp], #16
 ; nextln:  ret
+
+function %f25(i32, i32) -> i32 {
+block0(v0: i32, v1: i32):
+  v2 = iconst.i32 53
+  v3 = ishl.i32 v0, v2
+  v4 = isub.i32 v1, v3
+  return v4
+}
+
+; check:  stp fp, lr, [sp, #-16]!
+; nextln:  mov fp, sp
+; nextln:  sub w0, w1, w0, LSL 21
+; nextln:  mov sp, fp
+; nextln:  ldp fp, lr, [sp], #16
+; nextln:  ret