Skip to content

Commit

Permalink
Auto merge of #53804 - RalfJung:ptr-invalid, r=nagisa
Browse files Browse the repository at this point in the history
fix some uses of pointer intrinsics with invalid pointers

[Found by miri](rust-lang/miri#446):

* `Vec::into_iter` calls `ptr::read` (and the underlying `copy_nonoverlapping`) with an unaligned pointer to a ZST. [According to LLVM devs](https://bugs.llvm.org/show_bug.cgi?id=38583), this is UB because it contradicts the metadata we are attaching to that pointer.
* `HashMap` creation calls `ptr:.write_bytes` on a NULL pointer with a count of 0. This is likely not currently UB *currently*, but it violates the rules we are setting in #53783, and we might want to exploit those rules later (e.g. with more `nonnull` attributes for LLVM).

    Probably what `HashMap` really should do is use `NonNull::dangling()` instead of 0 for the empty case, but that would require a more careful analysis of the code.

It seems like ideally, we should do a review of usage of such intrinsics all over libstd to ensure that they use valid pointers even when the size is 0. Is it worth opening an issue for that?
  • Loading branch information
bors committed Sep 16, 2018
2 parents d3cba9b + 357c5da commit 8a2dec6
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 7 deletions.
10 changes: 4 additions & 6 deletions src/liballoc/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2410,9 +2410,8 @@ impl<T> Iterator for IntoIter<T> {
// same pointer.
self.ptr = arith_offset(self.ptr as *const i8, 1) as *mut T;

// Use a non-null pointer value
// (self.ptr might be null because of wrapping)
Some(ptr::read(1 as *mut T))
// Make up a value of this ZST.
Some(mem::zeroed())
} else {
let old = self.ptr;
self.ptr = self.ptr.offset(1);
Expand Down Expand Up @@ -2451,9 +2450,8 @@ impl<T> DoubleEndedIterator for IntoIter<T> {
// See above for why 'ptr.offset' isn't used
self.end = arith_offset(self.end as *const i8, -1) as *mut T;

// Use a non-null pointer value
// (self.end might be null because of wrapping)
Some(ptr::read(1 as *mut T))
// Make up a value of this ZST.
Some(mem::zeroed())
} else {
self.end = self.end.offset(-1);

Expand Down
4 changes: 3 additions & 1 deletion src/libstd/collections/hash/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,9 @@ impl<K, V> RawTable<K, V> {
) -> Result<RawTable<K, V>, CollectionAllocErr> {
unsafe {
let ret = RawTable::new_uninitialized_internal(capacity, fallibility)?;
ptr::write_bytes(ret.hashes.ptr(), 0, capacity);
if capacity > 0 {
ptr::write_bytes(ret.hashes.ptr(), 0, capacity);
}
Ok(ret)
}
}
Expand Down

0 comments on commit 8a2dec6

Please sign in to comment.