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

Run name-anon-globals after LTO passes as well #55609

Merged
merged 1 commit into from
Nov 7, 2018

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Nov 2, 2018

If we're going to emit bitcode (through ThinLTOBuffer), then we need to ensure that anon globals are named. This was already done after optimization passes, but also has to happen after LTO passes, as we always emit the final result in a ThinLTO-compatible manner.

I added the test as run-make. The important bit is that we emit bitcode in some way (e.g. --crate-type rlib or --emit=llvm-bc). Please tell me if there is a better way to test for that.

Fixes #51947

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 2, 2018
@nikic
Copy link
Contributor Author

nikic commented Nov 2, 2018

r? @nagisa

@rust-highfive rust-highfive assigned nagisa and unassigned matthewjasper Nov 2, 2018
@nagisa
Copy link
Member

nagisa commented Nov 2, 2018

@bors r+

Looks good, thanks!

@bors
Copy link
Contributor

bors commented Nov 2, 2018

📌 Commit 270849f87f50dfd822232d80460f446f1e73a55c has been approved by nagisa

@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 Nov 2, 2018
@bors
Copy link
Contributor

bors commented Nov 5, 2018

☔ The latest upstream changes (presumably #55593) made this pull request unmergeable. Please resolve the merge conflicts.

@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 Nov 5, 2018
@nikic
Copy link
Contributor Author

nikic commented Nov 5, 2018

Rebased

@nagisa
Copy link
Member

nagisa commented Nov 5, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Nov 5, 2018

📌 Commit 87129738d035f6c9b5c60788571d74afe1348a12 has been approved by nagisa

@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 Nov 5, 2018
@bors
Copy link
Contributor

bors commented Nov 6, 2018

⌛ Testing commit 87129738d035f6c9b5c60788571d74afe1348a12 with merge 720b6d089a3f42e1eac66b70f3ae4b8e287e4709...

@bors
Copy link
Contributor

bors commented Nov 6, 2018

💔 Test failed - status-travis

@rust-highfive
Copy link
Collaborator

The job wasm32-unknown 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.
[00:56:16] status: exit code: 2
[00:56:16] command: "make"
[00:56:16] stdout:
[00:56:16] ------------------------------------------
[00:56:16] LD_LIBRARY_PATH="/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/issue-51947/issue-51947:/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/issue-51947/issue-51947 -L /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/issue-51947/issue-51947  test.rs --crate-type rlib -O
[00:56:16] Makefile:4: recipe for target 'all' failed
[00:56:16] ------------------------------------------
[00:56:16] stderr:
[00:56:16] ------------------------------------------
[00:56:16] error[E0463]: can't find crate for `std`
[00:56:16] error[E0463]: can't find crate for `std`
[00:56:16] 
[00:56:16] error: aborting due to previous error
[00:56:16] 
[00:56:16] For more information about this error, try `rustc --explain E0463`.
[00:56:16] make: *** [all] Error 1
[00:56:16] ------------------------------------------
[00:56:16] 
[00:56:16] thread '[run-make] run-make/issue-51947' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3284:9
[00:56:16] note: Run with `RUST_BACKTRACE=1` for a backtrace.
---
[00:56:16] test result: FAILED. 8 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out
[00:56:16] 
[00:56:16] 
[00:56:16] 
[00:56:16] 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 -Zunstable-options " "--target-rustcflags" "-Crpath -O -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.0svn\n" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:56:16] 
[00:56:16] 
[00:56:16] 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
[00:56:16] Build completed unsuccessfully in 0:52:54
---
travis_time:end:00fce28c:start=1541509576247418052,finish=1541509576254868668,duration=7450616
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0b26f07a
$ 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:062645ac
travis_time:start:062645ac
$ 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:00ec8c20
$ 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 Nov 6, 2018
@nikic
Copy link
Contributor Author

nikic commented Nov 6, 2018

The failure is in the newly introduced test, so definitely legit. Not sure why though.

@kennytm kennytm 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 Nov 6, 2018
@nagisa
Copy link
Member

nagisa commented Nov 6, 2018

Hmm, this is a failure on WASM. I think WASM target has libstd… it is not like every test out there ignores wasm target. To work around the issue you could also just make this test #[no_std]?

@nikic
Copy link
Contributor Author

nikic commented Nov 6, 2018

@nagisa I feel like that shouldn't be necessary -- as you say, there'd have to be more tests skipping wasm. I'm wondering if something about the run-make test is not right. I checked that it's also possible to test this with a compile-pass ui test with a #![crate_type = "lib"] annotation, which would be using a more typical testing mechanism.

@nagisa
Copy link
Member

nagisa commented Nov 6, 2018

Yeah, lets give a good ol' compile-pass test a try. Its preferable to run-make either way.

If we're going to emit bitcode (through ThinLTOBuffer), then we
need to ensure that anon globals are named. This was already done
after optimization passes, but also has to happen after LTO passes,
as we always emit the final result in a ThinLTO-compatible manner.

Fixes rust-lang#51947.
@nikic
Copy link
Contributor Author

nikic commented Nov 6, 2018

Test changed to use compile-pass.

@nagisa
Copy link
Member

nagisa commented Nov 6, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Nov 6, 2018

📌 Commit 66702fc has been approved by nagisa

@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 Nov 6, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Nov 7, 2018
Run name-anon-globals after LTO passes as well

If we're going to emit bitcode (through ThinLTOBuffer), then we need to ensure that anon globals are named. This was already done after optimization passes, but also has to happen after LTO passes, as we always emit the final result in a ThinLTO-compatible manner.

I added the test as `run-make`. The important bit is that we emit bitcode in some way (e.g. `--crate-type rlib` or `--emit=llvm-bc`). Please tell me if there is a better way to test for that.

Fixes rust-lang#51947
bors added a commit that referenced this pull request Nov 7, 2018
Rollup of 14 pull requests

Successful merges:

 - #55377 (Slight copy-editing for `std::cell::Cell` docs)
 - #55441 (Remove unused re import in gdb_rust_pretty_printing)
 - #55453 (Choose predicates without inference variables over those with them)
 - #55495 (Don't print opt fuel messages to stdout because it breaks Rustbuild)
 - #55501 (Make `process_obligations`' computation of `completed` optional.)
 - #55510 (Fix feature gate only being checked on first repr attr.)
 - #55609 (Run name-anon-globals after LTO passes as well)
 - #55645 (do not print wrapping ranges like normal ranges in validity diagnostics)
 - #55688 (Standardised names and location of ui issue tests)
 - #55692 (-C remark: fix incorrect warning about requiring "--debuginfo" instead of "-C debuginfo=n")
 - #55702 (Add `aarch64-pc-windows-msvc` to deployed targets)
 - #55728 (Update lldb)
 - #55730 (Use trait impl method span when type param mismatch is due to impl Trait)
 - #55734 (refactor: use shorthand fields)
@bors bors merged commit 66702fc into rust-lang:master Nov 7, 2018
alexreg pushed a commit to alexreg/rust that referenced this pull request Nov 7, 2018
Rollup of 14 pull requests

Successful merges:

 - rust-lang#55377 (Slight copy-editing for `std::cell::Cell` docs)
 - rust-lang#55441 (Remove unused re import in gdb_rust_pretty_printing)
 - rust-lang#55453 (Choose predicates without inference variables over those with them)
 - rust-lang#55495 (Don't print opt fuel messages to stdout because it breaks Rustbuild)
 - rust-lang#55501 (Make `process_obligations`' computation of `completed` optional.)
 - rust-lang#55510 (Fix feature gate only being checked on first repr attr.)
 - rust-lang#55609 (Run name-anon-globals after LTO passes as well)
 - rust-lang#55645 (do not print wrapping ranges like normal ranges in validity diagnostics)
 - rust-lang#55688 (Standardised names and location of ui issue tests)
 - rust-lang#55692 (-C remark: fix incorrect warning about requiring "--debuginfo" instead of "-C debuginfo=n")
 - rust-lang#55702 (Add `aarch64-pc-windows-msvc` to deployed targets)
 - rust-lang#55728 (Update lldb)
 - rust-lang#55730 (Use trait impl method span when type param mismatch is due to impl Trait)
 - rust-lang#55734 (refactor: use shorthand fields)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

None yet

6 participants