Rollup merge of #142673 - oli-obk:uninit-read-mem, r=RalfJung

Show the offset, length and memory of uninit read errors

r? ``@RalfJung``

I want to improve memory dumps in general. Not sure yet how to do so best within rust diagnostics, but in a perfect world I could generate a dummy in-memory file (that contains the rendered memory dump) that we then can then provide regular rustc `Span`s to. So we'd basically report normal diagnostics for them with squiggly lines and everything.

Author: matthiaskrgr

compiler/rustc_const_eval/src/const_eval/error.rs

@@ -2,17 +2,17 @@ use std::mem;
 
 use rustc_errors::{Diag, DiagArgName, DiagArgValue, DiagMessage, IntoDiagArg};
 use rustc_middle::mir::AssertKind;
-use rustc_middle::mir::interpret::{AllocId, Provenance, ReportedErrorInfo};
+use rustc_middle::mir::interpret::{AllocId, Provenance, ReportedErrorInfo, UndefinedBehaviorInfo};
 use rustc_middle::query::TyCtxtAt;
+use rustc_middle::ty::ConstInt;
 use rustc_middle::ty::layout::LayoutError;
-use rustc_middle::ty::{ConstInt, TyCtxt};
 use rustc_span::{Span, Symbol};
 
 use super::CompileTimeMachine;
 use crate::errors::{self, FrameNote, ReportErrorExt};
 use crate::interpret::{
-    CtfeProvenance, ErrorHandled, Frame, InterpErrorInfo, InterpErrorKind, MachineStopType,
-    Pointer, err_inval, err_machine_stop,
+    CtfeProvenance, ErrorHandled, Frame, InterpCx, InterpErrorInfo, InterpErrorKind,
+    MachineStopType, Pointer, err_inval, err_machine_stop,
 };
 
 /// The CTFE machine has some custom error kinds.
@@ -163,7 +163,7 @@ pub fn get_span_and_frames<'tcx>(
 /// You can use it to add a stacktrace of current execution according to
 /// `get_span_and_frames` or just give context on where the const eval error happened.
 pub(super) fn report<'tcx, C, F>(
-    tcx: TyCtxt<'tcx>,
+    ecx: &InterpCx<'tcx, CompileTimeMachine<'tcx>>,
     error: InterpErrorKind<'tcx>,
     span: Span,
     get_span_and_frames: C,
