interpret: better error message for out-of-bounds pointer arithmetic and accesses

Author: RalfJung

compiler/rustc_const_eval/messages.ftl

@@ -12,6 +12,10 @@ const_eval_already_reported =
 const_eval_assume_false =
     `assume` called with `false`
 
+const_eval_bad_pointer_arithmetic = in-bounds pointer arithmetic failed
+const_eval_bad_pointer_dereferenceable = pointer not dereferenceable
+const_eval_bad_pointer_memory_access = memory access failed
+
 const_eval_bounds_check_failed =
     indexing out of bounds: the len is {$len} but the index is {$index}
 const_eval_call_nonzero_intrinsic =
@@ -39,9 +43,9 @@ const_eval_copy_nonoverlapping_overlapping =
     `copy_nonoverlapping` called on overlapping ranges
 
 const_eval_dangling_int_pointer =
-    {$bad_pointer_message}: {const_eval_expected_inbounds_pointer}, but got {$pointer} which is a dangling pointer (it has no provenance)
+    {$bad_pointer_message}: got {$pointer} which is a dangling pointer (it has no provenance)
 const_eval_dangling_null_pointer =
-    {$bad_pointer_message}: {const_eval_expected_inbounds_pointer}, but got a null pointer
+    {$bad_pointer_message}: got a null pointer
 
 const_eval_dangling_ptr_in_final = encountered dangling pointer in final value of {const_eval_intern_kind}
 const_eval_dead_local =
