Apply noundef metadata to loads of types that do not permit raw init #94158
Conversation
rustbot
added
T-compiler
erikdesjardins
force-pushed
the
more-more-noundef
branch
from
171bee8
to
36746b2
Compare
rustbot
added
the
S-waiting-on-review
rustbot
removed
the
S-blocked
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
rustbot
added
the
S-waiting-on-perf
|
⌛ Trying commit 36746b233d2953b441f0e5a3f16a340af5ea3d0b with merge 2f35d0d1b00e684f5860c2b557e45e09b0b74f68... |
|
☀️ Try build successful - checks-actions |
|
Queued 2f35d0d1b00e684f5860c2b557e45e09b0b74f68 with parent 6abd8cd, future comparison URL. |
|
Finished benchmarking commit (2f35d0d1b00e684f5860c2b557e45e09b0b74f68): comparison url. Summary: This benchmark run did not return any relevant results. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf. @bors rollup=never |
rustbot
removed
the
S-waiting-on-perf
This matches the noundef attributes we apply on arguments/return types.
erikdesjardins
force-pushed
the
more-more-noundef
branch
from
36746b2
to
fec4335
Compare
|
@bors r+ |
|
📌 Commit fec4335 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (427cf81): comparison url. Summary: This benchmark run did not return any relevant results. 14 results were found to be statistically significant but too small to be relevant. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
Add !align metadata on loads of &/&mut/Box Note that this refers to the alignment of what the loaded value points to, _not_ the alignment of the loaded value itself. r? `@ghost` (blocked on rust-lang#94158)
…t do not permit raw init" In rust-lang#94158, we started emitting `noundef`, which means that functions returning uninitialized references emit IR with is both `ret void` and also `noundef`: https://godbolt.org/z/hbjsKKfc3 More generally, this change makes `mem::uninitialized` dangerous in a way that it wasn't before. This `noundef` change was shipped in the past 2 stable releases, 1.61.0 and 1.62.0. This concerns me because in rust-lang#66151 we have thus far decided against issuing a panic for creating uninitialized references within arrays, on account of the breakage that shows up in crater. If this pattern is so widely-used that we're not willing to put in a runtime check for it, then it doesn't seem prudent to invite LLVM to exploit this UB. The pattern of creating uninit references in arrays shows up real code because the 0.11, 0.12, and 0.13 release series for `hyper` all use `mem::uninitialized` to construct arrays of `httparse::Header`, which contains a `&str` and `&[u8]`. There is no patch available within these release series, so a `cargo update` is not a viable way to escape the UB here. Also note that the 0.11 release series of `hyper` predates `MaybeUninit`, so any source-level patch for that will incur runtime overhead. Which would be unfortunate, but preferable to UB. I discussed this with @thomcc on the community Discord, and we think that it would be prudent to revert this introduction of `noundef` until we have a mitigation in place for the UB that this may unleash. We haven't been able to cook up any examples of surprising optimizations due to this pattern, but the whole point of `noundef` is to enable optimizations, and we know that there is code which uses it in a way which is definitely instant UB and which we have declined to inform users of. If possible, we would like to see `freeze` applied to the return value of `mem::uninitialized` as a mitigation for this problem. That may be able to keep old code functioning without introducing a performance hit. @rustbot labels add +T-compiler +I-compiler-nominated
This matches the noundef attributes we apply on arguments/return types.
Fixes (partially) #74378.