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

Multi-variant layouts for generators #59897

Merged
merged 13 commits into from May 4, 2019

Conversation

Projects
None yet
9 participants
@tmandry
Copy link
Contributor

commented Apr 12, 2019

This allows generators to overlap fields using variants, but doesn't do any such overlapping yet. It creates one variant for every state of the generator (unresumed, returned, panicked, plus one for every yield), and puts every stored local in each of the yield-point variants.

Required for optimizing generator layouts (#52924).

There was quite a lot of refactoring needed for this change. I've done my best in later commits to eliminate assumptions in the code that only certain kinds of types are multi-variant, and to centralize knowledge of the inner mechanics of generators in as few places as possible.

This change also emits debuginfo about the fields contained in each variant, as well as preserving debuginfo about stored locals while running in the generator.

Also, fixes #59972.

Future work:

  • Use this change for an optimization pass that actually overlaps locals within the generator struct (#52924)
  • In the type layout fields, don't include locals that are uninitialized for a particular variant, so miri and UB sanitizers can check our memory (see #59972 (comment))
  • Preserve debuginfo scopes across generator yield points
@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Apr 12, 2019

r? @varkor

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Apr 12, 2019

⚠️ Warning ⚠️

  • These commits modify submodules.

@tmandry tmandry force-pushed the tmandry:variantful-generators branch from a15b0f0 to 9432b3f Apr 12, 2019

@tmandry

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2019

@rust-highfive rust-highfive assigned eddyb and unassigned varkor Apr 12, 2019

@rust-highfive

This comment was marked as outdated.

Copy link
Collaborator

commented Apr 12, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (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.
travis_time:end:128cd2d4:start=1555030896926319452,finish=1555030899043856226,duration=2117536774
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---

[00:04:22] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:04:22] tidy error: /checkout/src/librustc_codegen_ssa/mir/mod.rs:658: TODO is deprecated; use FIXME
[00:04:22] tidy error: /checkout/src/librustc_codegen_llvm/debuginfo/metadata.rs:694: TODO is deprecated; use FIXME
[00:04:22] tidy error: /checkout/src/librustc_codegen_llvm/debuginfo/metadata.rs:698: TODO is deprecated; use FIXME
[00:04:22] tidy error: /checkout/src/librustc_codegen_llvm/debuginfo/metadata.rs:1824: TODO is deprecated; use FIXME
[00:04:24] some tidy checks failed
[00:04:24] 
[00:04:24] 
[00:04:24] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:04:24] 
[00:04:24] 
[00:04:24] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:24] Build completed unsuccessfully in 0:00:47
[00:04:24] Build completed unsuccessfully in 0:00:47
[00:04:24] Makefile:67: recipe for target 'tidy' failed
[00:04:24] make: *** [tidy] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:19b3ab46
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Fri Apr 12 01:06:15 UTC 2019
---
travis_time:end:3e9219fa:start=1555031175943857515,finish=1555031175948701806,duration=4844291
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:04b41768
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:3ac6daf9
travis_time:start:3ac6daf9
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:11c207c0
$ dmesg | grep -i kill

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 @TimNN. (Feature Requests)

@tmandry tmandry force-pushed the tmandry:variantful-generators branch from 9432b3f to 5b07b40 Apr 12, 2019

@rust-highfive

This comment was marked as outdated.

Copy link
Collaborator

