Is sdallocx safe to call with a null pointer?

[davidMcneil]

With recent changes to an application, I am running into the error memory allocation of n bytes failed (where n is a small number eg. 5, 1, 4) when using jemallocator. The application works fine when using the system allocator.

Running jemalloc in debug mode reveals that it is failing the assert that sdallocx was not called with a null pointer.

The jemalloc-sys docs imply it is safe to call sdallocx with a null pointer (it states "no action occurs"). I cannot find anything in jemalloc docs to confirm this, and by the assert in the code it appears as if it is explicitly not allowed.

Modifying sdallocx to simply return when called with a null pointer appears to fix the issue, but I am not sure of the ramifications of this change.

I apologize for not having a minimal example with which to replicate. The scenario in which the error occurs is fairly involved.

[gnzlbg]

According to jemalloc's docs:

• The dallocx() function causes the memory referenced by ptr to be made available for future allocations.

• The sdallocx() function is an extension of dallocx() with a size parameter to allow the caller to pass in the allocation size as an optimization. The minimum valid input size is the original requested size of the allocation, and the maximum valid input size is the corresponding value returned by nallocx() or sallocx().

so IIUC the following should work:

void* ptr = malloc(0); // Does this return NULL ? jemalloc docs don't say
sdallocx(ptr, 0);

yet that would always make the assert fail. So either this snippet is correct and the assert is incorrect, or the docs are incorrect, and this snippet would need to be written like this:

void* ptr = malloc(size);
if (size == 0) { free(ptr); } else { sdallocx(ptr, size); }

which IMO would make the API of jemalloc unnecessarily hard to use correctly.

@davidtgoldblatt could you clarify what the intent is here?

[gnzlbg]

@davidMcneil

The malloc() and calloc() functions return a pointer to the allocated memory if successful; otherwise a NULL pointer is returned and errno is set to ENOMEM.

That is, jemalloc memory allocation functions only returns NULL if the allocation failed, and sdallocx does not support being called with NULL, so the error is in the comment in this crate. Calling sdallocx with NULL is not allowed, you need to call it with an allocation that succeeded.

[davidMcneil]

Thanks for the response @gnzlbg! I am not using jemalloc-sys directly. The bad call to sdallocx is occurring in jemallocator's implementation of dealloc.

I will see if I can get a more minimal example replicating the issue.

[gnzlbg]

The behavior of calling {GlobalAlloc, Alloc}::dealloc with a null pointer or with a size of 0 is undefined, so the bug that's causing that must be somewhere else. I'm going to leave this issue open until the documentation bug for jemalloc-sys is fixed.

We should probably also add an assume!(!ptr.is_null()) to {GlobalAlloc, Alloc}::dealloc.

[davidtgoldblatt]

@davidtgoldblatt could you clarify what the intent is here?

We've historically tried to avoid taking a strong stance on this sort of implementation-defined thing (e.g. return value of malloc(0)). Wanting C++ interop has made this harder and harder, to the point that I think we might just as well commit to malloc(0) being non-NULL (barring OOM).

I'm not sure anyone ever thought too hard about the sdallocx-with-zero case. We used to assert against it, but took it away (again, for C++ interop reasons). The more I think about it, this less I'm convinced that it was ever the right thing to do. If we give out non-NULL pointers in response to a request for a particular size, we ought to accept that size back again in sdallocx.

[gnzlbg]

@davidtgoldblatt is there an issue in jemalloc upstream that we could cross-reference from here ?

In Rust, the behavior of requesting an allocation with zero-size is undefined, i.e., malloc(N) will always be called with N > 0, so if it returns a nullptr, then that must be because the allocation failed. Trying to deallocate such a nullptr (e.g. free(NULL), or sdallocx(NULL, N > 0)) is also UB in Rust, so these cases should never be triggered. That is, we will add asserts anyways to our wrappers to try to diagnose these issues in the Rust side.

[davidtgoldblatt]

@davidtgoldblatt is there an issue in jemalloc upstream that we could cross-reference from here ?

I don't think so; what specifically are you referring to though? (I think a catch-all "there should be better documentation around edge-cases" issue with a list would be useful; I don't think we have one at the moment).

[davidtgoldblatt]

I started a list at jemalloc/jemalloc#1716

[gnzlbg]

Thanks, that was precisely what I was referring to.