From 5a501f73ff753109c7e73d4981fe011633bd8e84 Mon Sep 17 00:00:00 2001 From: Andreas Liljeqvist Date: Sun, 22 Aug 2021 21:46:03 +0200 Subject: [PATCH 01/12] Use custom wrap-around type instead of Range --- compiler/rustc_codegen_llvm/src/builder.rs | 4 +- compiler/rustc_codegen_llvm/src/consts.rs | 9 ++- .../src/debuginfo/type_names.rs | 8 +- compiler/rustc_codegen_ssa/src/mir/rvalue.rs | 8 +- compiler/rustc_lint/src/types.rs | 2 +- compiler/rustc_middle/src/ty/layout.rs | 43 +++++++---- compiler/rustc_mir/src/interpret/validity.rs | 27 ++----- compiler/rustc_target/src/abi/mod.rs | 73 ++++++++++-------- .../consts/const-eval/ub-nonnull.64bit.stderr | 2 +- src/test/ui/layout/debug.stderr | 45 ++++++++--- src/test/ui/layout/hexagon-enum.stderr | 75 +++++++++++++++---- src/test/ui/layout/thumb-enum.stderr | 75 +++++++++++++++---- 12 files changed, 255 insertions(+), 116 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index 2139f9776b736..2b72b16742630 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -462,7 +462,7 @@ impl BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { load: &'ll Value, scalar: &abi::Scalar, ) { - let vr = scalar.valid_range.clone(); + let vr = scalar.valid_range; match scalar.value { abi::Int(..) => { let range = scalar.valid_range_exclusive(bx); @@ -470,7 +470,7 @@ impl BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { bx.range_metadata(load, range); } } - abi::Pointer if vr.start() < vr.end() && !vr.contains(&0) => { + abi::Pointer if vr.start < vr.end && !vr.contains(0) => { bx.nonnull_metadata(load); } _ => {} diff --git a/compiler/rustc_codegen_llvm/src/consts.rs b/compiler/rustc_codegen_llvm/src/consts.rs index e1baf95e1d9e5..01a93dd8857c2 100644 --- a/compiler/rustc_codegen_llvm/src/consts.rs +++ b/compiler/rustc_codegen_llvm/src/consts.rs @@ -16,7 +16,9 @@ use rustc_middle::mir::interpret::{ use rustc_middle::mir::mono::MonoItem; use rustc_middle::ty::{self, Instance, Ty}; use rustc_middle::{bug, span_bug}; -use rustc_target::abi::{AddressSpace, Align, HasDataLayout, LayoutOf, Primitive, Scalar, Size}; +use rustc_target::abi::{ + AddressSpace, Align, AllocationRange, HasDataLayout, LayoutOf, Primitive, Scalar, Size, +}; use tracing::debug; pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll Value { @@ -59,7 +61,10 @@ pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll Pointer::new(alloc_id, Size::from_bytes(ptr_offset)), &cx.tcx, ), - &Scalar { value: Primitive::Pointer, valid_range: 0..=!0 }, + &Scalar { + value: Primitive::Pointer, + valid_range: AllocationRange { start: 0, end: !0 }, + }, cx.type_i8p_ext(address_space), )); next_offset = offset + pointer_size; diff --git a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs index 81e905b1b5f57..f0b32c96309d6 100644 --- a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs +++ b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs @@ -406,11 +406,11 @@ fn push_debuginfo_type_name<'tcx>( let dataful_discriminant_range = &dataful_variant_layout.largest_niche.as_ref().unwrap().scalar.valid_range; - let min = dataful_discriminant_range.start(); - let min = tag.value.size(&tcx).truncate(*min); + let min = dataful_discriminant_range.start; + let min = tag.value.size(&tcx).truncate(min); - let max = dataful_discriminant_range.end(); - let max = tag.value.size(&tcx).truncate(*max); + let max = dataful_discriminant_range.end; + let max = tag.value.size(&tcx).truncate(max); let dataful_variant_name = def.variants[*dataful_variant].ident.as_str(); diff --git a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs index 7e432d2740224..90a29f24b8e0a 100644 --- a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs +++ b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs @@ -310,15 +310,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let er = scalar.valid_range_exclusive(bx.cx()); if er.end != er.start - && scalar.valid_range.end() >= scalar.valid_range.start() + && scalar.valid_range.end >= scalar.valid_range.start { // We want `table[e as usize ± k]` to not // have bound checks, and this is the most // convenient place to put the `assume`s. - if *scalar.valid_range.start() > 0 { + if scalar.valid_range.start > 0 { let enum_value_lower_bound = bx .cx() - .const_uint_big(ll_t_in, *scalar.valid_range.start()); + .const_uint_big(ll_t_in, scalar.valid_range.start); let cmp_start = bx.icmp( IntPredicate::IntUGE, llval, @@ -328,7 +328,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } let enum_value_upper_bound = - bx.cx().const_uint_big(ll_t_in, *scalar.valid_range.end()); + bx.cx().const_uint_big(ll_t_in, scalar.valid_range.end); let cmp_end = bx.icmp( IntPredicate::IntULE, llval, diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 34d342e66945e..82a23da3ed9ca 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -797,7 +797,7 @@ crate fn repr_nullable_ptr<'tcx>( // Return the nullable type this Option-like enum can be safely represented with. let field_ty_abi = &cx.layout_of(field_ty).unwrap().abi; if let Abi::Scalar(field_ty_scalar) = field_ty_abi { - match (field_ty_scalar.valid_range.start(), field_ty_scalar.valid_range.end()) { + match (field_ty_scalar.valid_range.start, field_ty_scalar.valid_range.end) { (0, _) => unreachable!("Non-null optimisation extended to a non-zero value."), (1, _) => { return Some(get_nullable_type(cx, field_ty).unwrap()); diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 3caca313ffddd..3b1990a3e675a 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -499,7 +499,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let scalar_unit = |value: Primitive| { let bits = value.size(dl).bits(); assert!(bits <= 128); - Scalar { value, valid_range: 0..=(!0 >> (128 - bits)) } + Scalar { value, valid_range: AllocationRange { start: 0, end: (!0 >> (128 - bits)) } } }; let scalar = |value: Primitive| tcx.intern_layout(Layout::scalar(self, scalar_unit(value))); @@ -512,11 +512,14 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { // Basic scalars. ty::Bool => tcx.intern_layout(Layout::scalar( self, - Scalar { value: Int(I8, false), valid_range: 0..=1 }, + Scalar { value: Int(I8, false), valid_range: AllocationRange { start: 0, end: 1 } }, )), ty::Char => tcx.intern_layout(Layout::scalar( self, - Scalar { value: Int(I32, false), valid_range: 0..=0x10FFFF }, + Scalar { + value: Int(I32, false), + valid_range: AllocationRange { start: 0, end: 0x10FFFF }, + }, )), ty::Int(ity) => scalar(Int(Integer::from_int_ty(dl, ity), true)), ty::Uint(ity) => scalar(Int(Integer::from_uint_ty(dl, ity), false)), @@ -526,7 +529,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { }), ty::FnPtr(_) => { let mut ptr = scalar_unit(Pointer); - ptr.valid_range = 1..=*ptr.valid_range.end(); + ptr.valid_range = AllocationRange { start: 1, end: ptr.valid_range.end }; tcx.intern_layout(Layout::scalar(self, ptr)) } @@ -544,7 +547,8 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { ty::Ref(_, pointee, _) | ty::RawPtr(ty::TypeAndMut { ty: pointee, .. }) => { let mut data_ptr = scalar_unit(Pointer); if !ty.is_unsafe_ptr() { - data_ptr.valid_range = 1..=*data_ptr.valid_range.end(); + data_ptr.valid_range = + AllocationRange { start: 1, end: data_ptr.valid_range.end }; } let pointee = tcx.normalize_erasing_regions(param_env, pointee); @@ -560,7 +564,8 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { ty::Slice(_) | ty::Str => scalar_unit(Int(dl.ptr_sized_integer(), false)), ty::Dynamic(..) => { let mut vtable = scalar_unit(Pointer); - vtable.valid_range = 1..=*vtable.valid_range.end(); + vtable.valid_range = + AllocationRange { start: 1, end: vtable.valid_range.end }; vtable } _ => return Err(LayoutError::Unknown(unsized_part)), @@ -933,14 +938,18 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { if let Bound::Included(start) = start { // FIXME(eddyb) this might be incorrect - it doesn't // account for wrap-around (end < start) ranges. - assert!(*scalar.valid_range.start() <= start); - scalar.valid_range = start..=*scalar.valid_range.end(); + assert!(scalar.valid_range.start <= start); + // scalar.valid_range = + // AllocationRange { start, end: scalar.valid_range.end }; + scalar.valid_range.start = start; } if let Bound::Included(end) = end { // FIXME(eddyb) this might be incorrect - it doesn't // account for wrap-around (end < start) ranges. - assert!(*scalar.valid_range.end() >= end); - scalar.valid_range = *scalar.valid_range.start()..=end; + assert!(scalar.valid_range.end >= end); + // scalar.valid_range = + // AllocationRange { start: scalar.valid_range.start, end }; + scalar.valid_range.end = end; } // Update `largest_niche` if we have introduced a larger niche. @@ -1256,7 +1265,10 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let tag_mask = !0u128 >> (128 - ity.size().bits()); let tag = Scalar { value: Int(ity, signed), - valid_range: (min as u128 & tag_mask)..=(max as u128 & tag_mask), + valid_range: AllocationRange { + start: (min as u128 & tag_mask), + end: (max as u128 & tag_mask), + }, }; let mut abi = Abi::Aggregate { sized: true }; if tag.value.size(dl) == size { @@ -1535,7 +1547,10 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let max_discr = (info.variant_fields.len() - 1) as u128; let discr_int = Integer::fit_unsigned(max_discr); let discr_int_ty = discr_int.to_ty(tcx, false); - let tag = Scalar { value: Primitive::Int(discr_int, false), valid_range: 0..=max_discr }; + let tag = Scalar { + value: Primitive::Int(discr_int, false), + valid_range: AllocationRange { start: 0, end: max_discr }, + }; let tag_layout = self.tcx.intern_layout(Layout::scalar(self, tag.clone())); let tag_layout = TyAndLayout { ty: discr_int_ty, layout: tag_layout }; @@ -2846,8 +2861,8 @@ where return; } - if scalar.valid_range.start() < scalar.valid_range.end() { - if *scalar.valid_range.start() > 0 { + if scalar.valid_range.start < scalar.valid_range.end { + if scalar.valid_range.start > 0 { attrs.set(ArgAttribute::NonNull); } } diff --git a/compiler/rustc_mir/src/interpret/validity.rs b/compiler/rustc_mir/src/interpret/validity.rs index 0c7f89c1a36ba..71e616ff560c0 100644 --- a/compiler/rustc_mir/src/interpret/validity.rs +++ b/compiler/rustc_mir/src/interpret/validity.rs @@ -7,7 +7,6 @@ use std::convert::TryFrom; use std::fmt::Write; use std::num::NonZeroUsize; -use std::ops::RangeInclusive; use rustc_data_structures::fx::FxHashSet; use rustc_hir as hir; @@ -15,7 +14,9 @@ use rustc_middle::mir::interpret::InterpError; use rustc_middle::ty; use rustc_middle::ty::layout::TyAndLayout; use rustc_span::symbol::{sym, Symbol}; -use rustc_target::abi::{Abi, LayoutOf, Scalar as ScalarAbi, Size, VariantIdx, Variants}; +use rustc_target::abi::{ + Abi, AllocationRange, LayoutOf, Scalar as ScalarAbi, Size, VariantIdx, Variants, +}; use std::hash::Hash; @@ -181,22 +182,10 @@ fn write_path(out: &mut String, path: &[PathElem]) { } } -// Test if a range that wraps at overflow contains `test` -fn wrapping_range_contains(r: &RangeInclusive, test: u128) -> bool { - let (lo, hi) = r.clone().into_inner(); - if lo > hi { - // Wrapped - (..=hi).contains(&test) || (lo..).contains(&test) - } else { - // Normal - r.contains(&test) - } -} - // Formats such that a sentence like "expected something {}" to mean // "expected something " makes sense. -fn wrapping_range_format(r: &RangeInclusive, max_hi: u128) -> String { - let (lo, hi) = r.clone().into_inner(); +fn wrapping_range_format(r: AllocationRange, max_hi: u128) -> String { + let AllocationRange { start: lo, end: hi } = r; assert!(hi <= max_hi); if lo > hi { format!("less or equal to {}, or greater or equal to {}", hi, lo) @@ -634,8 +623,8 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' scalar_layout: &ScalarAbi, ) -> InterpResult<'tcx> { let value = self.read_scalar(op)?; - let valid_range = &scalar_layout.valid_range; - let (lo, hi) = valid_range.clone().into_inner(); + let valid_range = scalar_layout.valid_range; + let AllocationRange { start: lo, end: hi } = valid_range; // Determine the allowed range // `max_hi` is as big as the size fits let max_hi = u128::MAX >> (128 - op.layout.size.bits()); @@ -684,7 +673,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' Ok(int) => int.assert_bits(op.layout.size), }; // Now compare. This is slightly subtle because this is a special "wrap-around" range. - if wrapping_range_contains(&valid_range, bits) { + if valid_range.contains(bits) { Ok(()) } else { throw_validation_failure!(self.path, diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs index 8ef6e142caecf..07687a4b104cf 100644 --- a/compiler/rustc_target/src/abi/mod.rs +++ b/compiler/rustc_target/src/abi/mod.rs @@ -677,32 +677,52 @@ impl Primitive { } } +/// Inclusive wrap-around range of valid values, that is, if +/// start > end, it represents `start..=MAX`, +/// followed by `0..=end`. +/// +/// That is, for an i8 primitive, a range of `254..=2` means following +/// sequence: +/// +/// 254 (-2), 255 (-1), 0, 1, 2 +/// +/// This is intended specifically to mirror LLVM’s `!range` metadata, +/// semantics. +#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)] +#[derive(HashStable_Generic)] +pub struct AllocationRange { + pub start: u128, + pub end: u128, +} + +impl AllocationRange { + /// Returns `true` if `v` is contained in the range. + #[inline] + pub fn contains(&self, v: u128) -> bool { + if self.start <= self.end { + self.start <= v && v <= self.end + } else { + self.start <= v || v <= self.end + } + } +} + /// Information about one scalar component of a Rust type. #[derive(Clone, PartialEq, Eq, Hash, Debug)] #[derive(HashStable_Generic)] pub struct Scalar { pub value: Primitive, - /// Inclusive wrap-around range of valid values, that is, if - /// start > end, it represents `start..=MAX`, - /// followed by `0..=end`. - /// - /// That is, for an i8 primitive, a range of `254..=2` means following - /// sequence: - /// - /// 254 (-2), 255 (-1), 0, 1, 2 - /// - /// This is intended specifically to mirror LLVM’s `!range` metadata, - /// semantics. // FIXME(eddyb) always use the shortest range, e.g., by finding // the largest space between two consecutive valid values and // taking everything else as the (shortest) valid range. - pub valid_range: RangeInclusive, + pub valid_range: AllocationRange, } impl Scalar { pub fn is_bool(&self) -> bool { - matches!(self.value, Int(I8, false)) && self.valid_range == (0..=1) + matches!(self.value, Int(I8, false)) + && matches!(self.valid_range, AllocationRange { start: 0, end: 1 }) } /// Returns the valid range as a `x..y` range. @@ -715,8 +735,8 @@ impl Scalar { let bits = self.value.size(cx).bits(); assert!(bits <= 128); let mask = !0u128 >> (128 - bits); - let start = *self.valid_range.start(); - let end = *self.valid_range.end(); + let start = self.valid_range.start; + let end = self.valid_range.end; assert_eq!(start, start & mask); assert_eq!(end, end & mask); start..(end.wrapping_add(1) & mask) @@ -965,20 +985,20 @@ impl Niche { } pub fn available(&self, cx: &C) -> u128 { - let Scalar { value, valid_range: ref v } = self.scalar; + let Scalar { value, valid_range: v } = self.scalar; let bits = value.size(cx).bits(); assert!(bits <= 128); let max_value = !0u128 >> (128 - bits); // Find out how many values are outside the valid range. - let niche = v.end().wrapping_add(1)..*v.start(); + let niche = v.end.wrapping_add(1)..v.start; niche.end.wrapping_sub(niche.start) & max_value } pub fn reserve(&self, cx: &C, count: u128) -> Option<(u128, Scalar)> { assert!(count > 0); - let Scalar { value, valid_range: ref v } = self.scalar; + let Scalar { value, valid_range: v } = self.scalar; let bits = value.size(cx).bits(); assert!(bits <= 128); let max_value = !0u128 >> (128 - bits); @@ -988,24 +1008,17 @@ impl Niche { } // Compute the range of invalid values being reserved. - let start = v.end().wrapping_add(1) & max_value; - let end = v.end().wrapping_add(count) & max_value; + let start = v.end.wrapping_add(1) & max_value; + let end = v.end.wrapping_add(count) & max_value; // If the `end` of our range is inside the valid range, // then we ran out of invalid values. // FIXME(eddyb) abstract this with a wraparound range type. - let valid_range_contains = |x| { - if v.start() <= v.end() { - *v.start() <= x && x <= *v.end() - } else { - *v.start() <= x || x <= *v.end() - } - }; - if valid_range_contains(end) { + if v.contains(end) { return None; } - Some((start, Scalar { value, valid_range: *v.start()..=end })) + Some((start, Scalar { value, valid_range: AllocationRange { start: v.start, end } })) } } @@ -1214,7 +1227,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> { if zero { let range = &s.valid_range; // The range must contain 0. - range.contains(&0) || (*range.start() > *range.end()) // wrap-around allows 0 + range.contains(0) || (range.start > range.end) // wrap-around allows 0 } else { // The range must include all values. `valid_range_exclusive` handles // the wrap-around using target arithmetic; with wrap-around then the full diff --git a/src/test/ui/consts/const-eval/ub-nonnull.64bit.stderr b/src/test/ui/consts/const-eval/ub-nonnull.64bit.stderr index 1ce87bc7c1ce8..41de900f1640a 100644 --- a/src/test/ui/consts/const-eval/ub-nonnull.64bit.stderr +++ b/src/test/ui/consts/const-eval/ub-nonnull.64bit.stderr @@ -52,7 +52,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-nonnull.rs:41:1 | LL | const BAD_RANGE1: RestrictedRange1 = unsafe { RestrictedRange1(42) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 42, but expected something in the range 10..=30 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 42, but expected something in the range AllocationRange { start: 10, end: 30 } | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. = note: the raw bytes of the constant (size: 4, align: 4) { diff --git a/src/test/ui/layout/debug.stderr b/src/test/ui/layout/debug.stderr index 1a371c6b17000..cbe9b6400c35f 100644 --- a/src/test/ui/layout/debug.stderr +++ b/src/test/ui/layout/debug.stderr @@ -15,7 +15,10 @@ error: layout_of(E) = Layout { I32, false, ), - valid_range: 0..=0, + valid_range: AllocationRange { + start: 0, + end: 0, + }, }, tag_encoding: Direct, tag_field: 0, @@ -91,7 +94,10 @@ error: layout_of(E) = Layout { I32, false, ), - valid_range: 0..=0, + valid_range: AllocationRange { + start: 0, + end: 0, + }, }, }, ), @@ -138,14 +144,20 @@ error: layout_of(S) = Layout { I32, true, ), - valid_range: 0..=4294967295, + valid_range: AllocationRange { + start: 0, + end: 4294967295, + }, }, Scalar { value: Int( I32, true, ), - valid_range: 0..=4294967295, + valid_range: AllocationRange { + start: 0, + end: 4294967295, + }, }, ), largest_niche: None, @@ -207,7 +219,10 @@ error: layout_of(std::result::Result) = Layout { I32, false, ), - valid_range: 0..=1, + valid_range: AllocationRange { + start: 0, + end: 1, + }, }, tag_encoding: Direct, tag_field: 0, @@ -276,14 +291,20 @@ error: layout_of(std::result::Result) = Layout { I32, false, ), - valid_range: 0..=1, + valid_range: AllocationRange { + start: 0, + end: 1, + }, }, Scalar { value: Int( I32, true, ), - valid_range: 0..=4294967295, + valid_range: AllocationRange { + start: 0, + end: 4294967295, + }, }, ), largest_niche: Some( @@ -296,7 +317,10 @@ error: layout_of(std::result::Result) = Layout { I32, false, ), - valid_range: 0..=1, + valid_range: AllocationRange { + start: 0, + end: 1, + }, }, }, ), @@ -326,7 +350,10 @@ error: layout_of(i32) = Layout { I32, true, ), - valid_range: 0..=4294967295, + valid_range: AllocationRange { + start: 0, + end: 4294967295, + }, }, ), largest_niche: None, diff --git a/src/test/ui/layout/hexagon-enum.stderr b/src/test/ui/layout/hexagon-enum.stderr index d4676a5afb25e..520ba12c39fb1 100644 --- a/src/test/ui/layout/hexagon-enum.stderr +++ b/src/test/ui/layout/hexagon-enum.stderr @@ -15,7 +15,10 @@ error: layout_of(A) = Layout { I8, false, ), - valid_range: 0..=0, + valid_range: AllocationRange { + start: 0, + end: 0, + }, }, tag_encoding: Direct, tag_field: 0, @@ -52,7 +55,10 @@ error: layout_of(A) = Layout { I8, false, ), - valid_range: 0..=0, + valid_range: AllocationRange { + start: 0, + end: 0, + }, }, ), largest_niche: Some( @@ -65,7 +71,10 @@ error: layout_of(A) = Layout { I8, false, ), - valid_range: 0..=0, + valid_range: AllocationRange { + start: 0, + end: 0, + }, }, }, ), @@ -103,7 +112,10 @@ error: layout_of(B) = Layout { I8, false, ), - valid_range: 255..=255, + valid_range: AllocationRange { + start: 255, + end: 255, + }, }, tag_encoding: Direct, tag_field: 0, @@ -140,7 +152,10 @@ error: layout_of(B) = Layout { I8, false, ), - valid_range: 255..=255, + valid_range: AllocationRange { + start: 255, + end: 255, + }, }, ), largest_niche: Some( @@ -153,7 +168,10 @@ error: layout_of(B) = Layout { I8, false, ), - valid_range: 255..=255, + valid_range: AllocationRange { + start: 255, + end: 255, + }, }, }, ), @@ -191,7 +209,10 @@ error: layout_of(C) = Layout { I16, false, ), - valid_range: 256..=256, + valid_range: AllocationRange { + start: 256, + end: 256, + }, }, tag_encoding: Direct, tag_field: 0, @@ -228,7 +249,10 @@ error: layout_of(C) = Layout { I16, false, ), - valid_range: 256..=256, + valid_range: AllocationRange { + start: 256, + end: 256, + }, }, ), largest_niche: Some( @@ -241,7 +265,10 @@ error: layout_of(C) = Layout { I16, false, ), - valid_range: 256..=256, + valid_range: AllocationRange { + start: 256, + end: 256, + }, }, }, ), @@ -279,7 +306,10 @@ error: layout_of(P) = Layout { I32, false, ), - valid_range: 268435456..=268435456, + valid_range: AllocationRange { + start: 268435456, + end: 268435456, + }, }, tag_encoding: Direct, tag_field: 0, @@ -316,7 +346,10 @@ error: layout_of(P) = Layout { I32, false, ), - valid_range: 268435456..=268435456, + valid_range: AllocationRange { + start: 268435456, + end: 268435456, + }, }, ), largest_niche: Some( @@ -329,7 +362,10 @@ error: layout_of(P) = Layout { I32, false, ), - valid_range: 268435456..=268435456, + valid_range: AllocationRange { + start: 268435456, + end: 268435456, + }, }, }, ), @@ -367,7 +403,10 @@ error: layout_of(T) = Layout { I32, true, ), - valid_range: 2164260864..=2164260864, + valid_range: AllocationRange { + start: 2164260864, + end: 2164260864, + }, }, tag_encoding: Direct, tag_field: 0, @@ -404,7 +443,10 @@ error: layout_of(T) = Layout { I32, true, ), - valid_range: 2164260864..=2164260864, + valid_range: AllocationRange { + start: 2164260864, + end: 2164260864, + }, }, ), largest_niche: Some( @@ -417,7 +459,10 @@ error: layout_of(T) = Layout { I32, true, ), - valid_range: 2164260864..=2164260864, + valid_range: AllocationRange { + start: 2164260864, + end: 2164260864, + }, }, }, ), diff --git a/src/test/ui/layout/thumb-enum.stderr b/src/test/ui/layout/thumb-enum.stderr index 898a61b904db5..50e37c5411632 100644 --- a/src/test/ui/layout/thumb-enum.stderr +++ b/src/test/ui/layout/thumb-enum.stderr @@ -15,7 +15,10 @@ error: layout_of(A) = Layout { I8, false, ), - valid_range: 0..=0, + valid_range: AllocationRange { + start: 0, + end: 0, + }, }, tag_encoding: Direct, tag_field: 0, @@ -52,7 +55,10 @@ error: layout_of(A) = Layout { I8, false, ), - valid_range: 0..=0, + valid_range: AllocationRange { + start: 0, + end: 0, + }, }, ), largest_niche: Some( @@ -65,7 +71,10 @@ error: layout_of(A) = Layout { I8, false, ), - valid_range: 0..=0, + valid_range: AllocationRange { + start: 0, + end: 0, + }, }, }, ), @@ -103,7 +112,10 @@ error: layout_of(B) = Layout { I8, false, ), - valid_range: 255..=255, + valid_range: AllocationRange { + start: 255, + end: 255, + }, }, tag_encoding: Direct, tag_field: 0, @@ -140,7 +152,10 @@ error: layout_of(B) = Layout { I8, false, ), - valid_range: 255..=255, + valid_range: AllocationRange { + start: 255, + end: 255, + }, }, ), largest_niche: Some( @@ -153,7 +168,10 @@ error: layout_of(B) = Layout { I8, false, ), - valid_range: 255..=255, + valid_range: AllocationRange { + start: 255, + end: 255, + }, }, }, ), @@ -191,7 +209,10 @@ error: layout_of(C) = Layout { I16, false, ), - valid_range: 256..=256, + valid_range: AllocationRange { + start: 256, + end: 256, + }, }, tag_encoding: Direct, tag_field: 0, @@ -228,7 +249,10 @@ error: layout_of(C) = Layout { I16, false, ), - valid_range: 256..=256, + valid_range: AllocationRange { + start: 256, + end: 256, + }, }, ), largest_niche: Some( @@ -241,7 +265,10 @@ error: layout_of(C) = Layout { I16, false, ), - valid_range: 256..=256, + valid_range: AllocationRange { + start: 256, + end: 256, + }, }, }, ), @@ -279,7 +306,10 @@ error: layout_of(P) = Layout { I32, false, ), - valid_range: 268435456..=268435456, + valid_range: AllocationRange { + start: 268435456, + end: 268435456, + }, }, tag_encoding: Direct, tag_field: 0, @@ -316,7 +346,10 @@ error: layout_of(P) = Layout { I32, false, ), - valid_range: 268435456..=268435456, + valid_range: AllocationRange { + start: 268435456, + end: 268435456, + }, }, ), largest_niche: Some( @@ -329,7 +362,10 @@ error: layout_of(P) = Layout { I32, false, ), - valid_range: 268435456..=268435456, + valid_range: AllocationRange { + start: 268435456, + end: 268435456, + }, }, }, ), @@ -367,7 +403,10 @@ error: layout_of(T) = Layout { I32, true, ), - valid_range: 2164260864..=2164260864, + valid_range: AllocationRange { + start: 2164260864, + end: 2164260864, + }, }, tag_encoding: Direct, tag_field: 0, @@ -404,7 +443,10 @@ error: layout_of(T) = Layout { I32, true, ), - valid_range: 2164260864..=2164260864, + valid_range: AllocationRange { + start: 2164260864, + end: 2164260864, + }, }, ), largest_niche: Some( @@ -417,7 +459,10 @@ error: layout_of(T) = Layout { I32, true, ), - valid_range: 2164260864..=2164260864, + valid_range: AllocationRange { + start: 2164260864, + end: 2164260864, + }, }, }, ), From 1b4064f04a5d9f2ff910f448f65e3396502ff280 Mon Sep 17 00:00:00 2001 From: Andreas Liljeqvist Date: Sun, 22 Aug 2021 22:45:51 +0200 Subject: [PATCH 02/12] fix 32bit err --- src/test/ui/consts/const-eval/ub-nonnull.32bit.stderr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/consts/const-eval/ub-nonnull.32bit.stderr b/src/test/ui/consts/const-eval/ub-nonnull.32bit.stderr index e44f324945481..01403a61e5e24 100644 --- a/src/test/ui/consts/const-eval/ub-nonnull.32bit.stderr +++ b/src/test/ui/consts/const-eval/ub-nonnull.32bit.stderr @@ -52,7 +52,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-nonnull.rs:41:1 | LL | const BAD_RANGE1: RestrictedRange1 = unsafe { RestrictedRange1(42) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 42, but expected something in the range 10..=30 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 42, but expected something in the range AllocationRange { start: 10, end: 30 } | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. = note: the raw bytes of the constant (size: 4, align: 4) { From c3fdefe14267e766e3a590fc5967168c5443c7ee Mon Sep 17 00:00:00 2001 From: Andreas Liljeqvist Date: Mon, 23 Aug 2021 11:21:27 +0200 Subject: [PATCH 03/12] remove commented code --- compiler/rustc_middle/src/ty/layout.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 3b1990a3e675a..671f956bf3151 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -939,16 +939,12 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { // FIXME(eddyb) this might be incorrect - it doesn't // account for wrap-around (end < start) ranges. assert!(scalar.valid_range.start <= start); - // scalar.valid_range = - // AllocationRange { start, end: scalar.valid_range.end }; scalar.valid_range.start = start; } if let Bound::Included(end) = end { // FIXME(eddyb) this might be incorrect - it doesn't // account for wrap-around (end < start) ranges. assert!(scalar.valid_range.end >= end); - // scalar.valid_range = - // AllocationRange { start: scalar.valid_range.start, end }; scalar.valid_range.end = end; } From 225a4bf9222dbef88dc6fbefe437602e55decf1b Mon Sep 17 00:00:00 2001 From: Andreas Liljeqvist Date: Mon, 23 Aug 2021 13:56:28 +0200 Subject: [PATCH 04/12] Removed fixed fixme --- compiler/rustc_target/src/abi/mod.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs index 07687a4b104cf..b30111c6788f0 100644 --- a/compiler/rustc_target/src/abi/mod.rs +++ b/compiler/rustc_target/src/abi/mod.rs @@ -1011,9 +1011,6 @@ impl Niche { let start = v.end.wrapping_add(1) & max_value; let end = v.end.wrapping_add(count) & max_value; - // If the `end` of our range is inside the valid range, - // then we ran out of invalid values. - // FIXME(eddyb) abstract this with a wraparound range type. if v.contains(end) { return None; } From d50abd024901176f8b21081713bf4a2779d9aadb Mon Sep 17 00:00:00 2001 From: Andreas Liljeqvist Date: Mon, 23 Aug 2021 14:18:48 +0200 Subject: [PATCH 05/12] Use ref --- compiler/rustc_mir/src/interpret/validity.rs | 2 +- compiler/rustc_target/src/abi/mod.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_mir/src/interpret/validity.rs b/compiler/rustc_mir/src/interpret/validity.rs index 71e616ff560c0..bf34430cece20 100644 --- a/compiler/rustc_mir/src/interpret/validity.rs +++ b/compiler/rustc_mir/src/interpret/validity.rs @@ -623,7 +623,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' scalar_layout: &ScalarAbi, ) -> InterpResult<'tcx> { let value = self.read_scalar(op)?; - let valid_range = scalar_layout.valid_range; + let valid_range = scalar_layout.valid_range.clone(); let AllocationRange { start: lo, end: hi } = valid_range; // Determine the allowed range // `max_hi` is as big as the size fits diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs index b30111c6788f0..c3ec9bb82336e 100644 --- a/compiler/rustc_target/src/abi/mod.rs +++ b/compiler/rustc_target/src/abi/mod.rs @@ -985,7 +985,7 @@ impl Niche { } pub fn available(&self, cx: &C) -> u128 { - let Scalar { value, valid_range: v } = self.scalar; + let Scalar { value, valid_range: ref v } = self.scalar; let bits = value.size(cx).bits(); assert!(bits <= 128); let max_value = !0u128 >> (128 - bits); @@ -998,7 +998,7 @@ impl Niche { pub fn reserve(&self, cx: &C, count: u128) -> Option<(u128, Scalar)> { assert!(count > 0); - let Scalar { value, valid_range: v } = self.scalar; + let Scalar { value, valid_range: ref v } = self.scalar; let bits = value.size(cx).bits(); assert!(bits <= 128); let max_value = !0u128 >> (128 - bits); From 70433955f4531f2742ddeb986e6ac19a8fd4792f Mon Sep 17 00:00:00 2001 From: Andreas Liljeqvist Date: Mon, 23 Aug 2021 14:20:38 +0200 Subject: [PATCH 06/12] implement contains_zero method --- compiler/rustc_codegen_llvm/src/builder.rs | 3 +-- compiler/rustc_middle/src/ty/layout.rs | 6 ++---- compiler/rustc_target/src/abi/mod.rs | 12 +++++++++--- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index 2b72b16742630..7986d1d9cb283 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -462,7 +462,6 @@ impl BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { load: &'ll Value, scalar: &abi::Scalar, ) { - let vr = scalar.valid_range; match scalar.value { abi::Int(..) => { let range = scalar.valid_range_exclusive(bx); @@ -470,7 +469,7 @@ impl BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { bx.range_metadata(load, range); } } - abi::Pointer if vr.start < vr.end && !vr.contains(0) => { + abi::Pointer if !scalar.valid_range.contains_zero() => { bx.nonnull_metadata(load); } _ => {} diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 671f956bf3151..c6caab3e798fa 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -2857,10 +2857,8 @@ where return; } - if scalar.valid_range.start < scalar.valid_range.end { - if scalar.valid_range.start > 0 { - attrs.set(ArgAttribute::NonNull); - } + if !scalar.valid_range.contains_zero() { + attrs.set(ArgAttribute::NonNull); } if let Some(pointee) = layout.pointee_info_at(cx, offset) { diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs index c3ec9bb82336e..5ce8906e6ac63 100644 --- a/compiler/rustc_target/src/abi/mod.rs +++ b/compiler/rustc_target/src/abi/mod.rs @@ -688,7 +688,7 @@ impl Primitive { /// /// This is intended specifically to mirror LLVM’s `!range` metadata, /// semantics. -#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)] +#[derive(Clone, PartialEq, Eq, Hash, Debug)] #[derive(HashStable_Generic)] pub struct AllocationRange { pub start: u128, @@ -705,6 +705,13 @@ impl AllocationRange { self.start <= v || v <= self.end } } + + /// Returns `true` if zero is contained in the range. + /// Equal to `range.contains(0)` but should be faster. + #[inline] + pub fn contains_zero(&self) -> bool { + !(self.start <= self.end && self.start != 0) + } } /// Information about one scalar component of a Rust type. @@ -1222,9 +1229,8 @@ impl<'a, Ty> TyAndLayout<'a, Ty> { { let scalar_allows_raw_init = move |s: &Scalar| -> bool { if zero { - let range = &s.valid_range; // The range must contain 0. - range.contains(0) || (range.start > range.end) // wrap-around allows 0 + s.valid_range.contains_zero() } else { // The range must include all values. `valid_range_exclusive` handles // the wrap-around using target arithmetic; with wrap-around then the full From e8e6d9bd86c9cf685666718ca99e016275e1751b Mon Sep 17 00:00:00 2001 From: Andreas Liljeqvist Date: Mon, 23 Aug 2021 14:24:34 +0200 Subject: [PATCH 07/12] Rename to WrappingRange --- compiler/rustc_codegen_llvm/src/consts.rs | 7 ++----- compiler/rustc_middle/src/ty/layout.rs | 16 ++++++++-------- compiler/rustc_mir/src/interpret/validity.rs | 8 ++++---- compiler/rustc_target/src/abi/mod.rs | 10 +++++----- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/consts.rs b/compiler/rustc_codegen_llvm/src/consts.rs index 01a93dd8857c2..ec92bd686d2df 100644 --- a/compiler/rustc_codegen_llvm/src/consts.rs +++ b/compiler/rustc_codegen_llvm/src/consts.rs @@ -17,7 +17,7 @@ use rustc_middle::mir::mono::MonoItem; use rustc_middle::ty::{self, Instance, Ty}; use rustc_middle::{bug, span_bug}; use rustc_target::abi::{ - AddressSpace, Align, AllocationRange, HasDataLayout, LayoutOf, Primitive, Scalar, Size, + AddressSpace, Align, HasDataLayout, LayoutOf, Primitive, Scalar, Size, WrappingRange, }; use tracing::debug; @@ -61,10 +61,7 @@ pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll Pointer::new(alloc_id, Size::from_bytes(ptr_offset)), &cx.tcx, ), - &Scalar { - value: Primitive::Pointer, - valid_range: AllocationRange { start: 0, end: !0 }, - }, + &Scalar { value: Primitive::Pointer, valid_range: WrappingRange { start: 0, end: !0 } }, cx.type_i8p_ext(address_space), )); next_offset = offset + pointer_size; diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index c6caab3e798fa..6b628cb041b57 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -499,7 +499,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let scalar_unit = |value: Primitive| { let bits = value.size(dl).bits(); assert!(bits <= 128); - Scalar { value, valid_range: AllocationRange { start: 0, end: (!0 >> (128 - bits)) } } + Scalar { value, valid_range: WrappingRange { start: 0, end: (!0 >> (128 - bits)) } } }; let scalar = |value: Primitive| tcx.intern_layout(Layout::scalar(self, scalar_unit(value))); @@ -512,13 +512,13 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { // Basic scalars. ty::Bool => tcx.intern_layout(Layout::scalar( self, - Scalar { value: Int(I8, false), valid_range: AllocationRange { start: 0, end: 1 } }, + Scalar { value: Int(I8, false), valid_range: WrappingRange { start: 0, end: 1 } }, )), ty::Char => tcx.intern_layout(Layout::scalar( self, Scalar { value: Int(I32, false), - valid_range: AllocationRange { start: 0, end: 0x10FFFF }, + valid_range: WrappingRange { start: 0, end: 0x10FFFF }, }, )), ty::Int(ity) => scalar(Int(Integer::from_int_ty(dl, ity), true)), @@ -529,7 +529,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { }), ty::FnPtr(_) => { let mut ptr = scalar_unit(Pointer); - ptr.valid_range = AllocationRange { start: 1, end: ptr.valid_range.end }; + ptr.valid_range = WrappingRange { start: 1, end: ptr.valid_range.end }; tcx.intern_layout(Layout::scalar(self, ptr)) } @@ -548,7 +548,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let mut data_ptr = scalar_unit(Pointer); if !ty.is_unsafe_ptr() { data_ptr.valid_range = - AllocationRange { start: 1, end: data_ptr.valid_range.end }; + WrappingRange { start: 1, end: data_ptr.valid_range.end }; } let pointee = tcx.normalize_erasing_regions(param_env, pointee); @@ -565,7 +565,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { ty::Dynamic(..) => { let mut vtable = scalar_unit(Pointer); vtable.valid_range = - AllocationRange { start: 1, end: vtable.valid_range.end }; + WrappingRange { start: 1, end: vtable.valid_range.end }; vtable } _ => return Err(LayoutError::Unknown(unsized_part)), @@ -1261,7 +1261,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let tag_mask = !0u128 >> (128 - ity.size().bits()); let tag = Scalar { value: Int(ity, signed), - valid_range: AllocationRange { + valid_range: WrappingRange { start: (min as u128 & tag_mask), end: (max as u128 & tag_mask), }, @@ -1545,7 +1545,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let discr_int_ty = discr_int.to_ty(tcx, false); let tag = Scalar { value: Primitive::Int(discr_int, false), - valid_range: AllocationRange { start: 0, end: max_discr }, + valid_range: WrappingRange { start: 0, end: max_discr }, }; let tag_layout = self.tcx.intern_layout(Layout::scalar(self, tag.clone())); let tag_layout = TyAndLayout { ty: discr_int_ty, layout: tag_layout }; diff --git a/compiler/rustc_mir/src/interpret/validity.rs b/compiler/rustc_mir/src/interpret/validity.rs index bf34430cece20..3ff149d6a7a25 100644 --- a/compiler/rustc_mir/src/interpret/validity.rs +++ b/compiler/rustc_mir/src/interpret/validity.rs @@ -15,7 +15,7 @@ use rustc_middle::ty; use rustc_middle::ty::layout::TyAndLayout; use rustc_span::symbol::{sym, Symbol}; use rustc_target::abi::{ - Abi, AllocationRange, LayoutOf, Scalar as ScalarAbi, Size, VariantIdx, Variants, + Abi, LayoutOf, Scalar as ScalarAbi, Size, VariantIdx, Variants, WrappingRange, }; use std::hash::Hash; @@ -184,8 +184,8 @@ fn write_path(out: &mut String, path: &[PathElem]) { // Formats such that a sentence like "expected something {}" to mean // "expected something " makes sense. -fn wrapping_range_format(r: AllocationRange, max_hi: u128) -> String { - let AllocationRange { start: lo, end: hi } = r; +fn wrapping_range_format(r: WrappingRange, max_hi: u128) -> String { + let WrappingRange { start: lo, end: hi } = r; assert!(hi <= max_hi); if lo > hi { format!("less or equal to {}, or greater or equal to {}", hi, lo) @@ -624,7 +624,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' ) -> InterpResult<'tcx> { let value = self.read_scalar(op)?; let valid_range = scalar_layout.valid_range.clone(); - let AllocationRange { start: lo, end: hi } = valid_range; + let WrappingRange { start: lo, end: hi } = valid_range; // Determine the allowed range // `max_hi` is as big as the size fits let max_hi = u128::MAX >> (128 - op.layout.size.bits()); diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs index 5ce8906e6ac63..d29b731e4f129 100644 --- a/compiler/rustc_target/src/abi/mod.rs +++ b/compiler/rustc_target/src/abi/mod.rs @@ -690,12 +690,12 @@ impl Primitive { /// semantics. #[derive(Clone, PartialEq, Eq, Hash, Debug)] #[derive(HashStable_Generic)] -pub struct AllocationRange { +pub struct WrappingRange { pub start: u128, pub end: u128, } -impl AllocationRange { +impl WrappingRange { /// Returns `true` if `v` is contained in the range. #[inline] pub fn contains(&self, v: u128) -> bool { @@ -723,13 +723,13 @@ pub struct Scalar { // FIXME(eddyb) always use the shortest range, e.g., by finding // the largest space between two consecutive valid values and // taking everything else as the (shortest) valid range. - pub valid_range: AllocationRange, + pub valid_range: WrappingRange, } impl Scalar { pub fn is_bool(&self) -> bool { matches!(self.value, Int(I8, false)) - && matches!(self.valid_range, AllocationRange { start: 0, end: 1 }) + && matches!(self.valid_range, WrappingRange { start: 0, end: 1 }) } /// Returns the valid range as a `x..y` range. @@ -1022,7 +1022,7 @@ impl Niche { return None; } - Some((start, Scalar { value, valid_range: AllocationRange { start: v.start, end } })) + Some((start, Scalar { value, valid_range: WrappingRange { start: v.start, end } })) } } From d230b92ba7a2a32000be5e207860aa27d1a11113 Mon Sep 17 00:00:00 2001 From: Andreas Liljeqvist Date: Mon, 23 Aug 2021 15:05:40 +0200 Subject: [PATCH 08/12] implement debug in similar way to RangeInclusive --- compiler/rustc_target/src/abi/mod.rs | 9 ++- .../consts/const-eval/ub-nonnull.32bit.stderr | 2 +- .../consts/const-eval/ub-nonnull.64bit.stderr | 2 +- src/test/ui/layout/debug.stderr | 45 +++-------- src/test/ui/layout/hexagon-enum.stderr | 75 ++++--------------- src/test/ui/layout/thumb-enum.stderr | 75 ++++--------------- 6 files changed, 49 insertions(+), 159 deletions(-) diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs index d29b731e4f129..c9d0b12e7396f 100644 --- a/compiler/rustc_target/src/abi/mod.rs +++ b/compiler/rustc_target/src/abi/mod.rs @@ -688,7 +688,7 @@ impl Primitive { /// /// This is intended specifically to mirror LLVM’s `!range` metadata, /// semantics. -#[derive(Clone, PartialEq, Eq, Hash, Debug)] +#[derive(Clone, PartialEq, Eq, Hash)] #[derive(HashStable_Generic)] pub struct WrappingRange { pub start: u128, @@ -714,6 +714,13 @@ impl WrappingRange { } } +impl fmt::Debug for WrappingRange { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(fmt, "{}..={}", self.start, self.end)?; + Ok(()) + } +} + /// Information about one scalar component of a Rust type. #[derive(Clone, PartialEq, Eq, Hash, Debug)] #[derive(HashStable_Generic)] diff --git a/src/test/ui/consts/const-eval/ub-nonnull.32bit.stderr b/src/test/ui/consts/const-eval/ub-nonnull.32bit.stderr index 01403a61e5e24..e44f324945481 100644 --- a/src/test/ui/consts/const-eval/ub-nonnull.32bit.stderr +++ b/src/test/ui/consts/const-eval/ub-nonnull.32bit.stderr @@ -52,7 +52,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-nonnull.rs:41:1 | LL | const BAD_RANGE1: RestrictedRange1 = unsafe { RestrictedRange1(42) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 42, but expected something in the range AllocationRange { start: 10, end: 30 } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 42, but expected something in the range 10..=30 | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. = note: the raw bytes of the constant (size: 4, align: 4) { diff --git a/src/test/ui/consts/const-eval/ub-nonnull.64bit.stderr b/src/test/ui/consts/const-eval/ub-nonnull.64bit.stderr index 41de900f1640a..1ce87bc7c1ce8 100644 --- a/src/test/ui/consts/const-eval/ub-nonnull.64bit.stderr +++ b/src/test/ui/consts/const-eval/ub-nonnull.64bit.stderr @@ -52,7 +52,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-nonnull.rs:41:1 | LL | const BAD_RANGE1: RestrictedRange1 = unsafe { RestrictedRange1(42) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 42, but expected something in the range AllocationRange { start: 10, end: 30 } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 42, but expected something in the range 10..=30 | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. = note: the raw bytes of the constant (size: 4, align: 4) { diff --git a/src/test/ui/layout/debug.stderr b/src/test/ui/layout/debug.stderr index cbe9b6400c35f..1a371c6b17000 100644 --- a/src/test/ui/layout/debug.stderr +++ b/src/test/ui/layout/debug.stderr @@ -15,10 +15,7 @@ error: layout_of(E) = Layout { I32, false, ), - valid_range: AllocationRange { - start: 0, - end: 0, - }, + valid_range: 0..=0, }, tag_encoding: Direct, tag_field: 0, @@ -94,10 +91,7 @@ error: layout_of(E) = Layout { I32, false, ), - valid_range: AllocationRange { - start: 0, - end: 0, - }, + valid_range: 0..=0, }, }, ), @@ -144,20 +138,14 @@ error: layout_of(S) = Layout { I32, true, ), - valid_range: AllocationRange { - start: 0, - end: 4294967295, - }, + valid_range: 0..=4294967295, }, Scalar { value: Int( I32, true, ), - valid_range: AllocationRange { - start: 0, - end: 4294967295, - }, + valid_range: 0..=4294967295, }, ), largest_niche: None, @@ -219,10 +207,7 @@ error: layout_of(std::result::Result) = Layout { I32, false, ), - valid_range: AllocationRange { - start: 0, - end: 1, - }, + valid_range: 0..=1, }, tag_encoding: Direct, tag_field: 0, @@ -291,20 +276,14 @@ error: layout_of(std::result::Result) = Layout { I32, false, ), - valid_range: AllocationRange { - start: 0, - end: 1, - }, + valid_range: 0..=1, }, Scalar { value: Int( I32, true, ), - valid_range: AllocationRange { - start: 0, - end: 4294967295, - }, + valid_range: 0..=4294967295, }, ), largest_niche: Some( @@ -317,10 +296,7 @@ error: layout_of(std::result::Result) = Layout { I32, false, ), - valid_range: AllocationRange { - start: 0, - end: 1, - }, + valid_range: 0..=1, }, }, ), @@ -350,10 +326,7 @@ error: layout_of(i32) = Layout { I32, true, ), - valid_range: AllocationRange { - start: 0, - end: 4294967295, - }, + valid_range: 0..=4294967295, }, ), largest_niche: None, diff --git a/src/test/ui/layout/hexagon-enum.stderr b/src/test/ui/layout/hexagon-enum.stderr index 520ba12c39fb1..d4676a5afb25e 100644 --- a/src/test/ui/layout/hexagon-enum.stderr +++ b/src/test/ui/layout/hexagon-enum.stderr @@ -15,10 +15,7 @@ error: layout_of(A) = Layout { I8, false, ), - valid_range: AllocationRange { - start: 0, - end: 0, - }, + valid_range: 0..=0, }, tag_encoding: Direct, tag_field: 0, @@ -55,10 +52,7 @@ error: layout_of(A) = Layout { I8, false, ), - valid_range: AllocationRange { - start: 0, - end: 0, - }, + valid_range: 0..=0, }, ), largest_niche: Some( @@ -71,10 +65,7 @@ error: layout_of(A) = Layout { I8, false, ), - valid_range: AllocationRange { - start: 0, - end: 0, - }, + valid_range: 0..=0, }, }, ), @@ -112,10 +103,7 @@ error: layout_of(B) = Layout { I8, false, ), - valid_range: AllocationRange { - start: 255, - end: 255, - }, + valid_range: 255..=255, }, tag_encoding: Direct, tag_field: 0, @@ -152,10 +140,7 @@ error: layout_of(B) = Layout { I8, false, ), - valid_range: AllocationRange { - start: 255, - end: 255, - }, + valid_range: 255..=255, }, ), largest_niche: Some( @@ -168,10 +153,7 @@ error: layout_of(B) = Layout { I8, false, ), - valid_range: AllocationRange { - start: 255, - end: 255, - }, + valid_range: 255..=255, }, }, ), @@ -209,10 +191,7 @@ error: layout_of(C) = Layout { I16, false, ), - valid_range: AllocationRange { - start: 256, - end: 256, - }, + valid_range: 256..=256, }, tag_encoding: Direct, tag_field: 0, @@ -249,10 +228,7 @@ error: layout_of(C) = Layout { I16, false, ), - valid_range: AllocationRange { - start: 256, - end: 256, - }, + valid_range: 256..=256, }, ), largest_niche: Some( @@ -265,10 +241,7 @@ error: layout_of(C) = Layout { I16, false, ), - valid_range: AllocationRange { - start: 256, - end: 256, - }, + valid_range: 256..=256, }, }, ), @@ -306,10 +279,7 @@ error: layout_of(P) = Layout { I32, false, ), - valid_range: AllocationRange { - start: 268435456, - end: 268435456, - }, + valid_range: 268435456..=268435456, }, tag_encoding: Direct, tag_field: 0, @@ -346,10 +316,7 @@ error: layout_of(P) = Layout { I32, false, ), - valid_range: AllocationRange { - start: 268435456, - end: 268435456, - }, + valid_range: 268435456..=268435456, }, ), largest_niche: Some( @@ -362,10 +329,7 @@ error: layout_of(P) = Layout { I32, false, ), - valid_range: AllocationRange { - start: 268435456, - end: 268435456, - }, + valid_range: 268435456..=268435456, }, }, ), @@ -403,10 +367,7 @@ error: layout_of(T) = Layout { I32, true, ), - valid_range: AllocationRange { - start: 2164260864, - end: 2164260864, - }, + valid_range: 2164260864..=2164260864, }, tag_encoding: Direct, tag_field: 0, @@ -443,10 +404,7 @@ error: layout_of(T) = Layout { I32, true, ), - valid_range: AllocationRange { - start: 2164260864, - end: 2164260864, - }, + valid_range: 2164260864..=2164260864, }, ), largest_niche: Some( @@ -459,10 +417,7 @@ error: layout_of(T) = Layout { I32, true, ), - valid_range: AllocationRange { - start: 2164260864, - end: 2164260864, - }, + valid_range: 2164260864..=2164260864, }, }, ), diff --git a/src/test/ui/layout/thumb-enum.stderr b/src/test/ui/layout/thumb-enum.stderr index 50e37c5411632..898a61b904db5 100644 --- a/src/test/ui/layout/thumb-enum.stderr +++ b/src/test/ui/layout/thumb-enum.stderr @@ -15,10 +15,7 @@ error: layout_of(A) = Layout { I8, false, ), - valid_range: AllocationRange { - start: 0, - end: 0, - }, + valid_range: 0..=0, }, tag_encoding: Direct, tag_field: 0, @@ -55,10 +52,7 @@ error: layout_of(A) = Layout { I8, false, ), - valid_range: AllocationRange { - start: 0, - end: 0, - }, + valid_range: 0..=0, }, ), largest_niche: Some( @@ -71,10 +65,7 @@ error: layout_of(A) = Layout { I8, false, ), - valid_range: AllocationRange { - start: 0, - end: 0, - }, + valid_range: 0..=0, }, }, ), @@ -112,10 +103,7 @@ error: layout_of(B) = Layout { I8, false, ), - valid_range: AllocationRange { - start: 255, - end: 255, - }, + valid_range: 255..=255, }, tag_encoding: Direct, tag_field: 0, @@ -152,10 +140,7 @@ error: layout_of(B) = Layout { I8, false, ), - valid_range: AllocationRange { - start: 255, - end: 255, - }, + valid_range: 255..=255, }, ), largest_niche: Some( @@ -168,10 +153,7 @@ error: layout_of(B) = Layout { I8, false, ), - valid_range: AllocationRange { - start: 255, - end: 255, - }, + valid_range: 255..=255, }, }, ), @@ -209,10 +191,7 @@ error: layout_of(C) = Layout { I16, false, ), - valid_range: AllocationRange { - start: 256, - end: 256, - }, + valid_range: 256..=256, }, tag_encoding: Direct, tag_field: 0, @@ -249,10 +228,7 @@ error: layout_of(C) = Layout { I16, false, ), - valid_range: AllocationRange { - start: 256, - end: 256, - }, + valid_range: 256..=256, }, ), largest_niche: Some( @@ -265,10 +241,7 @@ error: layout_of(C) = Layout { I16, false, ), - valid_range: AllocationRange { - start: 256, - end: 256, - }, + valid_range: 256..=256, }, }, ), @@ -306,10 +279,7 @@ error: layout_of(P) = Layout { I32, false, ), - valid_range: AllocationRange { - start: 268435456, - end: 268435456, - }, + valid_range: 268435456..=268435456, }, tag_encoding: Direct, tag_field: 0, @@ -346,10 +316,7 @@ error: layout_of(P) = Layout { I32, false, ), - valid_range: AllocationRange { - start: 268435456, - end: 268435456, - }, + valid_range: 268435456..=268435456, }, ), largest_niche: Some( @@ -362,10 +329,7 @@ error: layout_of(P) = Layout { I32, false, ), - valid_range: AllocationRange { - start: 268435456, - end: 268435456, - }, + valid_range: 268435456..=268435456, }, }, ), @@ -403,10 +367,7 @@ error: layout_of(T) = Layout { I32, true, ), - valid_range: AllocationRange { - start: 2164260864, - end: 2164260864, - }, + valid_range: 2164260864..=2164260864, }, tag_encoding: Direct, tag_field: 0, @@ -443,10 +404,7 @@ error: layout_of(T) = Layout { I32, true, ), - valid_range: AllocationRange { - start: 2164260864, - end: 2164260864, - }, + valid_range: 2164260864..=2164260864, }, ), largest_niche: Some( @@ -459,10 +417,7 @@ error: layout_of(T) = Layout { I32, true, ), - valid_range: AllocationRange { - start: 2164260864, - end: 2164260864, - }, + valid_range: 2164260864..=2164260864, }, }, ), From 32d7e5b723f97092e392abc33074d8e750a9cb23 Mon Sep 17 00:00:00 2001 From: Andreas Liljeqvist Date: Mon, 23 Aug 2021 15:44:56 +0200 Subject: [PATCH 09/12] add `with_start` and `with_end` --- compiler/rustc_middle/src/ty/layout.rs | 8 +++----- compiler/rustc_target/src/abi/mod.rs | 12 +++++++++++- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 6b628cb041b57..dcb56d5b2ba1b 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -529,7 +529,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { }), ty::FnPtr(_) => { let mut ptr = scalar_unit(Pointer); - ptr.valid_range = WrappingRange { start: 1, end: ptr.valid_range.end }; + ptr.valid_range = ptr.valid_range.with_start(1); tcx.intern_layout(Layout::scalar(self, ptr)) } @@ -547,8 +547,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { ty::Ref(_, pointee, _) | ty::RawPtr(ty::TypeAndMut { ty: pointee, .. }) => { let mut data_ptr = scalar_unit(Pointer); if !ty.is_unsafe_ptr() { - data_ptr.valid_range = - WrappingRange { start: 1, end: data_ptr.valid_range.end }; + data_ptr.valid_range = data_ptr.valid_range.with_start(1); } let pointee = tcx.normalize_erasing_regions(param_env, pointee); @@ -564,8 +563,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { ty::Slice(_) | ty::Str => scalar_unit(Int(dl.ptr_sized_integer(), false)), ty::Dynamic(..) => { let mut vtable = scalar_unit(Pointer); - vtable.valid_range = - WrappingRange { start: 1, end: vtable.valid_range.end }; + vtable.valid_range = vtable.valid_range.with_start(1); vtable } _ => return Err(LayoutError::Unknown(unsized_part)), diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs index c9d0b12e7396f..6e2f8962eef48 100644 --- a/compiler/rustc_target/src/abi/mod.rs +++ b/compiler/rustc_target/src/abi/mod.rs @@ -712,6 +712,16 @@ impl WrappingRange { pub fn contains_zero(&self) -> bool { !(self.start <= self.end && self.start != 0) } + + /// Returns new `WrappingRange` with replaced `start` + pub fn with_start(&self, start: u128) -> Self { + Self { start, end: self.end } + } + + /// Returns new `WrappingRange` with replaced `end` + pub fn with_end(&self, end: u128) -> Self { + Self { start: self.start, end } + } } impl fmt::Debug for WrappingRange { @@ -1029,7 +1039,7 @@ impl Niche { return None; } - Some((start, Scalar { value, valid_range: WrappingRange { start: v.start, end } })) + Some((start, Scalar { value, valid_range: v.with_end(end) })) } } From d92810646ed517376b16ac3b1fed329d8df2a8a9 Mon Sep 17 00:00:00 2001 From: Andreas Liljeqvist Date: Mon, 23 Aug 2021 15:52:47 +0200 Subject: [PATCH 10/12] Simplify zero check --- compiler/rustc_target/src/abi/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs index 6e2f8962eef48..b352243c0c44e 100644 --- a/compiler/rustc_target/src/abi/mod.rs +++ b/compiler/rustc_target/src/abi/mod.rs @@ -710,7 +710,7 @@ impl WrappingRange { /// Equal to `range.contains(0)` but should be faster. #[inline] pub fn contains_zero(&self) -> bool { - !(self.start <= self.end && self.start != 0) + self.start > self.end || self.start == 0 } /// Returns new `WrappingRange` with replaced `start` From e3f07b2e30eb29a737b13ab127db927d3825c22b Mon Sep 17 00:00:00 2001 From: Andreas Liljeqvist Date: Tue, 24 Aug 2021 10:18:07 +0200 Subject: [PATCH 11/12] Force inline: small functions and single call-site --- compiler/rustc_target/src/abi/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs index b352243c0c44e..49c06fca85acc 100644 --- a/compiler/rustc_target/src/abi/mod.rs +++ b/compiler/rustc_target/src/abi/mod.rs @@ -697,7 +697,7 @@ pub struct WrappingRange { impl WrappingRange { /// Returns `true` if `v` is contained in the range. - #[inline] + #[inline(always)] pub fn contains(&self, v: u128) -> bool { if self.start <= self.end { self.start <= v && v <= self.end @@ -708,17 +708,19 @@ impl WrappingRange { /// Returns `true` if zero is contained in the range. /// Equal to `range.contains(0)` but should be faster. - #[inline] + #[inline(always)] pub fn contains_zero(&self) -> bool { self.start > self.end || self.start == 0 } /// Returns new `WrappingRange` with replaced `start` + #[inline(always)] pub fn with_start(&self, start: u128) -> Self { Self { start, end: self.end } } /// Returns new `WrappingRange` with replaced `end` + #[inline(always)] pub fn with_end(&self, end: u128) -> Self { Self { start: self.start, end } } From f17e384a43dd8ca0aefb36bfcd8a69d9ad7f12cf Mon Sep 17 00:00:00 2001 From: Andreas Liljeqvist Date: Tue, 24 Aug 2021 19:41:58 +0200 Subject: [PATCH 12/12] use convention for with_* methods --- compiler/rustc_target/src/abi/mod.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs index 49c06fca85acc..d206df461200a 100644 --- a/compiler/rustc_target/src/abi/mod.rs +++ b/compiler/rustc_target/src/abi/mod.rs @@ -713,16 +713,18 @@ impl WrappingRange { self.start > self.end || self.start == 0 } - /// Returns new `WrappingRange` with replaced `start` + /// Returns `self` with replaced `start` #[inline(always)] - pub fn with_start(&self, start: u128) -> Self { - Self { start, end: self.end } + pub fn with_start(mut self, start: u128) -> Self { + self.start = start; + self } - /// Returns new `WrappingRange` with replaced `end` + /// Returns `self` with replaced `end` #[inline(always)] - pub fn with_end(&self, end: u128) -> Self { - Self { start: self.start, end } + pub fn with_end(mut self, end: u128) -> Self { + self.end = end; + self } } @@ -1024,7 +1026,7 @@ impl Niche { pub fn reserve(&self, cx: &C, count: u128) -> Option<(u128, Scalar)> { assert!(count > 0); - let Scalar { value, valid_range: ref v } = self.scalar; + let Scalar { value, valid_range: v } = self.scalar.clone(); let bits = value.size(cx).bits(); assert!(bits <= 128); let max_value = !0u128 >> (128 - bits);