diff options
| author | bors <bors@rust-lang.org> | 2018-09-16 18:03:39 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2018-09-16 18:03:39 +0000 |
| commit | 8a2dec6e583bc6425a91b277bdc6c602088845f1 (patch) | |
| tree | 0177aaa3b37f91f57eb87954ee64513ca66ff34e | |
| parent | d3cba9b4b4959d63eaaf5ffd647e4a5fc6d43e17 (diff) | |
| parent | 357c5dacee1015dc03583287d0c7a132d9fe7880 (diff) | |
Auto merge of #53804 - RalfJung:ptr-invalid, r=nagisa
fix some uses of pointer intrinsics with invalid pointers
[Found by miri](https://github.com/solson/miri/pull/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 https://github.com/rust-lang/rust/pull/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?
| -rw-r--r-- | src/liballoc/vec.rs | 10 | ||||
| -rw-r--r-- | src/libstd/collections/hash/table.rs | 4 |
2 files changed, 7 insertions, 7 deletions
diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs index e9c1a3df518..7fc4453fec5 100644 --- a/src/liballoc/vec.rs +++ b/src/liballoc/vec.rs @@ -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); @@ -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); diff --git a/src/libstd/collections/hash/table.rs b/src/libstd/collections/hash/table.rs index 768357ec8dc..547f97cc8ac 100644 --- a/src/libstd/collections/hash/table.rs +++ b/src/libstd/collections/hash/table.rs @@ -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) } } |
