4 ms·
Bizarre. I think I've been writing broken Rust code for a couple years. If I understand you correctly something like: let mut data = Vec::with_capacity(sz)
by nemothekid 1y ago
Bizarre. I think I've been writing broken Rust code for a couple years. If I understand you correctly something like:
let mut data = Vec::with_capacity(sz);
unsafe { data.set_len(sz) };
buf.copy_to_slice(data.as_mut_slice());
is UB?
- vgatherps 1y agoYes, this is the case that I ran into as well. You have to zero memory before reading and/or have some crazy combination of tracking what’s uninitialized capacity or initialized len, I think the rust stdlib write trait for &mut Vec got butchered over this concern. It’s strictly more complicated and slower than the obvious thing to do and only exists to satisfy the abstract machine.
- Arnavion 1y agoNo. The correct way to write that code is to use .spare_capacity_mut() to get a &mut [MaybeUninit<T>], then write your Ts into that using .write_copy_of_slice(), then .set_len(). And that will not be any slower (though obviously more complicated) than the original incorrect code.
- vgatherps 1y agoOh this is very nice, I think it was stabilized since I wrote said code.
- nemothekid 1y agowrite_copy_of_slice doesn't look to be stable. I'll mess around with godbolt, but my hope that whatever incantation is used compiles down to a memcpy
- Arnavion 1y agoAs I wrote in https://news.ycombinator.com/item?id=44048391 https://news.ycombinator.com/item?id=44048391 , you have to get used to copying the libstd impl when working with MaybeUninit. For my code I put a "TODO(rustup)" comment on such copies, to remind myself to revisit them every time I update the Rust version in toolchain.toml
- nemothekid 1y agoIn other words the """safe""" stable code looks like this: let mut data = Vec::with_capacity(sz); let mut dst_uninit = data.spare_capacity_mut(); let uninit_src: &[MaybeUninit<T>] = unsafe { transmute(buf) }; dst_uninit.copy_from_slice(uninit_src); unsafe { data.set_len(sz) };
- Arnavion 1y agoThat's correct.
- eptcyka 1y agoValgrind it :)
- vlovich123 1y agoValgrind doesn’t tell you about UB, just if the code did something incorrect with memory and that depends on what the optimizer did if you did write UB code. You’ll need Miri to tell you if this kind of code is triggering UB which works by evaluating and analyzing the mid level of compiler output to check if Rust rules about safety are followed.
- eptcyka 1y agoReading from uninitialised memory is a fault that valgrind will detect.
- vlovich123 1y agoBut that’s precisely NOT the problem that exists in OPs code. It’s a problem Valgrind will detect if and only if the optimizer does something weird to exploit the UB in the code which may or may not happen AND doesn’t even necessarily happen on that line of code which will leave you scratching your head. UB is weird and valgrind is not a tool for detecting UB. For that you want Miri or UBSAN. Valgrind’s equivalent is ASAN and MSAN which catch UB issues incidentally in some rare cases and not necessarily where the UB actually happened.
- ironhaven 1y agoThat's right. Line 3 is undefined behaviour because you are creating mutable references to the uninit spare capacity of the vec. copy_to_slice only works with writing to initialized slices. The proper way for you example to mess with the uninitialized memory on a vec would be only use raw pointers or calling the newly added Vec::spare_capacity_mut function on the vec that returns a slice of MaybeUninit
- NobodyNada 1y agoIt's an open question whether creating a reference to an uninitialized value is instant UB, or only UB if that reference is misused (e.g. if copy_to_slice reads an uninitialized byte). The specific discussion is whether the language requires "recursive validity for references", which would mean constructing a reference to an invalid value is "language UB" (your program is not well specified and the compiler is allowed to "miscompile" it) rather than "library UB" (your program is well-specified, but functions you call might not expect an uninitialized buffer and trigger language UB). See the discussion here: https://github.com/rust-lang/unsafe-code-guidelines/issues/346 https://github.com/rust-lang/unsafe-code-guidelines/issues/3... Currently, the team is leaning in the direction of not requiring recursive validity for references. This would mean your code is not language UB as long as you can assume `set_len` and `copy_to_slice` never read from 'data`. However, it's still considered library UB, as this assumption is not documented or specified anywhere and is not guaranteed -- changes to safe code in your program or in the standard library can turn this into language UB, so by doing something like this you're writing fragile code that gives up a lot of Rust's safety by design.
- bombela 1y agoWhy not simply: let mut data = Vec::with_capacity(sz); data.extend(&buf[..sz]); Vec::extend extends a container from an iterable. A Vec/slice is iterable. And from the doc: > This implementation is specialized for slice iterators, where it uses copy_from_slice to append the entire slice at once. Of course this trivial example could also be written as: let mut data = buf.clone();