Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Freeing slice returned via dst[0..d] doesn't free the dst[d..] portion #2

Open
ljamzin opened this issue Mar 28, 2024 · 2 comments
Open

Comments

@ljamzin
Copy link

ljamzin commented Mar 28, 2024

Caught in following test code:

fn printHex(comptime label: []const u8, data: []u8) void {
    print("{s}: ", .{label});
    for (data) |byte| print("{x}", .{byte});
    print("\n", .{});
}

test "first snappy exec" {
    {
        print("\n\n", .{});
        var alcr = testing.allocator;

        var _buffin: [64]u8 = [_]u8{0xFF} ** 64;
        var buffin: []u8 = &_buffin;

        try testing.expectEqual(64, _buffin.len);
        printHex("Input", buffin);

        var _outexp = [_]u8{ 0x40, 0x00, 0xFF, 0xFA, 0x01, 0x00 };
        var outexp: []u8 = &_outexp;
        printHex("Expected", outexp);

        var buffout = try snappy.encode(alcr, buffin);
        defer alcr.free(buffout);
        printHex("Output", buffout);

        try testing.expectEqualSlices(u8, outexp, buffout);

        print("\n", .{});
    }
}

The specifics code probably doesn't matter though, the leak should be reported by testing.allocator with pretty much any net positive encode invocation:

[gpa] (err): Allocation size 106 bytes does not match free size 6. Allocation:
C:\dev\24-03\qrnl\src\codec\snappy\snappy.zig:427:34: 0x7ff6e29d789e in encode (test.exe.obj)
    var dst = try allocator.alloc(u8, @as(usize, @intCast(encodedLen)));
                                 ^
C:\dev\24-03\qrnl\src\codec\snappy\test.zig:34:40: 0x7ff6e29dc1f2 in test.first snappy exec (test.exe.obj)
        var buffout = try snappy.encode(alcr, buffin);
                                       ^
C:\Users\Aleksandar\AppData\Roaming\Code\User\globalStorage\ziglang.vscode-zig\zig_install\lib\test_runner.zig:100:29: 0x7ff6e29d5e41 in mainServer (test.exe.obj)
                test_fn.func() catch |err| switch (err) {
                            ^
C:\Users\Aleksandar\AppData\Roaming\Code\User\globalStorage\ziglang.vscode-zig\zig_install\lib\test_runner.zig:34:26: 0x7ff6e29d12cb in main (test.exe.obj)
        return mainServer() catch @panic("internal test runner failure");
                         ^
C:\Users\Aleksandar\AppData\Roaming\Code\User\globalStorage\ziglang.vscode-zig\zig_install\lib\std\start.zig:339:65: 0x7ff6e29d1053 in WinStartup (test.exe.obj)
    std.os.windows.kernel32.ExitProcess(initEventLoopAndCallMain());
                                                                ^
???:?:?: 0x7fffa2e4257c in ??? (KERNEL32.DLL)
???:?:?: 0x7fffa3e6aa57 in ??? (ntdll.dll)
 Free:
C:\dev\24-03\qrnl\src\codec\snappy\test.zig:35:24: 0x7ff6e29dc301 in test.first snappy exec (test.exe.obj)
        defer alcr.free(buffout);
                       ^
C:\Users\Aleksandar\AppData\Roaming\Code\User\globalStorage\ziglang.vscode-zig\zig_install\lib\test_runner.zig:100:29: 0x7ff6e29d5e41 in mainServer (test.exe.obj)
                test_fn.func() catch |err| switch (err) {
                            ^
C:\Users\Aleksandar\AppData\Roaming\Code\User\globalStorage\ziglang.vscode-zig\zig_install\lib\test_runner.zig:34:26: 0x7ff6e29d12cb in main (test.exe.obj)
        return mainServer() catch @panic("internal test runner failure");
                         ^
C:\Users\Aleksandar\AppData\Roaming\Code\User\globalStorage\ziglang.vscode-zig\zig_install\lib\std\start.zig:339:65: 0x7ff6e29d1053 in WinStartup (test.exe.obj)
    std.os.windows.kernel32.ExitProcess(initEventLoopAndCallMain());
                                                                ^
???:?:?: 0x7fffa2e4257c in ??? (KERNEL32.DLL)
???:?:?: 0x7fffa3e6aa57 in ??? (ntdll.dll)

I'm also fairly new to zig and slowly trying to write a shitty parquet file reader. This seems to work for page decompression! I intend to at some point test if/how much using this and other pure zig codec implementations affects bundle size compared to using the C versions. I can share the results if you're curious.

@ljamzin
Copy link
Author

ljamzin commented Mar 28, 2024

var final = try allocator.alloc(u8, d);
@memcpy(final, dst[0..d]);
allocator.free(dst);
return final;
// return dst[0..d];

crude workaround

@gsquire gsquire mentioned this issue Mar 28, 2024
@gsquire
Copy link
Owner

gsquire commented Mar 28, 2024

Thanks for finding this! I should have had a test to catch this earlier. I added one in and fixed it with your suggestion. We can leave this issue open if you'd like to share your results. It'd be interesting to see!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants