unsafe fn malloc_size_of doesn't explicitly document its safety requirements

[anforowicz]

It would be great if the documentation of MallocSizeOfOps.malloc_size_of would explicitly list the safety requirements of this unsafe function. This would make clippy happy (see the missing_safety_doc lint) and would also make it easier to audit calls into the function (e.g. when auditing recent smallvec changes here).

It's a bit hard to reverse-engineer the safety requirements, because it is impossible to look at all the impls of the MallocSizeOfOps trait, but I have some guesses below as a tentative starting point:

• Maybe: ptr needs to be point to memory currently allocated by the allocator associated with the given impl of MallocSizeOfOps?
  • But if so, then it may mean that shallow_size_of in smallvec should be unsafe (because its safety requirement should require that ops corresponds to the right allocator).
• Maybe: ptr just needs to be valid for reading?
  • But if so, then it would mean that before looking up allocator-specific metadata (e.g. stored in bytes preceding the allocation) the implementation of malloc_size_of would need to somehow check if the pointer points to "currently allocated memory". And if so, then it should also be able to check if ptr is valid for reading.

The guesses above don't look quite right to me. I feel that the real safety requirements are different from the guesses above. I am probably missing an understanding of how this API is intended to work. Help please? :-)

[nicoburns]

Agree that this would be good.

For context, this is a crate that has lived in the Firefox/Servo codebases for some years but is now being published in order to allow for a more sensible dependency graph that still conforms to Rust's orphan rules for trait implementations. I didn't add safety comments (which weren't present in the version from Firefox's tree) because I wasn't myself sure of what they were.

Looking into this, the function that the implementation of MallocSizeOfOps in the Servo codebase is ultimately delegating to https://docs.rs/jemalloc-sys/latest/jemalloc_sys/fn.malloc_usable_size.html (Firefox has it's own mozjemalloc fork of jemalloc, but I would be surprised if this bit is different), which has the following documentation:

Errors

If ptr is null, 0 is returned.

Safety

The behavior is undefined if:

• ptr does not match a pointer earlier returned by the memory allocation functions of this crate, or
• the memory region referenced by ptr has been deallocated.

I'm assuming that "undefined" here means actual Undefined Behaviour.

---

Reasoning about the above, I think that both MallocSizeOf::size_of and MallocShallowSizeOf::shallow_size_of really ought to be unsafe as they are only safe to call if:

• They were allocated using the same allocator as the MallocSizeOfOps impl (or trivially if the type does not contain an allocation at all)
• In the case of MallocSizeOf::size_of, if all nested allocations were also allocated using that same allocator

It seems to me that this functionality would be quite tricky to use unless one is using the same allocator across the entire codebase, but then again I expect that there are many applications where this is the case (one allocator in the binary).

---

Looking at the jemalloc codebase it looks like they have fallible lookup functions (which return an error rather than triggering undefined behaviour when the pointer isn't tracked by the allocator), they just don't expose it publicly. Which is a bit of a shame as I think the whole interface could be safe if that was exposed.