Auto merge of #114932 - RalfJung:miri, r=RalfJung

update Miri

r? `@ghost`

Author: bors

src/tools/miri/README.md

@@ -53,7 +53,7 @@ behavior** in your program, and cannot run all programs:
   positives here, so if your program runs fine in Miri right now that is by no
   means a guarantee that it is UB-free when these questions get answered.
 
-    In particular, Miri does currently not check that references point to valid data.
+    In particular, Miri does not check that references point to valid data.
 * If the program relies on unspecified details of how data is laid out, it will
   still run fine in Miri -- but might break (including causing UB) on different
   compiler versions or different platforms.

src/tools/miri/rust-version

@@ -1 +1 @@
-9fa6bdd764a1f7bdf69eccceeace6d13f38cb2e1
+656ee47db32e882fb02913f6204e09cc7a41a50e

src/tools/miri/src/diagnostics.rs

@@ -304,11 +304,21 @@ pub fn report_error<'tcx, 'mir>(
                     (None, format!("this usually indicates that your program performed an invalid operation and caused Undefined Behavior")),
                     (None, format!("but due to `-Zmiri-symbolic-alignment-check`, alignment errors can also be false positives")),
                 ],
-            UndefinedBehavior(_) =>
-                vec![
+            UndefinedBehavior(info) => {
+                let mut helps = vec![
                     (None, format!("this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior")),
                     (None, format!("see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information")),
-                ],
+                ];
+                if let UndefinedBehaviorInfo::PointerUseAfterFree(alloc_id, _) | UndefinedBehaviorInfo::PointerOutOfBounds { alloc_id, .. } = info {
+                    if let Some(span) = ecx.machine.allocated_span(*alloc_id) {
+                        helps.push((Some(span), format!("{:?} was allocated here:", alloc_id)));
+                    }
+                    if let Some(span) = ecx.machine.deallocated_span(*alloc_id) {
+                        helps.push((Some(span), format!("{:?} was deallocated here:", alloc_id)));
+                    }
+                }
+                helps
+            }
             InvalidProgram(
                 InvalidProgramInfo::AlreadyReported(_)
             ) => {

src/tools/miri/src/helpers.rs

@@ -13,11 +13,11 @@ use rustc_index::IndexVec;
 use rustc_middle::mir;
 use rustc_middle::ty::{
     self,
-    layout::{LayoutOf, TyAndLayout},
-    List, TyCtxt,
+    layout::{IntegerExt as _, LayoutOf, TyAndLayout},
+    List, Ty, TyCtxt,
 };
 use rustc_span::{def_id::CrateNum, sym, Span, Symbol};
-use rustc_target::abi::{Align, FieldIdx, FieldsShape, Size, Variants};
+use rustc_target::abi::{Align, FieldIdx, FieldsShape, Integer, Size, Variants};
 use rustc_target::spec::abi::Abi;
 
 use rand::RngCore;
@@ -1011,6 +1011,65 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             None => tcx.item_name(def_id),
         }
     }
+
+    /// Converts `f` to integer type `dest_ty` after rounding with mode `round`.
+    /// Returns `None` if `f` is NaN or out of range.
+    fn float_to_int_checked<F>(
+        &self,
+        f: F,
+        dest_ty: Ty<'tcx>,
+        round: rustc_apfloat::Round,
+    ) -> Option<Scalar<Provenance>>
+    where
+        F: rustc_apfloat::Float + Into<Scalar<Provenance>>,
+    {
+        let this = self.eval_context_ref();
+
+        match dest_ty.kind() {
+            // Unsigned
+            ty::Uint(t) => {
+                let size = Integer::from_uint_ty(this, *t).size();
+                let res = f.to_u128_r(size.bits_usize(), round, &mut false);
+                if res.status.intersects(
+                    rustc_apfloat::Status::INVALID_OP
+                        | rustc_apfloat::Status::OVERFLOW
+                        | rustc_apfloat::Status::UNDERFLOW,
+                ) {
+                    // Floating point value is NaN (flagged with INVALID_OP) or outside the range
+                    // of values of the integer type (flagged with OVERFLOW or UNDERFLOW).
+                    None
+                } else {
+                    // Floating point value can be represented by the integer type after rounding.
+                    // The INEXACT flag is ignored on purpose to allow rounding.
+                    Some(Scalar::from_uint(res.value, size))
+                }
+            }
+            // Signed
+            ty::Int(t) => {
+                let size = Integer::from_int_ty(this, *t).size();
+                let res = f.to_i128_r(size.bits_usize(), round, &mut false);
+                if res.status.intersects(
+                    rustc_apfloat::Status::INVALID_OP
+                        | rustc_apfloat::Status::OVERFLOW
+                        | rustc_apfloat::Status::UNDERFLOW,
+                ) {
+                    // Floating point value is NaN (flagged with INVALID_OP) or outside the range
+                    // of values of the integer type (flagged with OVERFLOW or UNDERFLOW).
+                    None
+                } else {
+                    // Floating point value can be represented by the integer type after rounding.
+                    // The INEXACT flag is ignored on purpose to allow rounding.
+                    Some(Scalar::from_int(res.value, size))
+                }
+            }
+            // Nothing else
+            _ =>
+                span_bug!(
+                    this.cur_span(),
+                    "attempted float-to-int conversion with non-int output type {dest_ty:?}"
+                ),
+        }
+    }
 }
 
 impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {

src/tools/miri/src/machine.rs

@@ -25,7 +25,7 @@ use rustc_middle::{
     },
 };
 use rustc_span::def_id::{CrateNum, DefId};
