review

nia-e · nia-e · commit 94bf0d20d623 · 2025-07-15T13:42:42.000+02:00
diff --git a/src/shims/native_lib/mod.rs b/src/shims/native_lib/mod.rs
@@ -53,15 +53,6 @@ impl AccessEvent {
             AccessEvent::Write(access_range, _) => access_range.clone(),
         }
     }
-
-    fn is_read(&self) -> bool {
-        matches!(self, AccessEvent::Read(_))
-    }
-
-    fn definitely_happened(&self) -> bool {
-        // Anything except a maybe-write is definitive.
-        !matches!(self, AccessEvent::Write(_, false))
-    }
 }
 
 /// The memory touched by a given access.
@@ -226,46 +217,56 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
         let this = self.eval_context_mut();
 
         for evt in events.acc_events {
-            //eprintln!("evt: {evt:?}");
             let evt_rg = evt.get_range();
             // LLVM at least permits vectorising accesses to adjacent allocations,
             // so we cannot assume 1 access = 1 allocation. :(
             let mut rg = evt_rg.addr..evt_rg.end();
             while let Some(curr) = rg.next() {
-                let alloc_id = this
-                    .alloc_id_from_addr(curr.to_u64(), rg.len().try_into().unwrap(), true)
-                    .expect("Foreign code did an out-of-bounds access!");
+                let Some(alloc_id) =
+                    this.alloc_id_from_addr(curr.to_u64(), rg.len().try_into().unwrap(), true)
+                else {
+                    throw_ub_format!("Foreign code did an out-of-bounds access!")
+                };
                 let alloc = this.get_alloc_raw(alloc_id)?;
                 let alloc_addr = alloc.get_bytes_unchecked_raw().addr();
 
                 // Skip forward however many bytes of the access are contained in the current allocation.
+                // The start of the overlap range will be the greater of the alloc base address and the
+                // lowest remaining address in the access' range; the end will be the lesser of the end
+                // of the allocation and the end of the access' range. Both are then shifted by `alloc_addr`
+                // so that the overlap range would have index 0 as the first byte of the allocation (which
+                // is how the methods on the allocation expect it to be).
                 let overlap = std::cmp::max(alloc_addr, curr).strict_sub(alloc_addr)
                     ..std::cmp::min(alloc_addr.strict_add(alloc.len()), rg.end)
                         .strict_sub(alloc_addr);
+                // Since `overlap` is capped in length by `rg.end`, this will inherently always be
+                // Ok(()) so we can discard it.
                 let _ = rg.advance_by(overlap.len());
-                //eprintln!("overlap: {overlap:?}");
-
-                // Get an overlap range as an offset from the allocation base addr.
-                //let overlap = unshifted_overlap.start..unshifted_overlap.start.strict_add(unshifted_overlap.len());
-
-                // Reads are infallible, writes might not be.
-                if evt.is_read() {
-                    let p_map = alloc.provenance();
-                    for idx in overlap {
-                        // If a provenance was read by the foreign code, expose it.
-                        if let Some(prov) = p_map.get(Size::from_bytes(idx), this) {
-                            this.expose_provenance(prov)?;
+
+                match evt {
+                    AccessEvent::Read(_) => {
+                        let p_map = alloc.provenance();
+                        for idx in overlap {
+                            // If a provenance was read by the foreign code, expose it.
+                            if let Some(prov) = p_map.get(Size::from_bytes(idx), this) {
+                                this.expose_provenance(prov)?;
+                            }
+                        }
+                    }
+                    AccessEvent::Write(_, certain) => {
+                        // Sometimes we aren't certain if a write happened, in which case we
+                        // only initialise that data if the allocation is mutable.
+                        if certain || alloc.mutability.is_mut() {
+                            let (alloc, cx) = this.get_alloc_raw_mut(alloc_id)?;
+                            alloc.process_native_write(
+                                &cx.tcx,
+                                Some(AllocRange {
+                                    start: Size::from_bytes(overlap.start),
+                                    size: Size::from_bytes(overlap.len()),
+                                }),
+                            )
                         }
                     }
-                } else if evt.definitely_happened() || alloc.mutability.is_mut() {
-                    let (alloc, cx) = this.get_alloc_raw_mut(alloc_id)?;
-                    alloc.process_native_write(
-                        &cx.tcx,
-                        Some(AllocRange {
-                            start: Size::from_bytes(overlap.start),
-                            size: Size::from_bytes(overlap.len()),
-                        }),
-                    )
                 }
             }
         }
diff --git a/tests/native-lib/fail/trace_read.rs b/tests/native-lib/fail/trace_read.rs
diff --git a/tests/native-lib/fail/trace_read.stderr b/tests/native-lib/fail/trace_read.stderr
diff --git a/tests/native-lib/pass/ptr_read_access.rs b/tests/native-lib/pass/ptr_read_access.rs
@@ -1,8 +1,5 @@
 //@revisions: trace notrace
-//@[trace] only-target: linux
-//@[trace] only-target: gnu
-//@[trace] only-target: x86
-//@[trace] only-on-host
+//@[trace] only-target: x86_64-unknown-linux-gnu x86-unknown-linux-gnu
 //@[trace] compile-flags: -Zmiri-native-lib-enable-tracing
 
 fn main() {
diff --git a/tests/native-lib/pass/ptr_write_access.rs b/tests/native-lib/pass/ptr_write_access.rs
@@ -1,8 +1,5 @@
 //@revisions: trace notrace
-//@[trace] only-target: linux
-//@[trace] only-target: gnu
-//@[trace] only-target: x86
-//@[trace] only-on-host
+//@[trace] only-target: x86_64-unknown-linux-gnu x86-unknown-linux-gnu
 //@[trace] compile-flags: -Zmiri-native-lib-enable-tracing
 //@compile-flags: -Zmiri-permissive-provenance
 
diff --git a/tests/native-lib/ptr_read_access.c b/tests/native-lib/ptr_read_access.c
@@ -49,3 +49,9 @@ typedef struct Static {
 EXPORT int32_t access_static(const Static *s_ptr) {
   return s_ptr->recurse->recurse->value;
 }
+
+/* Test: unexposed_reachable_alloc */
+
+EXPORT uintptr_t do_one_deref(const int32_t ***ptr) {
+  return (uintptr_t)*ptr;
+}
diff --git a/tests/native-lib/scalar_arguments.c b/tests/native-lib/scalar_arguments.c
@@ -34,7 +34,3 @@ EXPORT int64_t add_short_to_long(int16_t x, int64_t y) {
 int32_t not_exported(void) {
   return 0;
 }
-
-EXPORT void do_nothing(void) {
-  return;
-}
diff --git a/tests/native-lib/tracing/fail/partial_init.rs b/tests/native-lib/tracing/fail/partial_init.rs
@@ -1,9 +1,3 @@
-//@only-target: linux
-//@only-target: gnu
-//@only-target: x86
-//@only-on-host
-//@compile-flags: -Zmiri-native-lib-enable-tracing
-
 extern "C" {
     fn init_n(n: i32, ptr: *mut u8);
 }
@@ -12,6 +6,8 @@ fn main() {
     partial_init();
 }
 
+// Initialise the first 2 elements of the slice from native code, and check
+// that the 3rd is correctly deemed uninit.
 fn partial_init() {
     let mut slice = std::mem::MaybeUninit::<[u8; 3]>::uninit();
     let slice_ptr = slice.as_mut_ptr().cast::<u8>();
diff --git a/tests/native-lib/tracing/fail/partial_init.stderr b/tests/native-lib/tracing/fail/partial_init.stderr
@@ -1,5 +1,5 @@
 warning: sharing memory with a native function called via FFI
-  --> tests/native-lib/fail/trace_write.rs:LL:CC
+  --> tests/native-lib/tracing/fail/partial_init.rs:LL:CC
    |
 LL |         init_n(2, slice_ptr);
    |         ^^^^^^^^^^^^^^^^^^^^ sharing memory with a native function
@@ -10,25 +10,25 @@ LL |         init_n(2, slice_ptr);
    = help: what this means is that Miri will easily miss Undefined Behavior related to incorrect usage of this shared memory, so you should not take a clean Miri run as a signal that your FFI code is UB-free
    = help: tracing memory accesses in native code is not yet fully implemented, so there can be further imprecisions beyond what is documented here
    = note: BACKTRACE:
-   = note: inside `partial_init` at tests/native-lib/fail/trace_write.rs:LL:CC
+   = note: inside `partial_init` at tests/native-lib/tracing/fail/partial_init.rs:LL:CC
 note: inside `main`
-  --> tests/native-lib/fail/trace_write.rs:LL:CC
+  --> tests/native-lib/tracing/fail/partial_init.rs:LL:CC
    |
 LL |     partial_init();
    |     ^^^^^^^^^^^^^^
 
 error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
-  --> tests/native-lib/fail/trace_write.rs:LL:CC
+  --> tests/native-lib/tracing/fail/partial_init.rs:LL:CC
    |
 LL |         let _val = *slice_ptr.offset(2);
    |                    ^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
    = note: BACKTRACE:
-   = note: inside `partial_init` at tests/native-lib/fail/trace_write.rs:LL:CC
+   = note: inside `partial_init` at tests/native-lib/tracing/fail/partial_init.rs:LL:CC
 note: inside `main`
-  --> tests/native-lib/fail/trace_write.rs:LL:CC
+  --> tests/native-lib/tracing/fail/partial_init.rs:LL:CC
    |
 LL |     partial_init();
    |     ^^^^^^^^^^^^^^
diff --git a/tests/native-lib/tracing/fail/unexposed_reachable_alloc.rs b/tests/native-lib/tracing/fail/unexposed_reachable_alloc.rs
@@ -0,0 +1,26 @@
+extern "C" {
+    fn do_one_deref(ptr: *const *const *const i32) -> usize;
+}
+
+fn main() {
+    unexposed_reachable_alloc();
+}
+
+// Expose 2 pointers by virtue of doing a native read and assert that the 3rd in
+// the chain remains properly unexposed.
+fn unexposed_reachable_alloc() {
+    let inner = 42;
+    let intermediate_a = &raw const inner;
+    let intermediate_b = &raw const intermediate_a;
+    let exposed = &raw const intermediate_b;
+    // Discard the return value; it's just there so the access in C doesn't get optimised away.
+    unsafe { do_one_deref(exposed) };
+    // Native read should have exposed the address of intermediate_b...
+    let valid: *const i32 = std::ptr::with_exposed_provenance(intermediate_b.addr());
+    // but not of intermediate_a.
+    let invalid: *const i32 = std::ptr::with_exposed_provenance(intermediate_a.addr());
+    unsafe {
+        let _ok = *valid;
+        let _not_ok = *invalid; //~ ERROR: Undefined Behavior: memory access failed: attempting to access
+    }
+}
diff --git a/tests/native-lib/tracing/fail/unexposed_reachable_alloc.stderr b/tests/native-lib/tracing/fail/unexposed_reachable_alloc.stderr
@@ -0,0 +1,58 @@
+warning: sharing memory with a native function called via FFI
+  --> tests/native-lib/tracing/fail/unexposed_reachable_alloc.rs:LL:CC
+   |
+LL |     unsafe { do_one_deref(exposed) };
+   |              ^^^^^^^^^^^^^^^^^^^^^ sharing memory with a native function
+   |
+   = help: when memory is shared with a native function call, Miri can only track initialisation and provenance on a best-effort basis
+   = help: in particular, Miri assumes that the native call initializes all memory it has written to
+   = help: Miri also assumes that any part of this memory may be a pointer that is permitted to point to arbitrary exposed memory
+   = help: what this means is that Miri will easily miss Undefined Behavior related to incorrect usage of this shared memory, so you should not take a clean Miri run as a signal that your FFI code is UB-free
+   = help: tracing memory accesses in native code is not yet fully implemented, so there can be further imprecisions beyond what is documented here
+   = note: BACKTRACE:
+   = note: inside `unexposed_reachable_alloc` at tests/native-lib/tracing/fail/unexposed_reachable_alloc.rs:LL:CC
+note: inside `main`
+  --> tests/native-lib/tracing/fail/unexposed_reachable_alloc.rs:LL:CC
+   |
+LL |     unexposed_reachable_alloc();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+warning: integer-to-pointer cast
+  --> tests/native-lib/tracing/fail/unexposed_reachable_alloc.rs:LL:CC
+   |
+LL |     let valid: *const i32 = std::ptr::with_exposed_provenance(intermediate_b.addr());
+   |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast
+   |
+   = help: this program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, which means that Miri might miss pointer bugs in this program
+   = help: see https://doc.rust-lang.org/nightly/std/ptr/fn.with_exposed_provenance.html for more details on that operation
+   = help: to ensure that Miri does not miss bugs in your program, use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead
+   = help: you can then set `MIRIFLAGS=-Zmiri-strict-provenance` to ensure you are not relying on `with_exposed_provenance` semantics
+   = help: alternatively, `MIRIFLAGS=-Zmiri-permissive-provenance` disables this warning
+   = note: BACKTRACE:
+   = note: inside `unexposed_reachable_alloc` at tests/native-lib/tracing/fail/unexposed_reachable_alloc.rs:LL:CC
+note: inside `main`
+  --> tests/native-lib/tracing/fail/unexposed_reachable_alloc.rs:LL:CC
+   |
+LL |     unexposed_reachable_alloc();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: Undefined Behavior: memory access failed: attempting to access 4 bytes, but got $HEX[noalloc] which is a dangling pointer (it has no provenance)
+  --> tests/native-lib/tracing/fail/unexposed_reachable_alloc.rs:LL:CC
+   |
+LL |         let _not_ok = *invalid;
+   |                       ^^^^^^^^ Undefined Behavior occurred here
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: BACKTRACE:
+   = note: inside `unexposed_reachable_alloc` at tests/native-lib/tracing/fail/unexposed_reachable_alloc.rs:LL:CC
+note: inside `main`
+  --> tests/native-lib/tracing/fail/unexposed_reachable_alloc.rs:LL:CC
+   |
+LL |     unexposed_reachable_alloc();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error; 2 warnings emitted
+
diff --git a/tests/ui.rs b/tests/ui.rs
@@ -206,6 +206,11 @@ fn run_tests(
         config.program.args.push(flag);
     }
 
+    // Pass on the flag for enabling tracing also when performing those tests.
+    if path.starts_with("tests/native-lib/tracing") {
+        config.program.args.push(std::ffi::OsString::from("-Zmiri-native-lib-enable-tracing"));
+    }
+
     eprintln!("   Compiler: {}", config.program.display());
     ui_test::run_tests_generic(
         // Only run one test suite. In the future we can add all test suites to one `Vec` and run
@@ -338,6 +343,20 @@ fn main() -> Result<()> {
     if cfg!(unix) && target == host {
         ui(Mode::Pass, "tests/native-lib/pass", &target, WithoutDependencies, tmpdir.path())?;
         ui(Mode::Fail, "tests/native-lib/fail", &target, WithoutDependencies, tmpdir.path())?;
+        if cfg!(all(
+            any(target_arch = "x86", target_arch = "x86_64"),
+            target_os = "linux",
+            target_env = "gnu"
+        )) {
+            // No pass tests here currently.
+            ui(
+                Mode::Fail,
+                "tests/native-lib/tracing/fail",
+                &target,
+                WithoutDependencies,
+                tmpdir.path(),
+            )?;
+        }
     }
 
     Ok(())

Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,3 @@ EXPORT int64_t add_short_to_long(int16_t x, int64_t y) {`
`34`	`34`	`int32_t not_exported(void) {`
`35`	`35`	`return 0;`
`36`	`36`	`}`
`37`		`-`
`38`		`-EXPORT void do_nothing(void) {`
`39`		`- return;`
`40`		`-}`