commented Apr 12, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (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.
travis_time:end:03a516b0:start=1555032592495089962,finish=1555032594580512897,duration=2085422935
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---

[00:04:05] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:04:05] tidy error: /checkout/src/librustc_codegen_ssa/mir/mod.rs:658: TODO is deprecated; use FIXME
[00:04:05] tidy error: /checkout/src/librustc_codegen_llvm/debuginfo/metadata.rs:694: TODO is deprecated; use FIXME
[00:04:05] tidy error: /checkout/src/librustc_codegen_llvm/debuginfo/metadata.rs:698: TODO is deprecated; use FIXME
[00:04:05] tidy error: /checkout/src/librustc_codegen_llvm/debuginfo/metadata.rs:1824: TODO is deprecated; use FIXME
[00:04:07] 
[00:04:07] 
[00:04:07] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:04:07] 
[00:04:07] 
[00:04:07] some tidy checks failed
[00:04:07] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:07] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:07] Build completed unsuccessfully in 0:00:43
[00:04:07] Makefile:67: recipe for target 'tidy' failed
[00:04:07] make: *** [tidy] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:129df13a
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Fri Apr 12 01:34:12 UTC 2019
---
travis_time:end:02d27c63:start=1555032853848726414,finish=1555032853853565970,duration=4839556
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:2096c7d8
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0500efec
travis_time:start:0500efec
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0997ef80
$ dmesg | grep -i kill

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 @TimNN. (Feature Requests)

@tmandry tmandry force-pushed the tmandry:variantful-generators branch from 5b07b40 to 167164f Apr 12, 2019

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Apr 12, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (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.
travis_time:end:0e07edfc:start=1555042089252633692,finish=1555042091318747217,duration=2066113525
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---

[00:03:52] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:03:52] tidy error: /checkout/src/librustc_codegen_ssa/mir/mod.rs:658: TODO is deprecated; use FIXME
[00:03:52] tidy error: /checkout/src/librustc_codegen_llvm/debuginfo/metadata.rs:694: TODO is deprecated; use FIXME
[00:03:52] tidy error: /checkout/src/librustc_codegen_llvm/debuginfo/metadata.rs:698: TODO is deprecated; use FIXME
[00:03:52] tidy error: /checkout/src/librustc_codegen_llvm/debuginfo/metadata.rs:1824: TODO is deprecated; use FIXME
[00:03:54] some tidy checks failed
[00:03:54] 
[00:03:54] 
[00:03:54] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:03:54] 
[00:03:54] 
[00:03:54] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:03:54] Build completed unsuccessfully in 0:00:46
[00:03:54] Build completed unsuccessfully in 0:00:46
[00:03:54] Makefile:67: recipe for target 'tidy' failed
[00:03:54] make: *** [tidy] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:1b596766
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Fri Apr 12 04:12:16 UTC 2019
---
travis_time:end:0cdb4488:start=1555042337737740424,finish=1555042337743025641,duration=5285217
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0c7a4282
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0468ebc8
travis_time:start:0468ebc8
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:2ad963c8
$ dmesg | grep -i kill

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 @TimNN. (Feature Requests)

@tmandry

This comment has been minimized.

Copy link
Contributor Author

commented Apr 13, 2019

One funky aspect of this is that we declare locals in debuginfo multiple times, one for each yield point their storage lives across (which, for now, is all of them). This will make more sense when/if we add scopes to those locals that correspond with the yield points. For now, I don't see any downsides to this, except bigger debuginfo.

@tmandry tmandry force-pushed the tmandry:variantful-generators branch from db8e691 to 955b245 Apr 13, 2019

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Apr 13, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (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.
travis_time:end:00a89e9a:start=1555114088719061578,finish=1555114170868550555,duration=82149488977
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---

[00:03:16] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:03:16] tidy error: /checkout/src/librustc_codegen_ssa/mir/mod.rs:658: TODO is deprecated; use FIXME
[00:03:16] tidy error: /checkout/src/librustc_codegen_llvm/debuginfo/metadata.rs:1381: TODO is deprecated; use FIXME
[00:03:17] some tidy checks failed
[00:03:17] 
[00:03:17] 
[00:03:17] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:03:17] 
[00:03:17] 
[00:03:17] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:03:17] Build completed unsuccessfully in 0:00:45
[00:03:17] Build completed unsuccessfully in 0:00:45
[00:03:17] make: *** [tidy] Error 1
[00:03:17] Makefile:67: recipe for target 'tidy' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:04bc1971
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Sat Apr 13 00:12:58 UTC 2019
---
travis_time:end:120e3ec3:start=1555114379110799108,finish=1555114379115483090,duration=4683982
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:00be1d64
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:352c7d98
travis_time:start:352c7d98
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:155f8d1a
$ dmesg | grep -i kill

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 @TimNN. (Feature Requests)

