From ff6812cd20da84f84b2cb8540110d08bd54e8060 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Tue, 10 Oct 2023 15:42:23 +0000 Subject: [PATCH] Move provenance checks out of interning method. --- .../rustc_const_eval/src/interpret/intern.rs | 14 ++----------- .../rustc_const_eval/src/interpret/memory.rs | 2 +- compiler/rustc_mir_transform/src/gvn.rs | 21 +++++++++++++++---- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/intern.rs b/compiler/rustc_const_eval/src/interpret/intern.rs index 4cac00c1be2..0d21ef7f327 100644 --- a/compiler/rustc_const_eval/src/interpret/intern.rs +++ b/compiler/rustc_const_eval/src/interpret/intern.rs @@ -450,7 +450,7 @@ pub fn intern_const_alloc_recursive< Ok(()) } -/// Intern `ret`, checking it references no other allocation. +/// Intern `ret`. This function assumes that `ret` references no other allocation. #[instrument(level = "debug", skip(ecx))] pub fn intern_const_alloc_for_constprop< 'mir, @@ -461,17 +461,7 @@ pub fn intern_const_alloc_for_constprop< ecx: &mut InterpCx<'mir, 'tcx, M>, ret: &MPlaceTy<'tcx>, ) -> InterpResult<'tcx, ()> { - let Some((size, _align)) = ecx.size_and_align_of_mplace(ret)? else { - throw_inval!(ConstPropNonsense) - }; - - let alloc_ref = ecx.get_ptr_alloc(ret.ptr(), size)?.unwrap(); - // Do not try interning a value that contains provenance. - if alloc_ref.has_provenance() { - throw_inval!(ConstPropNonsense) - } - - // remove allocation + // Move allocation to `tcx`. let alloc_id = ret.ptr().provenance.unwrap(); let Some((_, mut alloc)) = ecx.memory.alloc_map.remove(&alloc_id) else { // Pointer not found in local memory map. It is either a pointer to the global diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index d7c7e279849..16905e93bf1 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -1011,7 +1011,7 @@ impl<'tcx, 'a, Prov: Provenance, Extra, Bytes: AllocBytes> AllocRef<'a, 'tcx, Pr } /// Returns whether the allocation has provenance anywhere in the range of the `AllocRef`. - pub(crate) fn has_provenance(&self) -> bool { + pub fn has_provenance(&self) -> bool { !self.alloc.provenance().range_empty(self.range, &self.tcx) } } diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index ba50ca0b791..cced62ecf37 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -832,6 +832,7 @@ fn op_to_prop_const<'tcx>( // If this constant has scalar ABI, return it as a `ConstValue::Scalar`. if let Abi::Scalar(abi::Scalar::Initialized { .. }) = op.layout.abi && let Ok(scalar) = ecx.read_scalar(op) + && scalar.try_to_int().is_ok() { return Some(ConstValue::Scalar(scalar)); } @@ -840,6 +841,14 @@ fn op_to_prop_const<'tcx>( if let Either::Left(mplace) = op.as_mplace_or_imm() && let MemPlaceMeta::None = mplace.meta() { + let (size, _align) = ecx.size_and_align_of_mplace(&mplace).ok()??; + + // Do not try interning a value that contains provenance. + let alloc_ref = ecx.get_ptr_alloc(mplace.ptr(), size).ok()??; + if alloc_ref.has_provenance() { + return None; + } + intern_const_alloc_for_constprop(ecx, &mplace).ok()?; let pointer = mplace.ptr().into_pointer_or_addr().ok()?; let (alloc_id, offset) = pointer.into_parts(); @@ -853,7 +862,13 @@ fn op_to_prop_const<'tcx>( // Everything failed: create a new allocation to hold the data. let alloc_id = ecx.intern_with_temp_alloc(op.layout, |ecx, dest| ecx.copy_op(op, dest, false)).ok()?; - Some(ConstValue::Indirect { alloc_id, offset: Size::ZERO }) + let value = ConstValue::Indirect { alloc_id, offset: Size::ZERO }; + + if !value.has_provenance(*ecx.tcx, op.layout.size) { + return Some(value); + } + + None } impl<'tcx> VnState<'_, 'tcx> { @@ -905,9 +920,7 @@ impl<'tcx> VnState<'_, 'tcx> { // Check that we do not leak a pointer. // Those pointers may lose part of their identity in codegen. - if value.has_provenance(self.tcx, op.layout.size) { - return None; - } + assert!(!value.has_provenance(self.tcx, op.layout.size)); let const_ = Const::Val(value, op.layout.ty); Some(ConstOperand { span: rustc_span::DUMMY_SP, user_ty: None, const_ })