From 2668b2a0e7f5527649230765d2c6db6185bc51b5 Mon Sep 17 00:00:00 2001 From: SparrowLii Date: Thu, 2 Jun 2022 19:52:18 +0800 Subject: [PATCH 1/3] add overlap check when copying an imm --- .../rustc_const_eval/src/interpret/memory.rs | 37 ++++++++++++------- .../rustc_const_eval/src/interpret/place.rs | 14 ++++++- 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index 9c032c55fe544..3c83c1d9378cd 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -1096,23 +1096,18 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // The pointers above remain valid even if the `HashMap` table is moved around because they // point into the `Vec` storing the bytes. unsafe { - if src_alloc_id == dest_alloc_id { + if Self::check_ptr_overlap(src_alloc_id, src_offset, dest_alloc_id, dest_offset, size) { if nonoverlapping { - // `Size` additions - if (src_offset <= dest_offset && src_offset + size > dest_offset) - || (dest_offset <= src_offset && dest_offset + size > src_offset) - { - throw_ub_format!("copy_nonoverlapping called on overlapping ranges") + throw_ub_format!("copy_nonoverlapping called on overlapping ranges") + } else { + for i in 0..num_copies { + ptr::copy( + src_bytes, + dest_bytes.add((size * i).bytes_usize()), // `Size` multiplication + size.bytes_usize(), + ); } } - - for i in 0..num_copies { - ptr::copy( - src_bytes, - dest_bytes.add((size * i).bytes_usize()), // `Size` multiplication - size.bytes_usize(), - ); - } } else { for i in 0..num_copies { ptr::copy_nonoverlapping( @@ -1211,4 +1206,18 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { err_ub!(DanglingIntPointer(offset, CheckInAllocMsg::InboundsTest)).into() }) } + + /// Check if a `src` ptr overlaps with a `dest` ptr. + #[inline(always)] + pub fn check_ptr_overlap( + src_id: AllocId, + src_offset: Size, + dest_id: AllocId, + dest_offset: Size, + size: Size, + ) -> bool { + let overlaps = |a, b| a <= b && b < a + size; + src_id == dest_id + && (overlaps(src_offset, dest_offset) || overlaps(dest_offset, src_offset)) + } } diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs index 955480a1a7411..ba7cff010d518 100644 --- a/compiler/rustc_const_eval/src/interpret/place.rs +++ b/compiler/rustc_const_eval/src/interpret/place.rs @@ -4,6 +4,7 @@ use std::convert::TryFrom; use std::hash::Hash; +use std::ops::Deref; use rustc_ast::Mutability; use rustc_macros::HashStable; @@ -869,8 +870,17 @@ where Ok(src_val) => { assert!(!src.layout.is_unsized(), "cannot have unsized immediates"); // Yay, we got a value that we can write directly. - // FIXME: Add a check to make sure that if `src` is indirect, - // it does not overlap with `dest`. + // Then make sure `src` does not overlap with `dest`. + if let Operand::Indirect(src_mplace) = src.deref() + && let Place::Ptr(dest_mplace) = dest.deref() + && let Ok((src_id, src_offset, _)) = self.ptr_try_get_alloc_id(src_mplace.ptr) + && let Ok((dest_id, dest_offset, _)) = self.ptr_try_get_alloc_id(dest_mplace.ptr) + { + let size = src_val.layout.size; + if Self::check_ptr_overlap(src_id, src_offset, dest_id, dest_offset, size) { + throw_ub_format!("copy_nonoverlapping called on overlapping ranges") + } + } return self.write_immediate_no_validate(*src_val, dest); } Err(mplace) => mplace, From 03509a603c024a516173118a26079990645bf8f9 Mon Sep 17 00:00:00 2001 From: SparrowLii Date: Thu, 2 Jun 2022 20:43:51 +0800 Subject: [PATCH 2/3] correct overlap check --- compiler/rustc_const_eval/src/interpret/memory.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index 3c83c1d9378cd..ded4253edc209 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -1216,7 +1216,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { dest_offset: Size, size: Size, ) -> bool { - let overlaps = |a, b| a <= b && b < a + size; + let overlaps = |a, b| a <= b && b - a < size; src_id == dest_id && (overlaps(src_offset, dest_offset) || overlaps(dest_offset, src_offset)) } From 42c58cdef04f000f3de8298397216f5b1372b584 Mon Sep 17 00:00:00 2001 From: SparrowLii Date: Thu, 2 Jun 2022 21:11:00 +0800 Subject: [PATCH 3/3] correct condition statement --- compiler/rustc_const_eval/src/interpret/place.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs index ba7cff010d518..3e57214caf30c 100644 --- a/compiler/rustc_const_eval/src/interpret/place.rs +++ b/compiler/rustc_const_eval/src/interpret/place.rs @@ -870,15 +870,18 @@ where Ok(src_val) => { assert!(!src.layout.is_unsized(), "cannot have unsized immediates"); // Yay, we got a value that we can write directly. - // Then make sure `src` does not overlap with `dest`. - if let Operand::Indirect(src_mplace) = src.deref() + let size = src_val.layout.size; + if size != Size::ZERO + && let Operand::Indirect(src_mplace) = src.deref() && let Place::Ptr(dest_mplace) = dest.deref() - && let Ok((src_id, src_offset, _)) = self.ptr_try_get_alloc_id(src_mplace.ptr) - && let Ok((dest_id, dest_offset, _)) = self.ptr_try_get_alloc_id(dest_mplace.ptr) { - let size = src_val.layout.size; + // If `src` and `dest` are both `MemPlace`, we need to check if the memory + // they point overlap. + let (src_id, src_offset, _) = self.ptr_get_alloc_id(src_mplace.ptr)?; + let (dest_id, dest_offset, _) = self.ptr_get_alloc_id(dest_mplace.ptr)?; + if Self::check_ptr_overlap(src_id, src_offset, dest_id, dest_offset, size) { - throw_ub_format!("copy_nonoverlapping called on overlapping ranges") + throw_ub_format!("copy the operand to the overlapping place") } } return self.write_immediate_no_validate(*src_val, dest);