@tmandry tmandry force-pushed the tmandry:variantful-generators branch from 955b245 to 2cb4fad Apr 18, 2019

@tmandry tmandry changed the title [WIP] Variantful generators Refactor generators to use multi-variant layouts Apr 19, 2019

@tmandry tmandry changed the title Refactor generators to use multi-variant layouts Multi-variant layouts in generators Apr 19, 2019

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Apr 19, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (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.
travis_time:end:0193e452:start=1555631856635876007,finish=1555631949483916036,duration=92848040029
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---
[01:08:12] .................................................................................................... 500/2961
[01:08:23] .................................................................................................... 600/2961
[01:08:39] .................................................................................................... 700/2961
[01:08:50] .................................................................................................... 800/2961
[01:08:59] ..................................................F................................................. 900/2961
[01:09:28] .................................................................................................... 1100/2961
[01:09:37] .................................................................................................... 1200/2961
[01:09:47] .................................................................................................... 1300/2961
[01:09:59] .................................................................................................... 1400/2961
---
[01:13:51] failures:
[01:13:51] 
[01:13:51] ---- [run-pass] run-pass/generator/issue-58888.rs stdout ----
[01:13:51] 
[01:13:51] error: test compilation failed although it shouldn't!
[01:13:51] status: exit code: 101
[01:13:51] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-pass/generator/issue-58888.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/generator/issue-58888/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-g" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/generator/issue-58888/auxiliary"
[01:13:51] ------------------------------------------
[01:13:51] 
[01:13:51] ------------------------------------------
[01:13:51] stderr:
[01:13:51] stderr:
[01:13:51] ------------------------------------------
[01:13:51] error: internal compiler error: src/librustc/ty/mod.rs:2997: item_name: no name for DefPath { data: [DisambiguatedDefPathData { data: Impl, disambiguator: 0 }, DisambiguatedDefPathData { data: ValueNs("check_connection"), disambiguator: 0 }, DisambiguatedDefPathData { data: ClosureExpr, disambiguator: 0 }], krate: crate0 }
[01:13:51] thread 'rustc' panicked at 'Box<Any>', src/librustc_errors/lib.rs:635:9
[01:13:51] note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
[01:13:51] error: aborting due to previous error
[01:13:51] 
[01:13:51] 
[01:13:51] 
[01:13:51] note: the compiler unexpectedly panicked. this is a bug.
[01:13:51] 
[01:13:51] note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
[01:13:51] 
[01:13:51] note: rustc 1.36.0-dev running on x86_64-unknown-linux-gnu
[01:13:51] 
[01:13:51] note: compiler flags: -Z threads=1 -Z ui-testing -Z unstable-options -C prefer-dynamic -C rpath
[01:13:51] 
[01:13:51] ------------------------------------------
[01:13:51] 
[01:13:51] 
---
[01:13:51] thread 'main' panicked at 'Some tests failed', src/tools/compiletest/src/main.rs:517:22
[01:13:51] note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
[01:13:51] 
[01:13:51] 
[01:13:51] 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/run-pass" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "run-pass" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-6.0/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -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" "--quiet" "--llvm-version" "6.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[01:13:51] 
[01:13:51] 
[01:13:51] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:13:51] Build completed unsuccessfully in 0:10:54
[01:13:51] Build completed unsuccessfully in 0:10:54
[01:13:51] Makefile:48: recipe for target 'check' failed
[01:13:51] make: *** [check] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:19fc74b1
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Fri Apr 19 01:13:10 UTC 2019

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 @TimNN. (Feature Requests)

