Skip to content

Vec::from_raw_parts docs do not correctly handle empty buffers #119304

New issue

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

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

Already on GitHub? # to your account

Open
oskgo opened this issue Dec 25, 2023 · 2 comments · May be fixed by #129483
Open

Vec::from_raw_parts docs do not correctly handle empty buffers #119304

oskgo opened this issue Dec 25, 2023 · 2 comments · May be fixed by #129483
Assignees
Labels
A-collections Area: `std::collections` A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@oskgo
Copy link
Contributor

oskgo commented Dec 25, 2023

Location

https://doc.rust-lang.org/std/vec/struct.Vec.html#method.from_raw_parts

Summary

The docs state the precondition "ptr must have been allocated using the global allocator, such as via the alloc::alloc function", which means that the following code is unsound since Vec::new does not allocate to produce the pointer:

fn reassemble<T>(mut v: Vec<T>) -> Vec<T> {
  let capacity = v.capacity();
  let ptr = v.as_mut_ptr();
  let length = v.len();
  std::mem::forget(v);
  unsafe {Vec::from_raw_parts(ptr, length, capacity)}
}

I believe this to be highly surprising, and probably unintended.

Vec::from_raw_parts should allow ptr not to be obtained from an allocation if capacity times the size of T is zero.

If the documentation is correct InPlaceDstBufDrop is unsound: https://github.com/rust-lang/rust/blob/master/library/alloc/src/vec/in_place_drop.rs#L37

String::from_raw_parts and Vec::from_raw_parts_in have the same issue.

@oskgo oskgo added the A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools label Dec 25, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Dec 25, 2023
@oskgo oskgo changed the title Handle empty buffers in Vec::from_raw_parts docs Vec::from_raw_parts do not correctly handle empty buffers Dec 25, 2023
@oskgo oskgo changed the title Vec::from_raw_parts do not correctly handle empty buffers Vec::from_raw_parts does not correctly handle empty buffers Dec 25, 2023
@the8472
Copy link
Member

the8472 commented Dec 25, 2023

This is a documentation error. If the vec has zero bytes capacity then it must be a dangling pointer. See #113113 for a recent PR adjusting its behavior towards that.

For an unallocated Vec Drop does nothing:

fn drop(&mut self) {
if let Some((ptr, layout)) = self.current_memory() {
unsafe { self.alloc.deallocate(ptr, layout) }
}
}

@the8472 the8472 added A-collections Area: `std::collections` T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Dec 25, 2023
@oskgo oskgo changed the title Vec::from_raw_parts does not correctly handle empty buffers Vec::from_raw_parts docs do not correctly handle empty buffers Dec 27, 2023
@lolbinarycat
Copy link
Contributor

@rustbot claim

lolbinarycat added a commit to lolbinarycat/rust that referenced this issue Aug 23, 2024
they now reflect the fact that
zero-capacity collections do not allocate

fixes rust-lang#119304
lolbinarycat added a commit to lolbinarycat/rust that referenced this issue Aug 24, 2024
they now reflect the fact that
zero-capacity collections do not allocate

fixes rust-lang#119304
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-collections Area: `std::collections` A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants