about summary refs log tree commit diff
diff options
context:
space:
mode:
authorRalf Jung <post@ralfj.de>2019-07-29 15:27:01 +0200
committerRalf Jung <post@ralfj.de>2019-08-02 23:02:54 +0200
commit0de6873a319d86dc41b56d8339f75526473bac5f (patch)
treea0d3543b4afc1c0bc573e32f4c6b114106350f4f
parent30c63aa2b8fbc870f32c737d494b6b1ef5f2b912 (diff)
downloadrust-0de6873a319d86dc41b56d8339f75526473bac5f.tar.gz
rust-0de6873a319d86dc41b56d8339f75526473bac5f.zip
miri: validity checks alignment even when machine otherwise does not
-rw-r--r--src/librustc_mir/interpret/memory.rs17
-rw-r--r--src/librustc_mir/interpret/validity.rs4
-rw-r--r--src/test/ui/consts/const-eval/ub-ref.rs3
-rw-r--r--src/test/ui/consts/const-eval/ub-ref.stderr18
4 files changed, 33 insertions, 9 deletions
diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs
index fcefe1abd70..eb98d07f901 100644
--- a/src/librustc_mir/interpret/memory.rs
+++ b/src/librustc_mir/interpret/memory.rs
@@ -306,12 +306,25 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
     ///
     /// Most of the time you should use `check_mplace_access`, but when you just have a pointer,
     /// this method is still appropriate.
+    #[inline(always)]
     pub fn check_ptr_access(
         &self,
         sptr: Scalar<M::PointerTag>,
         size: Size,
         align: Align,
     ) -> InterpResult<'tcx, Option<Pointer<M::PointerTag>>> {
+        let align = if M::CHECK_ALIGN { Some(align) } else { None };
+        self.check_ptr_access_align(sptr, size, align)
+    }
+
+    /// Like `check_ptr_access`, but *definitely* checks alignment when `align`
+    /// is `Some` (overriding `M::CHECK_ALIGN`).
+    pub(super) fn check_ptr_access_align(
+        &self,
+        sptr: Scalar<M::PointerTag>,
+        size: Size,
+        align: Option<Align>,
+    ) -> InterpResult<'tcx, Option<Pointer<M::PointerTag>>> {
         fn check_offset_align(offset: u64, align: Align) -> InterpResult<'static> {
             if offset % align.bytes() == 0 {
                 Ok(())
@@ -343,7 +356,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
                     throw_unsup!(InvalidNullPointerUsage)
                 }
                 // Must be aligned.
-                if M::CHECK_ALIGN {
+                if let Some(align) = align {
                     check_offset_align(bits, align)?;
                 }
                 None
@@ -358,7 +371,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
                 end_ptr.check_in_alloc(allocation_size, CheckInAllocMsg::MemoryAccessTest)?;
                 // Test align. Check this last; if both bounds and alignment are violated
                 // we want the error to be about the bounds.
-                if M::CHECK_ALIGN {
+                if let Some(align) = align {
                     if alloc_align.bytes() < align.bytes() {
                         // The allocation itself is not aligned enough.
                         // FIXME: Alignment check is too strict, depending on the base address that
diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs
index 072c9afc50a..82d6d7db01c 100644
--- a/src/librustc_mir/interpret/validity.rs
+++ b/src/librustc_mir/interpret/validity.rs
@@ -398,7 +398,9 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
                     // alignment and size determined by the layout (size will be 0,
                     // alignment should take attributes into account).
                     .unwrap_or_else(|| (layout.size, layout.align.abi));
-                let ptr: Option<_> = match self.ecx.memory.check_ptr_access(ptr, size, align) {
+                let ptr: Option<_> = match
+                    self.ecx.memory.check_ptr_access_align(ptr, size, Some(align))
+                {
                     Ok(ptr) => ptr,
                     Err(err) => {
                         info!(
diff --git a/src/test/ui/consts/const-eval/ub-ref.rs b/src/test/ui/consts/const-eval/ub-ref.rs
index 7d2ac37f4e2..3b19f3b0775 100644
--- a/src/test/ui/consts/const-eval/ub-ref.rs
+++ b/src/test/ui/consts/const-eval/ub-ref.rs
@@ -3,7 +3,8 @@
 
 use std::mem;
 
-const UNALIGNED: &u16 = unsafe { mem::transmute(&[0u8; 4]) }; // Ok (CTFE does not check alignment)
+const UNALIGNED: &u16 = unsafe { mem::transmute(&[0u8; 4]) };
+//~^ ERROR it is undefined behavior to use this value
 
 const NULL: &u16 = unsafe { mem::transmute(0usize) };
 //~^ ERROR it is undefined behavior to use this value
diff --git a/src/test/ui/consts/const-eval/ub-ref.stderr b/src/test/ui/consts/const-eval/ub-ref.stderr
index d6a82517901..153c5381950 100644
--- a/src/test/ui/consts/const-eval/ub-ref.stderr
+++ b/src/test/ui/consts/const-eval/ub-ref.stderr
@@ -1,5 +1,13 @@
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref.rs:8:1
+  --> $DIR/ub-ref.rs:6:1
+   |
+LL | const UNALIGNED: &u16 = unsafe { mem::transmute(&[0u8; 4]) };
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered unaligned reference (required 2 byte alignment but found 1)
+   |
+   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
+
+error[E0080]: it is undefined behavior to use this value
+  --> $DIR/ub-ref.rs:9:1
    |
 LL | const NULL: &u16 = unsafe { mem::transmute(0usize) };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected something greater or equal to 1
@@ -7,7 +15,7 @@ LL | const NULL: &u16 = unsafe { mem::transmute(0usize) };
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref.rs:11:1
+  --> $DIR/ub-ref.rs:12:1
    |
 LL | const REF_AS_USIZE: usize = unsafe { mem::transmute(&0) };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain (non-pointer) bytes
@@ -15,7 +23,7 @@ LL | const REF_AS_USIZE: usize = unsafe { mem::transmute(&0) };
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref.rs:14:1
+  --> $DIR/ub-ref.rs:15:1
    |
 LL | const REF_AS_USIZE_SLICE: &[usize] = &[unsafe { mem::transmute(&0) }];
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer at .<deref>, but expected plain (non-pointer) bytes
@@ -23,13 +31,13 @@ LL | const REF_AS_USIZE_SLICE: &[usize] = &[unsafe { mem::transmute(&0) }];
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref.rs:17:1
+  --> $DIR/ub-ref.rs:18:1
    |
 LL | const USIZE_AS_REF: &'static u8 = unsafe { mem::transmute(1337usize) };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered dangling reference (created from integer)
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
-error: aborting due to 4 previous errors
+error: aborting due to 5 previous errors
 
 For more information about this error, try `rustc --explain E0080`.