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

Bump itoa version, don't use mem::uninit (0.13 backport) #2915

Closed
wants to merge 3 commits into from

Conversation

5225225
Copy link

@5225225 5225225 commented Jul 12, 2022

Marking this as a draft for now while I test some crates, patching their hyper version to be this one then seeing if a cargo update fixes them with strict init checks. The code is done, though.

See: #2914

@5225225 5225225 force-pushed the backport-mem-uninit-0.13 branch 2 times, most recently from 131bf52 to caf5a1e Compare July 12, 2022 07:51
@5225225
Copy link
Author

5225225 commented Jul 12, 2022

Hm.... I needed to bump pnet for the tests to pass with strict checks (otherwise you run into uninit memory in there), but that means the MSRV of 1.39 fails tests. And looks like a lot of dependencies depend on the 2021 edition in semver compatible versions with the ones we have in cargo.toml

@5225225
Copy link
Author

5225225 commented Jul 12, 2022

Also, i'm testing according to the checks that are enabled with -Zstrict-init-checks, which means "no uninit integers and no zeroed types that can't be zeroed". This is basically the checks that miri does, but is strictly speaking, stricter than what's been ruled to be UB yet (see rust-lang/rust#98919 ).

I figure it's best to fix all the panics that rust can legally insert now rather than needing to do another backport if/when we get the chance to make let x: [u8; 100] = mem::uninit() panic.

@RalfJung
Copy link
Contributor

This is basically the checks that miri does, but is strictly speaking, stricter than what's been ruled to be UB yet

Well, the reference says it is UB.

@5225225
Copy link
Author

5225225 commented Jul 14, 2022

Okay, I tested a few crates (arangoq, jqdata, oxidux, usgs-espa-client) that use 0.13 hyper by patching this into their dep tree, and running cargo update if http complained as well, which was only an issue for oxidux.

All 4 compiled and had their tests run fine, even with -Zstrict-init-checks.

@5225225 5225225 force-pushed the backport-mem-uninit-0.13 branch from caf5a1e to cf532bf Compare July 14, 2022 15:54
@5225225 5225225 marked this pull request as ready for review July 14, 2022 15:55
5225225 and others added 2 commits July 14, 2022 17:04
In newer versions of rust, mem::uninitialized and mem::zeroed will panic
when they are used for types that don't allow it in more cases.

This change allows the tests to run successfully even with the strictest
form of the checks.
@5225225 5225225 force-pushed the backport-mem-uninit-0.13 branch from cf532bf to 3e37e92 Compare July 14, 2022 16:08
@5225225 5225225 force-pushed the backport-mem-uninit-0.13 branch from 3e37e92 to 2a77117 Compare July 14, 2022 16:10
@5225225
Copy link
Author

5225225 commented Feb 26, 2023

Closing this as not really needed anymore, 0.13 is not maintained and there's no risk of anything actually going wrong even in far future LLVM because of the 0x01 filling.

@5225225 5225225 closed this Feb 26, 2023
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

Successfully merging this pull request may close these issues.

3 participants