implement zeroed and uninitialized with MaybeUninit#69922

RalfJung · 2020-03-11T13:32:55Z

This is the second attempt of doing such a change (first PR: #62150). The last change got reverted because it caused some issues in code that incorrectly used these functions.

Since then, the problematic code has been fixed, and rustc gained a lint that is able to detect many misuses of these functions statically and a dynamic check that panics instead of causing UB for some incorrect uses.

Fixes #62825

Centril · 2020-03-11T13:37:40Z

r? @oli-obk

src/test/ui/intrinsics/intrinsic-move-val.rs

oli-obk · 2020-03-11T22:32:54Z

@bors r+

bors · 2020-03-11T22:32:56Z

📌 Commit c7eb0f2 has been approved by oli-obk

RalfJung · 2020-03-12T09:11:54Z

@bors rollup=never to make this bisectable

Dylan-DPC-zz · 2020-03-13T01:07:53Z

@bors p=1

RalfJung · 2020-03-14T12:12:23Z

@bors p=0
I'd prefer my other PR #69999 to go first.

RalfJung · 2020-03-16T08:12:23Z

Giving back priority (#69922 (comment))
@bors p=1

bors · 2020-03-16T11:44:14Z

⌛ Testing commit c7eb0f2 with merge 954159622b131f800ed65cbd9e60585d03dac100...

rust-highfive · 2020-03-16T13:22:36Z

The job x86_64-gnu-nopt of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

2020-03-16T13:20:38.5925791Z thread 'main' panicked at 'Some tests failed', src/tools/compiletest/src/main.rs:348:22
2020-03-16T13:20:38.5927874Z note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
2020-03-16T13:20:38.5938708Z 
2020-03-16T13:20:38.5938949Z 
2020-03-16T13:20:38.5947596Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/checkout/obj/build/x86_64-unknown-linux-gnu/llvm/build/bin/FileCheck" "--host-rustcflags" "-Crpath -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--llvm-version" "9.0.1-rust-1.43.0-nightly\n" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
2020-03-16T13:20:38.5950748Z 
2020-03-16T13:20:38.5951040Z 
2020-03-16T13:20:38.5958754Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
2020-03-16T13:20:38.5959079Z Build completed unsuccessfully in 1:32:00
2020-03-16T13:20:38.5959079Z Build completed unsuccessfully in 1:32:00
2020-03-16T13:20:38.6022898Z == clock drift check ==
2020-03-16T13:20:38.6046394Z   local time: Mon Mar 16 13:20:38 UTC 2020
2020-03-16T13:20:38.8733954Z   network time: Mon, 16 Mar 2020 13:20:38 GMT
2020-03-16T13:20:38.8740928Z == end clock drift check ==
2020-03-16T13:20:39.3346594Z 
2020-03-16T13:20:39.3401413Z ##[error]Bash exited with code '1'.
2020-03-16T13:20:39.3464197Z ##[section]Starting: Checkout rust-lang/rust@auto to s
2020-03-16T13:20:39.3469654Z ==============================================================================
2020-03-16T13:20:39.3469964Z Task         : Get sources
2020-03-16T13:20:39.3470726Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @rust-lang/infra. (Feature Requests)

bors · 2020-03-16T13:22:50Z

💔 Test failed - checks-azure

RalfJung · 2020-03-16T13:36:31Z

src/test/ui/init-large-type.rs

    // do the test in a new thread to avoid (spurious?) stack overflows
    thread::spawn(|| {
-        let _memory: [u8; SIZE] = unsafe { init() };
+        let _memory: [u8; SIZE] = unsafe { mem::zeroed() };


This test is failing on the "noopt" builder. And that makes sense; there's now a function call "in the way" here where previously the intrinsic was expanded in-line to zero-initializing LLVM IR.

RalfJung · 2020-03-16T13:39:16Z

@oli-obk I added -O to the init-large-type test as that test now clearly needs inlining to work. Does this make sense?

oli-obk · 2020-03-16T15:40:54Z

Maybe we should mark the wrapper functions as #[inline(always)]? Is there any reason for them to never get inlined?

RalfJung · 2020-03-16T16:36:11Z

Sure, I can do that. Note that this already applied before this PR.

oli-obk · 2020-03-16T16:44:33Z

Oh, right. so it's the more complex code in MaybeUninit that llvm doesn't handle without optimizations. I guess that's alright, though maybe we can teach mir optimizations about that at some point.

@bors r+

bors · 2020-03-16T16:44:34Z

📌 Commit a2160e6 has been approved by oli-obk

implement zeroed and uninitialized with MaybeUninit This is the second attempt of doing such a change (first PR: rust-lang#62150). The last change [got reverted](rust-lang#63343) because it [caused](rust-lang#62825) some [issues](rust-lang#52898 (comment)) in [code that incorrectly used these functions](AltF02/x11-rs#99). Since then, the [problematic code has been fixed](AltF02/x11-rs#101), and rustc [gained a lint](rust-lang#63346) that is able to detect many misuses of these functions statically and a [dynamic check that panics](rust-lang#66059) instead of causing UB for some incorrect uses. Fixes rust-lang#62825

Centril · 2020-03-17T05:36:43Z

@bors retry rollup=maybe

Ignoring rollup=never directive due to constrained CI budget & combining with orthogonal PRs in rollup.

bors · 2020-03-17T05:48:38Z

⌛ Testing commit a2160e6 with merge 1d1d192555bc828cdc42a61cd9b6a02c6b260960...

implement zeroed and uninitialized with MaybeUninit This is the second attempt of doing such a change (first PR: rust-lang#62150). The last change [got reverted](rust-lang#63343) because it [caused](rust-lang#62825) some [issues](rust-lang#52898 (comment)) in [code that incorrectly used these functions](AltF02/x11-rs#99). Since then, the [problematic code has been fixed](AltF02/x11-rs#101), and rustc [gained a lint](rust-lang#63346) that is able to detect many misuses of these functions statically and a [dynamic check that panics](rust-lang#66059) instead of causing UB for some incorrect uses. Fixes rust-lang#62825

bors · 2020-03-17T09:04:56Z

💔 Test failed - checks-azure

RalfJung · 2020-03-17T09:40:07Z

"Received request to deprovision: The request was cancelled by the remote provider."...?

@bors retry

bors · 2020-03-17T11:13:01Z

⌛ Testing commit a2160e6 with merge 8da5b0313f3288a314a81f571c81ac9957b0df5e...

Centril · 2020-03-17T11:17:32Z

@bors retry rolled up.

@ghost

Rollup of 7 pull requests Successful merges: - #68746 (Make macro metavars respect (non-)hygiene) - #69688 (Move tidy check to mingw-check) - #69735 (bootstrap: Use hash to determine if sanitizers needs to be rebuilt) - #69922 (implement zeroed and uninitialized with MaybeUninit) - #69956 (Ensure HAS_FREE_LOCAL_NAMES is set for ReFree) - #70061 (Cosmetic fixes in documentation) - #70064 (Update books) Failed merges: r? @ghost

x11-dl was using std::mem::uninitialized incorrectly and when rustlang added MaybeUninit and intrinsic panics on UB caused by improper use of uninitialized (see rust-lang/rust/pull/69922) it caused issues with X11 initialization. x11-dl pr AltF02/x11-rs/pull/101 updated x11-dl to use MaybeUninit correctly

implement zeroed and uninitialized with MaybeUninit

d49306d

RalfJung mentioned this pull request Mar 11, 2020

re-land #62150 (Implement mem::{zeroed,uninitialized} in terms of MaybeUninit) #62825

Closed

rust-highfive assigned oli-obk Mar 11, 2020

RalfJung added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 11, 2020

This comment has been minimized.

Sign in to view

fix expand-to-unstable test

c7eb0f2

oli-obk reviewed Mar 11, 2020

View reviewed changes

src/test/ui/intrinsics/intrinsic-move-val.rs Show resolved Hide resolved

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 Mar 11, 2020

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 Mar 16, 2020

RalfJung commented Mar 16, 2020

View reviewed changes

init-large-type test needs optimizations

996a51b

make mem::{zeroed,uninitialized} inline(always)

a2160e6

Centril mentioned this pull request Mar 17, 2020

Rollup of 8 pull requests #70066

Closed

Centril mentioned this pull request Mar 17, 2020

Rollup of 8 pull requests #70067

Closed

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 Mar 17, 2020

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 Mar 17, 2020

Centril mentioned this pull request Mar 17, 2020

Rollup of 7 pull requests #70072

Merged

bors merged commit 7a7ca82 into rust-lang:master Mar 17, 2020

RalfJung deleted the less-intrinsic branch March 18, 2020 12:33

matthew-russo mentioned this pull request Mar 26, 2020

upgrades x11-dl to 2.18.5 to fix #376 rust-windowing/winit#1517

Merged

4 tasks

Uh oh!

Conversation

RalfJung commented Mar 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Centril commented Mar 11, 2020

Uh oh!

This comment has been minimized.

Uh oh!

oli-obk commented Mar 11, 2020

Uh oh!

bors commented Mar 11, 2020

Uh oh!

RalfJung commented Mar 12, 2020

Uh oh!

Dylan-DPC-zz commented Mar 13, 2020

Uh oh!

RalfJung commented Mar 14, 2020

Uh oh!

RalfJung commented Mar 16, 2020

Uh oh!

bors commented Mar 16, 2020

Uh oh!

rust-highfive commented Mar 16, 2020

Uh oh!

bors commented Mar 16, 2020

Uh oh!

RalfJung Mar 16, 2020

Choose a reason for hiding this comment

Uh oh!

RalfJung commented Mar 16, 2020

Uh oh!

oli-obk commented Mar 16, 2020

Uh oh!

RalfJung commented Mar 16, 2020

Uh oh!

oli-obk commented Mar 16, 2020

Uh oh!

bors commented Mar 16, 2020

Uh oh!

Centril commented Mar 17, 2020

Uh oh!

bors commented Mar 17, 2020

Uh oh!

bors commented Mar 17, 2020

Uh oh!

RalfJung commented Mar 17, 2020

Uh oh!

bors commented Mar 17, 2020

Uh oh!

Centril commented Mar 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

RalfJung commented Mar 11, 2020 •

edited

Loading