From bc3b7c9930d3500d69a39c2a34d7ef63c403d74d Mon Sep 17 00:00:00 2001 From: Daniel Paoliello Date: Tue, 19 Dec 2023 10:29:47 -0800 Subject: [PATCH] Enable address sanitizer for MSVC targets using INFERASANLIBS linker flag --- compiler/rustc_codegen_ssa/src/back/link.rs | 44 +++++++++++++------ compiler/rustc_session/src/session.rs | 5 ++- .../src/spec/targets/i686_pc_windows_msvc.rs | 3 +- .../spec/targets/x86_64_pc_windows_msvc.rs | 3 +- src/bootstrap/src/core/build_steps/compile.rs | 2 +- src/bootstrap/src/core/build_steps/test.rs | 23 ++++++++++ src/bootstrap/src/core/config/config.rs | 11 ++++- src/bootstrap/src/core/sanity.rs | 2 +- tests/run-make/sanitizer-cdylib-link/Makefile | 2 +- .../run-make/sanitizer-cdylib-link/program.rs | 2 + tests/run-make/sanitizer-dylib-link/Makefile | 2 +- .../run-make/sanitizer-dylib-link/program.rs | 2 + .../sanitizer-staticlib-link/program.rs | 2 +- tests/ui/sanitize/badfree.rs | 2 +- 14 files changed, 80 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 4ff497f2fdd3f..215649f33ff1d 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -1186,15 +1186,22 @@ mod win { } } -fn add_sanitizer_libraries(sess: &Session, crate_type: CrateType, linker: &mut dyn Linker) { - // On macOS the runtimes are distributed as dylibs which should be linked to - // both executables and dynamic shared objects. Everywhere else the runtimes - // are currently distributed as static libraries which should be linked to - // executables only. +fn add_sanitizer_libraries( + sess: &Session, + flavor: LinkerFlavor, + crate_type: CrateType, + linker: &mut dyn Linker, +) { + // On macOS and Windows using MSVC the runtimes are distributed as dylibs + // which should be linked to both executables and dynamic libraries. + // Everywhere else the runtimes are currently distributed as static + // libraries which should be linked to executables only. let needs_runtime = !sess.target.is_like_android && match crate_type { CrateType::Executable => true, - CrateType::Dylib | CrateType::Cdylib | CrateType::ProcMacro => sess.target.is_like_osx, + CrateType::Dylib | CrateType::Cdylib | CrateType::ProcMacro => { + sess.target.is_like_osx || sess.target.is_like_msvc + } CrateType::Rlib | CrateType::Staticlib => false, }; @@ -1204,26 +1211,31 @@ fn add_sanitizer_libraries(sess: &Session, crate_type: CrateType, linker: &mut d let sanitizer = sess.opts.unstable_opts.sanitizer; if sanitizer.contains(SanitizerSet::ADDRESS) { - link_sanitizer_runtime(sess, linker, "asan"); + link_sanitizer_runtime(sess, flavor, linker, "asan"); } if sanitizer.contains(SanitizerSet::LEAK) { - link_sanitizer_runtime(sess, linker, "lsan"); + link_sanitizer_runtime(sess, flavor, linker, "lsan"); } if sanitizer.contains(SanitizerSet::MEMORY) { - link_sanitizer_runtime(sess, linker, "msan"); + link_sanitizer_runtime(sess, flavor, linker, "msan"); } if sanitizer.contains(SanitizerSet::THREAD) { - link_sanitizer_runtime(sess, linker, "tsan"); + link_sanitizer_runtime(sess, flavor, linker, "tsan"); } if sanitizer.contains(SanitizerSet::HWADDRESS) { - link_sanitizer_runtime(sess, linker, "hwasan"); + link_sanitizer_runtime(sess, flavor, linker, "hwasan"); } if sanitizer.contains(SanitizerSet::SAFESTACK) { - link_sanitizer_runtime(sess, linker, "safestack"); + link_sanitizer_runtime(sess, flavor, linker, "safestack"); } } -fn link_sanitizer_runtime(sess: &Session, linker: &mut dyn Linker, name: &str) { +fn link_sanitizer_runtime( + sess: &Session, + flavor: LinkerFlavor, + linker: &mut dyn Linker, + name: &str, +) { fn find_sanitizer_runtime(sess: &Session, filename: &str) -> PathBuf { let session_tlib = filesearch::make_target_lib_path(&sess.sysroot, sess.opts.target_triple.triple()); @@ -1254,6 +1266,10 @@ fn link_sanitizer_runtime(sess: &Session, linker: &mut dyn Linker, name: &str) { let rpath = path.to_str().expect("non-utf8 component in path"); linker.args(&["-Wl,-rpath", "-Xlinker", rpath]); linker.link_dylib(&filename, false, true); + } else if sess.target.is_like_msvc && flavor == LinkerFlavor::Msvc(Lld::No) && name == "asan" { + // MSVC provides the `/INFERASANLIBS` argument to automatically find the + // compatible ASAN library. + linker.arg("/INFERASANLIBS"); } else { let filename = format!("librustc{channel}_rt.{name}.a"); let path = find_sanitizer_runtime(sess, &filename).join(&filename); @@ -2076,7 +2092,7 @@ fn linker_with_args<'a>( ); // Sanitizer libraries. - add_sanitizer_libraries(sess, crate_type, cmd); + add_sanitizer_libraries(sess, flavor, crate_type, cmd); // Object code from the current crate. // Take careful note of the ordering of the arguments we pass to the linker diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 9ee7625e5bfeb..c80990402a980 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -1274,7 +1274,10 @@ fn validate_commandline_args_with_session_available(sess: &Session) { } // Cannot enable crt-static with sanitizers on Linux - if sess.crt_static(None) && !sess.opts.unstable_opts.sanitizer.is_empty() { + if sess.crt_static(None) + && !sess.opts.unstable_opts.sanitizer.is_empty() + && !sess.target.is_like_msvc + { sess.dcx().emit_err(errors::CannotEnableCrtStaticLinux); } diff --git a/compiler/rustc_target/src/spec/targets/i686_pc_windows_msvc.rs b/compiler/rustc_target/src/spec/targets/i686_pc_windows_msvc.rs index ba80c23196e1d..5abc3017bf80c 100644 --- a/compiler/rustc_target/src/spec/targets/i686_pc_windows_msvc.rs +++ b/compiler/rustc_target/src/spec/targets/i686_pc_windows_msvc.rs @@ -1,9 +1,10 @@ -use crate::spec::{base, LinkerFlavor, Lld, Target}; +use crate::spec::{base, LinkerFlavor, Lld, SanitizerSet, Target}; pub fn target() -> Target { let mut base = base::windows_msvc::opts(); base.cpu = "pentium4".into(); base.max_atomic_width = Some(64); + base.supported_sanitizers = SanitizerSet::ADDRESS; base.add_pre_link_args( LinkerFlavor::Msvc(Lld::No), diff --git a/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs b/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs index 7d6276a0c2d57..3a4da91c2443f 100644 --- a/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs +++ b/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs @@ -1,10 +1,11 @@ -use crate::spec::{base, Target}; +use crate::spec::{base, SanitizerSet, Target}; pub fn target() -> Target { let mut base = base::windows_msvc::opts(); base.cpu = "x86-64".into(); base.plt_by_default = false; base.max_atomic_width = Some(64); + base.supported_sanitizers = SanitizerSet::ADDRESS; Target { llvm_target: "x86_64-pc-windows-msvc".into(), diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index dbb64583d561c..d699c4fe536c5 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -274,7 +274,7 @@ fn copy_third_party_objects( ) -> Vec<(PathBuf, DependencyType)> { let mut target_deps = vec![]; - if builder.config.sanitizers_enabled(target) && compiler.stage != 0 { + if builder.config.needs_sanitizer_runtime_built(target) && compiler.stage != 0 { // The sanitizers are only copied in stage1 or above, // to avoid creating dependency on LLVM. target_deps.extend( diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 92140b00da843..3ae3af38bf85b 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -1937,6 +1937,29 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the } } + // Special setup to enable running with sanitizers on MSVC. + if !builder.config.dry_run() + && target.contains("msvc") + && builder.config.sanitizers_enabled(target) + { + // Ignore interception failures: not all dlls in the process will have been built with + // address sanitizer enabled (e.g., ntdll.dll). + cmd.env("ASAN_WIN_CONTINUE_ON_INTERCEPTION_FAILURE", "1"); + // Add the address sanitizer runtime to the PATH - it is located next to cl.exe. + let asan_runtime_path = + builder.cc.borrow()[&target].path().parent().unwrap().to_path_buf(); + let old_path = cmd + .get_envs() + .find_map(|(k, v)| (k == "PATH").then_some(v)) + .flatten() + .map_or_else(|| env::var_os("PATH").unwrap_or_default(), |v| v.to_owned()); + let new_path = env::join_paths( + env::split_paths(&old_path).chain(std::iter::once(asan_runtime_path)), + ) + .expect("Could not add ASAN runtime path to PATH"); + cmd.env("PATH", new_path); + } + // Some UI tests trigger behavior in rustc where it reads $CARGO and changes behavior if it exists. // To make the tests work that rely on it not being set, make sure it is not set. cmd.env_remove("CARGO"); diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index c6bf71c883785..3ac3e54563148 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -2180,8 +2180,15 @@ impl Config { self.target_config.get(&target).map(|t| t.sanitizers).flatten().unwrap_or(self.sanitizers) } - pub fn any_sanitizers_enabled(&self) -> bool { - self.target_config.values().any(|t| t.sanitizers == Some(true)) || self.sanitizers + pub fn needs_sanitizer_runtime_built(&self, target: TargetSelection) -> bool { + // MSVC uses the Microsoft-provided sanitizer runtime, but all other runtimes we build. + !target.is_msvc() && self.sanitizers_enabled(target) + } + + pub fn any_sanitizers_to_build(&self) -> bool { + self.target_config + .iter() + .any(|(ts, t)| !ts.is_msvc() && t.sanitizers.unwrap_or(self.sanitizers)) } pub fn profiler_path(&self, target: TargetSelection) -> Option<&str> { diff --git a/src/bootstrap/src/core/sanity.rs b/src/bootstrap/src/core/sanity.rs index 9101d94ea881e..82755f418000d 100644 --- a/src/bootstrap/src/core/sanity.rs +++ b/src/bootstrap/src/core/sanity.rs @@ -96,7 +96,7 @@ pub fn check(build: &mut Build) { }) .any(|build_llvm_ourselves| build_llvm_ourselves); - let need_cmake = building_llvm || build.config.any_sanitizers_enabled(); + let need_cmake = building_llvm || build.config.any_sanitizers_to_build(); if need_cmake && cmd_finder.maybe_have("cmake").is_none() { eprintln!( " diff --git a/tests/run-make/sanitizer-cdylib-link/Makefile b/tests/run-make/sanitizer-cdylib-link/Makefile index 691585268bfe9..10d94afc39ed4 100644 --- a/tests/run-make/sanitizer-cdylib-link/Makefile +++ b/tests/run-make/sanitizer-cdylib-link/Makefile @@ -12,5 +12,5 @@ LOG := $(TMPDIR)/log.txt all: $(RUSTC) -g -Z sanitizer=address --crate-type cdylib --target $(TARGET) library.rs - $(RUSTC) -g -Z sanitizer=address --crate-type bin --target $(TARGET) -llibrary program.rs + $(RUSTC) -g -Z sanitizer=address --crate-type bin --target $(TARGET) program.rs LD_LIBRARY_PATH=$(TMPDIR) $(TMPDIR)/program 2>&1 | $(CGREP) stack-buffer-overflow diff --git a/tests/run-make/sanitizer-cdylib-link/program.rs b/tests/run-make/sanitizer-cdylib-link/program.rs index ef053aa2e7a3a..1026c7f89ba9f 100644 --- a/tests/run-make/sanitizer-cdylib-link/program.rs +++ b/tests/run-make/sanitizer-cdylib-link/program.rs @@ -1,3 +1,5 @@ +#[cfg_attr(windows, link(name = "library.dll.lib", modifiers = "+verbatim"))] +#[cfg_attr(not(windows), link(name = "library"))] extern "C" { fn overflow(); } diff --git a/tests/run-make/sanitizer-dylib-link/Makefile b/tests/run-make/sanitizer-dylib-link/Makefile index b0a91e5b197d7..c5a698db3a034 100644 --- a/tests/run-make/sanitizer-dylib-link/Makefile +++ b/tests/run-make/sanitizer-dylib-link/Makefile @@ -12,5 +12,5 @@ LOG := $(TMPDIR)/log.txt all: $(RUSTC) -g -Z sanitizer=address --crate-type dylib --target $(TARGET) library.rs - $(RUSTC) -g -Z sanitizer=address --crate-type bin --target $(TARGET) -llibrary program.rs + $(RUSTC) -g -Z sanitizer=address --crate-type bin --target $(TARGET) program.rs LD_LIBRARY_PATH=$(TMPDIR) $(TMPDIR)/program 2>&1 | $(CGREP) stack-buffer-overflow diff --git a/tests/run-make/sanitizer-dylib-link/program.rs b/tests/run-make/sanitizer-dylib-link/program.rs index ef053aa2e7a3a..1026c7f89ba9f 100644 --- a/tests/run-make/sanitizer-dylib-link/program.rs +++ b/tests/run-make/sanitizer-dylib-link/program.rs @@ -1,3 +1,5 @@ +#[cfg_attr(windows, link(name = "library.dll.lib", modifiers = "+verbatim"))] +#[cfg_attr(not(windows), link(name = "library"))] extern "C" { fn overflow(); } diff --git a/tests/run-make/sanitizer-staticlib-link/program.rs b/tests/run-make/sanitizer-staticlib-link/program.rs index ec59bdb11c889..5fac0e739669e 100644 --- a/tests/run-make/sanitizer-staticlib-link/program.rs +++ b/tests/run-make/sanitizer-staticlib-link/program.rs @@ -1,4 +1,4 @@ -#[link(name = "library")] +#[link(name = "library", kind = "static")] extern "C" { fn overflow(); } diff --git a/tests/ui/sanitize/badfree.rs b/tests/ui/sanitize/badfree.rs index c8d1ce7dff25d..4a230e11d9579 100644 --- a/tests/ui/sanitize/badfree.rs +++ b/tests/ui/sanitize/badfree.rs @@ -5,7 +5,7 @@ // compile-flags: -Z sanitizer=address -O // // run-fail -// error-pattern: AddressSanitizer: SEGV +// regex-error-pattern: AddressSanitizer: (SEGV|attempting free on address which was not malloc) use std::ffi::c_void;