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: include channel in sanitizers installed name #68994

Merged
merged 1 commit into from
Feb 12, 2020

Conversation

Keruspe
Copy link
Contributor

@Keruspe Keruspe commented Feb 9, 2020

Allows parallel install of different rust channels.

I'm not sure if the channel is the right thing to use there, but currently both beta and nightly try to install e.g. /usr/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_rt.asan.a when before (and in current stable) it used to be /usr/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_asan-45a4390180e83d28.rlib which contained a hash, making it unique.
With this patch, /usr/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc-nightly_rt.asan.a gets installed

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(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 Feb 9, 2020
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you verified that the sanitizers continue to be found and such? I would have expected us to need to change something outside the compiler, but I don't remember how we locate sanitizers...

src/bootstrap/compile.rs Outdated Show resolved Hide resolved
@Keruspe Keruspe changed the title rustbuild: include channel in sanitizers installed name [WIP] rustbuild: include channel in sanitizers installed name Feb 9, 2020
@Keruspe
Copy link
Contributor Author

Keruspe commented Feb 9, 2020

After double checking, it only worked because beta is also installed, without this patch, so it's using the one from beta. Marked the PR as WIP until I figure out where else the name needs to be changed

@Keruspe Keruspe force-pushed the sanitizers-conflict branch from f332408 to 25df314 Compare February 9, 2020 15:02
@tmiasko
Copy link
Contributor

tmiasko commented Feb 9, 2020

The code that links sanitizer runtimes and needs equivalent changes is here:

fn link_sanitizer_runtime(sess: &Session, crate_type: config::CrateType, linker: &mut dyn Linker) {
let sanitizer = match &sess.opts.debugging_opts.sanitizer {
Some(s) => s,
None => return,
};
if crate_type != config::CrateType::Executable {
return;
}
let name = match sanitizer {
Sanitizer::Address => "asan",
Sanitizer::Leak => "lsan",
Sanitizer::Memory => "msan",
Sanitizer::Thread => "tsan",
};
let default_sysroot = filesearch::get_or_default_sysroot();
let default_tlib =
filesearch::make_target_lib_path(&default_sysroot, sess.opts.target_triple.triple());
match sess.opts.target_triple.triple() {
"x86_64-apple-darwin" => {
// On Apple platforms, the sanitizer is always built as a dylib, and
// LLVM will link to `@rpath/*.dylib`, so we need to specify an
// rpath to the library as well (the rpath should be absolute, see
// PR #41352 for details).
let libname = format!("rustc_rt.{}", name);
let rpath = default_tlib.to_str().expect("non-utf8 component in path");
linker.args(&["-Wl,-rpath".into(), "-Xlinker".into(), rpath.into()]);
linker.link_dylib(Symbol::intern(&libname));
}
"x86_64-unknown-linux-gnu" | "x86_64-fuchsia" | "aarch64-fuchsia" => {
let filename = format!("librustc_rt.{}.a", name);
let path = default_tlib.join(&filename);
linker.link_whole_rlib(&path);
}
_ => {}
}
}

@Keruspe
Copy link
Contributor Author

Keruspe commented Feb 9, 2020

Found that with ripgrep, yep. testing in progress. Thanks for the hints

@Keruspe Keruspe changed the title [WIP] rustbuild: include channel in sanitizers installed name rustbuild: include channel in sanitizers installed name Feb 9, 2020
@Keruspe
Copy link
Contributor Author

Keruspe commented Feb 9, 2020

@Mark-Simulacrum it now properly works as expected

@Centril Centril added the A-sanitizers Area: Sanitizers for correctness and code quality label Feb 10, 2020
@rust-highfive
Copy link
Collaborator

The job mingw-check 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-02-10T08:36:29.2506312Z ========================== Starting Command Output ===========================
2020-02-10T08:36:29.2796011Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/820f4414-1a5f-4a85-90e5-750a565ef38c.sh
2020-02-10T08:36:29.2796061Z 
2020-02-10T08:36:29.2799622Z ##[section]Finishing: Disable git automatic line ending conversion
2020-02-10T08:36:29.2805323Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/68994/merge to s
2020-02-10T08:36:29.2806820Z Task         : Get sources
2020-02-10T08:36:29.2806854Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-02-10T08:36:29.2806900Z Version      : 1.0.0
2020-02-10T08:36:29.2806931Z Author       : Microsoft
---
2020-02-10T08:36:30.1914576Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-02-10T08:36:30.2023786Z ##[command]git config gc.auto 0
2020-02-10T08:36:30.2085215Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-02-10T08:36:30.2121233Z ##[command]git config --get-all http.proxy
2020-02-10T08:36:30.2293755Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/68994/merge:refs/remotes/pull/68994/merge
---
2020-02-10T08:45:16.3737789Z     Checking rustc_resolve v0.0.0 (/checkout/src/librustc_resolve)
2020-02-10T08:45:19.5452620Z     Checking rustc_plugin_impl v0.0.0 (/checkout/src/librustc_plugin_impl)
2020-02-10T08:45:19.7425029Z     Checking rustc_privacy v0.0.0 (/checkout/src/librustc_privacy)
2020-02-10T08:45:21.2498188Z error[E0599]: no method named `map_or_default` found for enum `std::option::Option<&str>` in the current scope
2020-02-10T08:45:21.2498556Z    --> src/librustc_codegen_ssa/back/link.rs:768:54
2020-02-10T08:45:21.2498841Z     |
2020-02-10T08:45:21.2499176Z 768 |     let channel = option_env!("CFG_RELEASE_CHANNEL").map_or_default(|channel| format!("-{}", channel));
2020-02-10T08:45:21.2499655Z 
2020-02-10T08:45:22.7096242Z error: aborting due to previous error
2020-02-10T08:45:22.7096401Z 
2020-02-10T08:45:22.7102053Z For more information about this error, try `rustc --explain E0599`.
2020-02-10T08:45:22.7102053Z For more information about this error, try `rustc --explain E0599`.
2020-02-10T08:45:22.7146008Z error: could not compile `rustc_codegen_ssa`.
2020-02-10T08:45:22.7146331Z warning: build failed, waiting for other jobs to finish...
2020-02-10T08:45:26.5501215Z error: build failed
2020-02-10T08:45:26.5539053Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "check" "-Zconfig-profile" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "2" "--release" "--color" "always" "--features" " llvm" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--message-format" "json-render-diagnostics"
2020-02-10T08:45:26.5549655Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap check
2020-02-10T08:45:26.5549950Z Build completed unsuccessfully in 0:05:51
2020-02-10T08:45:26.5599824Z == clock drift check ==
2020-02-10T08:45:26.5618823Z   local time: Mon Feb 10 08:45:26 UTC 2020
2020-02-10T08:45:26.5618823Z   local time: Mon Feb 10 08:45:26 UTC 2020
2020-02-10T08:45:26.8389611Z   network time: Mon, 10 Feb 2020 08:45:26 GMT
2020-02-10T08:45:26.8394892Z == end clock drift check ==
2020-02-10T08:45:27.6828023Z 
2020-02-10T08:45:27.6922090Z ##[error]Bash exited with code '1'.
2020-02-10T08:45:27.6935001Z ##[section]Finishing: Run build
2020-02-10T08:45:27.6952720Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/68994/merge to s
2020-02-10T08:45:27.6954398Z Task         : Get sources
2020-02-10T08:45:27.6954442Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-02-10T08:45:27.6954486Z Version      : 1.0.0
2020-02-10T08:45:27.6954543Z Author       : Microsoft
2020-02-10T08:45:27.6954543Z Author       : Microsoft
2020-02-10T08:45:27.6954587Z Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
2020-02-10T08:45:27.6954648Z ==============================================================================
2020-02-10T08:45:28.1144824Z Cleaning any cached credential from repository: rust-lang/rust (GitHub)
2020-02-10T08:45:28.1192447Z ##[section]Finishing: Checkout rust-lang/rust@refs/pull/68994/merge to s
2020-02-10T08:45:28.1346782Z Cleaning up task key
2020-02-10T08:45:28.1347600Z Start cleaning up orphan processes.
2020-02-10T08:45:28.1457309Z Terminate orphan process: pid (3674) (python)
2020-02-10T08:45:28.1644046Z ##[section]Finishing: Finalize Job

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)

