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

try_reserve tests execute UB, will fail in Rust 1.82 #34

Closed
saethlin opened this issue Sep 20, 2024 · 2 comments
Closed

try_reserve tests execute UB, will fail in Rust 1.82 #34

saethlin opened this issue Sep 20, 2024 · 2 comments

Comments

@saethlin
Copy link

I am filing this issue because this project's tests failed in the crater runs for Rust 1.82: rust-lang/rust#130579, you can reproduce this failure with cargo +beta test or cargo +nightly test.

There is some interesting wording in the failing test:

minivec/tests/vec.rs

Lines 1600 to 1603 in 7573a54

// On 16/32-bit, we check that allocations don't exceed isize::MAX,
// on 64-bit, we assume the OS will give an OOM for such a ridiculous size.
// Any platform that succeeds for these requests is technically broken with
// ptr::offset because LLVM is the worst.

You cannot rely on the underlying allocator to fail in this scenario. Even if the allocator does fail, the execution encounters UB before the abort due to allocation failure, so it is technically wrong to rely on the allocator failing.

@saethlin saethlin changed the title try_reserve tests execute UB try_reserve tests execute UB, will fail in Rust 1.82 Sep 20, 2024
@cmazakas
Copy link
Owner

cmazakas commented Sep 22, 2024

The actual test failure seemed to have been caused by an update to Layout. I've updated the code now.

I'm not sure where there's any UB tho. This test is a copy-paste of the stdlib test so I just do whatever they do. If they do UB, I do UB.

But I really do appreciate you calling my attention to the test failure. Been a hot minute since I've touched the code. The ping is much appreciated.

Closing because the failing test is now technically fixed cargo --nightly miri test && cargo --nightly test seems to pass fine.

@saethlin
Copy link
Author

The actual test failure seemed to have been caused by an update to Layout.

Yeah, by me. I added a special kind of debug assertion to it. It looks like the standard library code was changed in rust-lang/rust#95295, thanks for the heads-up that you pasted this test from the standard library. Some quick GitHub code search suggests a lot of other people have too 🙃

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