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

make it a compile error to call allocator.destroy on a non-single-item pointer #16540

Closed
Jarred-Sumner opened this issue Jul 25, 2023 · 2 comments · Fixed by #16544
Closed

make it a compile error to call allocator.destroy on a non-single-item pointer #16540

Jarred-Sumner opened this issue Jul 25, 2023 · 2 comments · Fixed by #16544
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@Jarred-Sumner
Copy link
Contributor

Jarred-Sumner commented Jul 25, 2023

Zig Version

0.11.0-dev.4006+bf827d0b5

Steps to Reproduce and Observed Output

Failing test:

test {
    var buffer = try @import("std").heap.page_allocator.alloc(u8, 0);
    @import("std").heap.page_allocator.destroy(buffer);
}
Test [1/1] test_0... thread 41234524 panic: incorrect alignment
/Users/jarred/zig/0.11.0-dev.4006+bf827d0b5/files/lib/std/heap/PageAllocator.zig:107:19: 0x102379ea3 in free (test)
        os.munmap(@alignCast(slice.ptr[0..buf_aligned_len]));
                  ^
/Users/jarred/zig/0.11.0-dev.4006+bf827d0b5/files/lib/std/mem/Allocator.zig:98:28: 0x102378acb in destroy__anon_1390 (test)
    return self.vtable.free(self.ptr, buf, log2_buf_align, ret_addr);
                           ^
/Users/jarred/Desktop/repro.zig:3:47: 0x102378773 in test_0 (test)
    @import("std").heap.page_allocator.destroy(buffer);
                                              ^
/Users/jarred/zig/0.11.0-dev.4006+bf827d0b5/files/lib/test_runner.zig:175:28: 0x10238098b in mainTerminal (test)
        } else test_fn.func();
                           ^
/Users/jarred/zig/0.11.0-dev.4006+bf827d0b5/files/lib/test_runner.zig:35:28: 0x10237919f in main (test)
        return mainTerminal();
                           ^
/Users/jarred/zig/0.11.0-dev.4006+bf827d0b5/files/lib/std/start.zig:598:22: 0x102378d2b in main (test)
            root.main();

Expected Output

allocator.destroy(slice) should be a compiler error since allocator.alloc says to use allocator.free

@Jarred-Sumner Jarred-Sumner added the error message This issue points out an error message that is unhelpful and should be improved. label Jul 25, 2023
@andrewrk
Copy link
Member

andrewrk commented Jul 25, 2023

incorrect API usage:

test {
    var buffer = try @import("std").heap.page_allocator.alloc(u8, 0);
-    @import("std").heap.page_allocator.destroy(buffer);
+    @import("std").heap.page_allocator.free(buffer);
}
$ zig test test.zig 
All 1 tests passed.

/// `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 {

@andrewrk andrewrk closed this as not planned Won't fix, can't repro, duplicate, stale Jul 25, 2023
@andrewrk andrewrk added question No questions on the issue tracker, please. and removed error message This issue points out an error message that is unhelpful and should be improved. labels Jul 25, 2023
@Jarred-Sumner
Copy link
Contributor Author

Why isn't it a compiler error?

Jarred-Sumner added a commit to oven-sh/bun that referenced this issue Jul 25, 2023
@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. standard library This issue involves writing Zig code for the standard library. and removed question No questions on the issue tracker, please. labels Jul 25, 2023
@andrewrk andrewrk changed the title allocator.destroy on a 0-length slice segfaults at runtime make it a compile error to call allocator.destroy on a non-single-item pointer Jul 25, 2023
@andrewrk andrewrk reopened this Jul 25, 2023
@andrewrk andrewrk added this to the 0.12.0 milestone Jul 25, 2023
@andrewrk andrewrk modified the milestones: 0.12.0, 0.11.0 Jul 25, 2023
Jarred-Sumner added a commit to oven-sh/bun that referenced this issue Jul 27, 2023
* Make os.cpus() faster on Linux

* Fix crash

See ziglang/zig#16540

* Handle watcher_count == 0

* Add assertion

* Clean up lifetimes of fs watcher a little

* ✂️

* Use `errdefer`

* Make the error better

* Make os.cpus() more lazy

* Please don't translate-c on the entire C standard library

* immediately closing works correctly is still bug

* ops

* fmt+fixeup

* add back verbose

* free instead of destroy

* remove destroy option for watcher tasks

* flush verbose and add debug log

* fixup files

* use log for debug

---------

Co-authored-by: Jarred Sumner <[email protected]>
Co-authored-by: cirospaciari <[email protected]>
trnxdev pushed a commit to trnxdev/bun that referenced this issue Aug 9, 2023
* Make os.cpus() faster on Linux

* Fix crash

See ziglang/zig#16540

* Handle watcher_count == 0

* Add assertion

* Clean up lifetimes of fs watcher a little

* ✂️

* Use `errdefer`

* Make the error better

* Make os.cpus() more lazy

* Please don't translate-c on the entire C standard library

* immediately closing works correctly is still bug

* ops

* fmt+fixeup

* add back verbose

* free instead of destroy

* remove destroy option for watcher tasks

* flush verbose and add debug log

* fixup files

* use log for debug

---------

Co-authored-by: Jarred Sumner <[email protected]>
Co-authored-by: cirospaciari <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants