Merge pull request #3620 from cfallin/release-0.32.1

Patch release 0.32.1.

RELEASES.md

@@ -2,6 +2,25 @@
 
 --------------------------------------------------------------------------------
 
+## 0.32.1
+
+Released 2021-12-17.
+
+### Fixed
+
+* Cranelift: remove recently-added build dependency on `sha2` to allow usage in
+ some dependency-sensitive environments, by computing ISLE manifest hashes
+ with a different hash function.
+ [#3619](https://github.com/bytecodealliance/wasmtime/pull/3619)
+
+* Cranelift: fixed 8- and 16-bit behavior of popcount (bit population count)
+ instruction. Does not affect Wasm frontend.
+ [#3617](https://github.com/bytecodealliance/wasmtime/pull/3617)
+
+* Cranelift: fixed miscompilation of 8- and 16-bit bit-rotate instructions.
+ Does not affect Wasm frontend.
+ [#3610](https://github.com/bytecodealliance/wasmtime/pull/3610)
+
 ## 0.32.0
 
 Released 2021-12-13.

cranelift/codegen/Cargo.toml

@@ -37,7 +37,6 @@ criterion = "0.3"
 cranelift-codegen-meta = { path = "meta", version = "0.79.0" }
 cranelift-isle = { path = "../isle/isle", version = "=0.79.0", optional = true }
 miette = { version = "3", features = ["fancy"], optional = true }
-sha2 = "0.9.8"
 
 [features]
 default = ["std", "unwind"]

cranelift/codegen/build.rs

@@ -16,7 +16,6 @@
 
 use cranelift_codegen_meta as meta;
 
-use sha2::{Digest, Sha512};
 use std::env;
 use std::io::Read;
 use std::process;
@@ -163,7 +162,28 @@ impl IsleCompilation {
  /// `<generated_filename>.manifest` and use it to verify that a
  /// rebuild was done if necessary.
  fn compute_manifest(&self) -> Result<String, Box<dyn std::error::Error + 'static>> {
+ // We use the deprecated SipHasher from std::hash in order to verify
+ // that ISLE sources haven't changed since the generated source was
+ // last regenerated.
+ //
+ // We use this instead of a stronger and more usual content hash, like
+ // SHA-{160,256,512}, because it's built into the standard library and
+ // we don't want to pull in a separate crate. We try to keep Cranelift
+ // crate dependencies as intentionally small as possible. In fact, we
+ // used to use the `sha2` crate for SHA-512 and this turns out to be
+ // undesirable for downstream consumers (see #3609).
+ //
+ // Why not the recommended replacement
+ // `std::collections::hash_map::DefaultHasher`? Because we need the
+ // hash to be deterministic, both between runs (i.e., not seeded with
+ // random state) and across Rust versions.
+ //
+ // If `SipHasher` is ever actually removed from `std`, we'll need to
+ // find a new option, either a very small crate or something else
+ // that's built-in.
+ #![allow(deprecated)]
  use std::fmt::Write;
+ use std::hash::{Hasher, SipHasher};
 
  let mut manifest = String::new();
 
@@ -176,11 +196,11 @@ impl IsleCompilation {
  // to `\r\n`; canonicalize the source that we hash to
  // Unix-style (`\n`) so hashes will match.
  let content = content.replace("\r\n", "\n");
- // One line in the manifest: <filename> <sha-512 hash>.
- let mut hasher = Sha512::default();
- hasher.update(content.as_bytes());
+ // One line in the manifest: <filename> <siphash>.
+ let mut hasher = SipHasher::new_with_keys(0, 0); // fixed keys for determinism
+ hasher.write(content.as_bytes());
  let filename = format!("{}", filename.display()).replace("\\", "/");
- writeln!(&mut manifest, "{} {:x}", filename, hasher.finalize())?;
+ writeln!(&mut manifest, "{} {:x}", filename, hasher.finish())?;
  }
 
  Ok(manifest)

cranelift/codegen/src/isa/aarch64/lower/isle/generated_code.manifest

@@ -1,4 +1,4 @@
-src/clif.isle be1359b4b6b153f378517c1dd95cd80f4a6bed0c7b86eaba11c088fd71b7bfe80a3c868ace245b2da0bfbbd6ded262ea9576c8e0eeacbf89d03c34a17a709602
-src/prelude.isle 9bd1fcb6a3604a24cf2e05e6b7eb04dcb3b9dc8fa9a2f1c8f29c25b6e3bf7f679b3b1b72dff87501497b72bc30fc92fd755b898db7e03f380235fae931b6a74b
-src/isa/aarch64/inst.isle 6e042ec14166fceae4b7133f681fdf604e20a2997e1d60f797e40acd683ccb34e33376189f6b7ed2f5eb441dc61d592cad2592256dfea51296330752181b9403
-src/isa/aarch64/lower.isle 64a725771537f69c445f44c728e04bffd8a715d6a4d87a5a2bf2e89714ee290b7497c5ca8b335bdddd775f6734be03318ff9aa67e2e4068949ebae06b0902b3f
+src/clif.isle f176ef3bba99365
+src/prelude.isle 53cfd4f9ccf7c050
+src/isa/aarch64/inst.isle 18c4784ff8c6f057
+src/isa/aarch64/lower.isle 85f6b03feadeb795

cranelift/codegen/src/isa/x64/inst.isle

@@ -77,9 +77,13 @@
  Size32
  Size64))
 
-;; Get the `OperandSize` for a given `Type`.
-(decl operand_size_of_type (Type) OperandSize)
-(extern constructor operand_size_of_type operand_size_of_type)
+;; Get the `OperandSize` for a given `Type`, rounding smaller types up to 32 bits.
+(decl operand_size_of_type_32_64 (Type) OperandSize)
+(extern constructor operand_size_of_type_32_64 operand_size_of_type_32_64)
+
+;; Get the true `OperandSize` for a given `Type`, with no rounding.
+(decl raw_operand_size_of_type (Type) OperandSize)
+(extern constructor raw_operand_size_of_type raw_operand_size_of_type)
 
 ;; Get the bit width of an `OperandSize`.
 (decl operand_size_bits (OperandSize) u16)
@@ -379,6 +383,10 @@
 (decl imm8_from_value (Imm8Reg) Value)
 (extern extractor imm8_from_value imm8_from_value)
 
+;; Mask an `Imm8Reg.Imm8`.
+(decl mask_imm8_const (Imm8Reg u64) Imm8Reg)
+(extern constructor mask_imm8_const mask_imm8_const)
+
 ;; Extract a constant `RegMemImm.Imm` from a value operand.
 (decl simm32_from_value (RegMemImm) Value)
 (extern extractor simm32_from_value simm32_from_value)
@@ -426,7 +434,7 @@
  (let ((from_bits u16 (ty_bits_u16 from_ty))
  ;; Use `operand_size_of_type` so that the we clamp the output to 32-
  ;; or 64-bit width types.
- (to_bits u16 (operand_size_bits (operand_size_of_type to_ty))))
+ (to_bits u16 (operand_size_bits (operand_size_of_type_32_64 to_ty))))
  (extend kind
  to_ty
  (ext_mode from_bits to_bits)
@@ -503,7 +511,7 @@
 (decl alu_rmi_r (Type AluRmiROpcode Reg RegMemImm) Reg)
 (rule (alu_rmi_r ty opcode src1 src2)
  (let ((dst WritableReg (temp_writable_reg ty))
- (size OperandSize (operand_size_of_type ty))
+ (size OperandSize (operand_size_of_type_32_64 ty))
  (_ Unit (emit (MInst.AluRmiR size opcode src1 src2 dst))))
  (writable_reg_to_reg dst)))
 
@@ -519,7 +527,7 @@
 (decl add_with_flags (Type Reg RegMemImm) ProducesFlags)
 (rule (add_with_flags ty src1 src2)
  (let ((dst WritableReg (temp_writable_reg ty)))
- (ProducesFlags.ProducesFlags (MInst.AluRmiR (operand_size_of_type ty)
+ (ProducesFlags.ProducesFlags (MInst.AluRmiR (operand_size_of_type_32_64 ty)
  (AluRmiROpcode.Add)
  src1
  src2
@@ -530,7 +538,7 @@
 (decl adc (Type Reg RegMemImm) ConsumesFlags)
 (rule (adc ty src1 src2)
  (let ((dst WritableReg (temp_writable_reg ty)))
- (ConsumesFlags.ConsumesFlags (MInst.AluRmiR (operand_size_of_type ty)
+ (ConsumesFlags.ConsumesFlags (MInst.AluRmiR (operand_size_of_type_32_64 ty)
  (AluRmiROpcode.Adc)
  src1
  src2
@@ -549,7 +557,7 @@
 (decl sub_with_flags (Type Reg RegMemImm) ProducesFlags)
 (rule (sub_with_flags ty src1 src2)
  (let ((dst WritableReg (temp_writable_reg ty)))
- (ProducesFlags.ProducesFlags (MInst.AluRmiR (operand_size_of_type ty)
+ (ProducesFlags.ProducesFlags (MInst.AluRmiR (operand_size_of_type_32_64 ty)
  (AluRmiROpcode.Sub)
  src1
  src2
@@ -560,7 +568,7 @@
 (decl sbb (Type Reg RegMemImm) ConsumesFlags)
 (rule (sbb ty src1 src2)
  (let ((dst WritableReg (temp_writable_reg ty)))
- (ConsumesFlags.ConsumesFlags (MInst.AluRmiR (operand_size_of_type ty)
+ (ConsumesFlags.ConsumesFlags (MInst.AluRmiR (operand_size_of_type_32_64 ty)
  (AluRmiROpcode.Sbb)
  src1
  src2
@@ -608,7 +616,7 @@
 ;; Integer immediates.
 (rule (imm ty simm64)
  (let ((dst WritableReg (temp_writable_reg ty))
- (size OperandSize (operand_size_of_type ty))
+ (size OperandSize (operand_size_of_type_32_64 ty))
  (_ Unit (emit (MInst.Imm size simm64 dst))))
  (writable_reg_to_reg dst)))
 
@@ -634,7 +642,7 @@
 (rule (imm ty 0)
  (let ((wr WritableReg (temp_writable_reg ty))
  (r Reg (writable_reg_to_reg wr))
- (size OperandSize (operand_size_of_type ty))
+ (size OperandSize (operand_size_of_type_32_64 ty))
  (_ Unit (emit (MInst.AluRmiR size
  (AluRmiROpcode.Xor)
  r
@@ -681,7 +689,9 @@
 (decl shift_r (Type ShiftKind Reg Imm8Reg) Reg)
 (rule (shift_r ty kind src1 src2)
  (let ((dst WritableReg (temp_writable_reg ty))
- (size OperandSize (operand_size_of_type ty))
+ ;; Use actual 8/16-bit instructions when appropriate: we
+ ;; rely on their shift-amount-masking semantics.
+ (size OperandSize (raw_operand_size_of_type ty))
  (_ Unit (emit (MInst.ShiftR size kind src1 src2 dst))))
  (writable_reg_to_reg dst)))
 
@@ -729,7 +739,7 @@
 (decl cmove (Type CC RegMem Reg) ConsumesFlags)
 (rule (cmove ty cc consequent alternative)
  (let ((dst WritableReg (temp_writable_reg ty))
- (size OperandSize (operand_size_of_type ty)))
+ (size OperandSize (operand_size_of_type_32_64 ty)))
  (ConsumesFlags.ConsumesFlags (MInst.Cmove size cc consequent alternative dst)
  (writable_reg_to_reg dst))))
 
@@ -1155,7 +1165,7 @@
 (rule (mul_hi ty signed src1 src2)
  (let ((dst_lo WritableReg (temp_writable_reg ty))
  (dst_hi WritableReg (temp_writable_reg ty))
- (size OperandSize (operand_size_of_type ty))
+ (size OperandSize (operand_size_of_type_32_64 ty))
  (_ Unit (emit (MInst.MulHi size
  signed
  src1
@@ -1224,6 +1234,6 @@
 (decl not (Type Reg) Reg)
 (rule (not ty src)
  (let ((dst WritableReg (temp_writable_reg ty))
- (size OperandSize (operand_size_of_type ty))
+ (size OperandSize (operand_size_of_type_32_64 ty))
  (_ Unit (emit (MInst.Not size src dst))))
  (writable_reg_to_reg dst)))

cranelift/codegen/src/isa/x64/lower.isle

@@ -633,15 +633,26 @@
 
 ;;;; Rules for `rotl` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
-;; `i64` and smaller.
+;; `i16` and `i8`: we need to extend the shift amount, or mask the
+;; constant.
+
+(rule (lower (has_type (ty_8_or_16 ty) (rotl src amt)))
+ (let ((amt_ Reg (extend_to_reg amt $I32 (ExtendKind.Zero))))
+ (value_reg (m_rotl ty (put_in_reg src) (Imm8Reg.Reg amt_)))))
+
+(rule (lower (has_type (ty_8_or_16 ty) (rotl src (imm8_from_value amt))))
+ (value_reg (m_rotl ty (put_in_reg src) (mask_imm8_const amt (ty_bits_mask ty)))))
+
+;; `i64` and `i32`: we can rely on x86's rotate-amount masking since
+;; we operate on the whole register.
 
-(rule (lower (has_type (fits_in_64 ty) (rotl src amt)))
+(rule (lower (has_type (ty_32_or_64 ty) (rotl src amt)))
  ;; NB: Only the low bits of `amt` matter since we logically mask the
  ;; shift amount to the value's bit width.
  (let ((amt_ Reg (lo_reg amt)))
  (value_reg (m_rotl ty (put_in_reg src) (Imm8Reg.Reg amt_)))))
 
-(rule (lower (has_type (fits_in_64 ty) (rotl src (imm8_from_value amt))))
+(rule (lower (has_type (ty_32_or_64 ty) (rotl src (imm8_from_value amt))))
  (value_reg (m_rotl ty (put_in_reg src) amt)))
 
 ;; `i128`.

cranelift/codegen/src/isa/x64/lower.rs

@@ -2236,14 +2236,25 @@ fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(
  Opcode::Popcnt => {
  let ty_tmp = ty.unwrap();
  if !ty_tmp.is_vector() {
- let (ext_spec, ty) = match ctx.input_ty(insn, 0) {
- types::I8 | types::I16 => (Some(ExtSpec::ZeroExtendTo32), types::I32),
- a if a == types::I32 || a == types::I64 || a == types::I128 => (None, a),
- _ => unreachable!(),
- };
+ let ty = ctx.input_ty(insn, 0);
 
  if isa_flags.use_popcnt() {
  match ty {
+ types::I8 | types::I16 => {
+ let src = RegMem::reg(extend_input_to_reg(
+ ctx,
+ inputs[0],
+ ExtSpec::ZeroExtendTo32,
+ ));
+ let dst = get_output_reg(ctx, outputs[0]).only_reg().unwrap();
+ ctx.emit(Inst::unary_rm_r(
+ OperandSize::from_ty(types::I32),
+ UnaryRmROpcode::Popcnt,
+ src,
+ dst,
+ ));
+ return Ok(());
+ }
  types::I32 | types::I64 => {
  let src = input_to_reg_mem(ctx, inputs[0]);
  let dst = get_output_reg(ctx, outputs[0]).only_reg().unwrap();
@@ -2299,6 +2310,12 @@ fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(
  }
  }
 
+ let (ext_spec, ty) = match ty {
+ types::I8 | types::I16 => (Some(ExtSpec::ZeroExtendTo32), types::I32),
+ a if a == types::I32 || a == types::I64 || a == types::I128 => (None, a),
+ _ => unreachable!(),
+ };
+
  let (srcs, ty): (SmallVec<[RegMem; 2]>, Type) = if let Some(ext_spec) = ext_spec {
  (
  smallvec![RegMem::reg(extend_input_to_reg(ctx, inputs[0], ext_spec))],