@tmandry tmandry changed the title Multi-variant layouts in generators Multi-variant layouts for generators Apr 20, 2019

@tmandry

This comment has been minimized.

Copy link
Contributor Author

commented Apr 20, 2019

Travis failure is for the wrong commit. Can someone re-run? cc @rust-lang/infra

@kennytm

This comment has been minimized.

Copy link
Member

commented Apr 20, 2019

@tmandry You could re-run it by closing & reopening the PR, or pushing a new commit. But the failure in https://travis-ci.com/rust-lang/rust/jobs/194180525 does look legit and referring to the correct HEAD commit 2cb4fad (merge commit d656918).

@tmandry

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2019

@kennytm Nevermind I'm dumb, I forgot about merge commits, sorry.

Still having trouble reproducing this on linux, even on the exact merge commit that's failing in Travis...

@tmandry tmandry force-pushed the tmandry:variantful-generators branch from 2cb4fad to b054710 Apr 22, 2019

@tmandry

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2019

Rebased in hopes that this error magically goes away :)

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Apr 23, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (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.
travis_time:end:2facbd8b:start=1555974372802925812,finish=1555974375196665459,duration=2393739647
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---
[01:09:37] .................................................................................................... 500/2954
[01:09:49] .................................................................................................... 600/2954
[01:10:04] .................................................................................................... 700/2954
[01:10:15] .................................................................................................... 800/2954
[01:10:25] .............................................F...................................................... 900/2954
[01:10:53] .................................................................................................... 1100/2954
[01:11:03] .................................................................................................... 1200/2954
[01:11:13] .................................................................................................... 1300/2954
[01:11:26] ......................ii............................................................................ 1400/2954
---
[01:15:27] failures:
[01:15:27] 
[01:15:27] ---- [run-pass] run-pass/generator/issue-58888.rs stdout ----
[01:15:27] 
[01:15:27] error: test compilation failed although it shouldn't!
[01:15:27] status: exit code: 101
[01:15:27] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-pass/generator/issue-58888.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/generator/issue-58888/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-g" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/generator/issue-58888/auxiliary"
[01:15:27] ------------------------------------------
[01:15:27] 
[01:15:27] ------------------------------------------
[01:15:27] stderr:
[01:15:27] stderr:
[01:15:27] ------------------------------------------
[01:15:27] error: internal compiler error: src/librustc/ty/mod.rs:2997: item_name: no name for DefPath { data: [DisambiguatedDefPathData { data: Impl, disambiguator: 0 }, DisambiguatedDefPathData { data: ValueNs("check_connection"), disambiguator: 0 }, DisambiguatedDefPathData { data: ClosureExpr, disambiguator: 0 }], krate: crate0 }
[01:15:27] thread 'rustc' panicked at 'Box<Any>', src/librustc_errors/lib.rs:636:9
[01:15:27] note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
[01:15:27] error: aborting due to previous error
[01:15:27] 
[01:15:27] 
[01:15:27] 
[01:15:27] note: the compiler unexpectedly panicked. this is a bug.
[01:15:27] 
[01:15:27] note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
[01:15:27] 
[01:15:27] note: rustc 1.36.0-dev running on x86_64-unknown-linux-gnu
[01:15:27] 
[01:15:27] note: compiler flags: -Z threads=1 -Z ui-testing -Z unstable-options -C prefer-dynamic -C rpath
[01:15:27] 
[01:15:27] ------------------------------------------
[01:15:27] 
[01:15:27] 
---
[01:15:27] thread 'main' panicked at 'Some tests failed', src/tools/compiletest/src/main.rs:517:22
[01:15:27] note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
[01:15:27] 
[01:15:27] 
[01:15:27] 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/run-pass" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "run-pass" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-6.0/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -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" "--quiet" "--llvm-version" "6.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[01:15:27] 
[01:15:27] 
[01:15:27] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:15:27] Build completed unsuccessfully in 0:11:16
[01:15:27] Build completed unsuccessfully in 0:11:16
[01:15:27] Makefile:48: recipe for target 'check' failed
[01:15:27] make: *** [check] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0181dec1
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Tue Apr 23 00:21:53 UTC 2019
---
travis_time:end:095a1f18:start=1555978915077102049,finish=1555978915081771365,duration=4669316
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:331999ea
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:19970612
travis_time:start:19970612
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:2dad3df0
$ dmesg | grep -i kill

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 @TimNN. (Feature Requests)

