From b0a463334c866ed479609adce1ee30911bf30cea Mon Sep 17 00:00:00 2001 From: Tavian Barnes Date: Fri, 3 Dec 2021 15:57:37 -0500 Subject: [PATCH 1/2] intptrcast: Never allocate two objects directly adjecent When two objects directly follow each other in memory, what is the provenance of an integer cast to a pointer that points directly between them? For a zero-size region, it could point into the end of the first object, or the start of the second. We can avoid answering this difficult question by simply never allocating two objects directly beside each other. This fixes some of the false positives from #1866. --- src/intptrcast.rs | 9 +++++---- tests/run-pass/adjacent-allocs.rs | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 tests/run-pass/adjacent-allocs.rs diff --git a/src/intptrcast.rs b/src/intptrcast.rs index 665a134184..91fd26ed75 100644 --- a/src/intptrcast.rs +++ b/src/intptrcast.rs @@ -1,5 +1,4 @@ use std::cell::RefCell; -use std::cmp::max; use std::collections::hash_map::Entry; use log::trace; @@ -107,9 +106,11 @@ impl<'mir, 'tcx> GlobalState { slack, ); - // Remember next base address. If this allocation is zero-sized, leave a gap - // of at least 1 to avoid two allocations having the same base address. - global_state.next_base_addr = base_addr.checked_add(max(size.bytes(), 1)).unwrap(); + // Remember next base address. Leave a gap of at least 1 to avoid two zero-sized allocations + // having the same base address, and to avoid ambiguous provenance for the address between two + // allocations. + let bytes = size.bytes().checked_add(1).unwrap(); + global_state.next_base_addr = base_addr.checked_add(bytes).unwrap(); // Given that `next_base_addr` increases in each allocation, pushing the // corresponding tuple keeps `int_to_ptr_map` sorted global_state.int_to_ptr_map.push((base_addr, alloc_id)); diff --git a/tests/run-pass/adjacent-allocs.rs b/tests/run-pass/adjacent-allocs.rs new file mode 100644 index 0000000000..812dfcc66c --- /dev/null +++ b/tests/run-pass/adjacent-allocs.rs @@ -0,0 +1,21 @@ +fn main() { + // The slack between allocations is random. + // Loop a few times to hit the zero-slack case. + for _ in 0..1024 { + let n = 0u64; + let ptr: *const u64 = &n; + + // Allocate a new stack variable whose lifetime quickly ends. + // If there's a chance that &m == ptr.add(1), then an int-to-ptr cast of + // that value will have ambiguous provenance between n and m. + // See https://github.com/rust-lang/miri/issues/1866#issuecomment-985770125 + { + let m = 0u64; + let _ = &m as *const u64; + } + + let iptr = ptr as usize; + let zst = (iptr + 8) as *const (); + unsafe { *zst } + } +} From 6a98c64c8b684ad12ae1a11fc7ad55a176b45bfe Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 5 Dec 2021 20:33:20 -0500 Subject: [PATCH 2/2] final tweaks --- src/intptrcast.rs | 6 +++--- tests/run-pass/adjacent-allocs.rs | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/intptrcast.rs b/src/intptrcast.rs index 91fd26ed75..6f4169e950 100644 --- a/src/intptrcast.rs +++ b/src/intptrcast.rs @@ -108,9 +108,9 @@ impl<'mir, 'tcx> GlobalState { // Remember next base address. Leave a gap of at least 1 to avoid two zero-sized allocations // having the same base address, and to avoid ambiguous provenance for the address between two - // allocations. - let bytes = size.bytes().checked_add(1).unwrap(); - global_state.next_base_addr = base_addr.checked_add(bytes).unwrap(); + // allocations (also see https://github.com/rust-lang/unsafe-code-guidelines/issues/313). + let size_plus_1 = size.bytes().checked_add(1).unwrap(); + global_state.next_base_addr = base_addr.checked_add(size_plus_1).unwrap(); // Given that `next_base_addr` increases in each allocation, pushing the // corresponding tuple keeps `int_to_ptr_map` sorted global_state.int_to_ptr_map.push((base_addr, alloc_id)); diff --git a/tests/run-pass/adjacent-allocs.rs b/tests/run-pass/adjacent-allocs.rs index 812dfcc66c..509965fe4f 100644 --- a/tests/run-pass/adjacent-allocs.rs +++ b/tests/run-pass/adjacent-allocs.rs @@ -16,6 +16,7 @@ fn main() { let iptr = ptr as usize; let zst = (iptr + 8) as *const (); + // This is a ZST ptr just at the end of `n`, so it should be valid to deref. unsafe { *zst } } }