-use rustc_span::Symbol;
+use rustc_span::{Span, SpanData, Symbol};
 use rustc_target::abi::{Align, Size};
 use rustc_target::spec::abi::Abi;
 
@@ -135,6 +135,19 @@ impl MayLeak for MiriMemoryKind {
     }
 }
 
+impl MiriMemoryKind {
+    /// Whether we have a useful allocation span for an allocation of this kind.
+    fn should_save_allocation_span(self) -> bool {
+        use self::MiriMemoryKind::*;
+        match self {
+            // Heap allocations are fine since the `Allocation` is created immediately.
+            Rust | Miri | C | WinHeap | Mmap => true,
+            // Everything else is unclear, let's not show potentially confusing spans.
+            Machine | Global | ExternStatic | Tls | Runtime => false,
+        }
+    }
+}
+
 impl fmt::Display for MiriMemoryKind {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         use self::MiriMemoryKind::*;
@@ -497,6 +510,10 @@ pub struct MiriMachine<'mir, 'tcx> {
 
     /// Whether to collect a backtrace when each allocation is created, just in case it leaks.
     pub(crate) collect_leak_backtraces: bool,
+
+    /// The spans we will use to report where an allocation was created and deallocated in
+    /// diagnostics.
+    pub(crate) allocation_spans: RefCell<FxHashMap<AllocId, (Span, Option<Span>)>>,
 }
 
 impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
@@ -621,6 +638,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
             stack_addr,
             stack_size,
             collect_leak_backtraces: config.collect_leak_backtraces,
+            allocation_spans: RefCell::new(FxHashMap::default()),
         }
     }
 
@@ -742,6 +760,21 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
     pub(crate) fn page_align(&self) -> Align {
         Align::from_bytes(self.page_size).unwrap()
     }
+
+    pub(crate) fn allocated_span(&self, alloc_id: AllocId) -> Option<SpanData> {
+        self.allocation_spans
+            .borrow()
+            .get(&alloc_id)
+            .map(|(allocated, _deallocated)| allocated.data())
+    }
+
+    pub(crate) fn deallocated_span(&self, alloc_id: AllocId) -> Option<SpanData> {
+        self.allocation_spans
+            .borrow()
+            .get(&alloc_id)
+            .and_then(|(_allocated, deallocated)| *deallocated)
+            .map(Span::data)
+    }
 }
 
 impl VisitTags for MiriMachine<'_, '_> {
@@ -791,6 +824,7 @@ impl VisitTags for MiriMachine<'_, '_> {
             stack_addr: _,
             stack_size: _,
             collect_leak_backtraces: _,
+            allocation_spans: _,
         } = self;
 
         threads.visit_tags(visit);
@@ -1051,6 +1085,14 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
             },
             |ptr| ecx.global_base_pointer(ptr),
         )?;
+
+        if matches!(kind, MemoryKind::Machine(kind) if kind.should_save_allocation_span()) {
+            ecx.machine
+                .allocation_spans
+                .borrow_mut()
+                .insert(id, (ecx.machine.current_span(), None));
+        }
+
         Ok(Cow::Owned(alloc))
     }
 
@@ -1181,6 +1223,10 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
         if let Some(borrow_tracker) = &mut alloc_extra.borrow_tracker {
             borrow_tracker.before_memory_deallocation(alloc_id, prove_extra, range, machine)?;
         }
+        if let Some((_, deallocated_at)) = machine.allocation_spans.borrow_mut().get_mut(&alloc_id)
+        {
+            *deallocated_at = Some(machine.current_span());
+        }
         Ok(())
     }
 

src/tools/miri/src/range_map.rs

