From c2b9621a7488b2fa69fc70bc057929df6541b5d5 Mon Sep 17 00:00:00 2001 From: leoconst <35425444+leoconst@users.noreply.github.com> Date: Thu, 2 Feb 2023 22:47:12 +0000 Subject: [PATCH 1/6] Raise a compile error for non-pointers passed to Allocator functions --- lib/std/mem/Allocator.zig | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/lib/std/mem/Allocator.zig b/lib/std/mem/Allocator.zig index 6e566078651d..cbc63b85b1f2 100644 --- a/lib/std/mem/Allocator.zig +++ b/lib/std/mem/Allocator.zig @@ -109,7 +109,7 @@ pub fn create(self: Allocator, comptime T: type) Error!*T { /// `ptr` should be the return value of `create`, or otherwise /// have the same address and alignment property. pub fn destroy(self: Allocator, ptr: anytype) void { - const info = @typeInfo(@TypeOf(ptr)).Pointer; + const info = EnsureSlice(@TypeOf(ptr), "destroy"); const T = info.child; if (@sizeOf(T) == 0) return; const non_const_ptr = @intToPtr([*]u8, @ptrToInt(ptr)); @@ -224,7 +224,7 @@ pub fn allocAdvancedWithRetAddr( /// the pointer, however the allocator implementation may refuse the resize /// request by returning `false`. pub fn resize(self: Allocator, old_mem: anytype, new_n: usize) bool { - const Slice = @typeInfo(@TypeOf(old_mem)).Pointer; + const Slice = EnsureSlice(@TypeOf(old_mem), "resize"); const T = Slice.child; if (new_n == 0) { self.free(old_mem); @@ -245,7 +245,7 @@ pub fn resize(self: Allocator, old_mem: anytype, new_n: usize) bool { /// can be larger, smaller, or the same size as the old memory allocation. /// If `new_n` is 0, this is the same as `free` and it always succeeds. pub fn realloc(self: Allocator, old_mem: anytype, new_n: usize) t: { - const Slice = @typeInfo(@TypeOf(old_mem)).Pointer; + const Slice = EnsureSlice(@TypeOf(old_mem), "realloc"); break :t Error![]align(Slice.alignment) Slice.child; } { return self.reallocAdvanced(old_mem, new_n, @returnAddress()); @@ -257,10 +257,10 @@ pub fn reallocAdvanced( new_n: usize, return_address: usize, ) t: { - const Slice = @typeInfo(@TypeOf(old_mem)).Pointer; + const Slice = EnsureSlice(@TypeOf(old_mem), "reallocAdvanced"); break :t Error![]align(Slice.alignment) Slice.child; } { - const Slice = @typeInfo(@TypeOf(old_mem)).Pointer; + const Slice = EnsureSlice(@TypeOf(old_mem), "reallocAdvanced"); const T = Slice.child; if (old_mem.len == 0) { return self.allocAdvancedWithRetAddr(T, Slice.alignment, new_n, return_address); @@ -293,7 +293,7 @@ pub fn reallocAdvanced( /// Free an array allocated with `alloc`. To free a single item, /// see `destroy`. pub fn free(self: Allocator, memory: anytype) void { - const Slice = @typeInfo(@TypeOf(memory)).Pointer; + const Slice = EnsureSlice(@TypeOf(memory), "free"); const bytes = mem.sliceAsBytes(memory); const bytes_len = bytes.len + if (Slice.sentinel != null) @sizeOf(Slice.child) else 0; if (bytes_len == 0) return; @@ -318,6 +318,13 @@ pub fn dupeZ(allocator: Allocator, comptime T: type, m: []const T) ![:0]T { return new_buf[0..m.len :0]; } +inline fn EnsureSlice(comptime Type: type, comptime function_name: []const u8) std.builtin.Type.Pointer { + return switch (@typeInfo(Type)) { + .Pointer => |pointer| pointer, + else => @compileError(function_name ++ " expects an array but received value of type `" ++ @typeName(Type) ++ "`"), + }; +} + /// TODO replace callsites with `@log2` after this proposal is implemented: /// https://github.com/ziglang/zig/issues/13642 inline fn log2a(x: anytype) switch (@typeInfo(@TypeOf(x))) { From 80249705dbe34a262426181ae0257ea30f87bb83 Mon Sep 17 00:00:00 2001 From: leoconst <35425444+leoconst@users.noreply.github.com> Date: Thu, 2 Feb 2023 23:47:15 +0000 Subject: [PATCH 2/6] Fix casing of ensureSlice function --- lib/std/mem/Allocator.zig | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/std/mem/Allocator.zig b/lib/std/mem/Allocator.zig index cbc63b85b1f2..a7fc872e40ae 100644 --- a/lib/std/mem/Allocator.zig +++ b/lib/std/mem/Allocator.zig @@ -109,7 +109,7 @@ pub fn create(self: Allocator, comptime T: type) Error!*T { /// `ptr` should be the return value of `create`, or otherwise /// have the same address and alignment property. pub fn destroy(self: Allocator, ptr: anytype) void { - const info = EnsureSlice(@TypeOf(ptr), "destroy"); + const info = ensureSlice(@TypeOf(ptr), "destroy"); const T = info.child; if (@sizeOf(T) == 0) return; const non_const_ptr = @intToPtr([*]u8, @ptrToInt(ptr)); @@ -224,7 +224,7 @@ pub fn allocAdvancedWithRetAddr( /// the pointer, however the allocator implementation may refuse the resize /// request by returning `false`. pub fn resize(self: Allocator, old_mem: anytype, new_n: usize) bool { - const Slice = EnsureSlice(@TypeOf(old_mem), "resize"); + const Slice = ensureSlice(@TypeOf(old_mem), "resize"); const T = Slice.child; if (new_n == 0) { self.free(old_mem); @@ -245,7 +245,7 @@ pub fn resize(self: Allocator, old_mem: anytype, new_n: usize) bool { /// can be larger, smaller, or the same size as the old memory allocation. /// If `new_n` is 0, this is the same as `free` and it always succeeds. pub fn realloc(self: Allocator, old_mem: anytype, new_n: usize) t: { - const Slice = EnsureSlice(@TypeOf(old_mem), "realloc"); + const Slice = ensureSlice(@TypeOf(old_mem), "realloc"); break :t Error![]align(Slice.alignment) Slice.child; } { return self.reallocAdvanced(old_mem, new_n, @returnAddress()); @@ -257,10 +257,10 @@ pub fn reallocAdvanced( new_n: usize, return_address: usize, ) t: { - const Slice = EnsureSlice(@TypeOf(old_mem), "reallocAdvanced"); + const Slice = ensureSlice(@TypeOf(old_mem), "reallocAdvanced"); break :t Error![]align(Slice.alignment) Slice.child; } { - const Slice = EnsureSlice(@TypeOf(old_mem), "reallocAdvanced"); + const Slice = ensureSlice(@TypeOf(old_mem), "reallocAdvanced"); const T = Slice.child; if (old_mem.len == 0) { return self.allocAdvancedWithRetAddr(T, Slice.alignment, new_n, return_address); @@ -293,7 +293,7 @@ pub fn reallocAdvanced( /// Free an array allocated with `alloc`. To free a single item, /// see `destroy`. pub fn free(self: Allocator, memory: anytype) void { - const Slice = EnsureSlice(@TypeOf(memory), "free"); + const Slice = ensureSlice(@TypeOf(memory), "free"); const bytes = mem.sliceAsBytes(memory); const bytes_len = bytes.len + if (Slice.sentinel != null) @sizeOf(Slice.child) else 0; if (bytes_len == 0) return; @@ -318,7 +318,7 @@ pub fn dupeZ(allocator: Allocator, comptime T: type, m: []const T) ![:0]T { return new_buf[0..m.len :0]; } -inline fn EnsureSlice(comptime Type: type, comptime function_name: []const u8) std.builtin.Type.Pointer { +inline fn ensureSlice(comptime Type: type, comptime function_name: []const u8) std.builtin.Type.Pointer { return switch (@typeInfo(Type)) { .Pointer => |pointer| pointer, else => @compileError(function_name ++ " expects an array but received value of type `" ++ @typeName(Type) ++ "`"), From cdb210a8a28f0a8a7aace8cdbe48ab4a1a38ec62 Mon Sep 17 00:00:00 2001 From: leoconst <35425444+leoconst@users.noreply.github.com> Date: Fri, 3 Feb 2023 16:36:13 +0000 Subject: [PATCH 3/6] Use @src().fn_name where possible --- lib/std/mem/Allocator.zig | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/std/mem/Allocator.zig b/lib/std/mem/Allocator.zig index a7fc872e40ae..9e603cffc77d 100644 --- a/lib/std/mem/Allocator.zig +++ b/lib/std/mem/Allocator.zig @@ -109,7 +109,7 @@ pub fn create(self: Allocator, comptime T: type) Error!*T { /// `ptr` should be the return value of `create`, or otherwise /// have the same address and alignment property. pub fn destroy(self: Allocator, ptr: anytype) void { - const info = ensureSlice(@TypeOf(ptr), "destroy"); + const info = ensureSlice(@TypeOf(ptr), @src().fn_name); const T = info.child; if (@sizeOf(T) == 0) return; const non_const_ptr = @intToPtr([*]u8, @ptrToInt(ptr)); @@ -224,7 +224,7 @@ pub fn allocAdvancedWithRetAddr( /// the pointer, however the allocator implementation may refuse the resize /// request by returning `false`. pub fn resize(self: Allocator, old_mem: anytype, new_n: usize) bool { - const Slice = ensureSlice(@TypeOf(old_mem), "resize"); + const Slice = ensureSlice(@TypeOf(old_mem), @src().fn_name); const T = Slice.child; if (new_n == 0) { self.free(old_mem); @@ -260,7 +260,7 @@ pub fn reallocAdvanced( const Slice = ensureSlice(@TypeOf(old_mem), "reallocAdvanced"); break :t Error![]align(Slice.alignment) Slice.child; } { - const Slice = ensureSlice(@TypeOf(old_mem), "reallocAdvanced"); + const Slice = ensureSlice(@TypeOf(old_mem), @src().fn_name); const T = Slice.child; if (old_mem.len == 0) { return self.allocAdvancedWithRetAddr(T, Slice.alignment, new_n, return_address); @@ -293,7 +293,7 @@ pub fn reallocAdvanced( /// Free an array allocated with `alloc`. To free a single item, /// see `destroy`. pub fn free(self: Allocator, memory: anytype) void { - const Slice = ensureSlice(@TypeOf(memory), "free"); + const Slice = ensureSlice(@TypeOf(memory), @src().fn_name); const bytes = mem.sliceAsBytes(memory); const bytes_len = bytes.len + if (Slice.sentinel != null) @sizeOf(Slice.child) else 0; if (bytes_len == 0) return; From 0ac8e7b4c1d588f7a8b94da081371d9bb835bf6d Mon Sep 17 00:00:00 2001 From: leoconst <35425444+leoconst@users.noreply.github.com> Date: Fri, 3 Feb 2023 16:39:16 +0000 Subject: [PATCH 4/6] Check size when ensuring type - breaks tests --- lib/std/mem/Allocator.zig | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/lib/std/mem/Allocator.zig b/lib/std/mem/Allocator.zig index 9e603cffc77d..5d11a0379d90 100644 --- a/lib/std/mem/Allocator.zig +++ b/lib/std/mem/Allocator.zig @@ -109,7 +109,7 @@ pub fn create(self: Allocator, comptime T: type) Error!*T { /// `ptr` should be the return value of `create`, or otherwise /// have the same address and alignment property. pub fn destroy(self: Allocator, ptr: anytype) void { - const info = ensureSlice(@TypeOf(ptr), @src().fn_name); + const info = ensureSlice(@TypeOf(ptr), @src().fn_name, .One); const T = info.child; if (@sizeOf(T) == 0) return; const non_const_ptr = @intToPtr([*]u8, @ptrToInt(ptr)); @@ -224,7 +224,7 @@ pub fn allocAdvancedWithRetAddr( /// the pointer, however the allocator implementation may refuse the resize /// request by returning `false`. pub fn resize(self: Allocator, old_mem: anytype, new_n: usize) bool { - const Slice = ensureSlice(@TypeOf(old_mem), @src().fn_name); + const Slice = ensureSlice(@TypeOf(old_mem), @src().fn_name, .Slice); const T = Slice.child; if (new_n == 0) { self.free(old_mem); @@ -245,7 +245,7 @@ pub fn resize(self: Allocator, old_mem: anytype, new_n: usize) bool { /// can be larger, smaller, or the same size as the old memory allocation. /// If `new_n` is 0, this is the same as `free` and it always succeeds. pub fn realloc(self: Allocator, old_mem: anytype, new_n: usize) t: { - const Slice = ensureSlice(@TypeOf(old_mem), "realloc"); + const Slice = ensureSlice(@TypeOf(old_mem), "realloc", .Slice); break :t Error![]align(Slice.alignment) Slice.child; } { return self.reallocAdvanced(old_mem, new_n, @returnAddress()); @@ -257,10 +257,10 @@ pub fn reallocAdvanced( new_n: usize, return_address: usize, ) t: { - const Slice = ensureSlice(@TypeOf(old_mem), "reallocAdvanced"); + const Slice = ensureSlice(@TypeOf(old_mem), "reallocAdvanced", .Slice); break :t Error![]align(Slice.alignment) Slice.child; } { - const Slice = ensureSlice(@TypeOf(old_mem), @src().fn_name); + const Slice = ensureSlice(@TypeOf(old_mem), @src().fn_name, .Slice); const T = Slice.child; if (old_mem.len == 0) { return self.allocAdvancedWithRetAddr(T, Slice.alignment, new_n, return_address); @@ -293,7 +293,7 @@ pub fn reallocAdvanced( /// Free an array allocated with `alloc`. To free a single item, /// see `destroy`. pub fn free(self: Allocator, memory: anytype) void { - const Slice = ensureSlice(@TypeOf(memory), @src().fn_name); + const Slice = ensureSlice(@TypeOf(memory), @src().fn_name, .Slice); const bytes = mem.sliceAsBytes(memory); const bytes_len = bytes.len + if (Slice.sentinel != null) @sizeOf(Slice.child) else 0; if (bytes_len == 0) return; @@ -318,11 +318,27 @@ pub fn dupeZ(allocator: Allocator, comptime T: type, m: []const T) ![:0]T { return new_buf[0..m.len :0]; } -inline fn ensureSlice(comptime Type: type, comptime function_name: []const u8) std.builtin.Type.Pointer { - return switch (@typeInfo(Type)) { - .Pointer => |pointer| pointer, - else => @compileError(function_name ++ " expects an array but received value of type `" ++ @typeName(Type) ++ "`"), +inline fn ensureSlice( + comptime Type: type, + comptime function_name: []const u8, + comptime expected_size: std.builtin.Type.Pointer.Size, +) std.builtin.Type.Pointer { + const expectation = switch (expected_size) { + .One => "a single item pointer", + .Slice => "a slice", + else => unreachable, }; + const type_info = @typeInfo(Type); + + if (type_info == .Pointer) { + const pointer = type_info.Pointer; + + if (pointer.size == expected_size) { + return pointer; + } + } + + @compileError(std.fmt.comptimePrint("{s} expects {s} but received a value of type `{s}`", .{ function_name, expectation, @typeName(Type) })); } /// TODO replace callsites with `@log2` after this proposal is implemented: From ada1d9547e068d0bd3a53dce44bb5af60c41f67c Mon Sep 17 00:00:00 2001 From: leoconst <35425444+leoconst@users.noreply.github.com> Date: Fri, 3 Feb 2023 17:36:36 +0000 Subject: [PATCH 5/6] Revert "Use @src().fn_name where possible" This reverts commit cdb210a8a28f0a8a7aace8cdbe48ab4a1a38ec62. --- lib/std/mem/Allocator.zig | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/std/mem/Allocator.zig b/lib/std/mem/Allocator.zig index 9e603cffc77d..a7fc872e40ae 100644 --- a/lib/std/mem/Allocator.zig +++ b/lib/std/mem/Allocator.zig @@ -109,7 +109,7 @@ pub fn create(self: Allocator, comptime T: type) Error!*T { /// `ptr` should be the return value of `create`, or otherwise /// have the same address and alignment property. pub fn destroy(self: Allocator, ptr: anytype) void { - const info = ensureSlice(@TypeOf(ptr), @src().fn_name); + const info = ensureSlice(@TypeOf(ptr), "destroy"); const T = info.child; if (@sizeOf(T) == 0) return; const non_const_ptr = @intToPtr([*]u8, @ptrToInt(ptr)); @@ -224,7 +224,7 @@ pub fn allocAdvancedWithRetAddr( /// the pointer, however the allocator implementation may refuse the resize /// request by returning `false`. pub fn resize(self: Allocator, old_mem: anytype, new_n: usize) bool { - const Slice = ensureSlice(@TypeOf(old_mem), @src().fn_name); + const Slice = ensureSlice(@TypeOf(old_mem), "resize"); const T = Slice.child; if (new_n == 0) { self.free(old_mem); @@ -260,7 +260,7 @@ pub fn reallocAdvanced( const Slice = ensureSlice(@TypeOf(old_mem), "reallocAdvanced"); break :t Error![]align(Slice.alignment) Slice.child; } { - const Slice = ensureSlice(@TypeOf(old_mem), @src().fn_name); + const Slice = ensureSlice(@TypeOf(old_mem), "reallocAdvanced"); const T = Slice.child; if (old_mem.len == 0) { return self.allocAdvancedWithRetAddr(T, Slice.alignment, new_n, return_address); @@ -293,7 +293,7 @@ pub fn reallocAdvanced( /// Free an array allocated with `alloc`. To free a single item, /// see `destroy`. pub fn free(self: Allocator, memory: anytype) void { - const Slice = ensureSlice(@TypeOf(memory), @src().fn_name); + const Slice = ensureSlice(@TypeOf(memory), "free"); const bytes = mem.sliceAsBytes(memory); const bytes_len = bytes.len + if (Slice.sentinel != null) @sizeOf(Slice.child) else 0; if (bytes_len == 0) return; From 1c643c40cc5dcf9a405101324c42bba0d7a594c7 Mon Sep 17 00:00:00 2001 From: leoconst <35425444+leoconst@users.noreply.github.com> Date: Sat, 4 Feb 2023 12:02:10 +0000 Subject: [PATCH 6/6] Convert page to slice before freeing --- lib/std/heap/general_purpose_allocator.zig | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/std/heap/general_purpose_allocator.zig b/lib/std/heap/general_purpose_allocator.zig index 4f8be3804c47..15427dc1ce28 100644 --- a/lib/std/heap/general_purpose_allocator.zig +++ b/lib/std/heap/general_purpose_allocator.zig @@ -397,7 +397,9 @@ pub fn GeneralPurposeAllocator(comptime config: Config) type { const prev = bucket.prev; if (config.never_unmap) { // free page that was intentionally leaked by never_unmap - self.backing_allocator.free(bucket.page[0..page_size]); + const array_ptr = bucket.page[0..page_size]; + comptime assert(@TypeOf(array_ptr) == *align(page_size) [page_size]u8); + self.backing_allocator.free(@as([]align(page_size) u8, array_ptr)); } // alloc_cursor was set to slot count when bucket added to empty_buckets self.freeBucket(bucket, @divExact(page_size, bucket.alloc_cursor)); @@ -814,7 +816,9 @@ pub fn GeneralPurposeAllocator(comptime config: Config) type { self.buckets[bucket_index] = bucket.prev; } if (!config.never_unmap) { - self.backing_allocator.free(bucket.page[0..page_size]); + const array_ptr = bucket.page[0..page_size]; + comptime assert(@TypeOf(array_ptr) == *align(page_size) [page_size]u8); + self.backing_allocator.free(@as([]align(page_size) u8, array_ptr)); } if (!config.retain_metadata) { self.freeBucket(bucket, size_class);