Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pointer problem in UnmanagedVector (backport #571) #574

Merged
merged 2 commits into from
Nov 14, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions libwasmvm/src/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,18 @@ impl UnmanagedVector {
match source {
Some(data) => {
let (ptr, len, cap) = {
// Can be replaced with Vec::into_raw_parts when stable
// https://doc.rust-lang.org/std/vec/struct.Vec.html#method.into_raw_parts
let mut data = mem::ManuallyDrop::new(data);
(data.as_mut_ptr(), data.len(), data.capacity())
if data.capacity() == 0 {
// we need to explicitly use a null pointer here, since `as_mut_ptr`
// always returns a dangling pointer (e.g. 0x01) on an empty Vec,
// which trips up Go's pointer checks.
// This is safe because the Vec has not allocated, so no memory is leaked.
(std::ptr::null_mut::<u8>(), 0, 0)
} else {
// Can be replaced with Vec::into_raw_parts when stable
// https://doc.rust-lang.org/std/vec/struct.Vec.html#method.into_raw_parts
let mut data = mem::ManuallyDrop::new(data);
(data.as_mut_ptr(), data.len(), data.capacity())
}
};
Self {
is_none: false,
Expand Down Expand Up @@ -261,6 +269,12 @@ impl UnmanagedVector {
pub fn consume(self) -> Option<Vec<u8>> {
if self.is_none {
None
} else if self.cap == 0 {
// capacity 0 means the vector was never allocated and
// the ptr field does not point to an actual byte buffer
// (we normalize to `null` in `UnmanagedVector::new`),
// so no memory is leaked by ignoring the ptr field here.
Some(Vec::new())
} else {
Some(unsafe { Vec::from_raw_parts(self.ptr, self.len, self.cap) })
}
Expand Down Expand Up @@ -349,7 +363,7 @@ mod test {
// Empty data
let x = UnmanagedVector::new(Some(vec![]));
assert!(!x.is_none);
assert_eq!(x.ptr as usize, 0x01); // We probably don't get any guarantee for this, but good to know where the 0x01 marker pointer can come from
assert_eq!(x.ptr as usize, 0);
assert_eq!(x.len, 0);
assert_eq!(x.cap, 0);

Expand All @@ -373,7 +387,7 @@ mod test {
// Empty data
let x = UnmanagedVector::some(vec![]);
assert!(!x.is_none);
assert_eq!(x.ptr as usize, 0x01); // We probably don't get any guarantee for this, but good to know where the 0x01 marker pointer can come from
assert_eq!(x.ptr as usize, 0);
assert_eq!(x.len, 0);
assert_eq!(x.cap, 0);
}
Expand Down
Loading