@@ -27,35 +27,27 @@ impl<T> RangeMap<T> {
     #[inline(always)]
     pub fn new(size: Size, init: T) -> RangeMap<T> {
         let size = size.bytes();
-        let mut map = RangeMap { v: Vec::new() };
-        if size > 0 {
-            map.v.push(Elem { range: 0..size, data: init });
-        }
-        map
+        let v = if size > 0 { vec![Elem { range: 0..size, data: init }] } else { Vec::new() };
+        RangeMap { v }
     }
 
     /// Finds the index containing the given offset.
     fn find_offset(&self, offset: u64) -> usize {
-        // We do a binary search.
-        let mut left = 0usize; // inclusive
-        let mut right = self.v.len(); // exclusive
-        loop {
-            debug_assert!(left < right, "find_offset: offset {offset} is out-of-bounds");
-            let candidate = left.checked_add(right).unwrap() / 2;
-            let elem = &self.v[candidate];
-            if offset < elem.range.start {
-                // We are too far right (offset is further left).
-                debug_assert!(candidate < right); // we are making progress
-                right = candidate;
-            } else if offset >= elem.range.end {
-                // We are too far left (offset is further right).
-                debug_assert!(candidate >= left); // we are making progress
-                left = candidate + 1;
-            } else {
-                // This is it!
-                return candidate;
-            }
-        }
+        self.v
+            .binary_search_by(|elem| -> std::cmp::Ordering {
+                if offset < elem.range.start {
+                    // We are too far right (offset is further left).
+                    // (`Greater` means that `elem` is greater than the desired target.)
+                    std::cmp::Ordering::Greater
+                } else if offset >= elem.range.end {
+                    // We are too far left (offset is further right).
+                    std::cmp::Ordering::Less
+                } else {
+                    // This is it!
+                    std::cmp::Ordering::Equal
+                }
+            })
+            .unwrap()
     }
 
     /// Provides read-only iteration over everything in the given range. This does

src/tools/miri/src/shims/foreign_items.rs

@@ -690,6 +690,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 let right = this.read_pointer(right)?;
                 let n = Size::from_bytes(this.read_target_usize(n)?);
 
+                // C requires that this must always be a valid pointer (C18 §7.1.4).
+                this.ptr_get_alloc_id(left)?;
+                this.ptr_get_alloc_id(right)?;
+
                 let result = {
                     let left_bytes = this.read_bytes_ptr_strip_provenance(left, n)?;
                     let right_bytes = this.read_bytes_ptr_strip_provenance(right, n)?;
@@ -714,6 +718,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)]
                 let val = val as u8;
 
+                // C requires that this must always be a valid pointer (C18 §7.1.4).
+                this.ptr_get_alloc_id(ptr)?;
+
                 if let Some(idx) = this
                     .read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(num))?
                     .iter()
@@ -738,6 +745,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)]
                 let val = val as u8;
 
+                // C requires that this must always be a valid pointer (C18 §7.1.4).
+                this.ptr_get_alloc_id(ptr)?;
+
                 let idx = this
                     .read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(num))?
                     .iter()
@@ -752,6 +762,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             "strlen" => {
                 let [ptr] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
                 let ptr = this.read_pointer(ptr)?;
+                // This reads at least 1 byte, so we are already enforcing that this is a valid pointer.
                 let n = this.read_c_str(ptr)?.len();
                 this.write_scalar(
                     Scalar::from_target_usize(u64::try_from(n).unwrap(), this),
@@ -791,6 +802,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 // pointer provenance is preserved by this implementation of `strcpy`.
                 // That is probably overly cautious, but there also is no fundamental
                 // reason to have `strcpy` destroy pointer provenance.
+                // This reads at least 1 byte, so we are already enforcing that this is a valid pointer.
                 let n = this.read_c_str(ptr_src)?.len().checked_add(1).unwrap();
                 this.mem_copy(
                     ptr_src,
@@ -942,6 +954,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 this.write_scalar(Scalar::from_u64(res.to_bits()), dest)?;
             }
 
+            // LLVM intrinsics
             "llvm.prefetch" => {
                 let [p, rw, loc, ty] =
                     this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
@@ -968,8 +981,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                     throw_unsup_format!("unsupported `llvm.prefetch` type argument: {}", ty);
                 }
             }
-
-            // Architecture-specific shims
             "llvm.x86.addcarry.64" if this.tcx.sess.target.arch == "x86_64" => {
                 // Computes u8+u64+u64, returning tuple (u8,u64) comprising the output carry and truncated sum.
                 let [c_in, a, b] = this.check_shim(abi, Abi::Unadjusted, link_name, args)?;