Skip to content

RawVecInner: add missing unsafe to unsafe fns #145067

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

btj
Copy link
Contributor

@btj btj commented Aug 7, 2025

Some (module-private) functions in library/alloc/src/raw_vec/mod.rs are unsafe (i.e. may cause UB when called from safe code) but are not marked unsafe. Specifically:

  • RawVecInner::grow_exact causes UB if called with len and additional arguments such that len + additional is less than the current capacity. Indeed, in that case it calls Allocator::grow with a new_layout that is smaller than old_layout, which violates a safety precondition.
  • The RawVecInner methods for resizing the buffer cause UB if called with an elem_layout different from the one used to initially allocate the buffer, because in that case Allocator::grow or Allocator::shrink are called with an old_layout that does not fit the allocated block, which violates a safety precondition.
  • RawVecInner::current_memory might cause UB if called with an elem_layout different from the one used to initially allocate the buffer, because the unchecked_mul might overflow.
  • Furthermore, these methods cause UB if called with an elem_layout where the size is not a multiple of the alignment. This is because Layout::repeat is used (in layout_array) to compute the allocation's layout when allocating, which includes padding to ensure alignment of array elements, but simple multiplication is used (in current_memory) to compute the old allocation's layout when resizing or deallocating, which would cause the layout used to resize or deallocate to not fit the allocated block, which violates a safety precondition.

I discovered these issues while performing formal verification of library/alloc/src/raw_vec/mod.rs per Challenge 19 of the AWS Rust Standard Library Verification Contest.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 7, 2025
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

- RawVecInner::grow_exact causes UB if called with len and additional
  arguments such that len + additional is less than the current
  capacity.  Indeed, in that case it calls Allocator::grow with a
  new_layout that is smaller than old_layout, which violates a safety
  precondition.

- All RawVecInner methods for resizing the buffer cause UB
  if called with an elem_layout different from the one used to initially
  allocate the buffer, because in that case Allocator::grow/shrink is called with
  an old_layout that does not fit the allocated block, which violates a
  safety precondition.

- RawVecInner::current_memory might cause UB if called with an elem_layout
  different from the one used to initially allocate the buffer, because
  the unchecked_mul might overflow.

- Furthermore, these methods cause UB if called with an elem_layout
  where the size is not a multiple of the alignment. This is because
  Layout::repeat is used (in layout_array) to compute the allocation's
  layout when allocating, which includes padding to ensure alignment of
  array elements, but simple multiplication is used (in current_memory) to
  compute the old allocation's layout when resizing or deallocating, which
  would cause the layout used to resize or deallocate to not fit the
  allocated block, which violates a safety precondition.
@btj btj marked this pull request as ready for review August 8, 2025 07:54
@rustbot
Copy link
Collaborator

rustbot commented Aug 8, 2025

r? @thomcc

rustbot has assigned @thomcc.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 8, 2025
Copy link
Contributor

@celinval celinval left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not an std reviewer, but I wanted to suggest that you add safety comments to the functions that you marked as unsafe.

You could even go one step further and try adding contracts to them, but be aware that it is still highly experimental, and it may not work.

@btj
Copy link
Contributor Author

btj commented Aug 8, 2025

@celinval I considered that, but I noticed that most of the safety preconditions of the functions that were already marked unsafe were not documented either, so concerns of adhering to existing practice and avoiding excessive "noise" kept me from adding them so far. Happy to add them if @thomcc thinks it makes sense. Mostly each resizing or deallocation method should get a safety precondition saying elem_layout must be the same one used in earlier calls and must have a size that is a multiple of the alignment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants