GeneralPurposeAllocator and testing allocator do not catch wrong frees

[rganesan]

Zig Version

0.11.0

Steps to Reproduce and Observed Behavior

const std = @import("std");
const mem = std.mem;

pub fn main() !void {
    var gpa = std.heap.GeneralPurposeAllocator(.{}){};
    const allocator = gpa.allocator();

    var buf = try allocator.alloc(u8, 10);
    buf = buf[0..3];
    allocator.free(buf);
}

const testing = std.testing;

test "wrong free" {
    const allocator = testing.allocator;
    var buf = try allocator.alloc(u8, 10);
    buf = buf[0..3];
    allocator.free(buf);
}

Consider the above code. Neither "zig run wrong-free.zig" or "zig test wrong-free.zig" complain that the wrong length is being freed for the slice or that there is a leak.

Expected Behavior

The allocators should catch this.

[nektro]

there is no leak since allocators only identify the data returned to you by the address of the first element.

[andrewrk]

@nektro I find your comment unconstructive. Please, when you respond to an issue, choose one of these two:

1. Put a lot of effort into it, and really make sure you are helping the person, addressing their use case in all of its aspects. Read a draft of your comment, and ask yourself, does my comment help the person submitting the issue, or others trying to resolve it? Write some code to verify your assumptions. Provide the code in your comment.
2. Refrain from making a comment.

[nektro]

I admit I could have been a bit more clear in my response earlier but I was only describing the current behavior and why it was working as designed and not a bug, since I do not have the ability to mark something as a proposal or not. I left the comment because I did feel it was addressing their use case in all of its aspects and have always been willing to add followup when folks have thought I was being too terse. As far as adding in my personal opinion re: it becoming a proposal, I thought that the current design/behavior was okay which is why that was left out.

[rganesan]

@andrewrk and @nektro FWIW, I didn't find the first response unconstructive. It lead me to look closer at the GPA implementation and I'm not convinced that @nektro is right in all cases. It's true that GPA searches buckets solely on the address, however if the original allocation was a "large object" and the free was called with a smaller slice, the behavior would be unexpected.

Besides, this doesn't seem to be guaranteed by the Allocator interface; an allocator implementation can legitimately use the size for it's internal implementation. Also this is clearly a programming error and would be nice to catch.

[mlugg]

Yeah, Zig's allocator interface explicitly allows using the size (if it didn't, we wouldn't bother passing it in!). This can be useful for some allocator designs such as binned allocators. Without having looked at the details of GPA's impl, I agree it'd make sense to always catch this error if possible. (Note that GPA in its current form is really more of a "debug allocator", it's slooowwwwww, so regressing perf a bit further wouldn't really matter.)