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

rustbuild: Simplify debuginfo configuration #60568

Merged
merged 4 commits into from
May 24, 2019
Merged

Conversation

petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented May 5, 2019

This is supposed to fix #52179

This PR introduces one option debuginfo-level replacing debuginfo and debuginfo-lines and corresponding to the rustc flag -C debuginfo=N.

debuginfo-level serves as a default for all Rust code built during bootstrap, but it can be overridden for specific subsets of code using finer-grained options debuginfo-level-{rustc,std,tools,tests} replacing debuginfo-only-std, debuginfo-tools and debuginfo-tests.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 5, 2019
@petrochenkov
Copy link
Contributor Author

petrochenkov commented May 5, 2019

Questions:

  • Does all rust code goes through bootstrap/bin/rustc.rs?
    Does all rust code goes through builder.rs/fn cargo?
    I had to add the debuginfo options in one more place for tests run by compiletest.
    Are there any other places? (Or perhaps compiletest didn't need special treatment?)
  • I'm not entirely sure the correspondence between Modes and debuginfo config "partitions" is right.
  • How to special-case the rustdoc tool in builder.rs/fn cargo so it gets debuginfo even when debuginfo-level-tools is disabled? Introduce a new "partition" for it? Treat it as debuginfo-level-rustc? Perhaps it doesn't need that special casing?
  • I also previously noticed that compiletest didn't get debuginfo properly even with debuginfo = true (or at least VTune didn't see it, I didn't look in more details). Could it be due to target.is_none() in bootstrap/bin/rustc.rs? In that case this PR should fix it (didn't check yet).

@Mark-Simulacrum
Copy link
Member

Does all rust code goes through bootstrap/bin/rustc.rs?

With the exception of rustbuild itself (which is compiled by bootstrap.py and cannot depend on itself), this should be the case. It might be necessary to update bootstrap.py to properly handle these changes, I'm not sure. We will sometimes skip using this file, though it's probably buggy to do so -- basically anywhere you see a call to builder.rustc (e.g., test::Clippy, test::Cargotest, test::Miri, etc) we're not using this file. I think this is mostly because these tools are generally run against stage 2 builds and as such everything "just works" for the most part. We should probably not be doing this regardless though.

Does all rust code goes through builder.rs/fn cargo?

Compiletest doesn't use Cargo internally, so no on that; it itself will be built via fn cargo though. I think we spawn rustdoc without cargo doc on individual files and via mdbook sometimes, so not quite. For the most part though this is true.

How to special-case the rustdoc tool in builder.rs/fn cargo so it gets debuginfo even when debuginfo-level-tools is disabled? Introduce a new "partition" for it? Treat it as debuginfo-level-rustc? Perhaps it doesn't need that special casing?

I would probably have it partner with rustc instead of tools, but I don't think it matters that much.

I also previously noticed that compiletest didn't get debuginfo properly even with debuginfo = true (or at least VTune didn't see it, I didn't look in more details). Could it be due to target.is_none() in bootstrap/bin/rustc.rs? In that case this PR should fix it (didn't check yet).

I suspect that this could be the reason, but would need to investigate to be able to answer for sure.

src/bootstrap/builder.rs Show resolved Hide resolved
src/bootstrap/config.rs Show resolved Hide resolved
src/bootstrap/config.rs Outdated Show resolved Hide resolved
src/bootstrap/test.rs Outdated Show resolved Hide resolved
@rust-highfive
Copy link
Collaborator

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:02b5c1f4:start=1557085473552639684,finish=1557085474339711111,duration=787071427
$ 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:10:54] ...iiiii............................................................................................ 1100/5493
[01:10:57] .................................................................................................... 1200/5493
[01:10:59] .................................................................................................... 1300/5493
[01:11:02] .................................................................................................... 1400/5493
[01:11:05] ........................F........................................................................... 1500/5493
[01:11:11] ..................i................................................................................. 1700/5493
[01:11:15] .................................................................................................... 1800/5493
[01:11:19] .................................................................................................... 1900/5493
[01:11:22] .................................................................................................... 2000/5493
---
[01:13:39] 
[01:13:39] - error: extern items cannot be `const`
[01:13:39] -   --> $DIR/extern-const.rs:15:5
[01:13:39] -    |
[01:13:39] - LL |     const rust_dbg_static_mut: libc::c_int;
[01:13:39] -    |     ^^^^^ help: try using a static value: `static`
[01:13:39] - error: aborting due to previous error
[01:13:39] - error: aborting due to previous error
[01:13:39] + error: unknown codegen option: `-Zunstable-options`
[01:13:39] 9 
[01:13:39] 
[01:13:39] 
[01:13:39] The actual stderr differed from the expected stderr.
[01:13:39] The actual stderr differed from the expected stderr.
[01:13:39] Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/extern/extern-const/extern-const.stderr
[01:13:39] diff of fixed:
[01:13:39] 
[01:13:39] 12 
[01:13:39] 13 #[link(name = "rust_test_helpers", kind = "static")]
[01:13:39] 14 extern "C" {
[01:13:39] -     static rust_dbg_static_mut: libc::c_int; //~ ERROR extern items cannot be `const`
[01:13:39] +     const rust_dbg_static_mut: libc::c_int; //~ ERROR extern items cannot be `const`
[01:13:39] 17 
[01:13:39] 18 fn main() {
[01:13:39] 
[01:13:39] 
[01:13:39] 
[01:13:39] The actual fixed differed from the expected fixed.
[01:13:39] Actual fixed saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/extern/extern-const/extern-const.fixed
[01:13:39] To update references, rerun the tests and pass the `--bless` flag
[01:13:39] To only update this specific test, also pass `--test-args extern/extern-const.rs`
[01:13:39] error: 2 errors occurred comparing output.
[01:13:39] status: exit code: 1
[01:13:39] status: exit code: 1
[01:13:39] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/extern/extern-const.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/ui/extern/extern-const/a" "-Crpath" "-O" "-C" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-g" "-Z" "continue-parse-after-error" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/extern/extern-const/auxiliary" "-A" "unused"
[01:13:39] ------------------------------------------
[01:13:39] 
[01:13:39] ------------------------------------------
[01:13:39] stderr:
[01:13:39] stderr:
[01:13:39] ------------------------------------------
[01:13:39] error: unknown codegen option: `-Zunstable-options`
[01:13:39] 
[01:13:39] ------------------------------------------
[01:13:39] 
[01:13:39] 
---
[01:13:39] thread 'main' panicked at 'Some tests failed', src/tools/compiletest/src/main.rs:519:22
[01:13:39] note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
[01:13:39] 
[01:13:39] 
[01:13:39] 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" "/usr/lib/llvm-6.0/bin/FileCheck" "--host-rustcflags" "-Crpath -O -C debuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -C debuginfo=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" "--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:39] 
[01:13:39] 
[01:13:39] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:13:39] Build completed unsuccessfully in 0:04:32
[01:13:39] Build completed unsuccessfully in 0:04:32
[01:13:39] make: *** [check] Error 1
[01:13:39] Makefile:48: recipe for target 'check' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:24277d64
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Sun May  5 20:58:25 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)

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 7, 2019
@petrochenkov petrochenkov changed the title [WIP] rustbuild: Simplify debuginfo configuration rustbuild: Simplify debuginfo configuration May 10, 2019
@petrochenkov
Copy link
Contributor Author

Updated.

@bors rollup=never

@petrochenkov petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 10, 2019
@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented May 11, 2019

📌 Commit 53fcac07b93ab3d4724e8801b92af3fe4868a708 has been approved by Mark-Simulacrum

@bors 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 May 11, 2019
@bors
Copy link
Contributor

bors commented May 11, 2019

⌛ Testing commit 53fcac07b93ab3d4724e8801b92af3fe4868a708 with merge d5f86b8e7f1fb04f1d0dcaeca5ab2cbefd2c93be...

@bors
Copy link
Contributor

bors commented May 11, 2019

💔 Test failed - checks-travis

@rust-highfive
Copy link
Collaborator

The job test-various 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.
[01:09:37] status: exit code: 2
[01:09:37] command: "make"
[01:09:37] stdout:
[01:09:37] ------------------------------------------
[01:09:37] LD_LIBRARY_PATH="/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/wasm-panic-small/wasm-panic-small:/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools/x86_64-unknown-linux-gnu/release/deps:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/lib" '/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc' --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/wasm-panic-small/wasm-panic-small -L /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/wasm-panic-small/wasm-panic-small  foo.rs -C lto -O --target wasm32-unknown-unknown --cfg a
[01:09:37] wc -c < /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/wasm-panic-small/wasm-panic-small/foo.wasm
[01:09:37] 50740
[01:09:37] [ "`wc -c < /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/wasm-panic-small/wasm-panic-small/foo.wasm`" -lt "1024" ]
[01:09:37] Makefile:6: recipe for target 'all' failed
[01:09:37] ------------------------------------------
[01:09:37] stderr:
[01:09:37] ------------------------------------------
[01:09:37] make: *** [all] Error 1
---
[01:09:37] status: exit code: 2
[01:09:37] command: "make"
[01:09:37] stdout:
[01:09:37] ------------------------------------------
[01:09:37] LD_LIBRARY_PATH="/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/wasm-stringify-ints-small/wasm-stringify-ints-small:/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools/x86_64-unknown-linux-gnu/release/deps:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/lib" '/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc' --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/wasm-stringify-ints-small/wasm-stringify-ints-small -L /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/wasm-stringify-ints-small/wasm-stringify-ints-small  foo.rs -C lto -O --target wasm32-unknown-unknown
[01:09:37] wc -c < /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/wasm-stringify-ints-small/wasm-stringify-ints-small/foo.wasm
[01:09:37] 189258
[01:09:37] [ "`wc -c < /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/wasm-stringify-ints-small/wasm-stringify-ints-small/foo.wasm`" -lt "20500" ]
[01:09:37] Makefile:5: recipe for target 'all' failed
[01:09:37] ------------------------------------------
[01:09:37] stderr:
[01:09:37] ------------------------------------------
[01:09:37] make: *** [all] Error 1
---
[01:09:37] test result: FAILED. 8 passed; 2 failed; 2 ignored; 0 measured; 0 filtered out
[01:09:37] 
[01:09:37] 
[01:09:37] 
[01:09:37] 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/wasm32-unknown-unknown/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/run-make" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make" "--stage-id" "stage2-wasm32-unknown-unknown" "--mode" "run-make" "--target" "wasm32-unknown-unknown" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/checkout/obj/build/x86_64-unknown-linux-gnu/llvm/build/bin/FileCheck" "--nodejs" "/node-v9.2.0-linux-x64/bin/node" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/wasm32-unknown-unknown/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--llvm-version" "8.0.0-rust-1.36.0-dev\n" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[01:09:37] 
[01:09:37] 
[01:09:37] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test --target wasm32-unknown-unknown src/test/run-make src/test/ui src/test/run-pass src/test/compile-fail src/test/mir-opt src/test/codegen-units src/libcore
[01:09:37] Build completed unsuccessfully in 1:00:45
---
travis_time:end:02897a22:start=1557591850229103594,finish=1557591850237500101,duration=8396507
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:038b514d
$ 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:0494db58
travis_time:start:0494db58
$ 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:27ab0bc4
$ 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 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 May 11, 2019
@Mark-Simulacrum
Copy link
Member

