about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2018-09-16 18:03:39 +0000
committerbors <bors@rust-lang.org>2018-09-16 18:03:39 +0000
commit8a2dec6e583bc6425a91b277bdc6c602088845f1 (patch)
tree0177aaa3b37f91f57eb87954ee64513ca66ff34e
parentd3cba9b4b4959d63eaaf5ffd647e4a5fc6d43e17 (diff)
parent357c5dacee1015dc03583287d0c7a132d9fe7880 (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.rs10
-rw-r--r--src/libstd/collections/hash/table.rs4
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)
         }
     }