From 551f481ffb5b510cdb8cdf4d89d71f105b9b29ba Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 11 Sep 2023 20:01:48 +0200 Subject: [PATCH 1/7] use AllocId instead of Allocation in ConstValue::ByRef --- .../rustc_codegen_cranelift/src/constant.rs | 14 +++-- .../src/intrinsics/simd.rs | 3 +- compiler/rustc_codegen_ssa/src/mir/operand.rs | 4 +- .../src/const_eval/eval_queries.rs | 5 +- .../rustc_const_eval/src/interpret/intern.rs | 14 +++-- .../rustc_const_eval/src/interpret/operand.rs | 5 +- .../rustc_middle/src/mir/interpret/value.rs | 10 ++-- compiler/rustc_middle/src/mir/mod.rs | 3 +- compiler/rustc_middle/src/mir/pretty.rs | 14 ++--- .../rustc_middle/src/ty/structural_impls.rs | 1 + .../rustc_mir_transform/src/const_prop.rs | 6 +-- .../rustc_mir_transform/src/large_enums.rs | 3 +- compiler/rustc_monomorphize/src/collector.rs | 5 +- compiler/rustc_smir/src/rustc_smir/alloc.rs | 3 +- src/tools/clippy/clippy_utils/src/consts.rs | 53 ++++++++++--------- 15 files changed, 80 insertions(+), 63 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/constant.rs b/compiler/rustc_codegen_cranelift/src/constant.rs index b9d4bc9ff291d..a60964f0f757a 100644 --- a/compiler/rustc_codegen_cranelift/src/constant.rs +++ b/compiler/rustc_codegen_cranelift/src/constant.rs @@ -200,11 +200,15 @@ pub(crate) fn codegen_const_value<'tcx>( CValue::by_val(val, layout) } }, - ConstValue::ByRef { alloc, offset } => CValue::by_ref( - pointer_for_allocation(fx, alloc) - .offset_i64(fx, i64::try_from(offset.bytes()).unwrap()), - layout, - ), + ConstValue::ByRef { alloc_id, offset } => { + let alloc = fx.tcx.global_alloc(alloc_id).unwrap_memory(); + // FIXME: avoid creating multiple allocations for the same AllocId? + CValue::by_ref( + pointer_for_allocation(fx, alloc) + .offset_i64(fx, i64::try_from(offset.bytes()).unwrap()), + layout, + ) + } ConstValue::Slice { data, start, end } => { let ptr = pointer_for_allocation(fx, data) .offset_i64(fx, i64::try_from(start).unwrap()) diff --git a/compiler/rustc_codegen_cranelift/src/intrinsics/simd.rs b/compiler/rustc_codegen_cranelift/src/intrinsics/simd.rs index 9863e40b5b7a1..e17d587076f9f 100644 --- a/compiler/rustc_codegen_cranelift/src/intrinsics/simd.rs +++ b/compiler/rustc_codegen_cranelift/src/intrinsics/simd.rs @@ -172,7 +172,8 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>( .expect("simd_shuffle idx not const"); let idx_bytes = match idx_const { - ConstValue::ByRef { alloc, offset } => { + ConstValue::ByRef { alloc_id, offset } => { + let alloc = fx.tcx.global_alloc(alloc_id).unwrap_memory(); let size = Size::from_bytes( 4 * ret_lane_count, /* size_of([u32; ret_lane_count]) */ ); diff --git a/compiler/rustc_codegen_ssa/src/mir/operand.rs b/compiler/rustc_codegen_ssa/src/mir/operand.rs index ef66aa6ce1c30..18c3fd0f1d12c 100644 --- a/compiler/rustc_codegen_ssa/src/mir/operand.rs +++ b/compiler/rustc_codegen_ssa/src/mir/operand.rs @@ -116,7 +116,9 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> { let b_llval = bx.const_usize((end - start) as u64); OperandValue::Pair(a_llval, b_llval) } - ConstValue::ByRef { alloc, offset } => { + ConstValue::ByRef { alloc_id, offset } => { + let alloc = bx.tcx().global_alloc(alloc_id).unwrap_memory(); + // FIXME: should we attempt to avoid building the same AllocId multiple times? return Self::from_const_alloc(bx, layout, alloc, offset); } }; diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index 369dc8821d646..7f336daa28a9a 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -148,10 +148,7 @@ pub(super) fn op_to_const<'tcx>( let to_const_value = |mplace: &MPlaceTy<'_>| { debug!("to_const_value(mplace: {:?})", mplace); match mplace.ptr().into_parts() { - (Some(alloc_id), offset) => { - let alloc = ecx.tcx.global_alloc(alloc_id).unwrap_memory(); - ConstValue::ByRef { alloc, offset } - } + (Some(alloc_id), offset) => ConstValue::ByRef { alloc_id, offset }, (None, offset) => { assert!(mplace.layout.is_zst()); assert_eq!( diff --git a/compiler/rustc_const_eval/src/interpret/intern.rs b/compiler/rustc_const_eval/src/interpret/intern.rs index 562f7c610fd65..8c0009cfdfd02 100644 --- a/compiler/rustc_const_eval/src/interpret/intern.rs +++ b/compiler/rustc_const_eval/src/interpret/intern.rs @@ -24,8 +24,8 @@ use rustc_middle::ty::{self, layout::TyAndLayout, Ty}; use rustc_ast::Mutability; use super::{ - AllocId, Allocation, ConstAllocation, InterpCx, MPlaceTy, Machine, MemoryKind, PlaceTy, - Projectable, ValueVisitor, + AllocId, Allocation, InterpCx, MPlaceTy, Machine, MemoryKind, PlaceTy, Projectable, + ValueVisitor, }; use crate::const_eval; use crate::errors::{DanglingPtrInFinal, UnsupportedUntypedPointer}; @@ -455,7 +455,7 @@ impl<'mir, 'tcx: 'mir, M: super::intern::CompileTimeMachine<'mir, 'tcx, !>> { /// A helper function that allocates memory for the layout given and gives you access to mutate /// it. Once your own mutation code is done, the backing `Allocation` is removed from the - /// current `Memory` and returned. + /// current `Memory` and interned as read-only into the global memory. pub fn intern_with_temp_alloc( &mut self, layout: TyAndLayout<'tcx>, @@ -463,11 +463,15 @@ impl<'mir, 'tcx: 'mir, M: super::intern::CompileTimeMachine<'mir, 'tcx, !>> &mut InterpCx<'mir, 'tcx, M>, &PlaceTy<'tcx, M::Provenance>, ) -> InterpResult<'tcx, ()>, - ) -> InterpResult<'tcx, ConstAllocation<'tcx>> { + ) -> InterpResult<'tcx, AllocId> { + // `allocate` picks a fresh AllocId that we will associate with its data below. let dest = self.allocate(layout, MemoryKind::Stack)?; f(self, &dest.clone().into())?; let mut alloc = self.memory.alloc_map.remove(&dest.ptr().provenance.unwrap()).unwrap().1; alloc.mutability = Mutability::Not; - Ok(self.tcx.mk_const_alloc(alloc)) + let alloc = self.tcx.mk_const_alloc(alloc); + let alloc_id = dest.ptr().provenance.unwrap(); // this was just allocated, it must have provenance + self.tcx.set_alloc_id_memory(alloc_id, alloc); + Ok(alloc_id) } } diff --git a/compiler/rustc_const_eval/src/interpret/operand.rs b/compiler/rustc_const_eval/src/interpret/operand.rs index 26dd9098bb5d6..f27c5bee8b52e 100644 --- a/compiler/rustc_const_eval/src/interpret/operand.rs +++ b/compiler/rustc_const_eval/src/interpret/operand.rs @@ -756,11 +756,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }; let layout = from_known_layout(self.tcx, self.param_env, layout, || self.layout_of(ty))?; let op = match val_val { - ConstValue::ByRef { alloc, offset } => { - let id = self.tcx.create_memory_alloc(alloc); + ConstValue::ByRef { alloc_id, offset } => { // We rely on mutability being set correctly in that allocation to prevent writes // where none should happen. - let ptr = self.global_base_pointer(Pointer::new(id, offset))?; + let ptr = self.global_base_pointer(Pointer::new(alloc_id, offset))?; Operand::Indirect(MemPlace::from_ptr(ptr.into())) } ConstValue::Scalar(x) => Operand::Immediate(adjust_scalar(x)?.into()), diff --git a/compiler/rustc_middle/src/mir/interpret/value.rs b/compiler/rustc_middle/src/mir/interpret/value.rs index 5345a65880328..634a22d5dd649 100644 --- a/compiler/rustc_middle/src/mir/interpret/value.rs +++ b/compiler/rustc_middle/src/mir/interpret/value.rs @@ -43,9 +43,13 @@ pub enum ConstValue<'tcx> { /// A value not represented/representable by `Scalar` or `Slice` ByRef { - /// The backing memory of the value, may contain more memory than needed for just the value - /// in order to share `ConstAllocation`s between values - alloc: ConstAllocation<'tcx>, + /// The backing memory of the value. May contain more memory than needed for just the value + /// if this points into some other larger ConstValue. + /// + /// We use an `AllocId` here instead of a `ConstAllocation<'tcx>` to make sure that when a + /// raw constant (which is basically just an `AllocId`) is turned into a `ConstValue` and + /// back, we can preserve the original `AllocId`. + alloc_id: AllocId, /// Offset into `alloc` offset: Size, }, diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 3b22ecfbe50c8..54eb6cc7b497a 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -2914,8 +2914,9 @@ fn pretty_print_const_value<'tcx>( _ => {} } } - (ConstValue::ByRef { alloc, offset }, ty::Array(t, n)) if *t == u8_type => { + (ConstValue::ByRef { alloc_id, offset }, ty::Array(t, n)) if *t == u8_type => { let n = n.try_to_target_usize(tcx).unwrap(); + let alloc = tcx.global_alloc(alloc_id).unwrap_memory(); // cast is ok because we already checked for pointer size (32 or 64 bit) above let range = AllocRange { start: offset, size: Size::from_bytes(n) }; let byte_str = alloc.inner().get_bytes_strip_provenance(&tcx, range).unwrap(); diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index 6b23a7f5bff5b..4b4efe64db2cd 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -701,15 +701,15 @@ pub fn write_allocations<'tcx>( fn alloc_ids_from_const_val(val: ConstValue<'_>) -> impl Iterator + '_ { match val { ConstValue::Scalar(interpret::Scalar::Ptr(ptr, _)) => { - Either::Left(Either::Left(std::iter::once(ptr.provenance))) + Either::Left(std::iter::once(ptr.provenance)) } - ConstValue::Scalar(interpret::Scalar::Int { .. }) => { - Either::Left(Either::Right(std::iter::empty())) - } - ConstValue::ZeroSized => Either::Left(Either::Right(std::iter::empty())), - ConstValue::ByRef { alloc, .. } | ConstValue::Slice { data: alloc, .. } => { - Either::Right(alloc_ids_from_alloc(alloc)) + ConstValue::Scalar(interpret::Scalar::Int { .. }) => Either::Right(std::iter::empty()), + ConstValue::ZeroSized => Either::Right(std::iter::empty()), + ConstValue::Slice { .. } => { + // `u8`/`str` slices, shouldn't contain pointers that we want to print. + Either::Right(std::iter::empty()) } + ConstValue::ByRef { alloc_id, .. } => Either::Left(std::iter::once(alloc_id)), } } struct CollectAllocIds(BTreeSet); diff --git a/compiler/rustc_middle/src/ty/structural_impls.rs b/compiler/rustc_middle/src/ty/structural_impls.rs index 99a19f01b2b22..1b29a83f23eba 100644 --- a/compiler/rustc_middle/src/ty/structural_impls.rs +++ b/compiler/rustc_middle/src/ty/structural_impls.rs @@ -510,6 +510,7 @@ TrivialTypeTraversalAndLiftImpls! { ::rustc_span::symbol::Ident, ::rustc_errors::ErrorGuaranteed, interpret::Scalar, + interpret::AllocId, rustc_target::abi::Size, ty::BoundVar, } diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index 0c44bccbb4f43..0ffd64904ecfb 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -535,7 +535,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { } trace!("replacing {:?} with {:?}", place, value); - // FIXME> figure out what to do when read_immediate_raw fails + // FIXME: figure out what to do when read_immediate_raw fails let imm = self.ecx.read_immediate_raw(&value).ok()?; let Right(imm) = imm else { return None }; @@ -544,7 +544,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { Some(ConstantKind::from_scalar(self.tcx, scalar, value.layout.ty)) } Immediate::ScalarPair(l, r) if l.try_to_int().is_ok() && r.try_to_int().is_ok() => { - let alloc = self + let alloc_id = self .ecx .intern_with_temp_alloc(value.layout, |ecx, dest| { ecx.write_immediate(*imm, dest) @@ -552,7 +552,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { .ok()?; Some(ConstantKind::Val( - ConstValue::ByRef { alloc, offset: Size::ZERO }, + ConstValue::ByRef { alloc_id, offset: Size::ZERO }, value.layout.ty, )) } diff --git a/compiler/rustc_mir_transform/src/large_enums.rs b/compiler/rustc_mir_transform/src/large_enums.rs index 19108dabdf417..6948956521274 100644 --- a/compiler/rustc_mir_transform/src/large_enums.rs +++ b/compiler/rustc_mir_transform/src/large_enums.rs @@ -139,7 +139,6 @@ impl EnumSizeOpt { let (adt_def, num_variants, alloc_id) = self.candidate(tcx, param_env, ty, &mut alloc_cache)?; - let alloc = tcx.global_alloc(alloc_id).unwrap_memory(); let tmp_ty = Ty::new_array(tcx, tcx.types.usize, num_variants as u64); @@ -154,7 +153,7 @@ impl EnumSizeOpt { span, user_ty: None, literal: ConstantKind::Val( - interpret::ConstValue::ByRef { alloc, offset: Size::ZERO }, + interpret::ConstValue::ByRef { alloc_id, offset: Size::ZERO }, tmp_ty, ), }; diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 8cbb68fc8c1b3..7a36e787c82b8 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -1470,8 +1470,9 @@ fn collect_const_value<'tcx>( ) { match value { ConstValue::Scalar(Scalar::Ptr(ptr, _size)) => collect_alloc(tcx, ptr.provenance, output), - ConstValue::Slice { data: alloc, start: _, end: _ } | ConstValue::ByRef { alloc, .. } => { - for &id in alloc.inner().provenance().ptrs().values() { + ConstValue::ByRef { alloc_id, .. } => collect_alloc(tcx, alloc_id, output), + ConstValue::Slice { data, start: _, end: _ } => { + for &id in data.inner().provenance().ptrs().values() { collect_alloc(tcx, id, output); } } diff --git a/compiler/rustc_smir/src/rustc_smir/alloc.rs b/compiler/rustc_smir/src/rustc_smir/alloc.rs index 166c8bda9e10a..3c50d5d2ddd28 100644 --- a/compiler/rustc_smir/src/rustc_smir/alloc.rs +++ b/compiler/rustc_smir/src/rustc_smir/alloc.rs @@ -72,7 +72,8 @@ pub fn new_allocation<'tcx>( .unwrap(); allocation.stable(tables) } - ConstValue::ByRef { alloc, offset } => { + ConstValue::ByRef { alloc_id, offset } => { + let alloc = tables.tcx.global_alloc(alloc_id).unwrap_memory(); let ty_size = tables .tcx .layout_of(rustc_middle::ty::ParamEnv::reveal_all().and(ty)) diff --git a/src/tools/clippy/clippy_utils/src/consts.rs b/src/tools/clippy/clippy_utils/src/consts.rs index adeb673b6b9dc..03341feffb8cd 100644 --- a/src/tools/clippy/clippy_utils/src/consts.rs +++ b/src/tools/clippy/clippy_utils/src/consts.rs @@ -684,34 +684,37 @@ pub fn miri_to_const<'tcx>(lcx: &LateContext<'tcx>, result: mir::ConstantKind<'t }, _ => None, }, - mir::ConstantKind::Val(ConstValue::ByRef { alloc, offset: _ }, _) => match result.ty().kind() { - ty::Adt(adt_def, _) if adt_def.is_struct() => Some(Constant::Adt(result)), - ty::Array(sub_type, len) => match sub_type.kind() { - ty::Float(FloatTy::F32) => match len.try_to_target_usize(lcx.tcx) { - Some(len) => alloc - .inner() - .inspect_with_uninit_and_ptr_outside_interpreter(0..(4 * usize::try_from(len).unwrap())) - .to_owned() - .array_chunks::<4>() - .map(|&chunk| Some(Constant::F32(f32::from_le_bytes(chunk)))) - .collect::>>>() - .map(Constant::Vec), - _ => None, - }, - ty::Float(FloatTy::F64) => match len.try_to_target_usize(lcx.tcx) { - Some(len) => alloc - .inner() - .inspect_with_uninit_and_ptr_outside_interpreter(0..(8 * usize::try_from(len).unwrap())) - .to_owned() - .array_chunks::<8>() - .map(|&chunk| Some(Constant::F64(f64::from_le_bytes(chunk)))) - .collect::>>>() - .map(Constant::Vec), + mir::ConstantKind::Val(ConstValue::ByRef { alloc_id, offset: _ }, _) => { + let alloc = lcx.tcx.global_alloc(alloc_id).unwrap_memory(); + match result.ty().kind() { + ty::Adt(adt_def, _) if adt_def.is_struct() => Some(Constant::Adt(result)), + ty::Array(sub_type, len) => match sub_type.kind() { + ty::Float(FloatTy::F32) => match len.try_to_target_usize(lcx.tcx) { + Some(len) => alloc + .inner() + .inspect_with_uninit_and_ptr_outside_interpreter(0..(4 * usize::try_from(len).unwrap())) + .to_owned() + .array_chunks::<4>() + .map(|&chunk| Some(Constant::F32(f32::from_le_bytes(chunk)))) + .collect::>>>() + .map(Constant::Vec), + _ => None, + }, + ty::Float(FloatTy::F64) => match len.try_to_target_usize(lcx.tcx) { + Some(len) => alloc + .inner() + .inspect_with_uninit_and_ptr_outside_interpreter(0..(8 * usize::try_from(len).unwrap())) + .to_owned() + .array_chunks::<8>() + .map(|&chunk| Some(Constant::F64(f64::from_le_bytes(chunk)))) + .collect::>>>() + .map(Constant::Vec), + _ => None, + }, _ => None, }, _ => None, - }, - _ => None, + } }, _ => None, } From 0f8908da27329eea3a1728492b6e842a1947211b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 12 Sep 2023 07:49:25 +0200 Subject: [PATCH 2/7] =?UTF-8?q?cleanup=20op=5Fto=5Fconst=20a=20bit;=20rena?= =?UTF-8?q?me=20ConstValue::ByRef=20=E2=86=92=20Indirect?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../rustc_codegen_cranelift/src/constant.rs | 4 +- .../src/intrinsics/simd.rs | 2 +- compiler/rustc_codegen_ssa/src/mir/operand.rs | 2 +- .../src/const_eval/eval_queries.rs | 39 +++++++------------ .../rustc_const_eval/src/interpret/operand.rs | 2 +- .../rustc_middle/src/mir/interpret/value.rs | 14 ++++--- compiler/rustc_middle/src/mir/mod.rs | 2 +- compiler/rustc_middle/src/mir/pretty.rs | 8 +++- .../rustc_mir_transform/src/const_prop.rs | 4 +- .../rustc_mir_transform/src/large_enums.rs | 2 +- compiler/rustc_monomorphize/src/collector.rs | 2 +- compiler/rustc_smir/src/rustc_smir/alloc.rs | 2 +- .../internal_lints/unnecessary_def_path.rs | 2 +- src/tools/clippy/clippy_utils/src/consts.rs | 2 +- 14 files changed, 40 insertions(+), 47 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/constant.rs b/compiler/rustc_codegen_cranelift/src/constant.rs index a60964f0f757a..12e492da6e947 100644 --- a/compiler/rustc_codegen_cranelift/src/constant.rs +++ b/compiler/rustc_codegen_cranelift/src/constant.rs @@ -116,7 +116,7 @@ pub(crate) fn codegen_const_value<'tcx>( } match const_val { - ConstValue::ZeroSized => unreachable!(), // we already handles ZST above + ConstValue::ZeroSized => unreachable!(), // we already handled ZST above ConstValue::Scalar(x) => match x { Scalar::Int(int) => { if fx.clif_type(layout.ty).is_some() { @@ -200,7 +200,7 @@ pub(crate) fn codegen_const_value<'tcx>( CValue::by_val(val, layout) } }, - ConstValue::ByRef { alloc_id, offset } => { + ConstValue::Indirect { alloc_id, offset } => { let alloc = fx.tcx.global_alloc(alloc_id).unwrap_memory(); // FIXME: avoid creating multiple allocations for the same AllocId? CValue::by_ref( diff --git a/compiler/rustc_codegen_cranelift/src/intrinsics/simd.rs b/compiler/rustc_codegen_cranelift/src/intrinsics/simd.rs index e17d587076f9f..c64a4008996d5 100644 --- a/compiler/rustc_codegen_cranelift/src/intrinsics/simd.rs +++ b/compiler/rustc_codegen_cranelift/src/intrinsics/simd.rs @@ -172,7 +172,7 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>( .expect("simd_shuffle idx not const"); let idx_bytes = match idx_const { - ConstValue::ByRef { alloc_id, offset } => { + ConstValue::Indirect { alloc_id, offset } => { let alloc = fx.tcx.global_alloc(alloc_id).unwrap_memory(); let size = Size::from_bytes( 4 * ret_lane_count, /* size_of([u32; ret_lane_count]) */ diff --git a/compiler/rustc_codegen_ssa/src/mir/operand.rs b/compiler/rustc_codegen_ssa/src/mir/operand.rs index 18c3fd0f1d12c..d3101df7a6a37 100644 --- a/compiler/rustc_codegen_ssa/src/mir/operand.rs +++ b/compiler/rustc_codegen_ssa/src/mir/operand.rs @@ -116,7 +116,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> { let b_llval = bx.const_usize((end - start) as u64); OperandValue::Pair(a_llval, b_llval) } - ConstValue::ByRef { alloc_id, offset } => { + ConstValue::Indirect { alloc_id, offset } => { let alloc = bx.tcx().global_alloc(alloc_id).unwrap_memory(); // FIXME: should we attempt to avoid building the same AllocId multiple times? return Self::from_const_alloc(bx, layout, alloc, offset); diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index 7f336daa28a9a..d4b7d6f4e2977 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -112,13 +112,13 @@ pub(super) fn op_to_const<'tcx>( ecx: &CompileTimeEvalContext<'_, 'tcx>, op: &OpTy<'tcx>, ) -> ConstValue<'tcx> { + // Handle ZST consistently and early. + if op.layout.is_zst() { + return ConstValue::ZeroSized; + } + // We do not have value optimizations for everything. // Only scalars and slices, since they are very common. - // Note that further down we turn scalars of uninitialized bits back to `ByRef`. These can result - // from scalar unions that are initialized with one of their zero sized variants. We could - // instead allow `ConstValue::Scalar` to store `ScalarMaybeUninit`, but that would affect all - // the usual cases of extracting e.g. a `usize`, without there being a real use case for the - // `Undef` situation. let try_as_immediate = match op.layout.abi { Abi::Scalar(abi::Scalar::Initialized { .. }) => true, Abi::ScalarPair(..) => match op.layout.ty.kind() { @@ -134,7 +134,7 @@ pub(super) fn op_to_const<'tcx>( let immediate = if try_as_immediate { Right(ecx.read_immediate(op).expect("normalization works on validated constants")) } else { - // It is guaranteed that any non-slice scalar pair is actually ByRef here. + // It is guaranteed that any non-slice scalar pair is actually `Indirect` here. // When we come back from raw const eval, we are always by-ref. The only way our op here is // by-val is if we are in destructure_mir_constant, i.e., if this is (a field of) something that we // "tried to make immediate" before. We wouldn't do that for non-slice scalar pairs or @@ -144,28 +144,15 @@ pub(super) fn op_to_const<'tcx>( debug!(?immediate); - // We know `offset` is relative to the allocation, so we can use `into_parts`. - let to_const_value = |mplace: &MPlaceTy<'_>| { - debug!("to_const_value(mplace: {:?})", mplace); - match mplace.ptr().into_parts() { - (Some(alloc_id), offset) => ConstValue::ByRef { alloc_id, offset }, - (None, offset) => { - assert!(mplace.layout.is_zst()); - assert_eq!( - offset.bytes() % mplace.layout.align.abi.bytes(), - 0, - "this MPlaceTy must come from a validated constant, thus we can assume the \ - alignment is correct", - ); - ConstValue::ZeroSized - } - } - }; match immediate { - Left(ref mplace) => to_const_value(mplace), + Left(ref mplace) => { + // We know `offset` is relative to the allocation, so we can use `into_parts`. + let (alloc_id, offset) = mplace.ptr().into_parts(); + let alloc_id = alloc_id.expect("cannot have `fake` place fot non-ZST type"); + ConstValue::Indirect { alloc_id, offset } + } // see comment on `let try_as_immediate` above Right(imm) => match *imm { - _ if imm.layout.is_zst() => ConstValue::ZeroSized, Immediate::Scalar(x) => ConstValue::Scalar(x), Immediate::ScalarPair(a, b) => { debug!("ScalarPair(a: {:?}, b: {:?})", a, b); @@ -186,7 +173,7 @@ pub(super) fn op_to_const<'tcx>( let len: usize = len.try_into().unwrap(); ConstValue::Slice { data, start, end: start + len } } - Immediate::Uninit => to_const_value(&op.assert_mem_place()), + Immediate::Uninit => bug!("`Uninit` is not a valid value for {}", op.layout.ty), }, } } diff --git a/compiler/rustc_const_eval/src/interpret/operand.rs b/compiler/rustc_const_eval/src/interpret/operand.rs index f27c5bee8b52e..d61a360ef0f46 100644 --- a/compiler/rustc_const_eval/src/interpret/operand.rs +++ b/compiler/rustc_const_eval/src/interpret/operand.rs @@ -756,7 +756,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }; let layout = from_known_layout(self.tcx, self.param_env, layout, || self.layout_of(ty))?; let op = match val_val { - ConstValue::ByRef { alloc_id, offset } => { + ConstValue::Indirect { alloc_id, offset } => { // We rely on mutability being set correctly in that allocation to prevent writes // where none should happen. let ptr = self.global_base_pointer(Pointer::new(alloc_id, offset))?; diff --git a/compiler/rustc_middle/src/mir/interpret/value.rs b/compiler/rustc_middle/src/mir/interpret/value.rs index 634a22d5dd649..a77d16e42e994 100644 --- a/compiler/rustc_middle/src/mir/interpret/value.rs +++ b/compiler/rustc_middle/src/mir/interpret/value.rs @@ -30,19 +30,21 @@ pub struct ConstAlloc<'tcx> { #[derive(Copy, Clone, Debug, Eq, PartialEq, TyEncodable, TyDecodable, Hash)] #[derive(HashStable, Lift)] pub enum ConstValue<'tcx> { - /// Used only for types with `layout::abi::Scalar` ABI. + /// Used for types with `layout::abi::Scalar` ABI. /// /// Not using the enum `Value` to encode that this must not be `Uninit`. Scalar(Scalar), - /// Only used for ZSTs. + /// Only for ZSTs. ZeroSized, - /// Used only for `&[u8]` and `&str` + /// Used for `&[u8]` and `&str`. + /// + /// This is worth the optimization since Rust has literals of that type. Slice { data: ConstAllocation<'tcx>, start: usize, end: usize }, - /// A value not represented/representable by `Scalar` or `Slice` - ByRef { + /// A value not representable by the other variants; needs to be stored in-memory. + Indirect { /// The backing memory of the value. May contain more memory than needed for just the value /// if this points into some other larger ConstValue. /// @@ -62,7 +64,7 @@ impl<'tcx> ConstValue<'tcx> { #[inline] pub fn try_to_scalar(&self) -> Option> { match *self { - ConstValue::ByRef { .. } | ConstValue::Slice { .. } | ConstValue::ZeroSized => None, + ConstValue::Indirect { .. } | ConstValue::Slice { .. } | ConstValue::ZeroSized => None, ConstValue::Scalar(val) => Some(val), } } diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 54eb6cc7b497a..6e3409f17ca0c 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -2914,7 +2914,7 @@ fn pretty_print_const_value<'tcx>( _ => {} } } - (ConstValue::ByRef { alloc_id, offset }, ty::Array(t, n)) if *t == u8_type => { + (ConstValue::Indirect { alloc_id, offset }, ty::Array(t, n)) if *t == u8_type => { let n = n.try_to_target_usize(tcx).unwrap(); let alloc = tcx.global_alloc(alloc_id).unwrap_memory(); // cast is ok because we already checked for pointer size (32 or 64 bit) above diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index 4b4efe64db2cd..c4fae27a7a537 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -460,7 +460,7 @@ impl<'tcx> Visitor<'tcx> for ExtraComments<'tcx> { ConstValue::ZeroSized => "".to_string(), ConstValue::Scalar(s) => format!("Scalar({s:?})"), ConstValue::Slice { .. } => "Slice(..)".to_string(), - ConstValue::ByRef { .. } => "ByRef(..)".to_string(), + ConstValue::Indirect { .. } => "ByRef(..)".to_string(), }; let fmt_valtree = |valtree: &ty::ValTree<'tcx>| match valtree { @@ -709,7 +709,11 @@ pub fn write_allocations<'tcx>( // `u8`/`str` slices, shouldn't contain pointers that we want to print. Either::Right(std::iter::empty()) } - ConstValue::ByRef { alloc_id, .. } => Either::Left(std::iter::once(alloc_id)), + ConstValue::Indirect { alloc_id, .. } => { + // FIXME: we don't actually want to print all of these, since some are printed nicely directly as values inline in MIR. + // Really we'd want `pretty_print_const_value` to decide which allocations to print, instead of having a separate visitor. + Either::Left(std::iter::once(alloc_id)) + } } } struct CollectAllocIds(BTreeSet); diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index 0ffd64904ecfb..00e3e3a8f9f88 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -143,7 +143,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx> #[inline(always)] fn enforce_alignment(_ecx: &InterpCx<'mir, 'tcx, Self>) -> CheckAlignment { // We do not check for alignment to avoid having to carry an `Align` - // in `ConstValue::ByRef`. + // in `ConstValue::Indirect`. CheckAlignment::No } @@ -552,7 +552,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { .ok()?; Some(ConstantKind::Val( - ConstValue::ByRef { alloc_id, offset: Size::ZERO }, + ConstValue::Indirect { alloc_id, offset: Size::ZERO }, value.layout.ty, )) } diff --git a/compiler/rustc_mir_transform/src/large_enums.rs b/compiler/rustc_mir_transform/src/large_enums.rs index 6948956521274..f6526b5de1edc 100644 --- a/compiler/rustc_mir_transform/src/large_enums.rs +++ b/compiler/rustc_mir_transform/src/large_enums.rs @@ -153,7 +153,7 @@ impl EnumSizeOpt { span, user_ty: None, literal: ConstantKind::Val( - interpret::ConstValue::ByRef { alloc_id, offset: Size::ZERO }, + interpret::ConstValue::Indirect { alloc_id, offset: Size::ZERO }, tmp_ty, ), }; diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 7a36e787c82b8..bf2581bc61e09 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -1470,7 +1470,7 @@ fn collect_const_value<'tcx>( ) { match value { ConstValue::Scalar(Scalar::Ptr(ptr, _size)) => collect_alloc(tcx, ptr.provenance, output), - ConstValue::ByRef { alloc_id, .. } => collect_alloc(tcx, alloc_id, output), + ConstValue::Indirect { alloc_id, .. } => collect_alloc(tcx, alloc_id, output), ConstValue::Slice { data, start: _, end: _ } => { for &id in data.inner().provenance().ptrs().values() { collect_alloc(tcx, id, output); diff --git a/compiler/rustc_smir/src/rustc_smir/alloc.rs b/compiler/rustc_smir/src/rustc_smir/alloc.rs index 3c50d5d2ddd28..f7966564b7f54 100644 --- a/compiler/rustc_smir/src/rustc_smir/alloc.rs +++ b/compiler/rustc_smir/src/rustc_smir/alloc.rs @@ -72,7 +72,7 @@ pub fn new_allocation<'tcx>( .unwrap(); allocation.stable(tables) } - ConstValue::ByRef { alloc_id, offset } => { + ConstValue::Indirect { alloc_id, offset } => { let alloc = tables.tcx.global_alloc(alloc_id).unwrap_memory(); let ty_size = tables .tcx diff --git a/src/tools/clippy/clippy_lints/src/utils/internal_lints/unnecessary_def_path.rs b/src/tools/clippy/clippy_lints/src/utils/internal_lints/unnecessary_def_path.rs index f66f33fee1669..4a5b6fa5c18d6 100644 --- a/src/tools/clippy/clippy_lints/src/utils/internal_lints/unnecessary_def_path.rs +++ b/src/tools/clippy/clippy_lints/src/utils/internal_lints/unnecessary_def_path.rs @@ -232,7 +232,7 @@ fn path_to_matched_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option match cx.tcx.const_eval_poly(def_id).ok()? { - ConstValue::ByRef { alloc, offset } if offset.bytes() == 0 => { + ConstValue::Indirect { alloc, offset } if offset.bytes() == 0 => { read_mir_alloc_def_path(cx, alloc.inner(), cx.tcx.type_of(def_id).instantiate_identity()) }, _ => None, diff --git a/src/tools/clippy/clippy_utils/src/consts.rs b/src/tools/clippy/clippy_utils/src/consts.rs index 03341feffb8cd..61b944667a4ae 100644 --- a/src/tools/clippy/clippy_utils/src/consts.rs +++ b/src/tools/clippy/clippy_utils/src/consts.rs @@ -684,7 +684,7 @@ pub fn miri_to_const<'tcx>(lcx: &LateContext<'tcx>, result: mir::ConstantKind<'t }, _ => None, }, - mir::ConstantKind::Val(ConstValue::ByRef { alloc_id, offset: _ }, _) => { + mir::ConstantKind::Val(ConstValue::Indirect { alloc_id, offset: _ }, _) => { let alloc = lcx.tcx.global_alloc(alloc_id).unwrap_memory(); match result.ty().kind() { ty::Adt(adt_def, _) if adt_def.is_struct() => Some(Constant::Adt(result)), From 430c386821ad0df43360cf41a4bb35ac0f1d2d77 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 12 Sep 2023 08:42:36 +0200 Subject: [PATCH 3/7] make it more clear which functions create fresh AllocId --- .../rustc_codegen_cranelift/src/constant.rs | 24 +++++++++---------- compiler/rustc_codegen_ssa/src/mir/operand.rs | 8 +++++-- .../rustc_const_eval/src/interpret/cast.rs | 4 ++-- .../rustc_const_eval/src/interpret/machine.rs | 2 +- .../rustc_const_eval/src/interpret/memory.rs | 7 ++---- .../rustc_const_eval/src/interpret/operand.rs | 2 +- .../rustc_const_eval/src/interpret/traits.rs | 2 +- .../rustc_middle/src/mir/interpret/mod.rs | 15 ++++++------ compiler/rustc_middle/src/ty/context.rs | 2 +- compiler/rustc_middle/src/ty/vtable.rs | 6 ++--- compiler/rustc_mir_build/src/thir/cx/expr.rs | 2 +- .../rustc_mir_transform/src/large_enums.rs | 2 +- compiler/rustc_smir/src/rustc_smir/alloc.rs | 2 +- src/tools/miri/src/eval.rs | 2 +- src/tools/miri/src/machine.rs | 2 +- src/tools/miri/src/shims/backtrace.rs | 4 ++-- .../miri/src/shims/unix/foreign_items.rs | 2 +- .../miri/src/shims/windows/foreign_items.rs | 2 +- 18 files changed, 45 insertions(+), 45 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/constant.rs b/compiler/rustc_codegen_cranelift/src/constant.rs index 12e492da6e947..8c67760a0b9e3 100644 --- a/compiler/rustc_codegen_cranelift/src/constant.rs +++ b/compiler/rustc_codegen_cranelift/src/constant.rs @@ -3,7 +3,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::mir::interpret::{ - read_target_uint, AllocId, ConstAllocation, ConstValue, ErrorHandled, GlobalAlloc, Scalar, + read_target_uint, AllocId, ConstValue, ErrorHandled, GlobalAlloc, Scalar, }; use cranelift_module::*; @@ -200,17 +200,14 @@ pub(crate) fn codegen_const_value<'tcx>( CValue::by_val(val, layout) } }, - ConstValue::Indirect { alloc_id, offset } => { - let alloc = fx.tcx.global_alloc(alloc_id).unwrap_memory(); - // FIXME: avoid creating multiple allocations for the same AllocId? - CValue::by_ref( - pointer_for_allocation(fx, alloc) - .offset_i64(fx, i64::try_from(offset.bytes()).unwrap()), - layout, - ) - } + ConstValue::Indirect { alloc_id, offset } => CValue::by_ref( + pointer_for_allocation(fx, alloc_id) + .offset_i64(fx, i64::try_from(offset.bytes()).unwrap()), + layout, + ), ConstValue::Slice { data, start, end } => { - let ptr = pointer_for_allocation(fx, data) + let alloc_id = fx.tcx.reserve_and_set_memory_alloc(data); + let ptr = pointer_for_allocation(fx, alloc_id) .offset_i64(fx, i64::try_from(start).unwrap()) .get_addr(fx); let len = fx @@ -224,9 +221,9 @@ pub(crate) fn codegen_const_value<'tcx>( fn pointer_for_allocation<'tcx>( fx: &mut FunctionCx<'_, '_, 'tcx>, - alloc: ConstAllocation<'tcx>, + alloc_id: AllocId, ) -> crate::pointer::Pointer { - let alloc_id = fx.tcx.create_memory_alloc(alloc); + let alloc = fx.tcx.global_alloc(alloc_id).unwrap_memory(); let data_id = data_id_for_alloc_id( &mut fx.constants_cx, &mut *fx.module, @@ -357,6 +354,7 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut Constant unreachable!() } }; + // FIXME: should we have a cache so we don't do this multiple times for the same `ConstAllocation`? let data_id = *cx.anon_allocs.entry(alloc_id).or_insert_with(|| { module.declare_anonymous_data(alloc.inner().mutability.is_mut(), false).unwrap() }); diff --git a/compiler/rustc_codegen_ssa/src/mir/operand.rs b/compiler/rustc_codegen_ssa/src/mir/operand.rs index d3101df7a6a37..1926bb8df52cc 100644 --- a/compiler/rustc_codegen_ssa/src/mir/operand.rs +++ b/compiler/rustc_codegen_ssa/src/mir/operand.rs @@ -105,7 +105,10 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> { bug!("from_const: invalid ScalarPair layout: {:#?}", layout); }; let a = Scalar::from_pointer( - Pointer::new(bx.tcx().create_memory_alloc(data), Size::from_bytes(start)), + Pointer::new( + bx.tcx().reserve_and_set_memory_alloc(data), + Size::from_bytes(start), + ), &bx.tcx(), ); let a_llval = bx.scalar_to_backend( @@ -118,7 +121,6 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> { } ConstValue::Indirect { alloc_id, offset } => { let alloc = bx.tcx().global_alloc(alloc_id).unwrap_memory(); - // FIXME: should we attempt to avoid building the same AllocId multiple times? return Self::from_const_alloc(bx, layout, alloc, offset); } }; @@ -184,6 +186,8 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> { _ if layout.is_zst() => OperandRef::zero_sized(layout), _ => { // Neither a scalar nor scalar pair. Load from a place + // FIXME: should we cache `const_data_from_alloc` to avoid repeating this for the + // same `ConstAllocation`? let init = bx.const_data_from_alloc(alloc); let base_addr = bx.static_addr_of(init, alloc_align, None); diff --git a/compiler/rustc_const_eval/src/interpret/cast.rs b/compiler/rustc_const_eval/src/interpret/cast.rs index 25c74b98611ba..4c826239eca47 100644 --- a/compiler/rustc_const_eval/src/interpret/cast.rs +++ b/compiler/rustc_const_eval/src/interpret/cast.rs @@ -84,7 +84,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ) .ok_or_else(|| err_inval!(TooGeneric))?; - let fn_ptr = self.create_fn_alloc_ptr(FnVal::Instance(instance)); + let fn_ptr = self.fn_ptr(FnVal::Instance(instance)); self.write_pointer(fn_ptr, dest)?; } _ => span_bug!(self.cur_span(), "reify fn pointer on {:?}", src.layout.ty), @@ -116,7 +116,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ty::ClosureKind::FnOnce, ) .ok_or_else(|| err_inval!(TooGeneric))?; - let fn_ptr = self.create_fn_alloc_ptr(FnVal::Instance(instance)); + let fn_ptr = self.fn_ptr(FnVal::Instance(instance)); self.write_pointer(fn_ptr, dest)?; } _ => span_bug!(self.cur_span(), "closure fn pointer on {:?}", src.layout.ty), diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs index 1353b7daf082c..c9fd210241846 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/compiler/rustc_const_eval/src/interpret/machine.rs @@ -555,7 +555,7 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) { def_id: DefId, ) -> InterpResult<$tcx, Pointer> { // Use the `AllocId` associated with the `DefId`. Any actual *access* will fail. - Ok(Pointer::new(ecx.tcx.create_static_alloc(def_id), Size::ZERO)) + Ok(Pointer::new(ecx.tcx.reserve_and_set_static_alloc(def_id), Size::ZERO)) } #[inline(always)] diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index 11bffedf54a61..436c4d521a51c 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -176,12 +176,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { M::adjust_alloc_base_pointer(self, ptr) } - pub fn create_fn_alloc_ptr( - &mut self, - fn_val: FnVal<'tcx, M::ExtraFnVal>, - ) -> Pointer { + pub fn fn_ptr(&mut self, fn_val: FnVal<'tcx, M::ExtraFnVal>) -> Pointer { let id = match fn_val { - FnVal::Instance(instance) => self.tcx.create_fn_alloc(instance), + FnVal::Instance(instance) => self.tcx.reserve_and_set_fn_alloc(instance), FnVal::Other(extra) => { // FIXME(RalfJung): Should we have a cache here? let id = self.tcx.reserve_alloc_id(); diff --git a/compiler/rustc_const_eval/src/interpret/operand.rs b/compiler/rustc_const_eval/src/interpret/operand.rs index d61a360ef0f46..8b33dbb677fad 100644 --- a/compiler/rustc_const_eval/src/interpret/operand.rs +++ b/compiler/rustc_const_eval/src/interpret/operand.rs @@ -768,7 +768,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // We rely on mutability being set correctly in `data` to prevent writes // where none should happen. let ptr = Pointer::new( - self.tcx.create_memory_alloc(data), + self.tcx.reserve_and_set_memory_alloc(data), Size::from_bytes(start), // offset: `start` ); Operand::Immediate(Immediate::new_slice( diff --git a/compiler/rustc_const_eval/src/interpret/traits.rs b/compiler/rustc_const_eval/src/interpret/traits.rs index fa15d466ac125..a9ca268a2a96f 100644 --- a/compiler/rustc_const_eval/src/interpret/traits.rs +++ b/compiler/rustc_const_eval/src/interpret/traits.rs @@ -27,7 +27,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ensure_monomorphic_enough(*self.tcx, ty)?; ensure_monomorphic_enough(*self.tcx, poly_trait_ref)?; - let vtable_symbolic_allocation = self.tcx.create_vtable_alloc(ty, poly_trait_ref); + let vtable_symbolic_allocation = self.tcx.reserve_and_set_vtable_alloc(ty, poly_trait_ref); let vtable_ptr = self.global_base_pointer(Pointer::from(vtable_symbolic_allocation))?; Ok(vtable_ptr.into()) } diff --git a/compiler/rustc_middle/src/mir/interpret/mod.rs b/compiler/rustc_middle/src/mir/interpret/mod.rs index 3543158bf82d2..f03a28d210dc1 100644 --- a/compiler/rustc_middle/src/mir/interpret/mod.rs +++ b/compiler/rustc_middle/src/mir/interpret/mod.rs @@ -389,7 +389,7 @@ impl<'s> AllocDecodingSession<'s> { trace!("creating fn alloc ID"); let instance = ty::Instance::decode(decoder); trace!("decoded fn alloc instance: {:?}", instance); - let alloc_id = decoder.interner().create_fn_alloc(instance); + let alloc_id = decoder.interner().reserve_and_set_fn_alloc(instance); alloc_id } AllocDiscriminant::VTable => { @@ -399,7 +399,8 @@ impl<'s> AllocDecodingSession<'s> { let poly_trait_ref = > as Decodable>::decode(decoder); trace!("decoded vtable alloc instance: {ty:?}, {poly_trait_ref:?}"); - let alloc_id = decoder.interner().create_vtable_alloc(ty, poly_trait_ref); + let alloc_id = + decoder.interner().reserve_and_set_vtable_alloc(ty, poly_trait_ref); alloc_id } AllocDiscriminant::Static => { @@ -407,7 +408,7 @@ impl<'s> AllocDecodingSession<'s> { trace!("creating extern static alloc ID"); let did = >::decode(decoder); trace!("decoded static def-ID: {:?}", did); - let alloc_id = decoder.interner().create_static_alloc(did); + let alloc_id = decoder.interner().reserve_and_set_static_alloc(did); alloc_id } } @@ -544,13 +545,13 @@ impl<'tcx> TyCtxt<'tcx> { /// Generates an `AllocId` for a static or return a cached one in case this function has been /// called on the same static before. - pub fn create_static_alloc(self, static_id: DefId) -> AllocId { + pub fn reserve_and_set_static_alloc(self, static_id: DefId) -> AllocId { self.reserve_and_set_dedup(GlobalAlloc::Static(static_id)) } /// Generates an `AllocId` for a function. Depending on the function type, /// this might get deduplicated or assigned a new ID each time. - pub fn create_fn_alloc(self, instance: Instance<'tcx>) -> AllocId { + pub fn reserve_and_set_fn_alloc(self, instance: Instance<'tcx>) -> AllocId { // Functions cannot be identified by pointers, as asm-equal functions can get deduplicated // by the linker (we set the "unnamed_addr" attribute for LLVM) and functions can be // duplicated across crates. @@ -575,7 +576,7 @@ impl<'tcx> TyCtxt<'tcx> { } /// Generates an `AllocId` for a (symbolic, not-reified) vtable. Will get deduplicated. - pub fn create_vtable_alloc( + pub fn reserve_and_set_vtable_alloc( self, ty: Ty<'tcx>, poly_trait_ref: Option>, @@ -588,7 +589,7 @@ impl<'tcx> TyCtxt<'tcx> { /// Statics with identical content will still point to the same `Allocation`, i.e., /// their data will be deduplicated through `Allocation` interning -- but they /// are different places in memory and as such need different IDs. - pub fn create_memory_alloc(self, mem: ConstAllocation<'tcx>) -> AllocId { + pub fn reserve_and_set_memory_alloc(self, mem: ConstAllocation<'tcx>) -> AllocId { let id = self.reserve_alloc_id(); self.set_alloc_id_memory(id, mem); id diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index eeb87d5561dbc..f748404875759 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -647,7 +647,7 @@ impl<'tcx> TyCtxt<'tcx> { // Create an allocation that just contains these bytes. let alloc = interpret::Allocation::from_bytes_byte_aligned_immutable(bytes); let alloc = self.mk_const_alloc(alloc); - self.create_memory_alloc(alloc) + self.reserve_and_set_memory_alloc(alloc) } /// Returns a range of the start/end indices specified with the diff --git a/compiler/rustc_middle/src/ty/vtable.rs b/compiler/rustc_middle/src/ty/vtable.rs index 97402caa0013b..62f41921d888a 100644 --- a/compiler/rustc_middle/src/ty/vtable.rs +++ b/compiler/rustc_middle/src/ty/vtable.rs @@ -84,7 +84,7 @@ pub(super) fn vtable_allocation_provider<'tcx>( let scalar = match entry { VtblEntry::MetadataDropInPlace => { let instance = ty::Instance::resolve_drop_in_place(tcx, ty); - let fn_alloc_id = tcx.create_fn_alloc(instance); + let fn_alloc_id = tcx.reserve_and_set_fn_alloc(instance); let fn_ptr = Pointer::from(fn_alloc_id); Scalar::from_pointer(fn_ptr, &tcx) } @@ -94,7 +94,7 @@ pub(super) fn vtable_allocation_provider<'tcx>( VtblEntry::Method(instance) => { // Prepare the fn ptr we write into the vtable. let instance = instance.polymorphize(tcx); - let fn_alloc_id = tcx.create_fn_alloc(instance); + let fn_alloc_id = tcx.reserve_and_set_fn_alloc(instance); let fn_ptr = Pointer::from(fn_alloc_id); Scalar::from_pointer(fn_ptr, &tcx) } @@ -112,5 +112,5 @@ pub(super) fn vtable_allocation_provider<'tcx>( } vtable.mutability = Mutability::Not; - tcx.create_memory_alloc(tcx.mk_const_alloc(vtable)) + tcx.reserve_and_set_memory_alloc(tcx.mk_const_alloc(vtable)) } diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index 6c1f7d7a60667..18b0e9a643e8a 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -950,7 +950,7 @@ impl<'tcx> Cx<'tcx> { let kind = if self.tcx.is_thread_local_static(id) { ExprKind::ThreadLocalRef(id) } else { - let alloc_id = self.tcx.create_static_alloc(id); + let alloc_id = self.tcx.reserve_and_set_static_alloc(id); ExprKind::StaticRef { alloc_id, ty, def_id: id } }; ExprKind::Deref { diff --git a/compiler/rustc_mir_transform/src/large_enums.rs b/compiler/rustc_mir_transform/src/large_enums.rs index f6526b5de1edc..8afbe418502c5 100644 --- a/compiler/rustc_mir_transform/src/large_enums.rs +++ b/compiler/rustc_mir_transform/src/large_enums.rs @@ -114,7 +114,7 @@ impl EnumSizeOpt { tcx.data_layout.ptr_sized_integer().align(&tcx.data_layout).abi, Mutability::Not, ); - let alloc = tcx.create_memory_alloc(tcx.mk_const_alloc(alloc)); + let alloc = tcx.reserve_and_set_memory_alloc(tcx.mk_const_alloc(alloc)); Some((*adt_def, num_discrs, *alloc_cache.entry(ty).or_insert(alloc))) } fn optim<'tcx>(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { diff --git a/compiler/rustc_smir/src/rustc_smir/alloc.rs b/compiler/rustc_smir/src/rustc_smir/alloc.rs index f7966564b7f54..35e65c19be05b 100644 --- a/compiler/rustc_smir/src/rustc_smir/alloc.rs +++ b/compiler/rustc_smir/src/rustc_smir/alloc.rs @@ -45,7 +45,7 @@ pub fn new_allocation<'tcx>( new_empty_allocation(align.abi) } ConstValue::Slice { data, start, end } => { - let alloc_id = tables.tcx.create_memory_alloc(data); + let alloc_id = tables.tcx.reserve_and_set_memory_alloc(data); let ptr = Pointer::new(alloc_id, rustc_target::abi::Size::from_bytes(start)); let scalar_ptr = rustc_middle::mir::interpret::Scalar::from_pointer(ptr, &tables.tcx); let scalar_len = rustc_middle::mir::interpret::Scalar::from_target_usize( diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index beb13ebdfe6fd..3946ce8ef9df2 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/src/eval.rs @@ -382,7 +382,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( .unwrap() .unwrap(); - let main_ptr = ecx.create_fn_alloc_ptr(FnVal::Instance(entry_instance)); + let main_ptr = ecx.fn_ptr(FnVal::Instance(entry_instance)); // Inlining of `DEFAULT` from // https://github.com/rust-lang/rust/blob/master/compiler/rustc_session/src/config/sigpipe.rs. diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 80f8318044802..ce7f47b5b4fee 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -711,7 +711,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { let layout = this.machine.layouts.const_raw_ptr; let dlsym = Dlsym::from_str("signal".as_bytes(), &this.tcx.sess.target.os)? .expect("`signal` must be an actual dlsym on android"); - let ptr = this.create_fn_alloc_ptr(FnVal::Other(dlsym)); + let ptr = this.fn_ptr(FnVal::Other(dlsym)); let val = ImmTy::from_scalar(Scalar::from_pointer(ptr, this), layout); Self::alloc_extern_static(this, "signal", val)?; // A couple zero-initialized pointer-sized extern statics. diff --git a/src/tools/miri/src/shims/backtrace.rs b/src/tools/miri/src/shims/backtrace.rs index c4c9f77fab7f9..bfec4833ac901 100644 --- a/src/tools/miri/src/shims/backtrace.rs +++ b/src/tools/miri/src/shims/backtrace.rs @@ -63,7 +63,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // to reconstruct the needed frame information in `handle_miri_resolve_frame`. // Note that we never actually read or write anything from/to this pointer - // all of the data is represented by the pointer value itself. - let fn_ptr = this.create_fn_alloc_ptr(FnVal::Instance(instance)); + let fn_ptr = this.fn_ptr(FnVal::Instance(instance)); fn_ptr.wrapping_offset(Size::from_bytes(pos.0), this) }) .collect(); @@ -159,7 +159,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // Reconstruct the original function pointer, // which we pass to user code. - let fn_ptr = this.create_fn_alloc_ptr(FnVal::Instance(fn_instance)); + let fn_ptr = this.fn_ptr(FnVal::Instance(fn_instance)); let num_fields = dest.layout.fields.count(); diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index 865f01931f706..4bcca5076cac2 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -232,7 +232,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let symbol = this.read_pointer(symbol)?; let symbol_name = this.read_c_str(symbol)?; if let Some(dlsym) = Dlsym::from_str(symbol_name, &this.tcx.sess.target.os)? { - let ptr = this.create_fn_alloc_ptr(FnVal::Other(dlsym)); + let ptr = this.fn_ptr(FnVal::Other(dlsym)); this.write_pointer(ptr, dest)?; } else { this.write_null(dest)?; diff --git a/src/tools/miri/src/shims/windows/foreign_items.rs b/src/tools/miri/src/shims/windows/foreign_items.rs index 28999268ba052..d76d01b07891a 100644 --- a/src/tools/miri/src/shims/windows/foreign_items.rs +++ b/src/tools/miri/src/shims/windows/foreign_items.rs @@ -335,7 +335,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { this.read_target_isize(hModule)?; let name = this.read_c_str(this.read_pointer(lpProcName)?)?; if let Some(dlsym) = Dlsym::from_str(name, &this.tcx.sess.target.os)? { - let ptr = this.create_fn_alloc_ptr(FnVal::Other(dlsym)); + let ptr = this.fn_ptr(FnVal::Other(dlsym)); this.write_pointer(ptr, dest)?; } else { this.write_null(dest)?; From 7aa44eee997e51154eb4dd04ee13a08fe7dc53af Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 12 Sep 2023 15:57:40 +0200 Subject: [PATCH 4/7] don't force all slice-typed ConstValue to be ConstValue::Slice --- .../src/const_eval/eval_queries.rs | 65 +++++++++---------- .../rustc_middle/src/mir/interpret/mod.rs | 2 +- .../rustc_middle/src/mir/interpret/value.rs | 23 ++----- 3 files changed, 36 insertions(+), 54 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index d4b7d6f4e2977..454baf2a745b9 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -18,9 +18,9 @@ use super::{CanAccessStatics, CompileTimeEvalContext, CompileTimeInterpreter}; use crate::errors; use crate::interpret::eval_nullary_intrinsic; use crate::interpret::{ - intern_const_alloc_recursive, Allocation, ConstAlloc, ConstValue, CtfeValidationMode, GlobalId, - Immediate, InternKind, InterpCx, InterpError, InterpResult, MPlaceTy, MemoryKind, OpTy, - RefTracking, StackPopCleanup, + intern_const_alloc_recursive, ConstAlloc, ConstValue, CtfeValidationMode, GlobalId, Immediate, + InternKind, InterpCx, InterpError, InterpResult, MPlaceTy, MemoryKind, OpTy, RefTracking, + StackPopCleanup, }; // Returns a pointer to where the result lives @@ -105,8 +105,7 @@ pub(super) fn mk_eval_cx<'mir, 'tcx>( ) } -/// This function converts an interpreter value into a constant that is meant for use in the -/// type system. +/// This function converts an interpreter value into a MIR constant. #[instrument(skip(ecx), level = "debug")] pub(super) fn op_to_const<'tcx>( ecx: &CompileTimeEvalContext<'_, 'tcx>, @@ -117,28 +116,25 @@ pub(super) fn op_to_const<'tcx>( return ConstValue::ZeroSized; } - // We do not have value optimizations for everything. - // Only scalars and slices, since they are very common. - let try_as_immediate = match op.layout.abi { + // All scalar types should be stored as `ConstValue::Scalar`. This is needed to make + // `ConstValue::try_to_scalar` efficient; we want that to work for *all* constants of scalar + // type (it's used throughout the compiler and having it work just on literals is not enough) + // and we want it to be fast (i.e., don't go to an `Allocation` and reconstruct the `Scalar` + // from its byte-serialized form). + let force_as_immediate = match op.layout.abi { Abi::Scalar(abi::Scalar::Initialized { .. }) => true, - Abi::ScalarPair(..) => match op.layout.ty.kind() { - ty::Ref(_, inner, _) => match *inner.kind() { - ty::Slice(elem) => elem == ecx.tcx.types.u8, - ty::Str => true, - _ => false, - }, - _ => false, - }, + // We don't *force* `ConstValue::Slice` for `ScalarPair`. This has the advantage that if the + // input `op` is a place, then turning it into a `ConstValue` and back into a `OpTy` will + // not have to generate any duplicate allocations (we preserve the original `AllocId` in + // `ConstValue::Indirect`). It means accessing the contents of a slice can be slow (since + // they can be stored as `ConstValue::Indirect`), but that's not relevant since we barely + // ever have to do this. (`try_get_slice_bytes_for_diagnostics` exists to provide this + // functionality.) _ => false, }; - let immediate = if try_as_immediate { + let immediate = if force_as_immediate { Right(ecx.read_immediate(op).expect("normalization works on validated constants")) } else { - // It is guaranteed that any non-slice scalar pair is actually `Indirect` here. - // When we come back from raw const eval, we are always by-ref. The only way our op here is - // by-val is if we are in destructure_mir_constant, i.e., if this is (a field of) something that we - // "tried to make immediate" before. We wouldn't do that for non-slice scalar pairs or - // structs containing such. op.as_mplace_or_imm() }; @@ -151,25 +147,22 @@ pub(super) fn op_to_const<'tcx>( let alloc_id = alloc_id.expect("cannot have `fake` place fot non-ZST type"); ConstValue::Indirect { alloc_id, offset } } - // see comment on `let try_as_immediate` above + // see comment on `let force_as_immediate` above Right(imm) => match *imm { Immediate::Scalar(x) => ConstValue::Scalar(x), Immediate::ScalarPair(a, b) => { debug!("ScalarPair(a: {:?}, b: {:?})", a, b); + // FIXME: assert that this has an appropriate type. + // Currently we actually get here for non-[u8] slices during valtree construction! + let msg = "`op_to_const` on an immediate scalar pair must only be used on slice references to actually allocated memory"; // We know `offset` is relative to the allocation, so we can use `into_parts`. - let (data, start) = match a.to_pointer(ecx).unwrap().into_parts() { - (Some(alloc_id), offset) => { - (ecx.tcx.global_alloc(alloc_id).unwrap_memory(), offset.bytes()) - } - (None, _offset) => ( - ecx.tcx.mk_const_alloc(Allocation::from_bytes_byte_aligned_immutable( - b"" as &[u8], - )), - 0, - ), - }; - let len = b.to_target_usize(ecx).unwrap(); - let start = start.try_into().unwrap(); + // We use `ConstValue::Slice` so that we don't have to generate an allocation for + // `ConstValue::Indirect` here. + let (alloc_id, offset) = a.to_pointer(ecx).expect(msg).into_parts(); + let alloc_id = alloc_id.expect(msg); + let data = ecx.tcx.global_alloc(alloc_id).unwrap_memory(); + let start = offset.bytes_usize(); + let len = b.to_target_usize(ecx).expect(msg); let len: usize = len.try_into().unwrap(); ConstValue::Slice { data, start, end: start + len } } diff --git a/compiler/rustc_middle/src/mir/interpret/mod.rs b/compiler/rustc_middle/src/mir/interpret/mod.rs index f03a28d210dc1..44d1dcbbe1791 100644 --- a/compiler/rustc_middle/src/mir/interpret/mod.rs +++ b/compiler/rustc_middle/src/mir/interpret/mod.rs @@ -149,7 +149,7 @@ pub use self::error::{ UnsupportedOpInfo, ValidationErrorInfo, ValidationErrorKind, }; -pub use self::value::{get_slice_bytes, ConstAlloc, ConstValue, Scalar}; +pub use self::value::{ConstAlloc, ConstValue, Scalar}; pub use self::allocation::{ alloc_range, AllocBytes, AllocError, AllocRange, AllocResult, Allocation, ConstAllocation, diff --git a/compiler/rustc_middle/src/mir/interpret/value.rs b/compiler/rustc_middle/src/mir/interpret/value.rs index a77d16e42e994..6207a35e4f315 100644 --- a/compiler/rustc_middle/src/mir/interpret/value.rs +++ b/compiler/rustc_middle/src/mir/interpret/value.rs @@ -12,7 +12,7 @@ use rustc_target::abi::{HasDataLayout, Size}; use crate::ty::{ParamEnv, ScalarInt, Ty, TyCtxt}; use super::{ - AllocId, AllocRange, ConstAllocation, InterpResult, Pointer, PointerArithmetic, Provenance, + AllocId, ConstAllocation, InterpResult, Pointer, PointerArithmetic, Provenance, ScalarSizeMismatch, }; @@ -40,10 +40,14 @@ pub enum ConstValue<'tcx> { /// Used for `&[u8]` and `&str`. /// - /// This is worth the optimization since Rust has literals of that type. + /// This is worth an optimized representation since Rust has literals of these types. + /// Not having to indirect those through an `AllocId` (or two, if we used `Indirect`) has shown + /// measurable performance improvements on stress tests. Slice { data: ConstAllocation<'tcx>, start: usize, end: usize }, /// A value not representable by the other variants; needs to be stored in-memory. + /// + /// Must *not* be used for scalars or ZST, but having `&str` or other slices in this variant is fine. Indirect { /// The backing memory of the value. May contain more memory than needed for just the value /// if this points into some other larger ConstValue. @@ -511,18 +515,3 @@ impl<'tcx, Prov: Provenance> Scalar { Ok(Double::from_bits(self.to_u64()?.into())) } } - -/// Gets the bytes of a constant slice value. -pub fn get_slice_bytes<'tcx>(cx: &impl HasDataLayout, val: ConstValue<'tcx>) -> &'tcx [u8] { - if let ConstValue::Slice { data, start, end } = val { - let len = end - start; - data.inner() - .get_bytes_strip_provenance( - cx, - AllocRange { start: Size::from_bytes(start), size: Size::from_bytes(len) }, - ) - .unwrap_or_else(|err| bug!("const slice is invalid: {:?}", err)) - } else { - bug!("expected const slice, but found another const value"); - } -} From 0a6e263b9f57ffeda85404943475b22302429bfa Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 12 Sep 2023 11:33:58 +0200 Subject: [PATCH 5/7] bless all --- .../checked_add.main.ConstProp.panic-abort.diff | 4 ++++ .../checked_add.main.ConstProp.panic-unwind.diff | 4 ++++ .../indirect.main.ConstProp.panic-abort.diff | 4 ++++ .../indirect.main.ConstProp.panic-unwind.diff | 4 ++++ .../inherit_overflow.main.ConstProp.panic-abort.diff | 4 ++++ ...inherit_overflow.main.ConstProp.panic-unwind.diff | 4 ++++ .../issue_66971.main.ConstProp.panic-abort.diff | 8 ++++++++ .../issue_66971.main.ConstProp.panic-unwind.diff | 8 ++++++++ .../issue_67019.main.ConstProp.panic-abort.diff | 12 ++++++++++++ .../issue_67019.main.ConstProp.panic-unwind.diff | 12 ++++++++++++ .../mutable_variable_aggregate.main.ConstProp.diff | 8 ++++++++ ...ble_unprop_assign.main.ConstProp.panic-abort.diff | 4 ++++ ...le_unprop_assign.main.ConstProp.panic-unwind.diff | 4 ++++ .../return_place.add.ConstProp.panic-abort.diff | 4 ++++ .../return_place.add.ConstProp.panic-unwind.diff | 4 ++++ ...eturn_place.add.PreCodegen.before.panic-abort.mir | 4 ++++ ...turn_place.add.PreCodegen.before.panic-unwind.mir | 4 ++++ ...teral_propagation.main.ConstProp.panic-abort.diff | 12 ++++++++++++ ...eral_propagation.main.ConstProp.panic-unwind.diff | 12 ++++++++++++ tests/mir-opt/enum_opt.cand.EnumSizeOpt.32bit.diff | 4 ++++ tests/mir-opt/enum_opt.cand.EnumSizeOpt.64bit.diff | 4 ++++ tests/mir-opt/enum_opt.unin.EnumSizeOpt.32bit.diff | 4 ++++ tests/mir-opt/enum_opt.unin.EnumSizeOpt.64bit.diff | 4 ++++ ...to_variable.main.ConstProp.32bit.panic-abort.diff | 4 ++++ ...o_variable.main.ConstProp.32bit.panic-unwind.diff | 4 ++++ ...to_variable.main.ConstProp.64bit.panic-abort.diff | 4 ++++ ...o_variable.main.ConstProp.64bit.panic-unwind.diff | 4 ++++ 27 files changed, 152 insertions(+) diff --git a/tests/mir-opt/const_prop/checked_add.main.ConstProp.panic-abort.diff b/tests/mir-opt/const_prop/checked_add.main.ConstProp.panic-abort.diff index 6daef87dd2c73..c2fd7f65f5eea 100644 --- a/tests/mir-opt/const_prop/checked_add.main.ConstProp.panic-abort.diff +++ b/tests/mir-opt/const_prop/checked_add.main.ConstProp.panic-abort.diff @@ -24,5 +24,9 @@ StorageDead(_1); return; } ++ } ++ ++ alloc3 (size: 8, align: 4) { ++ 02 00 00 00 00 __ __ __ │ .....░░░ } diff --git a/tests/mir-opt/const_prop/checked_add.main.ConstProp.panic-unwind.diff b/tests/mir-opt/const_prop/checked_add.main.ConstProp.panic-unwind.diff index 125407bf285a3..21a31f9aba31e 100644 --- a/tests/mir-opt/const_prop/checked_add.main.ConstProp.panic-unwind.diff +++ b/tests/mir-opt/const_prop/checked_add.main.ConstProp.panic-unwind.diff @@ -24,5 +24,9 @@ StorageDead(_1); return; } ++ } ++ ++ alloc3 (size: 8, align: 4) { ++ 02 00 00 00 00 __ __ __ │ .....░░░ } diff --git a/tests/mir-opt/const_prop/indirect.main.ConstProp.panic-abort.diff b/tests/mir-opt/const_prop/indirect.main.ConstProp.panic-abort.diff index ca0ce2888cd6e..c0efc87302925 100644 --- a/tests/mir-opt/const_prop/indirect.main.ConstProp.panic-abort.diff +++ b/tests/mir-opt/const_prop/indirect.main.ConstProp.panic-abort.diff @@ -29,5 +29,9 @@ StorageDead(_1); return; } ++ } ++ ++ alloc3 (size: 2, align: 1) { ++ 03 00 │ .. } diff --git a/tests/mir-opt/const_prop/indirect.main.ConstProp.panic-unwind.diff b/tests/mir-opt/const_prop/indirect.main.ConstProp.panic-unwind.diff index d63fb9255a326..2aee6f164aebb 100644 --- a/tests/mir-opt/const_prop/indirect.main.ConstProp.panic-unwind.diff +++ b/tests/mir-opt/const_prop/indirect.main.ConstProp.panic-unwind.diff @@ -29,5 +29,9 @@ StorageDead(_1); return; } ++ } ++ ++ alloc3 (size: 2, align: 1) { ++ 03 00 │ .. } diff --git a/tests/mir-opt/const_prop/inherit_overflow.main.ConstProp.panic-abort.diff b/tests/mir-opt/const_prop/inherit_overflow.main.ConstProp.panic-abort.diff index 51e17cf690aca..7ba51ccdbf6d4 100644 --- a/tests/mir-opt/const_prop/inherit_overflow.main.ConstProp.panic-abort.diff +++ b/tests/mir-opt/const_prop/inherit_overflow.main.ConstProp.panic-abort.diff @@ -35,5 +35,9 @@ _0 = const (); return; } ++ } ++ ++ alloc3 (size: 2, align: 1) { ++ 00 01 │ .. } diff --git a/tests/mir-opt/const_prop/inherit_overflow.main.ConstProp.panic-unwind.diff b/tests/mir-opt/const_prop/inherit_overflow.main.ConstProp.panic-unwind.diff index 5ef201497fbcd..545b7f22f6e0c 100644 --- a/tests/mir-opt/const_prop/inherit_overflow.main.ConstProp.panic-unwind.diff +++ b/tests/mir-opt/const_prop/inherit_overflow.main.ConstProp.panic-unwind.diff @@ -35,5 +35,9 @@ _0 = const (); return; } ++ } ++ ++ alloc3 (size: 2, align: 1) { ++ 00 01 │ .. } diff --git a/tests/mir-opt/const_prop/issue_66971.main.ConstProp.panic-abort.diff b/tests/mir-opt/const_prop/issue_66971.main.ConstProp.panic-abort.diff index 170c019782df3..18341ba7db923 100644 --- a/tests/mir-opt/const_prop/issue_66971.main.ConstProp.panic-abort.diff +++ b/tests/mir-opt/const_prop/issue_66971.main.ConstProp.panic-abort.diff @@ -18,5 +18,13 @@ StorageDead(_2); return; } ++ } ++ ++ alloc8 (size: 2, align: 1) { ++ 00 00 │ .. ++ } ++ ++ alloc7 (size: 2, align: 1) { ++ 00 00 │ .. } diff --git a/tests/mir-opt/const_prop/issue_66971.main.ConstProp.panic-unwind.diff b/tests/mir-opt/const_prop/issue_66971.main.ConstProp.panic-unwind.diff index 64227dfd78c2c..50763c10f0c22 100644 --- a/tests/mir-opt/const_prop/issue_66971.main.ConstProp.panic-unwind.diff +++ b/tests/mir-opt/const_prop/issue_66971.main.ConstProp.panic-unwind.diff @@ -18,5 +18,13 @@ StorageDead(_2); return; } ++ } ++ ++ alloc8 (size: 2, align: 1) { ++ 00 00 │ .. ++ } ++ ++ alloc7 (size: 2, align: 1) { ++ 00 00 │ .. } diff --git a/tests/mir-opt/const_prop/issue_67019.main.ConstProp.panic-abort.diff b/tests/mir-opt/const_prop/issue_67019.main.ConstProp.panic-abort.diff index e1f3f37b37074..015180db896b1 100644 --- a/tests/mir-opt/const_prop/issue_67019.main.ConstProp.panic-abort.diff +++ b/tests/mir-opt/const_prop/issue_67019.main.ConstProp.panic-abort.diff @@ -23,5 +23,17 @@ StorageDead(_2); return; } ++ } ++ ++ alloc12 (size: 2, align: 1) { ++ 01 02 │ .. ++ } ++ ++ alloc11 (size: 2, align: 1) { ++ 01 02 │ .. ++ } ++ ++ alloc8 (size: 2, align: 1) { ++ 01 02 │ .. } diff --git a/tests/mir-opt/const_prop/issue_67019.main.ConstProp.panic-unwind.diff b/tests/mir-opt/const_prop/issue_67019.main.ConstProp.panic-unwind.diff index aaa376a95cf66..8e41705c1af21 100644 --- a/tests/mir-opt/const_prop/issue_67019.main.ConstProp.panic-unwind.diff +++ b/tests/mir-opt/const_prop/issue_67019.main.ConstProp.panic-unwind.diff @@ -23,5 +23,17 @@ StorageDead(_2); return; } ++ } ++ ++ alloc12 (size: 2, align: 1) { ++ 01 02 │ .. ++ } ++ ++ alloc11 (size: 2, align: 1) { ++ 01 02 │ .. ++ } ++ ++ alloc8 (size: 2, align: 1) { ++ 01 02 │ .. } diff --git a/tests/mir-opt/const_prop/mutable_variable_aggregate.main.ConstProp.diff b/tests/mir-opt/const_prop/mutable_variable_aggregate.main.ConstProp.diff index 0f118c7f59fdf..3f3135ff4fe4b 100644 --- a/tests/mir-opt/const_prop/mutable_variable_aggregate.main.ConstProp.diff +++ b/tests/mir-opt/const_prop/mutable_variable_aggregate.main.ConstProp.diff @@ -25,5 +25,13 @@ StorageDead(_1); return; } ++ } ++ ++ alloc6 (size: 8, align: 4) { ++ 2a 00 00 00 63 00 00 00 │ *...c... ++ } ++ ++ alloc5 (size: 8, align: 4) { ++ 2a 00 00 00 2b 00 00 00 │ *...+... } diff --git a/tests/mir-opt/const_prop/mutable_variable_unprop_assign.main.ConstProp.panic-abort.diff b/tests/mir-opt/const_prop/mutable_variable_unprop_assign.main.ConstProp.panic-abort.diff index a85dcf9c7edf7..a1b433716c846 100644 --- a/tests/mir-opt/const_prop/mutable_variable_unprop_assign.main.ConstProp.panic-abort.diff +++ b/tests/mir-opt/const_prop/mutable_variable_unprop_assign.main.ConstProp.panic-abort.diff @@ -46,5 +46,9 @@ StorageDead(_1); return; } ++ } ++ ++ alloc7 (size: 8, align: 4) { ++ 01 00 00 00 02 00 00 00 │ ........ } diff --git a/tests/mir-opt/const_prop/mutable_variable_unprop_assign.main.ConstProp.panic-unwind.diff b/tests/mir-opt/const_prop/mutable_variable_unprop_assign.main.ConstProp.panic-unwind.diff index 15ef0fa4dfff7..2dc514194bc11 100644 --- a/tests/mir-opt/const_prop/mutable_variable_unprop_assign.main.ConstProp.panic-unwind.diff +++ b/tests/mir-opt/const_prop/mutable_variable_unprop_assign.main.ConstProp.panic-unwind.diff @@ -46,5 +46,9 @@ StorageDead(_1); return; } ++ } ++ ++ alloc7 (size: 8, align: 4) { ++ 01 00 00 00 02 00 00 00 │ ........ } diff --git a/tests/mir-opt/const_prop/return_place.add.ConstProp.panic-abort.diff b/tests/mir-opt/const_prop/return_place.add.ConstProp.panic-abort.diff index f3b30e0dcde46..6c9de4764658d 100644 --- a/tests/mir-opt/const_prop/return_place.add.ConstProp.panic-abort.diff +++ b/tests/mir-opt/const_prop/return_place.add.ConstProp.panic-abort.diff @@ -17,5 +17,9 @@ + _0 = const 4_u32; return; } ++ } ++ ++ alloc5 (size: 8, align: 4) { ++ 04 00 00 00 00 __ __ __ │ .....░░░ } diff --git a/tests/mir-opt/const_prop/return_place.add.ConstProp.panic-unwind.diff b/tests/mir-opt/const_prop/return_place.add.ConstProp.panic-unwind.diff index 79f85fcef110e..0f079278c43d9 100644 --- a/tests/mir-opt/const_prop/return_place.add.ConstProp.panic-unwind.diff +++ b/tests/mir-opt/const_prop/return_place.add.ConstProp.panic-unwind.diff @@ -17,5 +17,9 @@ + _0 = const 4_u32; return; } ++ } ++ ++ alloc5 (size: 8, align: 4) { ++ 04 00 00 00 00 __ __ __ │ .....░░░ } diff --git a/tests/mir-opt/const_prop/return_place.add.PreCodegen.before.panic-abort.mir b/tests/mir-opt/const_prop/return_place.add.PreCodegen.before.panic-abort.mir index c8f3f641a6d75..c2488f3944cbb 100644 --- a/tests/mir-opt/const_prop/return_place.add.PreCodegen.before.panic-abort.mir +++ b/tests/mir-opt/const_prop/return_place.add.PreCodegen.before.panic-abort.mir @@ -14,3 +14,7 @@ fn add() -> u32 { return; } } + +alloc5 (size: 8, align: 4) { + 04 00 00 00 00 __ __ __ │ .....░░░ +} diff --git a/tests/mir-opt/const_prop/return_place.add.PreCodegen.before.panic-unwind.mir b/tests/mir-opt/const_prop/return_place.add.PreCodegen.before.panic-unwind.mir index 9a06469746361..fa0b9c77eaf61 100644 --- a/tests/mir-opt/const_prop/return_place.add.PreCodegen.before.panic-unwind.mir +++ b/tests/mir-opt/const_prop/return_place.add.PreCodegen.before.panic-unwind.mir @@ -14,3 +14,7 @@ fn add() -> u32 { return; } } + +alloc5 (size: 8, align: 4) { + 04 00 00 00 00 __ __ __ │ .....░░░ +} diff --git a/tests/mir-opt/const_prop/tuple_literal_propagation.main.ConstProp.panic-abort.diff b/tests/mir-opt/const_prop/tuple_literal_propagation.main.ConstProp.panic-abort.diff index 9e705695ac0cd..6e676d3a0e2ef 100644 --- a/tests/mir-opt/const_prop/tuple_literal_propagation.main.ConstProp.panic-abort.diff +++ b/tests/mir-opt/const_prop/tuple_literal_propagation.main.ConstProp.panic-abort.diff @@ -29,5 +29,17 @@ StorageDead(_1); return; } ++ } ++ ++ alloc8 (size: 8, align: 4) { ++ 01 00 00 00 02 00 00 00 │ ........ ++ } ++ ++ alloc7 (size: 8, align: 4) { ++ 01 00 00 00 02 00 00 00 │ ........ ++ } ++ ++ alloc6 (size: 8, align: 4) { ++ 01 00 00 00 02 00 00 00 │ ........ } diff --git a/tests/mir-opt/const_prop/tuple_literal_propagation.main.ConstProp.panic-unwind.diff b/tests/mir-opt/const_prop/tuple_literal_propagation.main.ConstProp.panic-unwind.diff index 882dd97cc16b8..d3db3ad066b90 100644 --- a/tests/mir-opt/const_prop/tuple_literal_propagation.main.ConstProp.panic-unwind.diff +++ b/tests/mir-opt/const_prop/tuple_literal_propagation.main.ConstProp.panic-unwind.diff @@ -29,5 +29,17 @@ StorageDead(_1); return; } ++ } ++ ++ alloc8 (size: 8, align: 4) { ++ 01 00 00 00 02 00 00 00 │ ........ ++ } ++ ++ alloc7 (size: 8, align: 4) { ++ 01 00 00 00 02 00 00 00 │ ........ ++ } ++ ++ alloc6 (size: 8, align: 4) { ++ 01 00 00 00 02 00 00 00 │ ........ } diff --git a/tests/mir-opt/enum_opt.cand.EnumSizeOpt.32bit.diff b/tests/mir-opt/enum_opt.cand.EnumSizeOpt.32bit.diff index 9d9a7a1e4854b..ec5f5c1f1fc6e 100644 --- a/tests/mir-opt/enum_opt.cand.EnumSizeOpt.32bit.diff +++ b/tests/mir-opt/enum_opt.cand.EnumSizeOpt.32bit.diff @@ -64,5 +64,9 @@ StorageDead(_1); return; } ++ } ++ ++ alloc15 (size: 8, align: 4) { ++ 02 00 00 00 05 20 00 00 │ ..... .. } diff --git a/tests/mir-opt/enum_opt.cand.EnumSizeOpt.64bit.diff b/tests/mir-opt/enum_opt.cand.EnumSizeOpt.64bit.diff index 9d9a7a1e4854b..9bf8637ec7654 100644 --- a/tests/mir-opt/enum_opt.cand.EnumSizeOpt.64bit.diff +++ b/tests/mir-opt/enum_opt.cand.EnumSizeOpt.64bit.diff @@ -64,5 +64,9 @@ StorageDead(_1); return; } ++ } ++ ++ alloc15 (size: 16, align: 8) { ++ 02 00 00 00 00 00 00 00 05 20 00 00 00 00 00 00 │ ......... ...... } diff --git a/tests/mir-opt/enum_opt.unin.EnumSizeOpt.32bit.diff b/tests/mir-opt/enum_opt.unin.EnumSizeOpt.32bit.diff index 4306f38b82a55..7dc6d21a9076e 100644 --- a/tests/mir-opt/enum_opt.unin.EnumSizeOpt.32bit.diff +++ b/tests/mir-opt/enum_opt.unin.EnumSizeOpt.32bit.diff @@ -64,5 +64,9 @@ StorageDead(_1); return; } ++ } ++ ++ alloc14 (size: 8, align: 4) { ++ 05 20 00 00 01 00 00 00 │ . ...... } diff --git a/tests/mir-opt/enum_opt.unin.EnumSizeOpt.64bit.diff b/tests/mir-opt/enum_opt.unin.EnumSizeOpt.64bit.diff index 4306f38b82a55..0b000876a8660 100644 --- a/tests/mir-opt/enum_opt.unin.EnumSizeOpt.64bit.diff +++ b/tests/mir-opt/enum_opt.unin.EnumSizeOpt.64bit.diff @@ -64,5 +64,9 @@ StorageDead(_1); return; } ++ } ++ ++ alloc14 (size: 16, align: 8) { ++ 05 20 00 00 00 00 00 00 01 00 00 00 00 00 00 00 │ . .............. } diff --git a/tests/mir-opt/pre-codegen/optimizes_into_variable.main.ConstProp.32bit.panic-abort.diff b/tests/mir-opt/pre-codegen/optimizes_into_variable.main.ConstProp.32bit.panic-abort.diff index 2c607b4c055fa..681e9666e0fb0 100644 --- a/tests/mir-opt/pre-codegen/optimizes_into_variable.main.ConstProp.32bit.panic-abort.diff +++ b/tests/mir-opt/pre-codegen/optimizes_into_variable.main.ConstProp.32bit.panic-abort.diff @@ -55,5 +55,9 @@ StorageDead(_1); return; } ++ } ++ ++ alloc5 (size: 8, align: 4) { ++ 04 00 00 00 00 __ __ __ │ .....░░░ } diff --git a/tests/mir-opt/pre-codegen/optimizes_into_variable.main.ConstProp.32bit.panic-unwind.diff b/tests/mir-opt/pre-codegen/optimizes_into_variable.main.ConstProp.32bit.panic-unwind.diff index b6929f3f93c6c..db16b8d82d2cb 100644 --- a/tests/mir-opt/pre-codegen/optimizes_into_variable.main.ConstProp.32bit.panic-unwind.diff +++ b/tests/mir-opt/pre-codegen/optimizes_into_variable.main.ConstProp.32bit.panic-unwind.diff @@ -55,5 +55,9 @@ StorageDead(_1); return; } ++ } ++ ++ alloc5 (size: 8, align: 4) { ++ 04 00 00 00 00 __ __ __ │ .....░░░ } diff --git a/tests/mir-opt/pre-codegen/optimizes_into_variable.main.ConstProp.64bit.panic-abort.diff b/tests/mir-opt/pre-codegen/optimizes_into_variable.main.ConstProp.64bit.panic-abort.diff index 2c607b4c055fa..681e9666e0fb0 100644 --- a/tests/mir-opt/pre-codegen/optimizes_into_variable.main.ConstProp.64bit.panic-abort.diff +++ b/tests/mir-opt/pre-codegen/optimizes_into_variable.main.ConstProp.64bit.panic-abort.diff @@ -55,5 +55,9 @@ StorageDead(_1); return; } ++ } ++ ++ alloc5 (size: 8, align: 4) { ++ 04 00 00 00 00 __ __ __ │ .....░░░ } diff --git a/tests/mir-opt/pre-codegen/optimizes_into_variable.main.ConstProp.64bit.panic-unwind.diff b/tests/mir-opt/pre-codegen/optimizes_into_variable.main.ConstProp.64bit.panic-unwind.diff index b6929f3f93c6c..db16b8d82d2cb 100644 --- a/tests/mir-opt/pre-codegen/optimizes_into_variable.main.ConstProp.64bit.panic-unwind.diff +++ b/tests/mir-opt/pre-codegen/optimizes_into_variable.main.ConstProp.64bit.panic-unwind.diff @@ -55,5 +55,9 @@ StorageDead(_1); return; } ++ } ++ ++ alloc5 (size: 8, align: 4) { ++ 04 00 00 00 00 __ __ __ │ .....░░░ } From b18c0a8c4e641d97406fc574d965a74ec4cda3d1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 12 Sep 2023 14:44:38 +0200 Subject: [PATCH 6/7] fix clippy (and MIR printing) handling of ConstValue::Indirect slices --- .../rustc_middle/src/mir/interpret/value.rs | 53 ++++++++++++++++++- compiler/rustc_middle/src/mir/mod.rs | 35 ++++-------- src/tools/clippy/clippy_utils/src/consts.rs | 17 ++---- ...ble_variable_aggregate.main.ConstProp.diff | 2 +- ...ropagation.main.ConstProp.panic-abort.diff | 4 +- ...opagation.main.ConstProp.panic-unwind.diff | 4 +- 6 files changed, 71 insertions(+), 44 deletions(-) diff --git a/compiler/rustc_middle/src/mir/interpret/value.rs b/compiler/rustc_middle/src/mir/interpret/value.rs index 6207a35e4f315..c169733ad742c 100644 --- a/compiler/rustc_middle/src/mir/interpret/value.rs +++ b/compiler/rustc_middle/src/mir/interpret/value.rs @@ -9,7 +9,10 @@ use rustc_apfloat::{ use rustc_macros::HashStable; use rustc_target::abi::{HasDataLayout, Size}; -use crate::ty::{ParamEnv, ScalarInt, Ty, TyCtxt}; +use crate::{ + mir::interpret::alloc_range, + ty::{ParamEnv, ScalarInt, Ty, TyCtxt}, +}; use super::{ AllocId, ConstAllocation, InterpResult, Pointer, PointerArithmetic, Provenance, @@ -114,6 +117,54 @@ impl<'tcx> ConstValue<'tcx> { pub fn from_target_usize(i: u64, cx: &impl HasDataLayout) -> Self { ConstValue::Scalar(Scalar::from_target_usize(i, cx)) } + + /// Must only be called on constants of type `&str` or `&[u8]`! + pub fn try_get_slice_bytes_for_diagnostics(&self, tcx: TyCtxt<'tcx>) -> Option<&'tcx [u8]> { + let (data, start, end) = match self { + ConstValue::Scalar(_) | ConstValue::ZeroSized => { + bug!("`try_get_slice_bytes` on non-slice constant") + } + &ConstValue::Slice { data, start, end } => (data, start, end), + &ConstValue::Indirect { alloc_id, offset } => { + // The reference itself is stored behind an indirection. + // Load the reference, and then load the actual slice contents. + let a = tcx.global_alloc(alloc_id).unwrap_memory().inner(); + let ptr_size = tcx.data_layout.pointer_size; + if a.size() < offset + 2 * ptr_size { + // (partially) dangling reference + return None; + } + // Read the wide pointer components. + let ptr = a + .read_scalar( + &tcx, + alloc_range(offset, ptr_size), + /* read_provenance */ true, + ) + .ok()?; + let ptr = ptr.to_pointer(&tcx).ok()?; + let len = a + .read_scalar( + &tcx, + alloc_range(offset + ptr_size, ptr_size), + /* read_provenance */ false, + ) + .ok()?; + let len = len.to_target_usize(&tcx).ok()?; + let len: usize = len.try_into().ok()?; + if len == 0 { + return Some(&[]); + } + // Non-empty slice, must have memory. We know this is a relative pointer. + let (inner_alloc_id, offset) = ptr.into_parts(); + let data = tcx.global_alloc(inner_alloc_id?).unwrap_memory(); + (data, offset.bytes_usize(), offset.bytes_usize() + len) + } + }; + + // This is for diagnostics only, so we are okay to use `inspect_with_uninit_and_ptr_outside_interpreter`. + Some(data.inner().inspect_with_uninit_and_ptr_outside_interpreter(start..end)) + } } /// A `Scalar` represents an immediate, primitive value existing outside of a diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 6e3409f17ca0c..56b72d0b23599 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -2887,31 +2887,16 @@ fn pretty_print_const_value<'tcx>( let u8_type = tcx.types.u8; match (ct, ty.kind()) { // Byte/string slices, printed as (byte) string literals. - (ConstValue::Slice { data, start, end }, ty::Ref(_, inner, _)) => { - match inner.kind() { - ty::Slice(t) => { - if *t == u8_type { - // The `inspect` here is okay since we checked the bounds, and `u8` carries - // no provenance (we have an active slice reference here). We don't use - // this result to affect interpreter execution. - let byte_str = data - .inner() - .inspect_with_uninit_and_ptr_outside_interpreter(start..end); - pretty_print_byte_str(fmt, byte_str)?; - return Ok(()); - } - } - ty::Str => { - // The `inspect` here is okay since we checked the bounds, and `str` carries - // no provenance (we have an active `str` reference here). We don't use this - // result to affect interpreter execution. - let slice = data - .inner() - .inspect_with_uninit_and_ptr_outside_interpreter(start..end); - fmt.write_str(&format!("{:?}", String::from_utf8_lossy(slice)))?; - return Ok(()); - } - _ => {} + (_, ty::Ref(_, inner_ty, _)) if matches!(inner_ty.kind(), ty::Str) => { + if let Some(data) = ct.try_get_slice_bytes_for_diagnostics(tcx) { + fmt.write_str(&format!("{:?}", String::from_utf8_lossy(data)))?; + return Ok(()); + } + } + (_, ty::Ref(_, inner_ty, _)) if matches!(inner_ty.kind(), ty::Slice(t) if *t == u8_type) => { + if let Some(data) = ct.try_get_slice_bytes_for_diagnostics(tcx) { + pretty_print_byte_str(fmt, data)?; + return Ok(()); } } (ConstValue::Indirect { alloc_id, offset }, ty::Array(t, n)) if *t == u8_type => { diff --git a/src/tools/clippy/clippy_utils/src/consts.rs b/src/tools/clippy/clippy_utils/src/consts.rs index 61b944667a4ae..fcb90c63a6fc6 100644 --- a/src/tools/clippy/clippy_utils/src/consts.rs +++ b/src/tools/clippy/clippy_utils/src/consts.rs @@ -671,19 +671,10 @@ pub fn miri_to_const<'tcx>(lcx: &LateContext<'tcx>, result: mir::ConstantKind<'t ty::RawPtr(_) => Some(Constant::RawPtr(int.assert_bits(int.size()))), _ => None, }, - mir::ConstantKind::Val(ConstValue::Slice { data, start, end }, _) => match result.ty().kind() { - ty::Ref(_, tam, _) => match tam.kind() { - ty::Str => String::from_utf8( - data.inner() - .inspect_with_uninit_and_ptr_outside_interpreter(start..end) - .to_owned(), - ) - .ok() - .map(Constant::Str), - _ => None, - }, - _ => None, - }, + mir::ConstantKind::Val(cv, _) if matches!(result.ty().kind(), ty::Ref(_, inner_ty, _) if matches!(inner_ty.kind(), ty::Str)) => { + let data = cv.try_get_slice_bytes_for_diagnostics(lcx.tcx)?; + String::from_utf8(data.to_owned()).ok().map(Constant::Str) + } mir::ConstantKind::Val(ConstValue::Indirect { alloc_id, offset: _ }, _) => { let alloc = lcx.tcx.global_alloc(alloc_id).unwrap_memory(); match result.ty().kind() { diff --git a/tests/mir-opt/const_prop/mutable_variable_aggregate.main.ConstProp.diff b/tests/mir-opt/const_prop/mutable_variable_aggregate.main.ConstProp.diff index 3f3135ff4fe4b..56a127ae31ebe 100644 --- a/tests/mir-opt/const_prop/mutable_variable_aggregate.main.ConstProp.diff +++ b/tests/mir-opt/const_prop/mutable_variable_aggregate.main.ConstProp.diff @@ -27,7 +27,7 @@ } + } + -+ alloc6 (size: 8, align: 4) { ++ alloc7 (size: 8, align: 4) { + 2a 00 00 00 63 00 00 00 │ *...c... + } + diff --git a/tests/mir-opt/const_prop/tuple_literal_propagation.main.ConstProp.panic-abort.diff b/tests/mir-opt/const_prop/tuple_literal_propagation.main.ConstProp.panic-abort.diff index 6e676d3a0e2ef..988ef7dd2254a 100644 --- a/tests/mir-opt/const_prop/tuple_literal_propagation.main.ConstProp.panic-abort.diff +++ b/tests/mir-opt/const_prop/tuple_literal_propagation.main.ConstProp.panic-abort.diff @@ -31,11 +31,11 @@ } + } + -+ alloc8 (size: 8, align: 4) { ++ alloc9 (size: 8, align: 4) { + 01 00 00 00 02 00 00 00 │ ........ + } + -+ alloc7 (size: 8, align: 4) { ++ alloc8 (size: 8, align: 4) { + 01 00 00 00 02 00 00 00 │ ........ + } + diff --git a/tests/mir-opt/const_prop/tuple_literal_propagation.main.ConstProp.panic-unwind.diff b/tests/mir-opt/const_prop/tuple_literal_propagation.main.ConstProp.panic-unwind.diff index d3db3ad066b90..298446197203d 100644 --- a/tests/mir-opt/const_prop/tuple_literal_propagation.main.ConstProp.panic-unwind.diff +++ b/tests/mir-opt/const_prop/tuple_literal_propagation.main.ConstProp.panic-unwind.diff @@ -31,11 +31,11 @@ } + } + -+ alloc8 (size: 8, align: 4) { ++ alloc9 (size: 8, align: 4) { + 01 00 00 00 02 00 00 00 │ ........ + } + -+ alloc7 (size: 8, align: 4) { ++ alloc8 (size: 8, align: 4) { + 01 00 00 00 02 00 00 00 │ ........ + } + From 04a4df5f169c738a8c631874fa449d5107d52bbb Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 14 Sep 2023 11:47:11 +0200 Subject: [PATCH 7/7] add regression test for something we fixed --- ...issue-105536-const-val-roundtrip-ptr-eq.rs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 tests/ui/consts/issue-105536-const-val-roundtrip-ptr-eq.rs diff --git a/tests/ui/consts/issue-105536-const-val-roundtrip-ptr-eq.rs b/tests/ui/consts/issue-105536-const-val-roundtrip-ptr-eq.rs new file mode 100644 index 0000000000000..1615399be32aa --- /dev/null +++ b/tests/ui/consts/issue-105536-const-val-roundtrip-ptr-eq.rs @@ -0,0 +1,31 @@ +// run-pass + +// This does not reflect a stable guarantee (we guarantee very little for equality of pointers +// around `const`), but it would be good to understand what is happening if these assertions ever +// fail. +use std::ptr::NonNull; +use std::slice::from_raw_parts; + +const PTR_U8: *const u8 = NonNull::dangling().as_ptr(); +const CONST_U8_REF: &[u8] = unsafe { from_raw_parts(PTR_U8, 0) }; +const CONST_U8_PTR: *const u8 = unsafe { from_raw_parts(PTR_U8, 0).as_ptr() }; +static STATIC_U8_REF: &[u8] = unsafe { from_raw_parts(PTR_U8, 0) }; + +const PTR_U16: *const u16 = NonNull::dangling().as_ptr(); +const CONST_U16_REF: &[u16] = unsafe { from_raw_parts(PTR_U16, 0) }; + +const fn const_u8_fn() -> &'static [u8] { + unsafe { from_raw_parts(PTR_U8, 0) } +} + +fn main() { + let ptr_u8 = unsafe { from_raw_parts(PTR_U8, 0) }.as_ptr(); + let ptr_u16 = unsafe { from_raw_parts(PTR_U16, 0) }.as_ptr(); + + assert_eq!(ptr_u8, PTR_U8); + assert_eq!(ptr_u8, CONST_U8_PTR); + assert_eq!(ptr_u8, const_u8_fn().as_ptr()); + assert_eq!(ptr_u8, STATIC_U8_REF.as_ptr()); + assert_eq!(ptr_u16, CONST_U16_REF.as_ptr()); + assert_eq!(ptr_u8, CONST_U8_REF.as_ptr()); +}