might_permit_raw_init: also check arrays #79296
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @matthewjasper (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
|
r? @RalfJung |
|
This looks great, thanks! We should do a crater run. @pietroalbini if I schedule a crater run now, will it use the same lockfiles as what was used back then for #71274, or have they been updated since then? |
|
@bors try |
|
⌛ Trying commit a6f7780 with merge eafdee2dfae8c2233b74f98b123308be7ccefcc6... |
|
☀️ Try build successful - checks-actions |
|
@RalfJung for projects with an existing |
|
All right, that should work then. @craterbot test |
|
🚨 Error: failed to parse the command 🆘 If you have any trouble with Crater please ping |
|
@craterbot run |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
craterbot
added
S-waiting-on-crater
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
|
craterbot
added
S-waiting-on-review
|
All right, we got some data! And looks like we have quite a large amount of regressions again. @nico-abram let me know if you need any help analyzing this! I suggest you use some kind of shared editor so the intermediate results are easily shared (we often use hackmd.io for this). |
|
Do I mostly want to look at I started this hackmd (Not sure if I did it correctly) |
|
"regressed: root results" should be enough (but I admit I am also a bit confused by the classification here, in particular "4 different results" is odd). The goal in the analysis would be to figure out where the panic is coming from. at least roughly. As in, which crate is the one causing UB here? |
crlf0710
added
S-waiting-on-author
|
@nico-abram Ping from triage: any updates on this? |
|
@crlf0710 No updates, still have to analyze the crater report (I got started with it but haven't touched it in a couple weeks. Still planning to work on it, probably after december) |
|
@nico-abram Ping from triage: any updates on this? |
JohnCSimon
added
the
S-inactive
|
Ping from triage: |
Introduce stricter checks for might_permit_raw_init under a debug flag This is intended to be a version of the strict checks tried out in rust-lang#79296, but also checking number validity (under the assumption that `let _ = std::mem::uninitialized::<u32>()` is UB, which seems to be what rust-lang/unsafe-code-guidelines#71 is leaning towards.)
This is the next step for #66151: when doing mem::zeroed/mem::uninitialized, also recursively check arrays for whether they permit zero/uninit initialization.
cc @RalfJung