set memory to undefined in allocator implementations rather than interface

[andrewrk]

Follow-up from #4087, where I wrote:

Yes it makes sense to do it in the interface rather than in allocator implementations.

Having looked at the patch and seen this comment:

         // Note: can't set shrunk memory to undefined as memory shouldn't be modified on realloc failure 

I am now convinced that it should be done in the allocator implementations rather than the interface, where this problem can be solved. My reasoning is that a robust debug allocator will want to do this (handle realloc correctly with regards to defined), and while it wouldn't hurt semantically to have the memory set to undefined twice, this is an area where performance might matter a great deal, and it is better design to have one canonical place where the memory is set to undefined.

The allocator implementations may be able to much more efficiently set the data to undefined for allocations as well.

One concern is that allocator implementations will "forget" to do this. For this I propose that the standard library actually include a robust allocator test, which will work on any std.mem.Allocator API. People writing their own allocators should be able to call std.testing.allocatorInterfaceStressTest(&my_allocator, options); and be reasonably confident their allocator is working correctly, including with regards to setting memory to undefined. The options could include a flag to disable testing whether the memory gets set to undefined in debug/release-safe builds.

[dimdin]

There is another reason:
Setting bytes to undefined in free, hides double free detection.
The second free becomes use after free, when @memset to undefined happens.

[Cloudef]

I feel like this can easily lead to multiple useless memsets when one wraps multiple allocators on top of each other. There possibility should also be a way to allocate without memset for use-cases where you want to reserve big chunk of memory but not touch it

[gabe-lee]

So the tl;dr as I see it is:
CASE A: Allocator interface handles @memset(_, undefined)

• Difficulties undefining in certain cases because interface has limited access to underlying implementation
• Memory undefining performed only once and guaranteed by the interface (or is expected to be)

CASE B: Allocator internal implementation handles @memset(_, undefined)

• When using allocators that themselves take child/backing allocators, each level of allocator will run redundant @memset() on the same memory region
• Allocator implementation author is responsible for ensuring memory undefining, possible to forget

A third option (CASE C) I propose (that would require a breaking change to how some allocators are initialized) would be to have ALL allocators required to be built from a comptime factory function (like GeneralPurposeAllocator), and take a config struct with an option like set_undefined_on_free that tells the implementation whether they should be responsible for their own memory undefining or not.

The downside to this is that instead of the allocator AUTHOR being solely responsible for memory undefining, the allocator USER would also be responsible for making sure they pass the right setting to each level of allocator so that all memory is set to undefined at least once (idealy exactly once). This problem can be mitigated by giving the setting a default value of set_undefined_on_free: bool = true so that by default memory is always undefined, possibly multiple times.

Example code might look something like this:

// PageAllocNoUndef is just the std.heap.page_allocator that doesnt undefine memory on free
const PageAllocNoUndef: type = std.heap.PageAllocator(.{.set_undefined_on_free = false});
// MyAllocatorUndef will hande all undefining of memory
const MyAllocUndef: type = MyAllocator(.{.set_undefined_on_free = true});
var base_alloc: Allocator = PageAllocNoUndef.allocator();
var my_allocator: MyAllocUndef = MyAllocUndef.init(base_alloc);

Or alternatively

// PageAllocUndef is just the std.heap.page_allocator that DOES undefine memory on free
const PageAllocUndef: type = std.heap.PageAllocator(.{});
// MyAllocatorNoUndef will assume base allocator handles undefining
const MyAllocNoUndef: type = MyAllocator(.{.set_undefined_on_free = false});
var base_alloc: Allocator = PageAllocUndef.allocator();
var my_allocator: MyAllocNoUndef = MyAllocNoUndef.init(base_alloc);

If user doesn't know what to set, defaults ensure memory always undefined, albeit multiple times

const PageAlloc: type = std.heap.PageAllocator(.{});
const MyAlloc: type = MyAllocator(.{});
var base_alloc: Allocator = PageAlloc.allocator();
// Freed memory will be `@memset(_, undefined)` twice
var my_allocator: MyAlloc = MyAlloc.init(base_alloc);

If user explicitly sets all layers to not undefine, it could prevent detection of illegal behavior in Debug mode, but could ALSO be a 'feature', if the user (for some reason) wanted to manually set memory undefined somewhere else external to the allocator itself.

const PageAllocNoUndef: type = std.heap.PageAllocator(.{.set_undefined_on_free = false});
const MyAllocNoUndef: type = MyAllocator(.{.set_undefined_on_free = false});
var base_alloc: Allocator = PageAllocNoUndef.allocator();
// Freed memory will never be `@memset(_, undefined)`
var my_allocator: MyAlloc = MyAllocNoUndef.init(base_alloc);
// Do some stuff with memory
// Eventually free all memory and deinit allocators
setUsedMemoryAsUndefined(mem: []u8);