From 405866aaa3e8057218ad482ca60e284ea0c9e350 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 16 Nov 2019 11:44:32 +0100 Subject: [PATCH 1/5] re-add miri intrinsic ABI check --- src/librustc_mir/interpret/terminator.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 4f9e404b2c635..0134c77808b82 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -264,6 +264,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { match instance.def { ty::InstanceDef::Intrinsic(..) => { + if caller_abi != Abi::RustIntrinsic && caller_abi != Abi::PlatformIntrinsic { + throw_ub_format!("Rust intrinsic called with an ABI other than \ + `RustIntrinsic` and `PlatformIntrinsic`."); + } + let old_stack = self.cur_frame(); let old_bb = self.frame().block; M::call_intrinsic(self, span, instance, args, dest, ret, unwind)?; From 44b68116c522ad8870f0a8627550ba1f5c8fc797 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 16 Nov 2019 13:14:41 +0100 Subject: [PATCH 2/5] rename and move read_vector_ty --- src/librustc/ty/sty.rs | 18 ++++++++++++++---- src/librustc_mir/interpret/intrinsics.rs | 21 +++++++++++---------- src/librustc_mir/interpret/operand.rs | 11 ----------- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs index 8f6fc02ab4b37..d1d71a4287244 100644 --- a/src/librustc/ty/sty.rs +++ b/src/librustc/ty/sty.rs @@ -1814,20 +1814,30 @@ impl<'tcx> TyS<'tcx> { pub fn simd_type(&self, tcx: TyCtxt<'tcx>) -> Ty<'tcx> { match self.kind { - Adt(def, substs) => { - def.non_enum_variant().fields[0].ty(tcx, substs) - } + Adt(def, substs) => def.non_enum_variant().fields[0].ty(tcx, substs), _ => bug!("simd_type called on invalid type") } } - pub fn simd_size(&self, _cx: TyCtxt<'_>) -> usize { + pub fn simd_size(&self, _tcx: TyCtxt<'tcx>) -> usize { + // Parameter currently unused, but probably needed in the future to + // allow `#[repr(simd)] struct Simd([T; N]);`. match self.kind { Adt(def, _) => def.non_enum_variant().fields.len(), _ => bug!("simd_size called on invalid type") } } + pub fn simd_size_and_type(&self, tcx: TyCtxt<'tcx>) -> (usize, Ty<'tcx>) { + match self.kind { + Adt(def, substs) => { + let variant = def.non_enum_variant(); + (variant.fields.len(), variant.fields[0].ty(tcx, substs)) + } + _ => bug!("simd_size_and_type called on invalid type") + } + } + #[inline] pub fn is_region_ptr(&self) -> bool { match self.kind { diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index 6117cf4038a24..e43e6c0e43a80 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -302,10 +302,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.copy_op_transmute(args[0], dest)?; } "simd_insert" => { - let index = self.read_scalar(args[1])?.to_u32()? as u64; - let scalar = args[2]; + let index = u64::from(self.read_scalar(args[1])?.to_u32()?); + let elem = args[2]; let input = args[0]; - let (len, e_ty) = self.read_vector_ty(input); + let (len, e_ty) = input.layout.ty.simd_size_and_type(self.tcx.tcx); + let len = len as u64; assert!( index < len, "Index `{}` must be in bounds of vector type `{}`: `[0, {})`", @@ -317,15 +318,15 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { dest.layout.ty, input.layout.ty ); assert_eq!( - scalar.layout.ty, e_ty, - "Scalar type `{}` must match vector element type `{}`", - scalar.layout.ty, e_ty + elem.layout.ty, e_ty, + "Scalar element type `{}` must match vector element type `{}`", + elem.layout.ty, e_ty ); for i in 0..len { let place = self.place_field(dest, i)?; let value = if i == index { - scalar + elem } else { self.operand_field(input, i)? }; @@ -333,10 +334,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } "simd_extract" => { - let index = self.read_scalar(args[1])?.to_u32()? as _; - let (len, e_ty) = self.read_vector_ty(args[0]); + let index = u64::from(self.read_scalar(args[1])?.to_u32()?); + let (len, e_ty) = args[0].layout.ty.simd_size_and_type(self.tcx.tcx); assert!( - index < len, + index < len as u64, "index `{}` is out-of-bounds of vector type `{}` with length `{}`", index, e_ty, len ); diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 4d2ccdc20da65..970f76a9d6d7c 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -315,17 +315,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } - /// Read vector length and element type - pub fn read_vector_ty( - &self, op: OpTy<'tcx, M::PointerTag> - ) -> (u64, &rustc::ty::TyS<'tcx>) { - if let layout::Abi::Vector { .. } = op.layout.abi { - (op.layout.ty.simd_size(*self.tcx) as _, op.layout.ty.simd_type(*self.tcx)) - } else { - bug!("Type `{}` is not a SIMD vector type", op.layout.ty) - } - } - /// Read a scalar from a place pub fn read_scalar( &self, From 09180d71fd382c8d0471ff342147d91def3a1595 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 16 Nov 2019 13:31:09 +0100 Subject: [PATCH 3/5] make simd_size return a u64 --- src/librustc/ty/layout.rs | 2 +- src/librustc/ty/sty.rs | 8 ++++---- src/librustc_codegen_llvm/intrinsic.rs | 22 +++++++++++++--------- src/librustc_mir/interpret/intrinsics.rs | 3 +-- 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index 972452601ddd5..b9fc5f59b7bbc 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -697,7 +697,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { // SIMD vector types. ty::Adt(def, ..) if def.repr.simd() => { let element = self.layout_of(ty.simd_type(tcx))?; - let count = ty.simd_size(tcx) as u64; + let count = ty.simd_size(tcx); assert!(count > 0); let scalar = match element.abi { Abi::Scalar(ref scalar) => scalar.clone(), diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs index d1d71a4287244..b7e645d55a5fc 100644 --- a/src/librustc/ty/sty.rs +++ b/src/librustc/ty/sty.rs @@ -1819,20 +1819,20 @@ impl<'tcx> TyS<'tcx> { } } - pub fn simd_size(&self, _tcx: TyCtxt<'tcx>) -> usize { + pub fn simd_size(&self, _tcx: TyCtxt<'tcx>) -> u64 { // Parameter currently unused, but probably needed in the future to // allow `#[repr(simd)] struct Simd([T; N]);`. match self.kind { - Adt(def, _) => def.non_enum_variant().fields.len(), + Adt(def, _) => def.non_enum_variant().fields.len() as u64, _ => bug!("simd_size called on invalid type") } } - pub fn simd_size_and_type(&self, tcx: TyCtxt<'tcx>) -> (usize, Ty<'tcx>) { + pub fn simd_size_and_type(&self, tcx: TyCtxt<'tcx>) -> (u64, Ty<'tcx>) { match self.kind { Adt(def, substs) => { let variant = def.non_enum_variant(); - (variant.fields.len(), variant.fields[0].ty(tcx, substs)) + (variant.fields.len() as u64, variant.fields[0].ty(tcx, substs)) } _ => bug!("simd_size_and_type called on invalid type") } diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index e1ce7f622e2ef..fb5f457bb3a1c 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -28,6 +28,7 @@ use syntax_pos::Span; use std::cmp::Ordering; use std::{iter, i128, u128}; +use std::convert::TryFrom; fn get_simple_intrinsic(cx: &CodegenCx<'ll, '_>, name: &str) -> Option<&'ll Value> { let llvm_name = match name { @@ -1105,8 +1106,8 @@ fn generic_simd_intrinsic( let m_len = match in_ty.kind { // Note that this `.unwrap()` crashes for isize/usize, that's sort // of intentional as there's not currently a use case for that. - ty::Int(i) => i.bit_width().unwrap(), - ty::Uint(i) => i.bit_width().unwrap(), + ty::Int(i) => i.bit_width().unwrap() as u64, + ty::Uint(i) => i.bit_width().unwrap() as u64, _ => return_error!("`{}` is not an integral type", in_ty), }; require_simd!(arg_tys[1], "argument"); @@ -1116,7 +1117,7 @@ fn generic_simd_intrinsic( m_len, v_len ); let i1 = bx.type_i1(); - let i1xn = bx.type_vector(i1, m_len as u64); + let i1xn = bx.type_vector(i1, m_len); let m_i1s = bx.bitcast(args[0].immediate(), i1xn); return Ok(bx.select(m_i1s, args[1].immediate(), args[2].immediate())); } @@ -1166,7 +1167,7 @@ fn generic_simd_intrinsic( require_simd!(ret_ty, "return"); let out_len = ret_ty.simd_size(tcx); - require!(out_len == n, + require!(out_len == n as u64, "expected return type of length {}, found `{}` with length {}", n, ret_ty, out_len); require!(in_elem == ret_ty.simd_type(tcx), @@ -1251,7 +1252,7 @@ fn generic_simd_intrinsic( // trailing bits. let expected_int_bits = in_len.max(8); match ret_ty.kind { - ty::Uint(i) if i.bit_width() == Some(expected_int_bits) => (), + ty::Uint(i) if i.bit_width() == Some(expected_int_bits as usize) => (), _ => return_error!( "bitmask `{}`, expected `u{}`", ret_ty, expected_int_bits @@ -1276,7 +1277,8 @@ fn generic_simd_intrinsic( // Shift the MSB to the right by "in_elem_bitwidth - 1" into the first bit position. let shift_indices = vec![ - bx.cx.const_int(bx.type_ix(in_elem_bitwidth as _), (in_elem_bitwidth - 1) as _); in_len + bx.cx.const_int(bx.type_ix(in_elem_bitwidth as _), (in_elem_bitwidth - 1) as _); + in_len as _ ]; let i_xn_msb = bx.lshr(i_xn, bx.const_vector(shift_indices.as_slice())); // Truncate vector to an @@ -1291,7 +1293,7 @@ fn generic_simd_intrinsic( name: &str, in_elem: &::rustc::ty::TyS<'_>, in_ty: &::rustc::ty::TyS<'_>, - in_len: usize, + in_len: u64, bx: &mut Builder<'a, 'll, 'tcx>, span: Span, args: &[OperandRef<'tcx, &'ll Value>], @@ -1506,11 +1508,12 @@ fn generic_simd_intrinsic( // Truncate the mask vector to a vector of i1s: let (mask, mask_ty) = { let i1 = bx.type_i1(); - let i1xn = bx.type_vector(i1, in_len as u64); + let i1xn = bx.type_vector(i1, in_len); (bx.trunc(args[2].immediate(), i1xn), i1xn) }; // Type of the vector of pointers: + let in_len = usize::try_from(in_len).unwrap(); let llvm_pointer_vec_ty = llvm_vector_ty(bx, underlying_ty, in_len, pointer_count); let llvm_pointer_vec_str = llvm_vector_str(underlying_ty, in_len, pointer_count); @@ -1606,13 +1609,14 @@ fn generic_simd_intrinsic( // Truncate the mask vector to a vector of i1s: let (mask, mask_ty) = { let i1 = bx.type_i1(); - let i1xn = bx.type_vector(i1, in_len as u64); + let i1xn = bx.type_vector(i1, in_len); (bx.trunc(args[2].immediate(), i1xn), i1xn) }; let ret_t = bx.type_void(); // Type of the vector of pointers: + let in_len = usize::try_from(in_len).unwrap(); let llvm_pointer_vec_ty = llvm_vector_ty(bx, underlying_ty, in_len, pointer_count); let llvm_pointer_vec_str = llvm_vector_str(underlying_ty, in_len, pointer_count); diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index e43e6c0e43a80..23f7b1acb54d4 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -306,7 +306,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let elem = args[2]; let input = args[0]; let (len, e_ty) = input.layout.ty.simd_size_and_type(self.tcx.tcx); - let len = len as u64; assert!( index < len, "Index `{}` must be in bounds of vector type `{}`: `[0, {})`", @@ -337,7 +336,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let index = u64::from(self.read_scalar(args[1])?.to_u32()?); let (len, e_ty) = args[0].layout.ty.simd_size_and_type(self.tcx.tcx); assert!( - index < len as u64, + index < len, "index `{}` is out-of-bounds of vector type `{}` with length `{}`", index, e_ty, len ); From 8952c8aa42209919c2980e99f11694e36f2b6845 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 16 Nov 2019 14:10:07 +0100 Subject: [PATCH 4/5] ICE on invalid MIR --- src/librustc_mir/interpret/terminator.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 0134c77808b82..50c4a249c63c2 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -264,10 +264,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { match instance.def { ty::InstanceDef::Intrinsic(..) => { - if caller_abi != Abi::RustIntrinsic && caller_abi != Abi::PlatformIntrinsic { - throw_ub_format!("Rust intrinsic called with an ABI other than \ - `RustIntrinsic` and `PlatformIntrinsic`."); - } + assert!(caller_abi == Abi::RustIntrinsic || caller_abi == Abi::PlatformIntrinsic); let old_stack = self.cur_frame(); let old_bb = self.frame().block; From 5e115a25ca3799a9232f2c3712ed36626025c752 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 16 Nov 2019 16:09:45 +0100 Subject: [PATCH 5/5] avoid some casts --- src/librustc_codegen_llvm/intrinsic.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index fb5f457bb3a1c..4277ce1d1f754 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -28,7 +28,6 @@ use syntax_pos::Span; use std::cmp::Ordering; use std::{iter, i128, u128}; -use std::convert::TryFrom; fn get_simple_intrinsic(cx: &CodegenCx<'ll, '_>, name: &str) -> Option<&'ll Value> { let llvm_name = match name { @@ -1161,13 +1160,13 @@ fn generic_simd_intrinsic( } if name.starts_with("simd_shuffle") { - let n: usize = name["simd_shuffle".len()..].parse().unwrap_or_else(|_| + let n: u64 = name["simd_shuffle".len()..].parse().unwrap_or_else(|_| span_bug!(span, "bad `simd_shuffle` instruction only caught in codegen?")); require_simd!(ret_ty, "return"); let out_len = ret_ty.simd_size(tcx); - require!(out_len == n as u64, + require!(out_len == n, "expected return type of length {}, found `{}` with length {}", n, ret_ty, out_len); require!(in_elem == ret_ty.simd_type(tcx), @@ -1176,7 +1175,7 @@ fn generic_simd_intrinsic( in_elem, in_ty, ret_ty, ret_ty.simd_type(tcx)); - let total_len = in_len as u128 * 2; + let total_len = u128::from(in_len) * 2; let vector = args[2].immediate(); @@ -1402,7 +1401,7 @@ fn generic_simd_intrinsic( // FIXME: use: // https://github.com/llvm-mirror/llvm/blob/master/include/llvm/IR/Function.h#L182 // https://github.com/llvm-mirror/llvm/blob/master/include/llvm/IR/Intrinsics.h#L81 - fn llvm_vector_str(elem_ty: Ty<'_>, vec_len: usize, no_pointers: usize) -> String { + fn llvm_vector_str(elem_ty: Ty<'_>, vec_len: u64, no_pointers: usize) -> String { let p0s: String = "p0".repeat(no_pointers); match elem_ty.kind { ty::Int(v) => format!("v{}{}i{}", vec_len, p0s, v.bit_width().unwrap()), @@ -1412,7 +1411,7 @@ fn generic_simd_intrinsic( } } - fn llvm_vector_ty(cx: &CodegenCx<'ll, '_>, elem_ty: Ty<'_>, vec_len: usize, + fn llvm_vector_ty(cx: &CodegenCx<'ll, '_>, elem_ty: Ty<'_>, vec_len: u64, mut no_pointers: usize) -> &'ll Type { // FIXME: use cx.layout_of(ty).llvm_type() ? let mut elem_ty = match elem_ty.kind { @@ -1425,7 +1424,7 @@ fn generic_simd_intrinsic( elem_ty = cx.type_ptr_to(elem_ty); no_pointers -= 1; } - cx.type_vector(elem_ty, vec_len as u64) + cx.type_vector(elem_ty, vec_len) } @@ -1513,7 +1512,6 @@ fn generic_simd_intrinsic( }; // Type of the vector of pointers: - let in_len = usize::try_from(in_len).unwrap(); let llvm_pointer_vec_ty = llvm_vector_ty(bx, underlying_ty, in_len, pointer_count); let llvm_pointer_vec_str = llvm_vector_str(underlying_ty, in_len, pointer_count); @@ -1616,7 +1614,6 @@ fn generic_simd_intrinsic( let ret_t = bx.type_void(); // Type of the vector of pointers: - let in_len = usize::try_from(in_len).unwrap(); let llvm_pointer_vec_ty = llvm_vector_ty(bx, underlying_ty, in_len, pointer_count); let llvm_pointer_vec_str = llvm_vector_str(underlying_ty, in_len, pointer_count);