more review

Author: nia-e

src/shims/native_lib/mod.rs

@@ -222,21 +222,23 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
             // so we cannot assume 1 access = 1 allocation. :(
             let mut rg = evt_rg.addr..evt_rg.end();
             while let Some(curr) = rg.next() {
-                let Some(alloc_id) =
-                    this.alloc_id_from_addr(curr.to_u64(), rg.len().try_into().unwrap(), true)
-                else {
+                // FIXME: ProvenanceMap should have something like get_range().
+                let Some(alloc_id) = this.alloc_id_from_addr(
+                    curr.to_u64(),
+                    rg.len().try_into().unwrap(),
+                    /* only_exposed_allocations */ 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`
+                // The start of the overlap range will be `curr`; 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)
+                let overlap = 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

src/shims/native_lib/trace/parent.rs

@@ -389,7 +389,7 @@ fn capstone_find_events(
                         // FIXME: This could be made certain; either determine all cases where
                         // only reads happen, or have an intermediate mempr_* function to first
                         // map the page(s) as readonly and check if a segfault occurred.
-                        
+
                         // Per https://docs.rs/iced-x86/latest/iced_x86/enum.OpAccess.html,
                         // we know that the possible access types are Read, CondRead, Write,
                         // CondWrite, ReadWrite, and ReadCondWrite. Since we got a segfault

tests/native-lib/tracing/fail/partial_init.rs renamed to tests/native-lib/fail/tracing/partial_init.rs

@@ -1,3 +1,6 @@
+//@only-target: x86_64-unknown-linux-gnu i686-unknown-linux-gnu
+//@compile-flags: -Zmiri-native-lib-enable-tracing
+
 extern "C" {
     fn init_n(n: i32, ptr: *mut u8);
 }

tests/native-lib/tracing/fail/partial_init.stderr renamed to tests/native-lib/fail/tracing/partial_init.stderr

@@ -1,5 +1,5 @@
 warning: sharing memory with a native function called via FFI
-  --> tests/native-lib/tracing/fail/partial_init.rs:LL:CC
+  --> tests/native-lib/fail/tracing/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/tracing/fail/partial_init.rs:LL:CC
+   = note: inside `partial_init` at tests/native-lib/fail/tracing/partial_init.rs:LL:CC
 note: inside `main`
-  --> tests/native-lib/tracing/fail/partial_init.rs:LL:CC
+  --> tests/native-lib/fail/tracing/partial_init.rs:LL:CC
    |
 LL |     partial_init();
    |     ^^^^^^^^^^^^^^
 
 error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
-  --> tests/native-lib/tracing/fail/partial_init.rs:LL:CC
+  --> tests/native-lib/fail/tracing/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/tracing/fail/partial_init.rs:LL:CC
+   = note: inside `partial_init` at tests/native-lib/fail/tracing/partial_init.rs:LL:CC
 note: inside `main`
-  --> tests/native-lib/tracing/fail/partial_init.rs:LL:CC
+  --> tests/native-lib/fail/tracing/partial_init.rs:LL:CC
    |
 LL |     partial_init();
    |     ^^^^^^^^^^^^^^

tests/native-lib/tracing/fail/unexposed_reachable_alloc.rs renamed to tests/native-lib/fail/tracing/unexposed_reachable_alloc.rs

@@ -1,3 +1,6 @@
+//@only-target: x86_64-unknown-linux-gnu i686-unknown-linux-gnu
+//@compile-flags: -Zmiri-permissive-provenance -Zmiri-native-lib-enable-tracing
+
 extern "C" {
     fn do_one_deref(ptr: *const *const *const i32) -> usize;
 }

tests/native-lib/tracing/fail/unexposed_reachable_alloc.stderr renamed to tests/native-lib/fail/tracing/unexposed_reachable_alloc.stderr

@@ -1,5 +1,5 @@
 warning: sharing memory with a native function called via FFI
-  --> tests/native-lib/tracing/fail/unexposed_reachable_alloc.rs:LL:CC
+  --> tests/native-lib/fail/tracing/unexposed_reachable_alloc.rs:LL:CC
    |
 LL |     unsafe { do_one_deref(exposed) };
    |              ^^^^^^^^^^^^^^^^^^^^^ sharing memory with a native function
@@ -10,49 +10,30 @@ LL |     unsafe { do_one_deref(exposed) };
    = 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 `unexposed_reachable_alloc` at tests/native-lib/fail/tracing/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
+  --> tests/native-lib/fail/tracing/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
+  --> tests/native-lib/fail/tracing/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 `unexposed_reachable_alloc` at tests/native-lib/fail/tracing/unexposed_reachable_alloc.rs:LL:CC
 note: inside `main`
-  --> tests/native-lib/tracing/fail/unexposed_reachable_alloc.rs:LL:CC
+  --> tests/native-lib/fail/tracing/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
+error: aborting due to 1 previous error; 1 warning emitted
 

tests/native-lib/pass/ptr_read_access.rs

@@ -1,5 +1,5 @@
 //@revisions: trace notrace
-//@[trace] only-target: x86_64-unknown-linux-gnu x86-unknown-linux-gnu
+//@[trace] only-target: x86_64-unknown-linux-gnu i686-unknown-linux-gnu
 //@[trace] compile-flags: -Zmiri-native-lib-enable-tracing
 
 fn main() {

tests/native-lib/pass/ptr_write_access.rs

@@ -1,5 +1,5 @@
 //@revisions: trace notrace
-//@[trace] only-target: x86_64-unknown-linux-gnu x86-unknown-linux-gnu
+//@[trace] only-target: x86_64-unknown-linux-gnu i686-unknown-linux-gnu
 //@[trace] compile-flags: -Zmiri-native-lib-enable-tracing
 //@compile-flags: -Zmiri-permissive-provenance
 

tests/ui.rs

@@ -343,20 +343,6 @@ 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(())