@bors

This comment has been minimized.

Copy link
Contributor

commented May 4, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: eddyb
Pushing e232636 to master...

@bors bors added the merged-by-bors label May 4, 2019

@bors bors merged commit 77a6d29 into rust-lang:master May 4, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
homu Test successful
Details
@RalfJung

This comment has been minimized.

Copy link
Member

commented May 4, 2019

This broke the generator traversal logic in Miri at

ty::Generator(..) => {
let field = v.project_field(self.ecx(), 0)?;
self.visit_aggregate(v, std::iter::once(Ok(field)))?;
for i in 1..offsets.len() {
let field = v.project_field(self.ecx(), i as u64)?;
self.visit_generator_field(v, i, field)?;
}
Ok(())
}

Any advise for how to fix it?

@RalfJung

This comment has been minimized.

Copy link
Member

commented May 4, 2019

Looks like even though generators have variants, they still also contain "wrong" fields in their variants, meaning fields that are not actually going to be initialized when that variant is active. So generators still need a special exception. I think for now I'll just treat the entire generator as if it was a union.

Are there any plans to make the layout information correct, in the sense that when a field is present in a variant according to the layout, then it will also always be initialized?

@Zoxc

This comment has been minimized.

Copy link
Contributor

commented May 4, 2019

Are there any plans to make the layout information correct, in the sense that when a field is present in a variant according to the layout, then it will also always be initialized?

No. Generators are basically a function frame, not an enum. Even for enums it is not true as they can be partially moved out of. A similar thing can happen with generators where a value in the generator is partially moved out of, even when the generator is not executing. There might be other edge cases due to generators being function frames. #60187 should make the layout closer to the actual values contained though.

@RalfJung

This comment has been minimized.

Copy link
Member

commented May 4, 2019

Moving out is not a problem -- that leaves the bits in the old place untouched, so they still satisfy the validity invariant.

What is a problem is having a bool field, and not making sure that it definitely is either true or false. At that point the layout is just not correctly reflecting the data being stored there. This can lead to all sorts of problems, and already requires some awful hack to not break.

@eddyb However, this does somehow conflict with your proposal to make Move overwrite the moved-out data with Undef.

bors added a commit that referenced this pull request May 5, 2019

Auto merge of #60541 - RalfJung:miri-visitor-generators, r=oli-obk
fix Miri visiting generators

Fixes fall-out caused by #59897.

r? @oli-obk

@tmandry tmandry deleted the tmandry:variantful-generators branch May 5, 2019

@tmandry

This comment has been minimized.

Copy link
Contributor Author

commented May 5, 2019

@RalfJung Yeah, #60187 will get us closer. As @eddyb pointed out, we still need to save liveness (not just storage-liveness) information in GeneratorLayout. We also still have to worry about aggregates that are only ever partially initialized.

The short-term fix we discussed will be to wrap the types of every field with MaybeUnitialized in the layout code, so you don't have to handle it specially in miri. Still, I'd like to get a higher fidelity picture out at some point.

@tmandry tmandry referenced this pull request May 6, 2019

Merged

Add test for #59972 #60572

@RalfJung

This comment has been minimized.

Copy link
Member

commented May 6, 2019

The short-term fix we discussed will be to wrap the types of every field with MaybeUnitialized in the layout code, so you don't have to handle it specially in miri. Still, I'd like to get a higher fidelity picture out at some point.

