UB while loading Vec<bool>
Closed this issue · 1 comments
nes-emulator/src/serialization.rs
Line 144 in 0b9e321
impl<T: Savable> Savable for Vec<T> {
...
fn load(&mut self, fh: &mut Read) {
let mut len = 0usize;
len.load(fh);
self.truncate(0);
self.reserve(len);
for i in 0..len {
let mut x:T = unsafe { std::mem::uninitialized() };
x.load(fh);
self.push(x);
}
}
In case of Vec<bool>
we got:
let mut x:bool = unsafe { std::mem::uninitialized() };
Which is UB by definition: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
See comment rust-lang/rust#53491 (comment)
Thanks for catching this. I've pushed a commit
that uses a Default
trait instead. The only T
that is instantiated is u8
- used for the video recording feature - which already implements Default
.
I notice that your NewSavable
trait creates new objects rather than replacing the existing ones. Since my emulator currently uses pointers in places like CpuPpuInterconnect
Line 452 in f7cfcb9
it would actually be dangerous to replace all of the save
/load
functionality to return new objects because these pointers wouldn't be updated to point to them. I think you only changed the input recording feature, which is safe, but I'd prefer not to have two different serialization traits.