@@ -77,21 +81,6 @@ const_eval_error = {$error_kind ->
 const_eval_exact_div_has_remainder =
     exact_div: {$a} cannot be divided by {$b} without remainder
 
-const_eval_expected_inbounds_pointer =
-    expected a pointer to {$inbounds_size_abs ->
-        [0] some allocation
-        *[x] {$inbounds_size_is_neg ->
-            [false] {$inbounds_size_abs ->
-                    [1] 1 byte of memory
-                    *[x] {$inbounds_size_abs} bytes of memory
-                }
-            *[true] the end of {$inbounds_size_abs ->
-                    [1] 1 byte of memory
-                    *[x] {$inbounds_size_abs} bytes of memory
-                }
-        }
-    }
-
 const_eval_extern_static =
     cannot access extern static `{$did}`
 const_eval_extern_type_field = `extern type` field does not have a known offset
@@ -111,7 +100,6 @@ const_eval_frame_note_inner = inside {$where_ ->
 
 const_eval_frame_note_last = the failure occurred here
 
-const_eval_in_bounds_test = out-of-bounds pointer use
 const_eval_incompatible_calling_conventions =
     calling a function with calling convention {$callee_conv} using calling convention {$caller_conv}
 
@@ -206,7 +194,6 @@ const_eval_long_running =
 
 const_eval_max_num_nodes_in_const = maximum number of nodes exceeded in constant {$global_const_id}
 
-const_eval_memory_access_test = memory access failed
 const_eval_memory_exhausted =
     tried to allocate more memory than available to compiler
 
@@ -287,8 +274,6 @@ const_eval_offset_from_out_of_bounds =
     `{$name}` called on two different pointers where the memory range between them is not in-bounds of an allocation
 const_eval_offset_from_overflow =
     `{$name}` called when first pointer is too far ahead of second
-const_eval_offset_from_test =
-    out-of-bounds `offset_from` origin
 const_eval_offset_from_underflow =
     `{$name}` called when first pointer is too far before second
 const_eval_offset_from_unsigned_overflow =
@@ -312,24 +297,36 @@ const_eval_partial_pointer_overwrite =
     unable to overwrite parts of a pointer in memory at {$ptr}
 const_eval_pointer_arithmetic_overflow =
     overflowing pointer arithmetic: the total offset in bytes does not fit in an `isize`
-const_eval_pointer_arithmetic_test = out-of-bounds pointer arithmetic
-const_eval_pointer_out_of_bounds =
-    {$bad_pointer_message}: {const_eval_expected_inbounds_pointer}, but got {$pointer} {$ptr_offset_is_neg ->
-        [true] which points to before the beginning of the allocation
-        *[false] {$inbounds_size_is_neg ->
-            [true] {$ptr_offset_abs ->
-                [0] which is at the beginning of the allocation
-                *[other] which does not have enough space to the beginning of the allocation
-            }
-            *[false] {$alloc_size_minus_ptr_offset ->
-                [0] which is at or beyond the end of the allocation of size {$alloc_size ->
+
+const_eval_pointer_out_of_bounds_access =
+    {$bad_pointer_message}: attempting to access {$inbounds_size ->
+        [1] 1 byte
+        *[x] {$inbounds_size} bytes
+    } at {$pointer}, but {const_eval_pointer_out_of_bounds_details}
+const_eval_pointer_out_of_bounds_arithmetic =
+    {$bad_pointer_message}: attempting to offset pointer to {$pointer} by {$inbounds_size ->
+        [1] 1 byte
+        *[x] {$inbounds_size} bytes
+    }, but {const_eval_pointer_out_of_bounds_details}
+const_eval_pointer_out_of_bounds_dereferenceable =
+    {$bad_pointer_message}: pointer to {$pointer} must be dereferenceable for {$inbounds_size ->
+        [1] 1 byte
+        *[x] {$inbounds_size} bytes
+    }, but {const_eval_pointer_out_of_bounds_details}
+const_eval_pointer_out_of_bounds_details =
+    {$inbounds_size_is_neg ->
+        [false] the pointer {$alloc_size_minus_ptr_offset ->
+                [0] is at or beyond the end of the allocation of size {$alloc_size ->
                     [1] 1 byte
                     *[x] {$alloc_size} bytes
                 }
-                [1] which is only 1 byte from the end of the allocation
-                *[x] which is only {$alloc_size_minus_ptr_offset} bytes from the end of the allocation
+                [1] is only 1 byte from the end of the allocation
+                *[x] is only {$alloc_size_minus_ptr_offset} bytes from the end of the allocation
+            }
+        *[true] the pointer {$ptr_offset_abs ->
+                [0] is at the beginning of the allocation
+                *[other] is only {$ptr_offset_abs} bytes from the beginning of the allocation
             }
-        }
     }
 const_eval_pointer_use_after_free =
     {$bad_pointer_message}: {$alloc_id} has been freed, so this pointer is dangling

compiler/rustc_const_eval/src/errors.rs

@@ -502,10 +502,9 @@ fn bad_pointer_message(msg: CheckInAllocMsg, dcx: DiagCtxtHandle<'_>) -> String
     use crate::fluent_generated::*;
 
     let msg = match msg {
-        CheckInAllocMsg::MemoryAccessTest => const_eval_memory_access_test,
-        CheckInAllocMsg::PointerArithmeticTest => const_eval_pointer_arithmetic_test,
-        CheckInAllocMsg::OffsetFromTest => const_eval_offset_from_test,
-        CheckInAllocMsg::InboundsTest => const_eval_in_bounds_test,
+        CheckInAllocMsg::MemoryAccess => const_eval_bad_pointer_memory_access,
+        CheckInAllocMsg::InboundsPointerArithmetic => const_eval_bad_pointer_arithmetic,
+        CheckInAllocMsg::Dereferenceable => const_eval_bad_pointer_dereferenceable,
     };
 
     dcx.eagerly_translate_to_string(msg, [].into_iter())
@@ -534,7 +533,14 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
             InvalidMeta(InvalidMetaKind::TooBig) => const_eval_invalid_meta,
             UnterminatedCString(_) => const_eval_unterminated_c_string,
             PointerUseAfterFree(_, _) => const_eval_pointer_use_after_free,
-            PointerOutOfBounds { .. } => const_eval_pointer_out_of_bounds,
+            PointerOutOfBounds { msg, .. } => {
+                use CheckInAllocMsg::*;
+                match msg {
+                    MemoryAccess => const_eval_pointer_out_of_bounds_access,
+                    InboundsPointerArithmetic => const_eval_pointer_out_of_bounds_arithmetic,
+                    Dereferenceable => const_eval_pointer_out_of_bounds_dereferenceable,
+                }
+            }
             DanglingIntPointer { addr: 0, .. } => const_eval_dangling_null_pointer,
             DanglingIntPointer { .. } => const_eval_dangling_int_pointer,
             AlignmentCheckFailed { .. } => const_eval_alignment_check_failed,
@@ -627,8 +633,10 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
                     }
                     out
                 });
+                diag.arg("inbounds_size", inbounds_size);
                 diag.arg("inbounds_size_is_neg", inbounds_size < 0);
                 diag.arg("inbounds_size_abs", inbounds_size.unsigned_abs());
+                diag.arg("ptr_offset", ptr_offset);
                 diag.arg("ptr_offset_is_neg", ptr_offset < 0);
                 diag.arg("ptr_offset_abs", ptr_offset.unsigned_abs());
                 diag.arg(

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -349,7 +349,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
 
                 // Check that the memory between them is dereferenceable at all, starting from the
                 // origin pointer: `dist` is `a - b`, so it is based on `b`.
-                self.check_ptr_access_signed(b, dist, CheckInAllocMsg::OffsetFromTest)
+                self.check_ptr_access_signed(b, dist, CheckInAllocMsg::Dereferenceable)
                     .map_err_kind(|_| {
                         // This could mean they point to different allocations, or they point to the same allocation
                         // but not the entire range between the pointers is in-bounds.
@@ -373,7 +373,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                 self.check_ptr_access_signed(
                     a,
                     dist.checked_neg().unwrap(), // i64::MIN is impossible as no allocation can be that large
-                    CheckInAllocMsg::OffsetFromTest,
+                    CheckInAllocMsg::Dereferenceable,
                 )
                 .map_err_kind(|_| {
                     // Make the error more specific.
@@ -652,7 +652,11 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         offset_bytes: i64,
     ) -> InterpResult<'tcx, Pointer<Option<M::Provenance>>> {
         // The offset must be in bounds starting from `ptr`.
-        self.check_ptr_access_signed(ptr, offset_bytes, CheckInAllocMsg::PointerArithmeticTest)?;
+        self.check_ptr_access_signed(
+            ptr,
+            offset_bytes,
+            CheckInAllocMsg::InboundsPointerArithmetic,
+        )?;
         // This also implies that there is no overflow, so we are done.
         interp_ok(ptr.wrapping_signed_offset(offset_bytes, self))
     }

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -351,7 +351,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                         kind = "static_mem"
                     )
                 }
-                None => err_ub!(PointerUseAfterFree(alloc_id, CheckInAllocMsg::MemoryAccessTest)),
+                None => err_ub!(PointerUseAfterFree(alloc_id, CheckInAllocMsg::MemoryAccess)),
             })
             .into();
         };
@@ -414,10 +414,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             self,
             ptr,
             size,
-            CheckInAllocMsg::MemoryAccessTest,
+            CheckInAllocMsg::MemoryAccess,
             |this, alloc_id, offset, prov| {
-                let (size, align) = this
-                    .get_live_alloc_size_and_align(alloc_id, CheckInAllocMsg::MemoryAccessTest)?;
+                let (size, align) =
+                    this.get_live_alloc_size_and_align(alloc_id, CheckInAllocMsg::MemoryAccess)?;
                 interp_ok((size, align, (alloc_id, offset, prov)))
             },
         )