@Keruspe Keruspe force-pushed the sanitizers-conflict branch from 461214a to 3ad2652 Compare February 10, 2020 09:12
@Mark-Simulacrum
Copy link
Member

r=me with commits squashed into one

Allows parallel install of different rust channels

Signed-off-by: Marc-Antoine Perennou <Marc-Antoine@Perennou.com>
@Keruspe Keruspe force-pushed the sanitizers-conflict branch from 3ad2652 to 1bba9cf Compare February 11, 2020 08:41
@Keruspe
Copy link
Contributor Author

Keruspe commented Feb 11, 2020

squashed

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Feb 11, 2020

📌 Commit 1bba9cf 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 Feb 11, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 12, 2020
…-Simulacrum

rustbuild: include channel in sanitizers installed name

Allows parallel install of different rust channels.

I'm not sure if the channel is the right thing to use there, but currently both beta and nightly try to install e.g. `/usr/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_rt.asan.a` when before (and in current stable) it used to be `/usr/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_asan-45a4390180e83d28.rlib` which contained a hash, making it unique.
With this patch, `/usr/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc-nightly_rt.asan.a` gets installed
bors added a commit that referenced this pull request Feb 12, 2020
Rollup of 8 pull requests

Successful merges:

 - #67585 (Improve `char::is_ascii_*` codegen)
 - #68914 (Speed up `SipHasher128`.)
 - #68994 (rustbuild: include channel in sanitizers installed name)
 - #69032 (ICE in nightly-2020-02-08: handle TerminatorKind::Yield in librustc_mir::transform::promote_consts::Validator method)
 - #69034 (parser: Remove `Parser::prev_token_kind`)
 - #69042 (Remove backtrace header text)
 - #69059 (Remove a few unused objects)
 - #69089 (Properly use the darwin archive format on Apple targets)

Failed merges:

r? @ghost
@bors bors merged commit 1bba9cf into rust-lang:master Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sanitizers Area: Sanitizers for correctness and code quality 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.

7 participants