@@ -173,6 +173,7 @@ where
     C: FnOnce() -> (Span, Vec<FrameNote>),
     F: FnOnce(&mut Diag<'_>, Span, Vec<FrameNote>),
 {
+    let tcx = ecx.tcx.tcx;
     // Special handling for certain errors
     match error {
         // Don't emit a new diagnostic for these errors, they are already reported elsewhere or
@@ -198,6 +199,20 @@ where
                 InterpErrorKind::ResourceExhaustion(_) | InterpErrorKind::InvalidProgram(_)
             );
 
+            if let InterpErrorKind::UndefinedBehavior(UndefinedBehaviorInfo::InvalidUninitBytes(
+                Some((alloc_id, _access)),
+            )) = error
+            {
+                let bytes = ecx.print_alloc_bytes_for_diagnostics(alloc_id);
+                let info = ecx.get_alloc_info(alloc_id);
+                let raw_bytes = errors::RawBytesNote {
+                    size: info.size.bytes(),
+                    align: info.align.bytes(),
+                    bytes,
+                };
+                err.subdiagnostic(raw_bytes);
+            }
+
             error.add_args(&mut err);
 
             mk(&mut err, span, frames);

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -411,7 +411,7 @@ fn report_eval_error<'tcx>(
     let instance = with_no_trimmed_paths!(cid.instance.to_string());
 
     super::report(
-        *ecx.tcx,
+        ecx,
         error,
         DUMMY_SP,
         || super::get_span_and_frames(ecx.tcx, ecx.stack()),
@@ -451,7 +451,7 @@ fn report_validation_error<'tcx>(
         errors::RawBytesNote { size: info.size.bytes(), align: info.align.bytes(), bytes };
 
     crate::const_eval::report(
-        *ecx.tcx,
+        ecx,
         error,
         DUMMY_SP,
         || crate::const_eval::get_span_and_frames(ecx.tcx, ecx.stack()),

compiler/rustc_const_eval/src/interpret/validity.rs

@@ -394,7 +394,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
         interp_ok(try_validation!(
             self.ecx.read_immediate(val),
             self.path,
-            Ub(InvalidUninitBytes(None)) =>
+            Ub(InvalidUninitBytes(_)) =>
                 Uninit { expected },
             // The `Unsup` cases can only occur during CTFE
             Unsup(ReadPointerAsInt(_)) =>

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

@@ -702,8 +702,11 @@ impl<Prov: Provenance, Extra, Bytes: AllocBytes> Allocation<Prov, Extra, Bytes>
         read_provenance: bool,
     ) -> AllocResult<Scalar<Prov>> {
         // First and foremost, if anything is uninit, bail.
-        if self.init_mask.is_range_initialized(range).is_err() {
-            return Err(AllocError::InvalidUninitBytes(None));
+        if let Err(bad) = self.init_mask.is_range_initialized(range) {
+            return Err(AllocError::InvalidUninitBytes(Some(BadBytesAccess {
+                access: range,
+                bad,
+            })));
         }
 
         // Get the integer part of the result. We HAVE TO check provenance before returning this!

src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_double_destroy.rs

@@ -1,4 +1,6 @@
 //@ignore-target: windows # No pthreads on Windows
+//@ normalize-stderr-test: "(\n)ALLOC \(.*\) \{\n(.*\n)*\}(\n)" -> "${1}ALLOC DUMP${3}"
+//@ normalize-stderr-test: "\[0x[0-9a-z]..0x[0-9a-z]\]" -> "[0xX..0xY]"
 
 /// Test that destroying a pthread_cond twice fails, even without a check for number validity
 
@@ -15,6 +17,6 @@ fn main() {
         libc::pthread_cond_destroy(cond.as_mut_ptr());
 
         libc::pthread_cond_destroy(cond.as_mut_ptr());
-        //~^ ERROR: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
+        //~^ ERROR: /Undefined Behavior: reading memory .*, but memory is uninitialized/
     }
 }

src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_double_destroy.stderr

@@ -1,4 +1,4 @@
-error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
+error: Undefined Behavior: reading memory at ALLOC[0xX..0xY], but memory is uninitialized at [0xX..0xY], and this operation requires initialized memory
   --> tests/fail-dep/concurrency/libc_pthread_cond_double_destroy.rs:LL:CC
    |
 LL |         libc::pthread_cond_destroy(cond.as_mut_ptr());
@@ -9,6 +9,9 @@ LL |         libc::pthread_cond_destroy(cond.as_mut_ptr());
    = note: BACKTRACE:
    = note: inside `main` at tests/fail-dep/concurrency/libc_pthread_cond_double_destroy.rs:LL:CC
 
+Uninitialized memory occurred at ALLOC[0xX..0xY], in this allocation:
+ALLOC DUMP
+
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
 
 error: aborting due to 1 previous error

src/tools/miri/tests/fail-dep/concurrency/libc_pthread_condattr_double_destroy.rs

@@ -1,5 +1,7 @@
 //@ignore-target: windows # No pthreads on Windows
 //@ignore-target: apple # Our macOS condattr don't have any fields so we do not notice this.
+//@ normalize-stderr-test: "(\n)ALLOC \(.*\) \{\n(.*\n)*\}(\n)" -> "${1}ALLOC DUMP${3}"
+//@ normalize-stderr-test: "\[0x[0-9a-z]..0x[0-9a-z]\]" -> "[0xX..0xY]"
 
 /// Test that destroying a pthread_condattr twice fails, even without a check for number validity
 
@@ -13,6 +15,6 @@ fn main() {
         libc::pthread_condattr_destroy(attr.as_mut_ptr());
 
         libc::pthread_condattr_destroy(attr.as_mut_ptr());
-        //~^ ERROR: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
+        //~^ ERROR: /Undefined Behavior: reading memory .*, but memory is uninitialized/
     }
 }

src/tools/miri/tests/fail-dep/concurrency/libc_pthread_condattr_double_destroy.stderr

@@ -1,4 +1,4 @@
-error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
+error: Undefined Behavior: reading memory at ALLOC[0xX..0xY], but memory is uninitialized at [0xX..0xY], and this operation requires initialized memory
   --> tests/fail-dep/concurrency/libc_pthread_condattr_double_destroy.rs:LL:CC
    |
 LL |         libc::pthread_condattr_destroy(attr.as_mut_ptr());
@@ -9,6 +9,9 @@ LL |         libc::pthread_condattr_destroy(attr.as_mut_ptr());
    = note: BACKTRACE:
    = note: inside `main` at tests/fail-dep/concurrency/libc_pthread_condattr_double_destroy.rs:LL:CC
 
+Uninitialized memory occurred at ALLOC[0xX..0xY], in this allocation:
+ALLOC DUMP
+
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
 
 error: aborting due to 1 previous error

src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_double_destroy.rs

@@ -1,4 +1,6 @@
 //@ignore-target: windows # No pthreads on Windows
+//@ normalize-stderr-test: "(\n)ALLOC \(.*\) \{\n(.*\n)*\}(\n)" -> "${1}ALLOC DUMP${3}"
+//@ normalize-stderr-test: "\[0x[0-9a-z]..0x[0-9a-z]\]" -> "[0xX..0xY]"
 
 /// Test that destroying a pthread_mutex twice fails, even without a check for number validity
 
@@ -16,6 +18,6 @@ fn main() {
         libc::pthread_mutex_destroy(mutex.as_mut_ptr());
 
         libc::pthread_mutex_destroy(mutex.as_mut_ptr());
-        //~^ ERROR: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
+        //~^ ERROR: /Undefined Behavior: reading memory .*, but memory is uninitialized/
     }
 }

src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_double_destroy.stderr

@@ -1,4 +1,4 @@
-error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
+error: Undefined Behavior: reading memory at ALLOC[0xX..0xY], but memory is uninitialized at [0xX..0xY], and this operation requires initialized memory
   --> tests/fail-dep/concurrency/libc_pthread_mutex_double_destroy.rs:LL:CC
    |
 LL |         libc::pthread_mutex_destroy(mutex.as_mut_ptr());
@@ -9,6 +9,9 @@ LL |         libc::pthread_mutex_destroy(mutex.as_mut_ptr());
    = note: BACKTRACE:
    = note: inside `main` at tests/fail-dep/concurrency/libc_pthread_mutex_double_destroy.rs:LL:CC
 
+Uninitialized memory occurred at ALLOC[0xX..0xY], in this allocation:
+ALLOC DUMP
+
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
 
 error: aborting due to 1 previous error