Zero allocation results in non-portable behavior

[isaachier]

I decided to revisit the use of clang-tidy on the C++ code. As I browsed through the warnings I came across an issue involving an allocation of zero bytes. So, I just updated the util.hpp code to check for zero sized allocations, and in those cases return nullptr. When running the zig tests, I hit an assertion, tried tweaking it, hit another assertion, etc. So instead of going through the whole process of cleaning this up on my own (probably incorrectly), let me show you the code and the failed assertions. If this is a real issue, I'd be happy to follow up on it. I will submit the diff in a PR momentarily.

EDIT: The reason this is non-portable behavior is that malloc on zero is allowed to return null. It seems on the platforms used to test zig, this is not the case. A special pointer is returned with a non-null value. That is the only way I can explain the assertions failing when returning null.

[andrewrk]

Can we solve this whole thing by adding...

if (count == 0)
    return nullptr;

...to all the allocation functions in util.hpp?

[isaachier]

Ya that's the point. But now the assertions fail.

[bnoordhuis]

if (count == 0) return nullptr;

Passing nullptr to memcpy/memcmp/etc. is technically UB, even when n == 0, so unless you're really sure that never happens anywhere, it might be better to turn it into a 1-element allocation:

if (count == 0)
  count = 1;

[andrewrk]

Oops my mistake, sorry for not reading your text correctly @isaachier.

I'll go through the assertion failures you pointed out.

[isaachier]

@bnoordhuis the alternative isn't great either. Right now there are assertions passing because a pointer is the equivalent of 0XDEADBEEF as opposed to null when neither signify a non-zero allocation.

[isaachier]

But @bnoordhuis I see your point about making it 1 byte. I think C++ (at least before C++20) might do the equivalent for empty structs.

[PavelVozenilek]

In C:

Upon successful completion with size not equal to 0, malloc() shall return a pointer to the allocated space. If size is 0, either a null pointer or a unique pointer that can be successfully passed to free() shall be returned. Otherwise, it shall return a null pointer and set errno to indicate the error.

http://pubs.opengroup.org/onlinepubs/009695399/functions/malloc.html

[PavelVozenilek]

The fix (if (size == 0) return nullptr) is not wise. If this ever happens, it would most likely trigger some OOM mechanim, or exit the program. Hardly anyone would check for such specific situation.

Allocator should return one or more bytes, in debug mode filled with something and then check these byte(s) when freeing.

[isaachier]

I think sqlite uses the first 4 bytes to store the length of the allocation. That sort of thing is useful and resolves the issue.