From bb26e07dd59088fc3a308ad7a56283ba4e53b5f8 Mon Sep 17 00:00:00 2001 From: Valentin Tolmer Date: Wed, 27 Feb 2019 17:10:59 +0100 Subject: [PATCH 1/6] Add debug assertions to write_bytes and copy* --- src/libcore/intrinsics.rs | 33 ++++++++++++++++++++++++++++++++- src/libcore/slice/mod.rs | 6 +++--- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index b30eff8baa9c8..93450cb4798a7 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -41,6 +41,8 @@ since = "1.18.0")] pub use crate::ptr::drop_in_place; +use crate::mem; + extern "rust-intrinsic" { // N.B., these intrinsics take raw pointers because they mutate aliased // memory, which is not valid for either `&` or `&mut`. @@ -1331,6 +1333,26 @@ extern "rust-intrinsic" { // (`transmute` also falls into this category, but it cannot be wrapped due to the // check that `T` and `U` have the same size.) +/// Checks whether `ptr` is properly aligned with respect to +/// `align_of::()`. +pub(crate) fn is_aligned_and_not_null(ptr: *const T) -> bool { + return !ptr.is_null() && ptr as usize % mem::align_of::() == 0; +} + +/// Checks whether the regions of memory starting at `src` and `dst` of size +/// `count * size_of::()` overlap. +fn overlaps(src: *const T, dst: *const T, count: usize) -> bool { + let src_usize = src as usize; + let dst_usize = dst as usize; + let size = mem::size_of::().checked_mul(count).unwrap(); + let diff = if src_usize > dst_usize { + src_usize - dst_usize + } else { + dst_usize - src_usize + }; + size > diff +} + /// Copies `count * size_of::()` bytes from `src` to `dst`. The source /// and destination must *not* overlap. /// @@ -1420,7 +1442,11 @@ pub unsafe fn copy_nonoverlapping(src: *const T, dst: *mut T, count: usize) { extern "rust-intrinsic" { fn copy_nonoverlapping(src: *const T, dst: *mut T, count: usize); } - copy_nonoverlapping(src, dst, count); + + debug_assert!(is_aligned_and_not_null(src), "attempt to copy from unaligned or null pointer"); + debug_assert!(is_aligned_and_not_null(dst), "attempt to copy to unaligned or null pointer"); + debug_assert!(!overlaps(src, dst, count), "attempt to copy to overlapping memory"); + copy_nonoverlapping(src, dst, count) } /// Copies `count * size_of::()` bytes from `src` to `dst`. The source @@ -1480,6 +1506,9 @@ pub unsafe fn copy(src: *const T, dst: *mut T, count: usize) { extern "rust-intrinsic" { fn copy(src: *const T, dst: *mut T, count: usize); } + + debug_assert!(is_aligned_and_not_null(src), "attempt to copy from unaligned or null pointer"); + debug_assert!(is_aligned_and_not_null(dst), "attempt to copy to unaligned or null pointer"); copy(src, dst, count) } @@ -1561,6 +1590,8 @@ pub unsafe fn write_bytes(dst: *mut T, val: u8, count: usize) { extern "rust-intrinsic" { fn write_bytes(dst: *mut T, val: u8, count: usize); } + + debug_assert!(is_aligned_and_not_null(dst), "attempt to write to unaligned or null pointer"); write_bytes(dst, val, count) } diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index c6d44324ef5ee..dba9a1445e84c 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -25,7 +25,7 @@ use crate::cmp::Ordering::{self, Less, Equal, Greater}; use crate::cmp; use crate::fmt; -use crate::intrinsics::{assume, exact_div, unchecked_sub}; +use crate::intrinsics::{assume, exact_div, unchecked_sub, is_aligned_and_not_null}; use crate::isize; use crate::iter::*; use crate::ops::{FnMut, Try, self}; @@ -5213,7 +5213,7 @@ unsafe impl<'a, T> TrustedRandomAccess for RChunksExactMut<'a, T> { #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T] { - debug_assert!(data as usize % mem::align_of::() == 0, "attempt to create unaligned slice"); + debug_assert!(is_aligned_and_not_null(data), "attempt to create unaligned or null slice"); debug_assert!(mem::size_of::().saturating_mul(len) <= isize::MAX as usize, "attempt to create slice covering half the address space"); &*ptr::slice_from_raw_parts(data, len) @@ -5234,7 +5234,7 @@ pub unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T] { #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub unsafe fn from_raw_parts_mut<'a, T>(data: *mut T, len: usize) -> &'a mut [T] { - debug_assert!(data as usize % mem::align_of::() == 0, "attempt to create unaligned slice"); + debug_assert!(is_aligned_and_not_null(data), "attempt to create unaligned or null slice"); debug_assert!(mem::size_of::().saturating_mul(len) <= isize::MAX as usize, "attempt to create slice covering half the address space"); &mut *ptr::slice_from_raw_parts_mut(data, len) From 0533f129ecfb54ecae7e329949d7b1572368d0ae Mon Sep 17 00:00:00 2001 From: Valentin Tolmer Date: Wed, 27 Mar 2019 15:57:14 +0100 Subject: [PATCH 2/6] Handle null from LLVMRustGetSectionName --- src/librustc_codegen_llvm/llvm/ffi.rs | 4 +++- src/librustc_codegen_llvm/metadata.rs | 11 +++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index a5c295cd4525c..708ba79ec3ab2 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/src/librustc_codegen_llvm/llvm/ffi.rs @@ -1736,7 +1736,9 @@ extern "C" { pub fn LLVMRustArchiveIteratorFree(AIR: &'a mut ArchiveIterator<'a>); pub fn LLVMRustDestroyArchive(AR: &'static mut Archive); - pub fn LLVMRustGetSectionName(SI: &SectionIterator<'_>, data: &mut *const c_char) -> size_t; + #[allow(improper_ctypes)] + pub fn LLVMRustGetSectionName(SI: &SectionIterator<'_>, + data: &mut Option>) -> size_t; #[allow(improper_ctypes)] pub fn LLVMRustWriteTwineToString(T: &Twine, s: &RustString); diff --git a/src/librustc_codegen_llvm/metadata.rs b/src/librustc_codegen_llvm/metadata.rs index 7cf497cb5d036..9bddd29d2e88f 100644 --- a/src/librustc_codegen_llvm/metadata.rs +++ b/src/librustc_codegen_llvm/metadata.rs @@ -8,7 +8,6 @@ use rustc_data_structures::owning_ref::OwningRef; use rustc_codegen_ssa::METADATA_FILENAME; use std::path::Path; -use std::ptr; use std::slice; use rustc_fs_util::path_to_c_string; @@ -67,10 +66,14 @@ fn search_meta_section<'a>(of: &'a ObjectFile, unsafe { let si = mk_section_iter(of.llof); while llvm::LLVMIsSectionIteratorAtEnd(of.llof, si.llsi) == False { - let mut name_buf = ptr::null(); + let mut name_buf = None; let name_len = llvm::LLVMRustGetSectionName(si.llsi, &mut name_buf); - let name = slice::from_raw_parts(name_buf as *const u8, name_len as usize).to_vec(); - let name = String::from_utf8(name).unwrap(); + let name = name_buf.map_or( + "".to_string(), + |buf| String::from_utf8( + slice::from_raw_parts(buf.as_ptr() as *const u8, + name_len as usize) + .to_vec()).unwrap()); debug!("get_metadata_section: name {}", name); if read_metadata_section_name(target) == name { let cbuf = llvm::LLVMGetSectionContents(si.llsi); From 8a4573f841f219df32c97d25f400effee94e2474 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 24 Jun 2019 23:15:37 +0200 Subject: [PATCH 3/6] format a bit --- src/librustc_codegen_llvm/metadata.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/librustc_codegen_llvm/metadata.rs b/src/librustc_codegen_llvm/metadata.rs index 9bddd29d2e88f..42e91ef408f9d 100644 --- a/src/librustc_codegen_llvm/metadata.rs +++ b/src/librustc_codegen_llvm/metadata.rs @@ -69,11 +69,13 @@ fn search_meta_section<'a>(of: &'a ObjectFile, let mut name_buf = None; let name_len = llvm::LLVMRustGetSectionName(si.llsi, &mut name_buf); let name = name_buf.map_or( - "".to_string(), + String::new(), // we got a NULL ptr, ignore `name_len` |buf| String::from_utf8( slice::from_raw_parts(buf.as_ptr() as *const u8, name_len as usize) - .to_vec()).unwrap()); + .to_vec() + ).unwrap() + ); debug!("get_metadata_section: name {}", name); if read_metadata_section_name(target) == name { let cbuf = llvm::LLVMGetSectionContents(si.llsi); From 4159b27c61e991c0abe7e87ad381bd0c07077dfc Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 24 Jun 2019 23:19:04 +0200 Subject: [PATCH 4/6] order things more traditionally --- src/libcore/intrinsics.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index 93450cb4798a7..2d47f08d70154 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -36,13 +36,13 @@ issue = "0")] #![allow(missing_docs)] +use crate::mem; + #[stable(feature = "drop_in_place", since = "1.8.0")] #[rustc_deprecated(reason = "no longer an intrinsic - use `ptr::drop_in_place` directly", since = "1.18.0")] pub use crate::ptr::drop_in_place; -use crate::mem; - extern "rust-intrinsic" { // N.B., these intrinsics take raw pointers because they mutate aliased // memory, which is not valid for either `&` or `&mut`. From a68afc56bdf61b2ddd4f38ba4e3cabd7a4399c9b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 25 Jun 2019 09:40:50 +0200 Subject: [PATCH 5/6] ignore some codegen tests in debug mode --- src/test/codegen/issue-45222.rs | 1 + src/test/codegen/issue-45466.rs | 1 + src/test/codegen/swap-small-types.rs | 1 + 3 files changed, 3 insertions(+) diff --git a/src/test/codegen/issue-45222.rs b/src/test/codegen/issue-45222.rs index da65f2dfca5d1..7f99ca724cf73 100644 --- a/src/test/codegen/issue-45222.rs +++ b/src/test/codegen/issue-45222.rs @@ -1,4 +1,5 @@ // compile-flags: -O +// ignore-debug: the debug assertions get in the way #![crate_type = "lib"] diff --git a/src/test/codegen/issue-45466.rs b/src/test/codegen/issue-45466.rs index 7d6e31cc740f5..c79542767774a 100644 --- a/src/test/codegen/issue-45466.rs +++ b/src/test/codegen/issue-45466.rs @@ -1,4 +1,5 @@ // compile-flags: -O +// ignore-debug: the debug assertions get in the way #![crate_type="rlib"] diff --git a/src/test/codegen/swap-small-types.rs b/src/test/codegen/swap-small-types.rs index c8466fed7d1bd..6205e6a6559c9 100644 --- a/src/test/codegen/swap-small-types.rs +++ b/src/test/codegen/swap-small-types.rs @@ -1,5 +1,6 @@ // compile-flags: -O // only-x86_64 +// ignore-debug: the debug assertions get in the way #![crate_type = "lib"] From d3e1bf96dacab3e5c7c86321eb79fc916b28db87 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 25 Jun 2019 09:42:25 +0200 Subject: [PATCH 6/6] nits --- src/libcore/intrinsics.rs | 2 +- src/librustc_codegen_llvm/metadata.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index 2d47f08d70154..d9b68612785e9 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -1336,7 +1336,7 @@ extern "rust-intrinsic" { /// Checks whether `ptr` is properly aligned with respect to /// `align_of::()`. pub(crate) fn is_aligned_and_not_null(ptr: *const T) -> bool { - return !ptr.is_null() && ptr as usize % mem::align_of::() == 0; + !ptr.is_null() && ptr as usize % mem::align_of::() == 0 } /// Checks whether the regions of memory starting at `src` and `dst` of size diff --git a/src/librustc_codegen_llvm/metadata.rs b/src/librustc_codegen_llvm/metadata.rs index 42e91ef408f9d..cd7255888118c 100644 --- a/src/librustc_codegen_llvm/metadata.rs +++ b/src/librustc_codegen_llvm/metadata.rs @@ -69,7 +69,7 @@ fn search_meta_section<'a>(of: &'a ObjectFile, let mut name_buf = None; let name_len = llvm::LLVMRustGetSectionName(si.llsi, &mut name_buf); let name = name_buf.map_or( - String::new(), // we got a NULL ptr, ignore `name_len` + String::new(), // We got a NULL ptr, ignore `name_len`. |buf| String::from_utf8( slice::from_raw_parts(buf.as_ptr() as *const u8, name_len as usize)