Skip to content

Soundness/documentation issue: CallocBackingStore::new is not safety documented and potentially unsound #20

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
veluca93 opened this issue May 28, 2024 · 0 comments

Comments

@veluca93
Copy link

The unsafe function CallocBackingStore::new does not have documentation for its safety invariants - and neither does the define_allocator_memory_pool macro calling it.

pub unsafe fn new(num_elements : usize, alloc : AllocatorC, free : unsafe extern "C" fn (*mut u8), should_free : bool) -> Self{

I believe the safety contract should mention that:

  • the caller must guarantee that the allocator passed in can be safely called (including the corner-case of having a 0 argument for size), and returns buffers of the appropriate size.
  • the correct relationship between alloc and free is upheld (free must be a valid function to free the memory allocated by alloc)
  • computing num_elements * sizeof(T) must not overflow -- unless that is checked in the body of the function.

Moreover, using the macro with malloc is always unsound unless T is MaybeUninit<_>: new will end up creating a reference to a slice of uninitialized data, which is UB.
Even using the function with calloc might be unsound if 0 is not a valid bit pattern for T.
Similarly, when using a custom allocator, the bitpatterns returned by the custom allocator should be valid for T.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant