Apply noundef attribute to all scalar types which do not permit raw init #94157

erikdesjardins · 2022-02-19T15:04:50Z

Beyond &/&mut/Box, this covers char, enum discriminants, NonZero*, etc.
All such types currently cause a Miri error if left uninitialized,
and an invalid_value lint in cases like mem::uninitialized::<char>().

Note that this does not change whether or not it is UB for u64 (or
other integer types with no invalid values) to be undef.

Fixes (partially) #74378.

r? @ghost (blocked on #94127)

@rustbot label S-blocked

nikic · 2022-02-26T14:34:59Z

@bors try @rust-timer queue

rust-timer · 2022-02-26T14:35:00Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-02-26T14:35:08Z

⌛ Trying commit fa8c9719d2749752db1d388d043c177f9b6eb62a with merge f71dbc524c99e197ea8d54be1a5801bdab7c37aa...

bors · 2022-02-26T16:07:51Z

☀️ Try build successful - checks-actions
Build commit: f71dbc524c99e197ea8d54be1a5801bdab7c37aa (f71dbc524c99e197ea8d54be1a5801bdab7c37aa)

rust-timer · 2022-02-26T16:07:53Z

Queued f71dbc524c99e197ea8d54be1a5801bdab7c37aa with parent 8c9640e, future comparison URL.

compiler/rustc_middle/src/ty/layout.rs

src/test/codegen/call-metadata.rs

rust-timer · 2022-02-26T17:38:07Z

Finished benchmarking commit (f71dbc524c99e197ea8d54be1a5801bdab7c37aa): comparison url.

Summary: This benchmark run did not return any relevant results. 6 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.

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 label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

Beyond `&`/`&mut`/`Box`, this covers `char`, discriminants, `NonZero*`, etc. All such types currently cause a Miri error if left uninitialized, and an `invalid_value` lint in cases like `mem::uninitialized::<char>()` Note that this _does not_ change whether or not it is UB for `u64` (or other integer types with no invalid values) to be undef.

nikic · 2022-02-26T22:43:53Z

@bors r+

bors · 2022-02-26T22:43:54Z

📌 Commit 5979b68 has been approved by nikic

bors · 2022-02-27T00:44:38Z

⌛ Testing commit 5979b68 with merge d77ce1b08ed049da00de734d8928cf3166b77210...

bors · 2022-02-27T01:23:43Z

💔 Test failed - checks-actions

nikic · 2022-02-27T19:33:26Z

@bors r+

bors · 2022-02-27T19:33:28Z

📌 Commit 45ee3fc has been approved by nikic

bors · 2022-02-27T21:41:08Z

⌛ Testing commit 45ee3fc with merge 9fbff89...

bors · 2022-02-28T00:09:54Z

☀️ Test successful - checks-actions
Approved by: nikic
Pushing 9fbff89 to master...

rust-timer · 2022-02-28T01:39:47Z

Finished benchmarking commit (9fbff89): comparison url.

Summary: This benchmark run shows 5 relevant improvements 🎉 to instruction counts.

Arithmetic mean of relevant improvements: -1.0%
Largest improvement in instruction counts: -1.7% on incr-full builds of coercions debug

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

RalfJung · 2022-03-04T00:08:50Z

compiler/rustc_middle/src/ty/layout.rs

@@ -3053,6 +3053,11 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
                return;
            }

+            // Scalars which have invalid values cannot be undef.
+            if !scalar.is_always_valid(self) {
+                attrs.set(ArgAttribute::NoUndef);


With this logic, can't we remove some of the logic from #93670 since those types also are "not always valid"?

I initially thought the same, but then we don't put noundef on Option<&T/&mut T/Box<T>>>, since they have no invalid values.

Oh, I didn't realize those would get the attribute.

OTOH that still means we are missing Option<NonZero*>, so maybe there's a more general way to handle this by special-casing enums? Like, if the scalar has Multiple layout, then that one scalar contains its discriminant and hence we can definitely set NoUndef? (This might miss newtypes around such enums, not sure if that's a problem.)

You're right.

I'm a bit hesistant to add more complex logic here. I think the missing cases are fairly rare, just Option<NonZero*/NonNull> (and equivalent types), and enums with exactly 256 (or 65536, etc.) variants. In the long run we can switch to the uninit tracking added by #94527 and handle these cases without having to inspect the shape of the layout at all.

Sure. :) I just hope we won't forget to clean up that duplication of logic in two different places ;)

rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Feb 19, 2022

erikdesjardins mentioned this pull request Feb 19, 2022

Apply noundef metadata to loads of types that do not permit raw init #94158

Merged

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 26, 2022

nikic reviewed Feb 26, 2022

View reviewed changes

compiler/rustc_middle/src/ty/layout.rs Outdated Show resolved Hide resolved

src/test/codegen/call-metadata.rs Show resolved Hide resolved

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Feb 26, 2022

erikdesjardins force-pushed the more-noundef branch from fa8c971 to 5979b68 Compare February 26, 2022 21:43

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 27, 2022

This comment has been minimized.

Sign in to view

make pgo-branch-weights test not dependent on argument attributes

45ee3fc

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 27, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 28, 2022

bors merged commit 9fbff89 into rust-lang:master Feb 28, 2022

rustbot added this to the 1.61.0 milestone Feb 28, 2022

bors mentioned this pull request Feb 28, 2022

Add !align metadata on loads of &/&mut/Box #94159

Merged

erikdesjardins deleted the more-noundef branch February 28, 2022 04:13

erikdesjardins mentioned this pull request Mar 3, 2022

Let CTFE to handle partially uninitialized unions without marking the entire value as uninitialized. #94527

Merged

RalfJung reviewed Mar 4, 2022

View reviewed changes

compiler-errors mentioned this pull request Nov 4, 2022

Lifetimes are parsed without errors when they occur in most ExpressionWithBlocks in front of the contained block expression #103983

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply noundef attribute to all scalar types which do not permit raw init #94157

Apply noundef attribute to all scalar types which do not permit raw init #94157

erikdesjardins commented Feb 19, 2022

nikic commented Feb 26, 2022

rust-timer commented Feb 26, 2022

bors commented Feb 26, 2022

bors commented Feb 26, 2022

rust-timer commented Feb 26, 2022

rust-timer commented Feb 26, 2022

nikic commented Feb 26, 2022

bors commented Feb 26, 2022

bors commented Feb 27, 2022

bors commented Feb 27, 2022

This comment has been minimized.

nikic commented Feb 27, 2022

bors commented Feb 27, 2022

bors commented Feb 27, 2022

bors commented Feb 28, 2022

rust-timer commented Feb 28, 2022

RalfJung Mar 4, 2022

erikdesjardins Mar 4, 2022

RalfJung Mar 4, 2022

erikdesjardins Mar 4, 2022 •

edited

Loading

RalfJung Mar 4, 2022

Apply noundef attribute to all scalar types which do not permit raw init #94157

Apply noundef attribute to all scalar types which do not permit raw init #94157

Conversation

erikdesjardins commented Feb 19, 2022

nikic commented Feb 26, 2022

rust-timer commented Feb 26, 2022

bors commented Feb 26, 2022

bors commented Feb 26, 2022

rust-timer commented Feb 26, 2022

rust-timer commented Feb 26, 2022

nikic commented Feb 26, 2022

bors commented Feb 26, 2022

bors commented Feb 27, 2022

bors commented Feb 27, 2022

This comment has been minimized.

nikic commented Feb 27, 2022

bors commented Feb 27, 2022

bors commented Feb 27, 2022

bors commented Feb 28, 2022

rust-timer commented Feb 28, 2022

RalfJung Mar 4, 2022

Choose a reason for hiding this comment

erikdesjardins Mar 4, 2022

Choose a reason for hiding this comment

RalfJung Mar 4, 2022

Choose a reason for hiding this comment

erikdesjardins Mar 4, 2022 • edited Loading

Choose a reason for hiding this comment

RalfJung Mar 4, 2022

Choose a reason for hiding this comment

erikdesjardins Mar 4, 2022 •

edited

Loading