mem.Allocator: Make ptr optional to avoid UB when comparing Allocator instances

Previously, Allocator implementations that did not store any state would set `ptr` to `undefined`. However, the use of undefined meant that it was always unsafe/illegal to compare the ptr fields of Allocator. This restriction was not mentioned anywhere, though, and the ptr field *was* being compared in real code, e.g. `std.process.Child.collectOutput`, which can lead to undefined behavior in release modes (see ziglang#21756).

There are a few ways to address this:
1. Codify that comparing `ptr` is always illegal/unsafe, and remove all existing comparisons.
2. Set `ptr` to a legal dummy value, similar to the value returned by Allocator.alloc when a zero-length slice is requested.
3. Make `ptr` optional and set it to `null` for implementations that don't store any state

The first option seems like footgun central (at least until usage of `undefined` is fully safety-checked in safe modes), and the second option has its own issues since the dummy value could inadvertently be equal to a real mapped pointer, so the 3rd option was chosen.

Author: squeek502

lib/std/crypto/tlcsprng.zig

@@ -44,7 +44,7 @@ var install_atfork_handler = std.once(struct {
 
 threadlocal var wipe_mem: []align(mem.page_size) u8 = &[_]u8{};
 
-fn tlsCsprngFill(_: *anyopaque, buffer: []u8) void {
+fn tlsCsprngFill(_: ?*anyopaque, buffer: []u8) void {
     if (os_has_arc4random) {
         // arc4random is already a thread-local CSPRNG.
         return std.c.arc4random_buf(buffer.ptr, buffer.len);

lib/std/heap.zig

@@ -103,7 +103,7 @@ const CAllocator = struct {
     }
 
     fn alloc(
-        _: *anyopaque,
+        _: ?*anyopaque,
         len: usize,
         log2_align: u8,
         return_address: usize,
@@ -114,7 +114,7 @@ const CAllocator = struct {
     }
 
     fn resize(
-        _: *anyopaque,
+        _: ?*anyopaque,
         buf: []u8,
         log2_buf_align: u8,
         new_len: usize,
@@ -135,7 +135,7 @@ const CAllocator = struct {
     }
 
     fn free(
-        _: *anyopaque,
+        _: ?*anyopaque,
         buf: []u8,
         log2_buf_align: u8,
         return_address: usize,
@@ -150,7 +150,7 @@ const CAllocator = struct {
 /// `malloc_usable_size` if available. For an allocator that directly calls
 /// `malloc`/`free`, see `raw_c_allocator`.
 pub const c_allocator = Allocator{
-    .ptr = undefined,
+    .ptr = null,
     .vtable = &c_allocator_vtable,
 };
 const c_allocator_vtable = Allocator.VTable{
@@ -165,7 +165,7 @@ const c_allocator_vtable = Allocator.VTable{
 /// `ArenaAllocator` for example and is more optimal in such a case
 /// than `c_allocator`.
 pub const raw_c_allocator = Allocator{
-    .ptr = undefined,
+    .ptr = null,
     .vtable = &raw_c_allocator_vtable,
 };
 const raw_c_allocator_vtable = Allocator.VTable{
@@ -175,7 +175,7 @@ const raw_c_allocator_vtable = Allocator.VTable{
 };
 
 fn rawCAlloc(
-    _: *anyopaque,
+    _: ?*anyopaque,
     len: usize,
     log2_ptr_align: u8,
     ret_addr: usize,
@@ -192,7 +192,7 @@ fn rawCAlloc(
 }
 
 fn rawCResize(
-    _: *anyopaque,
+    _: ?*anyopaque,
     buf: []u8,
     log2_old_align: u8,
     new_len: usize,
@@ -213,7 +213,7 @@ fn rawCResize(
 }
 
 fn rawCFree(
-    _: *anyopaque,
+    _: ?*anyopaque,
     buf: []u8,
     log2_old_align: u8,
     ret_addr: usize,
@@ -231,17 +231,17 @@ pub const page_allocator = if (@hasDecl(root, "os") and
     root.os.heap.page_allocator
 else if (builtin.target.isWasm())
     Allocator{
-        .ptr = undefined,
+        .ptr = null,
         .vtable = &WasmPageAllocator.vtable,
     }
 else if (builtin.target.os.tag == .plan9)
     Allocator{
-        .ptr = undefined,
+        .ptr = null,
         .vtable = &SbrkAllocator(std.os.plan9.sbrk).vtable,
     }
 else
     Allocator{
-        .ptr = undefined,
+        .ptr = null,
         .vtable = &PageAllocator.vtable,
     };
 
@@ -251,7 +251,7 @@ else
 /// and wasm64 architectures.
 /// Until then, it is available here to play with.
 pub const wasm_allocator = Allocator{
-    .ptr = undefined,
+    .ptr = null,
     .vtable = &std.heap.WasmAllocator.vtable,
 };
 
@@ -296,13 +296,13 @@ pub const HeapAllocator = switch (builtin.os.tag) {
         }
 
         fn alloc(
-            ctx: *anyopaque,
+            ctx: ?*anyopaque,
             n: usize,
             log2_ptr_align: u8,
             return_address: usize,
         ) ?[*]u8 {
             _ = return_address;
-            const self: *HeapAllocator = @ptrCast(@alignCast(ctx));
+            const self: *HeapAllocator = @ptrCast(@alignCast(ctx.?));
 
             const ptr_align = @as(usize, 1) << @as(Allocator.Log2Align, @intCast(log2_ptr_align));
             const amt = n + ptr_align - 1 + @sizeOf(usize);
@@ -323,15 +323,15 @@ pub const HeapAllocator = switch (builtin.os.tag) {
         }
 
         fn resize(
-            ctx: *anyopaque,
+            ctx: ?*anyopaque,
             buf: []u8,
             log2_buf_align: u8,
             new_size: usize,
             return_address: usize,
         ) bool {
             _ = log2_buf_align;
             _ = return_address;
-            const self: *HeapAllocator = @ptrCast(@alignCast(ctx));
+            const self: *HeapAllocator = @ptrCast(@alignCast(ctx.?));
 
             const root_addr = getRecordPtr(buf).*;
             const align_offset = @intFromPtr(buf.ptr) - root_addr;
@@ -348,14 +348,14 @@ pub const HeapAllocator = switch (builtin.os.tag) {
         }
 
         fn free(
-            ctx: *anyopaque,
+            ctx: ?*anyopaque,
             buf: []u8,
             log2_buf_align: u8,
             return_address: usize,
         ) void {
             _ = log2_buf_align;
             _ = return_address;
-            const self: *HeapAllocator = @ptrCast(@alignCast(ctx));
+            const self: *HeapAllocator = @ptrCast(@alignCast(ctx.?));
             windows.HeapFree(self.heap_handle.?, 0, @as(*anyopaque, @ptrFromInt(getRecordPtr(buf).*)));
         }
     },
@@ -423,8 +423,8 @@ pub const FixedBufferAllocator = struct {
         return buf.ptr + buf.len == self.buffer.ptr + self.end_index;
     }
 
-    fn alloc(ctx: *anyopaque, n: usize, log2_ptr_align: u8, ra: usize) ?[*]u8 {
-        const self: *FixedBufferAllocator = @ptrCast(@alignCast(ctx));
+    fn alloc(ctx: ?*anyopaque, n: usize, log2_ptr_align: u8, ra: usize) ?[*]u8 {
+        const self: *FixedBufferAllocator = @ptrCast(@alignCast(ctx.?));
         _ = ra;
         const ptr_align = @as(usize, 1) << @as(Allocator.Log2Align, @intCast(log2_ptr_align));
         const adjust_off = mem.alignPointerOffset(self.buffer.ptr + self.end_index, ptr_align) orelse return null;
@@ -436,13 +436,13 @@ pub const FixedBufferAllocator = struct {
     }
 
     fn resize(
-        ctx: *anyopaque,
+        ctx: ?*anyopaque,
         buf: []u8,
         log2_buf_align: u8,
         new_size: usize,
         return_address: usize,
     ) bool {
-        const self: *FixedBufferAllocator = @ptrCast(@alignCast(ctx));
+        const self: *FixedBufferAllocator = @ptrCast(@alignCast(ctx.?));
         _ = log2_buf_align;
         _ = return_address;
         assert(@inComptime() or self.ownsSlice(buf));
@@ -466,12 +466,12 @@ pub const FixedBufferAllocator = struct {
     }
 
     fn free(
-        ctx: *anyopaque,
+        ctx: ?*anyopaque,
         buf: []u8,
         log2_buf_align: u8,
         return_address: usize,
     ) void {
-        const self: *FixedBufferAllocator = @ptrCast(@alignCast(ctx));
+        const self: *FixedBufferAllocator = @ptrCast(@alignCast(ctx.?));
         _ = log2_buf_align;
         _ = return_address;
         assert(@inComptime() or self.ownsSlice(buf));
@@ -481,8 +481,8 @@ pub const FixedBufferAllocator = struct {
         }
     }
 
-    fn threadSafeAlloc(ctx: *anyopaque, n: usize, log2_ptr_align: u8, ra: usize) ?[*]u8 {
-        const self: *FixedBufferAllocator = @ptrCast(@alignCast(ctx));
+    fn threadSafeAlloc(ctx: ?*anyopaque, n: usize, log2_ptr_align: u8, ra: usize) ?[*]u8 {
+        const self: *FixedBufferAllocator = @ptrCast(@alignCast(ctx.?));
         _ = ra;
         const ptr_align = @as(usize, 1) << @as(Allocator.Log2Align, @intCast(log2_ptr_align));
         var end_index = @atomicLoad(usize, &self.end_index, .seq_cst);
@@ -551,24 +551,24 @@ pub fn StackFallbackAllocator(comptime size: usize) type {
         pub const allocator = @compileError("use 'const allocator = stackFallback(N).get();' instead");
 
         fn alloc(
-            ctx: *anyopaque,
+            ctx: ?*anyopaque,
             len: usize,
             log2_ptr_align: u8,
             ra: usize,
         ) ?[*]u8 {
-            const self: *Self = @ptrCast(@alignCast(ctx));
+            const self: *Self = @ptrCast(@alignCast(ctx.?));
             return FixedBufferAllocator.alloc(&self.fixed_buffer_allocator, len, log2_ptr_align, ra) orelse
                 return self.fallback_allocator.rawAlloc(len, log2_ptr_align, ra);
         }
 
         fn resize(
-            ctx: *anyopaque,
+            ctx: ?*anyopaque,
             buf: []u8,
             log2_buf_align: u8,
             new_len: usize,
             ra: usize,
         ) bool {
-            const self: *Self = @ptrCast(@alignCast(ctx));
+            const self: *Self = @ptrCast(@alignCast(ctx.?));
             if (self.fixed_buffer_allocator.ownsPtr(buf.ptr)) {
                 return FixedBufferAllocator.resize(&self.fixed_buffer_allocator, buf, log2_buf_align, new_len, ra);
             } else {
@@ -577,12 +577,12 @@ pub fn StackFallbackAllocator(comptime size: usize) type {
         }
 
         fn free(
-            ctx: *anyopaque,
+            ctx: ?*anyopaque,
             buf: []u8,
             log2_buf_align: u8,
             ra: usize,
         ) void {
-            const self: *Self = @ptrCast(@alignCast(ctx));
+            const self: *Self = @ptrCast(@alignCast(ctx.?));
             if (self.fixed_buffer_allocator.ownsPtr(buf.ptr)) {
                 return FixedBufferAllocator.free(&self.fixed_buffer_allocator, buf, log2_buf_align, ra);
             } else {

lib/std/heap/PageAllocator.zig

@@ -14,7 +14,7 @@ pub const vtable = Allocator.VTable{
     .free = free,
 };
 
-fn alloc(_: *anyopaque, n: usize, log2_align: u8, ra: usize) ?[*]u8 {
+fn alloc(_: ?*anyopaque, n: usize, log2_align: u8, ra: usize) ?[*]u8 {
     _ = ra;
     _ = log2_align;
     assert(n > 0);
@@ -51,7 +51,7 @@ fn alloc(_: *anyopaque, n: usize, log2_align: u8, ra: usize) ?[*]u8 {
 }
 
 fn resize(
-    _: *anyopaque,
+    _: ?*anyopaque,
     buf_unaligned: []u8,
     log2_buf_align: u8,
     new_size: usize,
@@ -100,7 +100,7 @@ fn resize(
     return false;
 }
 
-fn free(_: *anyopaque, slice: []u8, log2_buf_align: u8, return_address: usize) void {
+fn free(_: ?*anyopaque, slice: []u8, log2_buf_align: u8, return_address: usize) void {
     _ = log2_buf_align;
     _ = return_address;
 

lib/std/heap/ThreadSafeAllocator.zig

@@ -14,25 +14,25 @@ pub fn allocator(self: *ThreadSafeAllocator) Allocator {
     };
 }
 
-fn alloc(ctx: *anyopaque, n: usize, log2_ptr_align: u8, ra: usize) ?[*]u8 {
-    const self: *ThreadSafeAllocator = @ptrCast(@alignCast(ctx));
+fn alloc(ctx: ?*anyopaque, n: usize, log2_ptr_align: u8, ra: usize) ?[*]u8 {
+    const self: *ThreadSafeAllocator = @ptrCast(@alignCast(ctx.?));
     self.mutex.lock();
     defer self.mutex.unlock();
 
     return self.child_allocator.rawAlloc(n, log2_ptr_align, ra);
 }
 
-fn resize(ctx: *anyopaque, buf: []u8, log2_buf_align: u8, new_len: usize, ret_addr: usize) bool {
-    const self: *ThreadSafeAllocator = @ptrCast(@alignCast(ctx));
+fn resize(ctx: ?*anyopaque, buf: []u8, log2_buf_align: u8, new_len: usize, ret_addr: usize) bool {
+    const self: *ThreadSafeAllocator = @ptrCast(@alignCast(ctx.?));
 
     self.mutex.lock();
     defer self.mutex.unlock();
 
     return self.child_allocator.rawResize(buf, log2_buf_align, new_len, ret_addr);
 }
 
-fn free(ctx: *anyopaque, buf: []u8, log2_buf_align: u8, ret_addr: usize) void {
-    const self: *ThreadSafeAllocator = @ptrCast(@alignCast(ctx));
+fn free(ctx: ?*anyopaque, buf: []u8, log2_buf_align: u8, ret_addr: usize) void {
+    const self: *ThreadSafeAllocator = @ptrCast(@alignCast(ctx.?));
 
     self.mutex.lock();
     defer self.mutex.unlock();

lib/std/heap/WasmAllocator.zig

@@ -43,7 +43,7 @@ var frees = [1]usize{0} ** size_class_count;
 /// For each big size class, points to the freed pointer.
 var big_frees = [1]usize{0} ** big_size_class_count;
 
-fn alloc(ctx: *anyopaque, len: usize, log2_align: u8, return_address: usize) ?[*]u8 {
+fn alloc(ctx: ?*anyopaque, len: usize, log2_align: u8, return_address: usize) ?[*]u8 {
     _ = ctx;
     _ = return_address;
     // Make room for the freelist next pointer.
@@ -81,7 +81,7 @@ fn alloc(ctx: *anyopaque, len: usize, log2_align: u8, return_address: usize) ?[*
 }
 
 fn resize(
-    ctx: *anyopaque,
+    ctx: ?*anyopaque,
     buf: []u8,
     log2_buf_align: u8,
     new_len: usize,
@@ -109,7 +109,7 @@ fn resize(
 }
 
 fn free(
-    ctx: *anyopaque,
+    ctx: ?*anyopaque,
     buf: []u8,
     log2_buf_align: u8,
     return_address: usize,
@@ -158,7 +158,7 @@ fn allocBigPages(n: usize) usize {
 }
 
 const test_ally = Allocator{
-    .ptr = undefined,
+    .ptr = null,
     .vtable = &vtable,
 };
 

lib/std/heap/WasmPageAllocator.zig

@@ -101,7 +101,7 @@ fn nPages(memsize: usize) usize {
     return mem.alignForward(usize, memsize, mem.page_size) / mem.page_size;
 }
 
-fn alloc(ctx: *anyopaque, len: usize, log2_align: u8, ra: usize) ?[*]u8 {
+fn alloc(ctx: ?*anyopaque, len: usize, log2_align: u8, ra: usize) ?[*]u8 {
     _ = ctx;
     _ = ra;
     if (len > maxInt(usize) - (mem.page_size - 1)) return null;
@@ -159,7 +159,7 @@ fn freePages(start: usize, end: usize) void {
 }
 
 fn resize(
-    ctx: *anyopaque,
+    ctx: ?*anyopaque,
     buf: []u8,
     log2_buf_align: u8,
     new_len: usize,
@@ -180,7 +180,7 @@ fn resize(
 }
 
 fn free(
-    ctx: *anyopaque,
+    ctx: ?*anyopaque,
     buf: []u8,
     log2_buf_align: u8,
     return_address: usize,