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

expand osx sanitizer support (lsan, dylib, cdylib, staticlib) #62681

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/bootstrap/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ fn copy_apple_sanitizer_dylibs(
platform: &str,
into: &Path,
) {
for &sanitizer in &["asan", "tsan"] {
for &sanitizer in &["asan", "lsan", "tsan"] {
let filename = format!("lib__rustc__clang_rt.{}_{}_dynamic.dylib", sanitizer, platform);
let mut src_path = native_dir.join(sanitizer);
src_path.push("build");
Expand Down
6 changes: 2 additions & 4 deletions src/ci/azure-pipelines/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@ jobs:
- template: steps/run.yml
strategy:
matrix:
x86_64-gnu-llvm-6.0:
IMAGE: x86_64-gnu-llvm-6.0
mingw-check:
IMAGE: mingw-check
x86_64-gnu:
IMAGE: x86_64-gnu

- job: LinuxTools
timeoutInMinutes: 600
Expand Down
10 changes: 7 additions & 3 deletions src/librustc/middle/dependency_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,8 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: config::CrateType) -> DependencyList {
// quite yet, so do so here.
activate_injected_dep(*sess.injected_panic_runtime.get(), &mut ret,
&|cnum| tcx.is_panic_runtime(cnum));
activate_injected_dep(*sess.injected_sanitizer_runtime.get(), &mut ret,
&|cnum| tcx.is_sanitizer_runtime(cnum));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure if this is necessary. It would only really affect linux targets which I don't have access to for testing. My thinking was changing the sanitizer crate depkind to Implicit requires adding activate_injected_dep in suitable places.


// When dylib B links to dylib A, then when using B we must also link to A.
// It could be the case, however, that the rlib for A is present (hence we
Expand Down Expand Up @@ -285,11 +287,13 @@ fn attempt_static(tcx: TyCtxt<'_>) -> Option<DependencyList> {
}
}).collect::<Vec<_>>();

// Our allocator/panic runtime may not have been linked above if it wasn't
// explicitly linked, which is the case for any injected dependency. Handle
// that here and activate them.
// Our allocator/panic/sanitizer runtime may not have been linked above if
// it wasn't explicitly linked, which is the case for any injected
// dependency. Handle that here and activate them.
activate_injected_dep(*sess.injected_panic_runtime.get(), &mut ret,
&|cnum| tcx.is_panic_runtime(cnum));
activate_injected_dep(*sess.injected_sanitizer_runtime.get(), &mut ret,
&|cnum| tcx.is_sanitizer_runtime(cnum));

Some(ret)
}
Expand Down
8 changes: 5 additions & 3 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,12 @@ pub struct Session {
/// The maximum number of stackframes allowed in const eval.
pub const_eval_stack_frame_limit: usize,

/// The metadata::creader module may inject an allocator/panic_runtime
/// dependency if it didn't already find one, and this tracks what was
/// injected.
/// The metadata::creader module may inject an allocator/panic_runtime/
/// sanitizer dependency if it didn't already find one, and this tracks what
/// was injected.
pub allocator_kind: Once<Option<AllocatorKind>>,
pub injected_panic_runtime: Once<Option<CrateNum>>,
pub injected_sanitizer_runtime: Once<Option<CrateNum>>,

/// Map from imported macro spans (which consist of
/// the localized span for the macro body) to the
Expand Down Expand Up @@ -1249,6 +1250,7 @@ fn build_session_(
next_node_id: OneThread::new(Cell::new(NodeId::from_u32(1))),
allocator_kind: Once::new(),
injected_panic_runtime: Once::new(),
injected_sanitizer_runtime: Once::new(),
imported_macro_spans: OneThread::new(RefCell::new(FxHashMap::default())),
incr_comp_session: OneThread::new(RefCell::new(IncrCompSession::NotInitialized)),
cgu_reuse_tracker,
Expand Down
1 change: 1 addition & 0 deletions src/librustc_lsan/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ fn main() {
.out_dir(&native.out_dir)
.build_target(&target)
.build();
native.fixup_sanitizer_lib_name("lsan");
}
println!("cargo:rerun-if-env-changed=LLVM_CONFIG");
}
176 changes: 83 additions & 93 deletions src/librustc_metadata/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -762,106 +762,96 @@ impl<'a> CrateLoader<'a> {
}

fn inject_sanitizer_runtime(&mut self) {
if let Some(ref sanitizer) = self.sess.opts.debugging_opts.sanitizer {
// Sanitizers can only be used on some tested platforms with
// executables linked to `std`
const ASAN_SUPPORTED_TARGETS: &[&str] = &["x86_64-unknown-linux-gnu",
"x86_64-apple-darwin"];
const TSAN_SUPPORTED_TARGETS: &[&str] = &["x86_64-unknown-linux-gnu",
"x86_64-apple-darwin"];
const LSAN_SUPPORTED_TARGETS: &[&str] = &["x86_64-unknown-linux-gnu"];
const MSAN_SUPPORTED_TARGETS: &[&str] = &["x86_64-unknown-linux-gnu"];

let supported_targets = match *sanitizer {
Sanitizer::Address => ASAN_SUPPORTED_TARGETS,
Sanitizer::Thread => TSAN_SUPPORTED_TARGETS,
Sanitizer::Leak => LSAN_SUPPORTED_TARGETS,
Sanitizer::Memory => MSAN_SUPPORTED_TARGETS,
};
if !supported_targets.contains(&&*self.sess.opts.target_triple.triple()) {
self.sess.err(&format!("{:?}Sanitizer only works with the `{}` target",
sanitizer,
supported_targets.join("` or `")
));
// Sanitizers can only be used on some tested platforms with
// executables linked to `std`
const ASAN_SUPPORTED_TARGETS: &[&str] = &["x86_64-unknown-linux-gnu",
"x86_64-apple-darwin"];
const TSAN_SUPPORTED_TARGETS: &[&str] = &["x86_64-unknown-linux-gnu",
"x86_64-apple-darwin"];
const LSAN_SUPPORTED_TARGETS: &[&str] = &["x86_64-unknown-linux-gnu",
"x86_64-apple-darwin"];
const MSAN_SUPPORTED_TARGETS: &[&str] = &["x86_64-unknown-linux-gnu"];

let sanitizer = match self.sess.opts.debugging_opts.sanitizer {
Some(ref sanitizer) => sanitizer,
None => {
self.sess.injected_sanitizer_runtime.set(None);
return
}
};

// firstyear 2017 - during testing I was unable to access an OSX machine
// to make this work on different crate types. As a result, today I have
// only been able to test and support linux as a target.
if self.sess.opts.target_triple.triple() == "x86_64-unknown-linux-gnu" {
if !self.sess.crate_types.borrow().iter().all(|ct| {
match *ct {
// Link the runtime
config::CrateType::Staticlib |
config::CrateType::Executable => true,
// This crate will be compiled with the required
// instrumentation pass
config::CrateType::Rlib |
config::CrateType::Dylib |
config::CrateType::Cdylib =>
false,
_ => {
self.sess.err(&format!("Only executables, staticlibs, \
cdylibs, dylibs and rlibs can be compiled with \
`-Z sanitizer`"));
false
}
}
}) {
return
}
} else {
if !self.sess.crate_types.borrow().iter().all(|ct| {
match *ct {
// Link the runtime
config::CrateType::Executable => true,
// This crate will be compiled with the required
// instrumentation pass
config::CrateType::Rlib => false,
_ => {
self.sess.err(&format!("Only executables and rlibs can be \
compiled with `-Z sanitizer`"));
false
}
}
}) {
return
}
}
let supported_targets = match *sanitizer {
Sanitizer::Address => ASAN_SUPPORTED_TARGETS,
Sanitizer::Thread => TSAN_SUPPORTED_TARGETS,
Sanitizer::Leak => LSAN_SUPPORTED_TARGETS,
Sanitizer::Memory => MSAN_SUPPORTED_TARGETS,
};
if !supported_targets.contains(&&*self.sess.opts.target_triple.triple()) {
self.sess.err(&format!("{:?}Sanitizer only works with the `{}` target",
sanitizer,
supported_targets.join("` or `")
));
return
}

let mut uses_std = false;
self.cstore.iter_crate_data(|_, data| {
if data.name == sym::std {
uses_std = true;
}
});
let any_non_rlib = self.sess.crate_types.borrow().iter().any(|ct| {
*ct != config::CrateType::Rlib
});
if !any_non_rlib {
info!("sanitizer runtime injection skipped, only generating rlib");
self.sess.injected_sanitizer_runtime.set(None);
return
}

if uses_std {
let name = match *sanitizer {
Sanitizer::Address => "rustc_asan",
Sanitizer::Leak => "rustc_lsan",
Sanitizer::Memory => "rustc_msan",
Sanitizer::Thread => "rustc_tsan",
};
info!("loading sanitizer: {}", name);

let symbol = Symbol::intern(name);
let dep_kind = DepKind::Explicit;
let (_, data) =
self.resolve_crate(&None, symbol, symbol, None, None, DUMMY_SP,
PathKind::Crate, dep_kind)
.unwrap_or_else(|err| err.report());

// Sanity check the loaded crate to ensure it is indeed a sanitizer runtime
if !data.root.sanitizer_runtime {
self.sess.err(&format!("the crate `{}` is not a sanitizer runtime",
name));
}
} else {
self.sess.err("Must link std to be compiled with `-Z sanitizer`");
let all_supported = self.sess.crate_types.borrow().iter().all(|ct| {
match *ct {
config::CrateType::Executable |
config::CrateType::Staticlib |
config::CrateType::Cdylib |
config::CrateType::Dylib |
config::CrateType::Rlib => true,
_ => false,
}
});
if !all_supported {
self.sess.err("Only executables, staticlibs, cdylibs, dylibs and rlibs can be compiled \
with `-Z sanitizer`");
self.sess.injected_sanitizer_runtime.set(None);
return
}

let mut uses_std = false;
self.cstore.iter_crate_data(|_, data| {
if data.name == sym::std {
uses_std = true;
}
});
if !uses_std {
self.sess.err("Must link std to be compiled with `-Z sanitizer`");
return
}

let name = match *sanitizer {
Sanitizer::Address => "rustc_asan",
Sanitizer::Leak => "rustc_lsan",
Sanitizer::Memory => "rustc_msan",
Sanitizer::Thread => "rustc_tsan",
};
info!("loading sanitizer: {}", name);

let symbol = Symbol::intern(name);
let dep_kind = DepKind::Implicit;
let (cnum, data) =
self.resolve_crate(&None, symbol, symbol, None, None, DUMMY_SP,
PathKind::Crate, dep_kind)
.unwrap_or_else(|err| err.report());

// Sanity check the loaded crate to ensure it is indeed a sanitizer runtime
if !data.root.sanitizer_runtime {
self.sess.err(&format!("the crate `{}` is not a sanitizer runtime",
name));
}
self.sess.injected_sanitizer_runtime.set(Some(cnum));
}

fn inject_profiler_runtime(&mut self) {
Expand Down
1 change: 1 addition & 0 deletions src/libstd/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ rand = "0.6.1"

[target.x86_64-apple-darwin.dependencies]
rustc_asan = { path = "../librustc_asan" }
rustc_lsan = { path = "../librustc_lsan" }
rustc_tsan = { path = "../librustc_tsan" }

[target.x86_64-unknown-linux-gnu.dependencies]
Expand Down
6 changes: 0 additions & 6 deletions src/test/run-make-fulldeps/sanitizer-address/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,7 @@
LOG := $(TMPDIR)/log.txt

# NOTE the address sanitizer only supports x86_64 linux and macOS

ifeq ($(TARGET),x86_64-apple-darwin)
EXTRA_RUSTFLAG=-C rpath
else
ifeq ($(TARGET),x86_64-unknown-linux-gnu)

# Apparently there are very specific Linux kernels, notably the one that's
# currently on Travis CI, which contain a buggy commit that triggers failures in
# the ASan implementation, detailed at google/sanitizers#837. As noted in
Expand All @@ -20,7 +15,6 @@ ifeq ($(TARGET),x86_64-unknown-linux-gnu)
# flags again.
EXTRA_RUSTFLAG=-C relocation-model=dynamic-no-pic
endif
endif

all:
$(RUSTC) -g -Z sanitizer=address -Z print-link-args $(EXTRA_RUSTFLAG) overflow.rs | $(CGREP) librustc_asan
Expand Down
20 changes: 20 additions & 0 deletions src/test/run-make-fulldeps/sanitizer-c-staticlib-link/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# needs-sanitizer-support
# only-x86_64
-include ../tools.mk

ifeq ($(TARGET),x86_64-apple-darwin)
# sanitizers are always built as dylibs on osx
EXTRACFLAGS=-L$(TARGET_RPATH_DIR) -l__rustc__clang_rt.asan_osx_dynamic
LD_LIBRARY_PATH="$(TARGET_RPATH_DIR):$(TMPDIR)"
else
LD_LIBRARY_PATH=$(TMPDIR)
endif

# This test builds a staticlib, then an executable that links to it.
# The staticlib is compiled with address sanitizer, and we assert that a fault
# in the staticlib is correctly detected.

all:
$(RUSTC) -g -Z sanitizer=address --crate-type staticlib --target $(TARGET) library.rs
$(CC) program.c $(call STATICLIB,library) $(call OUT_EXE,program) $(EXTRACFLAGS) $(EXTRACXXFLAGS)
LD_LIBRARY_PATH=$(LD_LIBRARY_PATH) $(TMPDIR)/program 2>&1 | $(CGREP) stack-buffer-overflow
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#[no_mangle]
pub extern fn overflow() {
let xs = [0, 1, 2, 3];
let _y = unsafe { *xs.as_ptr().offset(4) };
}
3 changes: 2 additions & 1 deletion src/test/run-make-fulldeps/sanitizer-cdylib-link/Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# needs-sanitizer-support
# only-x86_64
# only-linux

-include ../tools.mk

Expand All @@ -12,7 +11,9 @@ LOG := $(TMPDIR)/log.txt
# is correctly detected.

# See comment in sanitizer-address/Makefile for why this is here
ifeq ($(TARGET),x86_64-unknown-linux-gnu)
EXTRA_RUSTFLAG=-C relocation-model=dynamic-no-pic
endif

all:
$(RUSTC) -g -Z sanitizer=address --crate-type cdylib --target $(TARGET) $(EXTRA_RUSTFLAG) library.rs
Expand Down
3 changes: 2 additions & 1 deletion src/test/run-make-fulldeps/sanitizer-dylib-link/Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# needs-sanitizer-support
# only-x86_64
# only-linux

-include ../tools.mk

Expand All @@ -12,7 +11,9 @@ LOG := $(TMPDIR)/log.txt
# is correctly detected.

# See comment in sanitizer-address/Makefile for why this is here
ifeq ($(TARGET),x86_64-unknown-linux-gnu)
EXTRA_RUSTFLAG=-C relocation-model=dynamic-no-pic
endif

all:
$(RUSTC) -g -Z sanitizer=address --crate-type dylib --target $(TARGET) $(EXTRA_RUSTFLAG) library.rs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,9 @@
-include ../tools.mk
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not expect this test to pass. Is it run for PRs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

None of the tests tagged with # needs-sanitizer-support are run because the Dockerfile for x86_64-gnu-llvm-6.0 does not have --enable-sanitizers set. The Dockerfile for x86_64-gnu does have --enable-sanitizers, but is not currently being run on azure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should pass now - if it were run.

Copy link
Member

Choose a reason for hiding this comment

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

cc @rust-lang/infra is there anything that should/could be done here to get these tests running?

Copy link
Member

Choose a reason for hiding this comment

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

You can edit .azure-pipelines/pr.yml to temporarily run the x86_64-gnu builder on this one PR, and then that change would be reverted before approving the PR


# NOTE the address sanitizer only supports x86_64 linux and macOS

ifeq ($(TARGET),x86_64-apple-darwin)
EXTRA_RUSTFLAG=-C rpath
else
ifeq ($(TARGET),x86_64-unknown-linux-gnu)
EXTRA_RUSTFLAG=
endif
endif

all:
$(RUSTC) -Z sanitizer=address --crate-type proc-macro --target $(TARGET) hello.rs 2>&1 | $(CGREP) '-Z sanitizer'
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@

all:
$(RUSTC) -Z sanitizer=leak --target i686-unknown-linux-gnu hello.rs 2>&1 | \
$(CGREP) 'LeakSanitizer only works with the `x86_64-unknown-linux-gnu` target'
$(CGREP) 'LeakSanitizer only works with the `x86_64-unknown-linux-gnu` or `x86_64-apple-darwin` target'
3 changes: 0 additions & 3 deletions src/test/run-make-fulldeps/sanitizer-leak/Makefile
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
-include ../tools.mk

# needs-sanitizer-support
# only-linux
# only-x86_64
# ignore-test
# FIXME(#46126) ThinLTO for libstd broke this test

all:
$(RUSTC) -C opt-level=1 -g -Z sanitizer=leak -Z print-link-args leak.rs | $(CGREP) librustc_lsan
Expand Down
Loading