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

Detect incorrect use of ZST in the allocator? #3208

Closed
RalfJung opened this issue Dec 4, 2023 · 2 comments
Closed

Detect incorrect use of ZST in the allocator? #3208

RalfJung opened this issue Dec 4, 2023 · 2 comments

Comments

@RalfJung
Copy link
Member

RalfJung commented Dec 4, 2023

rust-lang/rust#113113 fixed some bugs in ZST handling of the core collections. I wonder if there is a way that Miri could help find those bugs?

Unfortunately the Rust global allocator says that allocating ZST is UB, so when the collections use the global allocator then ZST allocations never reach Miri -- or put differently, there's not actually a bug with Vec<T, Global>. So I guess to detect this we'd have to explicitly use Vec<T, System>? Then we could make the Miri implementation of malloc actually create a new (zero-sized) allocation on malloc(0), and so failing to pass that to free would be a leak. Passing a dangling pointer to free is already UB (except if it is NULL) so we should also be able to detect that case.

However, currently I don't think that would help. System::alloc ends up here so we never end up even calling malloc(0). I guess we'd need our own allocator for this test that wraps malloc more directly?

Cc @thomcc @Amanieu maybe you have some further ideas here.

@Amanieu
Copy link
Member

Amanieu commented Dec 4, 2023

rust-lang/rust#113113 was only concerned with the Allocator interface which, unlike GlobalAlloc, does allow allocating ZSTs. The problem was that ZSTs were being freed while not allocated and/or allocated and then leaked.

I don't think that it's Miri's job to catch this since this is library UB (the requirements of the Allocator trait are being violated). You would want some form of ZST-tracking allocator to verify this, somewhat like the one used in the tests of that PR.

@RalfJung
Copy link
Member Author

Okay, let's close this then.

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