@@ -613,7 +613,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             }
             Some(GlobalAlloc::Function { .. }) => throw_ub!(DerefFunctionPointer(id)),
             Some(GlobalAlloc::VTable(..)) => throw_ub!(DerefVTablePointer(id)),
-            None => throw_ub!(PointerUseAfterFree(id, CheckInAllocMsg::MemoryAccessTest)),
+            None => throw_ub!(PointerUseAfterFree(id, CheckInAllocMsg::MemoryAccess)),
             Some(GlobalAlloc::Static(def_id)) => {
                 assert!(self.tcx.is_static(def_id));
                 // Thread-local statics do not have a constant address. They *must* be accessed via
@@ -707,7 +707,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             self,
             ptr,
             size_i64,
-            CheckInAllocMsg::MemoryAccessTest,
+            CheckInAllocMsg::MemoryAccess,
             |this, alloc_id, offset, prov| {
                 let alloc = this.get_alloc_raw(alloc_id)?;
                 interp_ok((alloc.size(), alloc.align, (alloc_id, offset, prov, alloc)))
@@ -809,7 +809,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             self,
             ptr,
             size_i64,
-            CheckInAllocMsg::MemoryAccessTest,
+            CheckInAllocMsg::MemoryAccess,
             |this, alloc_id, offset, prov| {
                 let (alloc, machine) = this.get_alloc_raw_mut(alloc_id)?;
                 interp_ok((alloc.size(), alloc.align, (alloc_id, offset, prov, alloc, machine)))
@@ -1615,7 +1615,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                 err_ub!(DanglingIntPointer {
                     addr: offset,
                     inbounds_size: size,
-                    msg: CheckInAllocMsg::InboundsTest
+                    msg: CheckInAllocMsg::Dereferenceable
                 })
             })
             .into()

compiler/rustc_const_eval/src/interpret/validity.rs

@@ -510,7 +510,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
             self.ecx.check_ptr_access(
                 place.ptr(),
                 size,
-                CheckInAllocMsg::InboundsTest, // will anyway be replaced by validity message
+                CheckInAllocMsg::Dereferenceable, // will anyway be replaced by validity message
             ),
             self.path,
             Ub(DanglingIntPointer { addr: 0, .. }) => NullPtr { ptr_kind },

compiler/rustc_middle/src/mir/interpret/error.rs

@@ -221,13 +221,11 @@ pub enum InvalidProgramInfo<'tcx> {
 #[derive(Debug, Copy, Clone)]
 pub enum CheckInAllocMsg {
     /// We are access memory.
-    MemoryAccessTest,
+    MemoryAccess,
     /// We are doing pointer arithmetic.
-    PointerArithmeticTest,
-    /// We are doing pointer offset_from.
-    OffsetFromTest,
+    InboundsPointerArithmetic,
     /// None of the above -- generic/unspecific inbounds test.
-    InboundsTest,
+    Dereferenceable,
 }
 
 /// Details of which pointer is not aligned.

src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs

@@ -594,7 +594,7 @@ trait EvalContextPrivExt<'tcx, 'ecx>: crate::MiriInterpCxExt<'tcx> {
     ) -> InterpResult<'tcx, Option<Provenance>> {
         let this = self.eval_context_mut();
         // Ensure we bail out if the pointer goes out-of-bounds (see miri#1050).