Looks like we're generating a lot larger wasm files than before; presumably this is due to debuginfo...

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 12, 2019
@bors
Copy link
Contributor

bors commented May 14, 2019

⌛ Testing commit 53fcac07b93ab3d4724e8801b92af3fe4868a708 with merge 808576e0206ce070428486d45a171c9da21e087c...

@bors
Copy link
Contributor

bors commented May 14, 2019

💔 Test failed - checks-travis

@petrochenkov petrochenkov added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 23, 2019
@petrochenkov
Copy link
Contributor Author

I pushed one more commit untying debuginfo-level-tests from debuginfo-level.

From my experience of working on this branch

  • Not all tests pass with debuginfo enabled.
  • If necessary, debuginfo for tests can be enabled without rebuilding anything else.
  • I personally never needed to enable debuginfo for tests, but needed for other things.

Based on that, I would expect having

debuginfo-level = 1
debuginfo-level-tests = 0

in my config.toml most of the time.

So, I realized that the old behavior of not enabling debuginfo-tests together with debuginfo was pretty reasonable and restored it.
Any opinions?

config.toml.example Show resolved Hide resolved
config.toml.example Outdated Show resolved Hide resolved
@Mark-Simulacrum
Copy link
Member

I think this looks good to me. I'm not confident about the rustdoc special case removal, but it does seem like a reasonable simplification. It can be readded later anyway if we decide it's necessary.

@bors r+

@bors
Copy link
Contributor

bors commented May 24, 2019

📌 Commit f5ae0b4cc33a13249fb56329e54ed036110ae154 has been approved by Mark-Simulacrum

@bors 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 May 24, 2019
@Mark-Simulacrum
Copy link
Member

@bors r-

Totally forgot I had left a review comment. r=me with it fixed though.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 24, 2019
@petrochenkov
Copy link
Contributor Author

@bors r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented May 24, 2019

📌 Commit 780e406 has been approved by Mark-Simulacrum

@bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 24, 2019
@bors
Copy link
Contributor

bors commented May 24, 2019

⌛ Testing commit 780e406 with merge fc45382...

bors added a commit that referenced this pull request May 24, 2019
rustbuild: Simplify debuginfo configuration

This is supposed to fix #52179

This PR introduces one option `debuginfo-level` replacing `debuginfo` and `debuginfo-lines` and corresponding to the `rustc` flag `-C debuginfo=N`.

`debuginfo-level` serves as a default for all Rust code built during bootstrap, but it can be overridden for specific subsets of code using finer-grained options `debuginfo-level-{rustc,std,tools,tests}` replacing `debuginfo-only-std`, `debuginfo-tools` and `debuginfo-tests`.
@bors
Copy link
Contributor

bors commented May 24, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: Mark-Simulacrum
Pushing fc45382 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 24, 2019
@bors bors merged commit 780e406 into rust-lang:master May 24, 2019
@varkor
Copy link
Member

varkor commented May 24, 2019

https://rust-lang.github.io/rustc-guide/how-to-build-and-run.html needs to be updated after this.

@varkor

This comment has been minimized.

@petrochenkov petrochenkov deleted the debi branch June 5, 2019 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

config.toml: debuginfo option documentation is confusing
7 participants