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

Does the Allocator API allow sending pointer ownership across an FFI boundary? #550

Open
not-an-aardvark opened this issue Jan 3, 2025 · 1 comment

Comments

@not-an-aardvark
Copy link

not-an-aardvark commented Jan 3, 2025

Does the following code have UB?

extern "C" {
    // Hooks for a well-behaved memory allocator implemented in another language.
    fn ffi_malloc(...) -> NonNull<[u8]>;
    fn ffi_free(...);

    // Returns a pointer which was allocated using ffi_malloc. The caller is responsible for freeing it using ffi_free.
    fn get_foo_data() -> *mut u8;
}

struct FfiAllocator{};
impl std::alloc::Allocator for FfiAllocator {
    // ...Implements allocate() and deallocate() by passing through to ffi_malloc() and ffi_free()
    // (omitted for brevity)
};

fn main() {
    // Safety(?): get_foo_data() returns a pointer which is owned by the caller and was allocated by the FFI allocator
    let _ = unsafe {
        Box::from_raw_in(get_foo_data(), FfiAllocator{})
    };
}

The summary is:

  • A pointer is allocated using FFI. The allocation does not directly use an Allocator::allocate call in Rust because it's implemented in another language.
  • The pointer is deallocated using Allocator::deallocate, using a custom implementation of Allocator which forwards to the appropriate FFI method to free the memory.

Based on a strict reading of https://doc.rust-lang.org/std/alloc/trait.Allocator.html#currently-allocated-memory, this is a bit dubious. The result of get_foo_data() is not "currently allocated" via FfiAllocator{} because it was not previously returned by FfiAllocator::allocate.

However, the "currently allocated" precondition is a requirement imposed on the caller of Allocator::deallocate. Are specific implementations of the Allocator trait allowed to loosen this requirement? (For example, FfiAllocator::deallocate could document that it also accepts live pointers that were previously returned from ffi_malloc, even if those pointers were not returned from FfiAllocator::allocate.)

For a "normal" trait, a specific implementation of the trait would be allowed to weaken preconditions like this. However, the Allocator trait has some special interactions with opsem (e.g. allocating memory "mints" a new provenance #442, and memory allocations can nondeterministically be removed #328), and the details of this are still being figured out. So it's unclear whether allocators are allowed to do this.

Reasons it could theoretically be useful to allow this:

  • Allows using the allocator API for allocations that are created by another language. Arguably we could bless this to provide a baseline level of usability for Allocator, even as we figure out the other questions surrounding Allocator.

Reasons it could theoretically be useful to disallow this:

  • If we allow this, and there are multiple implementations of FfiAllocator (FfiAllocator1, FfiAllocator2, etc.) which all pass through to the same implementations of ffi_malloc and ffi_free, then it would probably be valid to allocate a pointer with FfiAllocator1 and deallocate the same pointer with FfiAllocator2. This could interfere with alias analysis in the following scenario:

    let ptr1 = FfiAllocator1::allocate(...)?;
    let ptr2 = black_box(ptr1, ...);
    {
        // performance-sensitive code involving ptr1 and ptr2
    }
    FfiAllocator2::deallocate(ptr2);
    

    Hypothetically it might be useful for the compiler to be able to assume that ptr1 and ptr2 don't alias. It's not clear how realistic this is though.

@CAD97
Copy link

CAD97 commented Jan 3, 2025

(this is my own opinion and not necessarily a reflection of consensus)

Based on a strict reading of [the trait documentation], this is a bit dubious.

If you are using a specific impl Allocator, then the trait’s requirements do not matter; known implementations of a trait are always allowed to provide stronger guarantees than required by the generic interface.

However, you are correct that allocation is tied into the opsem, and the details are being figured out, so this isn't 100% guaranteed with the Allocator trait; it would be nice to be able to optimize out drop(Box::new_in(0, MyAlloc)) like with drop(Box::new(0)).

I believe there's a prior issue tracking “where” the magic applies. It seems like the best way to semantically represent it is a Magic<A> type which provides "magic" allocation semantics backed by the configured backing allocator. (Global uses this to wrap around the provided #[global_allocator], through the GlobalAlloc thin waist.)

This allows there to be nothing magic about unsafe impl Allocator, which is nice1.

Found the prior issue, which includes this:

Footnotes

  1. I think the only allowed trait impl with magic semantics are Copy (has structural requirement) and Drop (can't call Drop::drop).

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

2 participants