From 98c5649cab296f5ace1201837fb8364bb6b8fb60 Mon Sep 17 00:00:00 2001 From: Erik Desjardins Date: Wed, 31 Mar 2021 00:06:01 -0400 Subject: [PATCH 1/2] use undef for uninitialized bytes in constants --- compiler/rustc_codegen_llvm/src/consts.rs | 43 +++++++++++----- .../src/mir/interpret/allocation.rs | 51 +++++++++++++++++++ .../rustc_middle/src/mir/interpret/mod.rs | 4 +- src/test/codegen/consts.rs | 4 +- src/test/codegen/uninit-consts.rs | 32 ++++++++++++ 5 files changed, 118 insertions(+), 16 deletions(-) create mode 100644 src/test/codegen/uninit-consts.rs diff --git a/compiler/rustc_codegen_llvm/src/consts.rs b/compiler/rustc_codegen_llvm/src/consts.rs index 99046839973d5..245bdcd6409c8 100644 --- a/compiler/rustc_codegen_llvm/src/consts.rs +++ b/compiler/rustc_codegen_llvm/src/consts.rs @@ -8,15 +8,17 @@ use crate::value::Value; use cstr::cstr; use libc::c_uint; use rustc_codegen_ssa::traits::*; +use rustc_data_structures::captures::Captures; use rustc_hir::def_id::DefId; use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs}; use rustc_middle::mir::interpret::{ - read_target_uint, Allocation, ErrorHandled, GlobalAlloc, Pointer, + read_target_uint, Allocation, ErrorHandled, GlobalAlloc, InitChunk, Pointer, }; 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 std::ops::Range; use tracing::debug; pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll Value { @@ -24,6 +26,29 @@ pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll let dl = cx.data_layout(); let pointer_size = dl.pointer_size.bytes() as usize; + // Note: this function may call `inspect_with_uninit_and_ptr_outside_interpreter`, + // so `range` must be within the bounds of `alloc` and not within a relocation. + fn chunks_of_init_and_uninit_bytes<'ll, 'a, 'b>( + cx: &'a CodegenCx<'ll, 'b>, + alloc: &'a Allocation, + range: Range, + ) -> impl Iterator + Captures<'a> + Captures<'b> { + alloc + .init_mask() + .range_as_init_chunks(Size::from_bytes(range.start), Size::from_bytes(range.end)) + .map(move |chunk| match chunk { + InitChunk::Init(range) => { + let range = (range.start.bytes() as usize)..(range.end.bytes() as usize); + let bytes = alloc.inspect_with_uninit_and_ptr_outside_interpreter(range); + cx.const_bytes(bytes) + } + InitChunk::Uninit(range) => { + let len = range.end.bytes() - range.start.bytes(); + cx.const_undef(cx.type_array(cx.type_i8(), len)) + } + }) + } + let mut next_offset = 0; for &(offset, ((), alloc_id)) in alloc.relocations().iter() { let offset = offset.bytes(); @@ -32,12 +57,8 @@ pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll if offset > next_offset { // This `inspect` is okay since we have checked that it is not within a relocation, it // is within the bounds of the allocation, and it doesn't affect interpreter execution - // (we inspect the result after interpreter execution). Any undef byte is replaced with - // some arbitrary byte value. - // - // FIXME: relay undef bytes to codegen as undef const bytes - let bytes = alloc.inspect_with_uninit_and_ptr_outside_interpreter(next_offset..offset); - llvals.push(cx.const_bytes(bytes)); + // (we inspect the result after interpreter execution). + llvals.extend(chunks_of_init_and_uninit_bytes(cx, alloc, next_offset..offset)); } let ptr_offset = read_target_uint( dl.endian, @@ -65,12 +86,8 @@ pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll let range = next_offset..alloc.len(); // This `inspect` is okay since we have check that it is after all relocations, it is // within the bounds of the allocation, and it doesn't affect interpreter execution (we - // inspect the result after interpreter execution). Any undef byte is replaced with some - // arbitrary byte value. - // - // FIXME: relay undef bytes to codegen as undef const bytes - let bytes = alloc.inspect_with_uninit_and_ptr_outside_interpreter(range); - llvals.push(cx.const_bytes(bytes)); + // inspect the result after interpreter execution). + llvals.extend(chunks_of_init_and_uninit_bytes(cx, alloc, range)); } cx.const_struct(&llvals, true) diff --git a/compiler/rustc_middle/src/mir/interpret/allocation.rs b/compiler/rustc_middle/src/mir/interpret/allocation.rs index 766d6a06f7e59..96cf9b0216fed 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation.rs @@ -775,6 +775,13 @@ impl InitMask { } } + /// Returns an iterator, yielding a range of byte indexes for each contiguous region + /// of initialized or uninitialized bytes inside the range `start..end` (end-exclusive). + #[inline] + pub fn range_as_init_chunks(&self, start: Size, end: Size) -> InitChunkIter<'_> { + InitChunkIter::new(self, start, end) + } + pub fn set_range(&mut self, start: Size, end: Size, new_state: bool) { let len = self.len; if end > len { @@ -867,6 +874,50 @@ impl InitMask { } } +/// Yields [`InitChunk`]s. See [`InitMask::range_as_init_chunks`]. +pub struct InitChunkIter<'a> { + init_mask: &'a InitMask, + /// The current byte index into `init_mask`. + start: Size, + /// The end byte index into `init_mask`. + end: Size, +} + +/// A contiguous chunk of initialized or uninitialized memory. +pub enum InitChunk { + Init(Range), + Uninit(Range), +} + +impl<'a> InitChunkIter<'a> { + fn new(init_mask: &'a InitMask, start: Size, end: Size) -> Self { + assert!(start <= end); + assert!(end <= init_mask.len); + Self { init_mask, start, end } + } +} + +impl<'a> Iterator for InitChunkIter<'a> { + type Item = InitChunk; + + fn next(&mut self) -> Option { + if self.start >= self.end { + return None; + } + + let is_init = self.init_mask.get(self.start); + let end_of_chunk = (self.start.bytes()..self.end.bytes()) + .map(Size::from_bytes) + .find(|&i| self.init_mask.get(i) != is_init) + .unwrap_or(self.end); + let range = self.start..end_of_chunk; + + self.start = end_of_chunk; + + Some(if is_init { InitChunk::Init(range) } else { InitChunk::Uninit(range) }) + } +} + #[inline] fn bit_index(bits: Size) -> (usize, usize) { let bits = bits.bytes(); diff --git a/compiler/rustc_middle/src/mir/interpret/mod.rs b/compiler/rustc_middle/src/mir/interpret/mod.rs index 55fe5f971e718..81051f122b97c 100644 --- a/compiler/rustc_middle/src/mir/interpret/mod.rs +++ b/compiler/rustc_middle/src/mir/interpret/mod.rs @@ -125,7 +125,9 @@ pub use self::error::{ pub use self::value::{get_slice_bytes, ConstAlloc, ConstValue, Scalar, ScalarMaybeUninit}; -pub use self::allocation::{Allocation, AllocationExtra, InitMask, Relocations}; +pub use self::allocation::{ + Allocation, AllocationExtra, InitChunk, InitChunkIter, InitMask, Relocations, +}; pub use self::pointer::{Pointer, PointerArithmetic}; diff --git a/src/test/codegen/consts.rs b/src/test/codegen/consts.rs index fcb9002986a1c..a6e611ce5b593 100644 --- a/src/test/codegen/consts.rs +++ b/src/test/codegen/consts.rs @@ -43,7 +43,7 @@ pub fn inline_enum_const() -> E { #[no_mangle] pub fn low_align_const() -> E { // Check that low_align_const and high_align_const use the same constant - // CHECK: memcpy.p0i8.p0i8.i{{(32|64)}}(i8* align 2 %1, i8* align 2 getelementptr inbounds (<{ [8 x i8] }>, <{ [8 x i8] }>* [[LOW_HIGH]], i32 0, i32 0, i32 0), i{{(32|64)}} 8, i1 false) + // CHECK: memcpy.p0i8.p0i8.i{{(32|64)}}(i8* align 2 %1, i8* align 2 getelementptr inbounds (<{ [4 x i8], [4 x i8] }>, <{ [4 x i8], [4 x i8] }>* [[LOW_HIGH]], i32 0, i32 0, i32 0), i{{(32|64)}} 8, i1 false) *&E::A(0) } @@ -51,6 +51,6 @@ pub fn low_align_const() -> E { #[no_mangle] pub fn high_align_const() -> E { // Check that low_align_const and high_align_const use the same constant - // CHECK: memcpy.p0i8.p0i8.i{{(32|64)}}(i8* align 4 %1, i8* align 4 getelementptr inbounds (<{ [8 x i8] }>, <{ [8 x i8] }>* [[LOW_HIGH]], i32 0, i32 0, i32 0), i{{(32|64)}} 8, i1 false) + // CHECK: memcpy.p0i8.p0i8.i{{(32|64)}}(i8* align 4 %1, i8* align 4 getelementptr inbounds (<{ [4 x i8], [4 x i8] }>, <{ [4 x i8], [4 x i8] }>* [[LOW_HIGH]], i32 0, i32 0, i32 0), i{{(32|64)}} 8, i1 false) *&E::A(0) } diff --git a/src/test/codegen/uninit-consts.rs b/src/test/codegen/uninit-consts.rs new file mode 100644 index 0000000000000..b1a55eccc0399 --- /dev/null +++ b/src/test/codegen/uninit-consts.rs @@ -0,0 +1,32 @@ +// compile-flags: -C no-prepopulate-passes +// ignore-tidy-linelength + +// Check that we use undef (and not zero) for uninitialized bytes in constants. + +#![crate_type = "lib"] + +use std::mem::MaybeUninit; + +pub struct PartiallyUninit { + x: u32, + y: MaybeUninit<[u8; 10]> +} + +// CHECK: [[FULLY_UNINIT:@[0-9]+]] = private unnamed_addr constant <{ [10 x i8] }> undef +// CHECK: [[PARTIALLY_UNINIT:@[0-9]+]] = private unnamed_addr constant <{ [4 x i8], [12 x i8] }> <{ [4 x i8] c"\EF\BE\AD\DE", [12 x i8] undef }>, align 4 + +// CHECK-LABEL: @fully_uninit +#[no_mangle] +pub const fn fully_uninit() -> MaybeUninit<[u8; 10]> { + const M: MaybeUninit<[u8; 10]> = MaybeUninit::uninit(); + // CHECK: call void @llvm.memcpy.p0i8.p0i8.i{{(32|64)}}(i8* align 1 %1, i8* align 1 getelementptr inbounds (<{ [10 x i8] }>, <{ [10 x i8] }>* [[FULLY_UNINIT]], i32 0, i32 0, i32 0), i{{(32|64)}} 10, i1 false) + M +} + +// CHECK-LABEL: @partially_uninit +#[no_mangle] +pub const fn partially_uninit() -> PartiallyUninit { + const X: PartiallyUninit = PartiallyUninit { x: 0xdeadbeef, y: MaybeUninit::uninit() }; + // CHECK: call void @llvm.memcpy.p0i8.p0i8.i{{(32|64)}}(i8* align 4 %1, i8* align 4 getelementptr inbounds (<{ [4 x i8], [12 x i8] }>, <{ [4 x i8], [12 x i8] }>* [[PARTIALLY_UNINIT]], i32 0, i32 0, i32 0), i{{(32|64)}} 16, i1 false) + X +} From be66c2e551c177c9607f257ccadefb0c64820645 Mon Sep 17 00:00:00 2001 From: Erik Desjardins Date: Wed, 31 Mar 2021 17:46:49 -0400 Subject: [PATCH 2/2] impl Step for Size; copy-paste fixme --- .../src/mir/interpret/allocation.rs | 14 +++---- compiler/rustc_target/src/abi/mod.rs | 38 +++++++++++++++++++ compiler/rustc_target/src/lib.rs | 3 ++ 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_middle/src/mir/interpret/allocation.rs b/compiler/rustc_middle/src/mir/interpret/allocation.rs index 96cf9b0216fed..d6bd1ed7933e3 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation.rs @@ -761,14 +761,11 @@ impl InitMask { } // FIXME(oli-obk): optimize this for allocations larger than a block. - let idx = (start.bytes()..end.bytes()).map(Size::from_bytes).find(|&i| !self.get(i)); + let idx = (start..end).find(|&i| !self.get(i)); match idx { Some(idx) => { - let uninit_end = (idx.bytes()..end.bytes()) - .map(Size::from_bytes) - .find(|&i| self.get(i)) - .unwrap_or(end); + let uninit_end = (idx..end).find(|&i| self.get(i)).unwrap_or(end); Err(idx..uninit_end) } None => Ok(()), @@ -906,10 +903,9 @@ impl<'a> Iterator for InitChunkIter<'a> { } let is_init = self.init_mask.get(self.start); - let end_of_chunk = (self.start.bytes()..self.end.bytes()) - .map(Size::from_bytes) - .find(|&i| self.init_mask.get(i) != is_init) - .unwrap_or(self.end); + // FIXME(oli-obk): optimize this for allocations larger than a block. + let end_of_chunk = + (self.start..self.end).find(|&i| self.init_mask.get(i) != is_init).unwrap_or(self.end); let range = self.start..end_of_chunk; self.start = end_of_chunk; diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs index e2618da749fb3..22e67ea4da0ed 100644 --- a/compiler/rustc_target/src/abi/mod.rs +++ b/compiler/rustc_target/src/abi/mod.rs @@ -5,6 +5,7 @@ use crate::spec::Target; use std::convert::{TryFrom, TryInto}; use std::fmt; +use std::iter::Step; use std::num::NonZeroUsize; use std::ops::{Add, AddAssign, Deref, Mul, Range, RangeInclusive, Sub}; use std::str::FromStr; @@ -433,6 +434,43 @@ impl AddAssign for Size { } } +unsafe impl Step for Size { + #[inline] + fn steps_between(start: &Self, end: &Self) -> Option { + u64::steps_between(&start.bytes(), &end.bytes()) + } + + #[inline] + fn forward_checked(start: Self, count: usize) -> Option { + u64::forward_checked(start.bytes(), count).map(Self::from_bytes) + } + + #[inline] + fn forward(start: Self, count: usize) -> Self { + Self::from_bytes(u64::forward(start.bytes(), count)) + } + + #[inline] + unsafe fn forward_unchecked(start: Self, count: usize) -> Self { + Self::from_bytes(u64::forward_unchecked(start.bytes(), count)) + } + + #[inline] + fn backward_checked(start: Self, count: usize) -> Option { + u64::backward_checked(start.bytes(), count).map(Self::from_bytes) + } + + #[inline] + fn backward(start: Self, count: usize) -> Self { + Self::from_bytes(u64::backward(start.bytes(), count)) + } + + #[inline] + unsafe fn backward_unchecked(start: Self, count: usize) -> Self { + Self::from_bytes(u64::backward_unchecked(start.bytes(), count)) + } +} + /// Alignment of a type in bytes (always a power of two). #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Encodable, Decodable)] #[derive(HashStable_Generic)] diff --git a/compiler/rustc_target/src/lib.rs b/compiler/rustc_target/src/lib.rs index fb747dfcbd337..f2badef8e7a2c 100644 --- a/compiler/rustc_target/src/lib.rs +++ b/compiler/rustc_target/src/lib.rs @@ -15,6 +15,9 @@ #![feature(never_type)] #![feature(associated_type_bounds)] #![feature(exhaustive_patterns)] +#![feature(step_trait)] +#![feature(step_trait_ext)] +#![feature(unchecked_math)] #[macro_use] extern crate rustc_macros;