That would also work (and will also do the right thing for inhabitedness and layout computation).

Centril added a commit to Centril/rust that referenced this pull request May 8, 2019

Centril added a commit to Centril/rust that referenced this pull request May 8, 2019

Centril added a commit to Centril/rust that referenced this pull request May 8, 2019

Centril added a commit to Centril/rust that referenced this pull request May 8, 2019

bors added a commit that referenced this pull request Jun 12, 2019

Auto merge of #60187 - tmandry:generator-optimization, r=eddyb
Generator optimization: Overlap locals that never have storage live at the same time

The specific goal of this optimization is to optimize async fns which use `await!`. Notably, `await!` has an enclosing scope around the futures it awaits ([definition](https://github.com/rust-lang/rust/blob/08bfe16129b0621bc90184f8704523d4929695ef/src/libstd/macros.rs#L365-L381)), which we rely on to implement the optimization.

More generally, the optimization allows overlapping the storage of some locals which are never storage-live at the same time. **We care about storage-liveness when computing the layout, because knowing a field is `StorageDead` is the only way to prove it will not be accessed, either directly or through a reference.**

To determine whether we can overlap two locals in the generator layout, we look at whether they might *both* be `StorageLive` at any point in the MIR. We use the `MaybeStorageLive` dataflow analysis for this. We iterate over every location in the MIR, and build a bitset for each local of the locals it might potentially conflict with.

Next, we assign every saved local to one or more variants. The variants correspond to suspension points, and we include the set of locals live across a given suspension point in the variant. (Note that we use liveness instead of storage-liveness here; this ensures that the local has actually been initialized in each variant it has been included in. If the local is not live across a suspension point, then it doesn't need to be included in that variant.). It's important to note that the variants are a "view" into our layout.

For the layout computation, we use a simplified approach.

1. Start with the set of locals assigned to only one variant. The rest are disqualified.
2. For each pair of locals which may conflict *and are not assigned to the same variant*, we pick one local to disqualify from overlapping.

Disqualified locals go into a non-overlapping "prefix" at the beginning of our layout. This means they always have space reserved for them. All the locals that are allowed to overlap in each variant are then laid out after this prefix, in the "overlap zone".

So, if A and B were disqualified, and X, Y, and Z were all eligible for overlap, our generator might look something like this:

You can think of a generator as an enum, where some fields are shared between variants. e.g.

```rust
enum Generator {
  Unresumed,
  Poisoned,
  Returned,
  Suspend0(A, B, X),
  Suspend1(B),
  Suspend2(A, Y, Z),
}
```

where every mention of `A` and `B` refer to the same field, which does not move when changing variants. Note that `A` and `B` would automatically be sent to the prefix in this example. Assuming that `X` is never `StorageLive` at the same time as either `Y` or `Z`, it would be allowed to overlap with them.

Note that if two locals (`Y` and `Z` in this case) are assigned to the same variant in our generator, their memory would never overlap in the layout. Thus they can both be eligible for the overlapping section, even if they are storage-live at the same time.

---

Depends on:
- [x] #59897 Multi-variant layouts for generators
- [x] #60840 Preserve local scopes in generator MIR
- [x] #61373 Emit StorageDead along unwind paths for generators

Before merging:

- [x] ~Wrap the types of all generator fields in `MaybeUninitialized` in layout::ty::field~ (opened #60889)
- [x] Make PR description more complete (e.g. explain why storage liveness is important and why we have to check every location)
- [x] Clean up TODO
- [x] Fix the layout code to enforce that the same field never moves around in the generator
- [x] Add tests for async/await
- [x] ~Reduce # bits we store by half, since the conflict relation is symmetric~ (note: decided not to do this, for simplicity)
- [x] Store liveness information for each yield point in our `GeneratorLayout`, that way we can emit more useful debuginfo AND tell miri which fields are definitely initialized for a given variant (see discussion at #59897 (comment))

Centril added a commit to Centril/rust that referenced this pull request Jun 12, 2019

Rollup merge of rust-lang#60187 - tmandry:generator-optimization, r=e…
…ddyb

Generator optimization: Overlap locals that never have storage live at the same time

The specific goal of this optimization is to optimize async fns which use `await!`. Notably, `await!` has an enclosing scope around the futures it awaits ([definition](https://github.com/rust-lang/rust/blob/08bfe16129b0621bc90184f8704523d4929695ef/src/libstd/macros.rs#L365-L381)), which we rely on to implement the optimization.

More generally, the optimization allows overlapping the storage of some locals which are never storage-live at the same time. **We care about storage-liveness when computing the layout, because knowing a field is `StorageDead` is the only way to prove it will not be accessed, either directly or through a reference.**

To determine whether we can overlap two locals in the generator layout, we look at whether they might *both* be `StorageLive` at any point in the MIR. We use the `MaybeStorageLive` dataflow analysis for this. We iterate over every location in the MIR, and build a bitset for each local of the locals it might potentially conflict with.

Next, we assign every saved local to one or more variants. The variants correspond to suspension points, and we include the set of locals live across a given suspension point in the variant. (Note that we use liveness instead of storage-liveness here; this ensures that the local has actually been initialized in each variant it has been included in. If the local is not live across a suspension point, then it doesn't need to be included in that variant.). It's important to note that the variants are a "view" into our layout.

For the layout computation, we use a simplified approach.

1. Start with the set of locals assigned to only one variant. The rest are disqualified.
2. For each pair of locals which may conflict *and are not assigned to the same variant*, we pick one local to disqualify from overlapping.

Disqualified locals go into a non-overlapping "prefix" at the beginning of our layout. This means they always have space reserved for them. All the locals that are allowed to overlap in each variant are then laid out after this prefix, in the "overlap zone".

So, if A and B were disqualified, and X, Y, and Z were all eligible for overlap, our generator might look something like this:

You can think of a generator as an enum, where some fields are shared between variants. e.g.

```rust
enum Generator {
  Unresumed,
  Poisoned,
  Returned,
  Suspend0(A, B, X),
  Suspend1(B),
  Suspend2(A, Y, Z),
}
```

where every mention of `A` and `B` refer to the same field, which does not move when changing variants. Note that `A` and `B` would automatically be sent to the prefix in this example. Assuming that `X` is never `StorageLive` at the same time as either `Y` or `Z`, it would be allowed to overlap with them.

Note that if two locals (`Y` and `Z` in this case) are assigned to the same variant in our generator, their memory would never overlap in the layout. Thus they can both be eligible for the overlapping section, even if they are storage-live at the same time.

---

Depends on:
- [x] rust-lang#59897 Multi-variant layouts for generators
- [x] rust-lang#60840 Preserve local scopes in generator MIR
- [x] rust-lang#61373 Emit StorageDead along unwind paths for generators

Before merging:

- [x] ~Wrap the types of all generator fields in `MaybeUninitialized` in layout::ty::field~ (opened rust-lang#60889)
- [x] Make PR description more complete (e.g. explain why storage liveness is important and why we have to check every location)
- [x] Clean up TODO
- [x] Fix the layout code to enforce that the same field never moves around in the generator
- [x] Add tests for async/await
- [x] ~Reduce # bits we store by half, since the conflict relation is symmetric~ (note: decided not to do this, for simplicity)
- [x] Store liveness information for each yield point in our `GeneratorLayout`, that way we can emit more useful debuginfo AND tell miri which fields are definitely initialized for a given variant (see discussion at rust-lang#59897 (comment))

@tmandry tmandry referenced this pull request Jun 14, 2019

Merged

Issue #60709 test #61818

Centril added a commit to Centril/rust that referenced this pull request Jun 14, 2019

Centril added a commit to Centril/rust that referenced this pull request Jun 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.