-        this.check_ptr_access(place.ptr(), size, CheckInAllocMsg::InboundsTest)?;
+        this.check_ptr_access(place.ptr(), size, CheckInAllocMsg::Dereferenceable)?;
 
         // It is crucial that this gets called on all code paths, to ensure we track tag creation.
         let log_creation = |this: &MiriInterpCx<'tcx>,

src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs

@@ -197,7 +197,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         // Make sure the new permission makes sense as the initial permission of a fresh tag.
         assert!(new_perm.initial_state.is_initial());
         // Ensure we bail out if the pointer goes out-of-bounds (see miri#1050).
-        this.check_ptr_access(place.ptr(), ptr_size, CheckInAllocMsg::InboundsTest)?;
+        this.check_ptr_access(place.ptr(), ptr_size, CheckInAllocMsg::Dereferenceable)?;
 
         // It is crucial that this gets called on all code paths, to ensure we track tag creation.
         let log_creation = |this: &MiriInterpCx<'tcx>,

src/tools/miri/src/shims/unix/android/thread.rs

@@ -42,7 +42,7 @@ pub fn prctl<'tcx>(
             ecx.check_ptr_access(
                 name.to_pointer(ecx)?,
                 Size::from_bytes(TASK_COMM_LEN),
-                CheckInAllocMsg::MemoryAccessTest,
+                CheckInAllocMsg::MemoryAccess,
             )?;
             let res = ecx.pthread_getname_np(thread, name, len, /* truncate*/ false)?;
             assert_eq!(res, ThreadNameResult::Ok);

src/tools/miri/src/shims/unix/fd.rs

@@ -226,7 +226,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         trace!("Reading from FD {}, size {}", fd_num, count);
 
         // Check that the *entire* buffer is actually valid memory.
-        this.check_ptr_access(buf, Size::from_bytes(count), CheckInAllocMsg::MemoryAccessTest)?;
+        this.check_ptr_access(buf, Size::from_bytes(count), CheckInAllocMsg::MemoryAccess)?;
 
         // We cap the number of read bytes to the largest value that we are able to fit in both the
         // host's and target's `isize`. This saves us from having to handle overflows later.
@@ -292,7 +292,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         // Isolation check is done via `FileDescription` trait.
 
         // Check that the *entire* buffer is actually valid memory.
-        this.check_ptr_access(buf, Size::from_bytes(count), CheckInAllocMsg::MemoryAccessTest)?;
+        this.check_ptr_access(buf, Size::from_bytes(count), CheckInAllocMsg::MemoryAccess)?;
 
         // We cap the number of written bytes to the largest value that we are able to fit in both the
         // host's and target's `isize`. This saves us from having to handle overflows later.