Skip to content

Commit

Permalink
Auto merge of #65497 - choller:master, r=tmiasko
Browse files Browse the repository at this point in the history
Avoid injecting sanitizer runtimes into staticlibs (#64629).

This fixes the remaining issue in `creader.rs` and also fixes the expected test failure. I have explicitly turned the `$(CC)` call into a negative check with the `!` to ensure that this command is really failing (if it is not, then either the runtime is attached to the lib or the lib has not been instrumented and both would be an error).

I've also borrowed `program.rs` and the additional `rustc` invocation from @tmiasko 's PR since he pointed out that using `-fsanitize=address` with `$(CC)` for linking could fail if the sanitizer runtimes on the system are incompatible.

With this toolchain I was able to compile Firefox locally without any linker errors. I am still seeing races with Rust in TSan but I assume that is because I did not build with `-Z build-std`.
  • Loading branch information
bors committed Oct 20, 2019
2 parents 7bf377f + a2feb9c commit 857a55b
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/librustc_metadata/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -738,10 +738,10 @@ impl<'a> CrateLoader<'a> {
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::Staticlib |
config::CrateType::Rlib |
config::CrateType::Dylib |
config::CrateType::Cdylib =>
Expand Down
14 changes: 10 additions & 4 deletions src/test/run-make-fulldeps/sanitizer-staticlib-link/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,18 @@

-include ../tools.mk

# This test builds a staticlib, then an executable that links to it.
# The staticlib and executable both are compiled with address sanitizer,
# and we assert that a fault in the staticlib is correctly detected.
# This test first builds a staticlib with AddressSanitizer and checks that
# linking it to an executable fails due to the missing sanitizer runtime.
# It then builds an executable linking to the staticlib and checks that
# the fault in the staticlib is detected correctly.

# Note that checking for the link failure actually checks two things at once:
# 1) That the library has the sanitizer intrumentation
# 2) and that library does not have the sanitizer runtime

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)
! $(CC) program.c $(call STATICLIB,library) $(call OUT_EXE,program) $(EXTRACFLAGS) $(EXTRACXXFLAGS)
$(RUSTC) -g -Z sanitizer=address --crate-type bin --target $(TARGET) -L . program.rs
LD_LIBRARY_PATH=$(TMPDIR) $(TMPDIR)/program 2>&1 | $(CGREP) stack-buffer-overflow

10 changes: 10 additions & 0 deletions src/test/run-make-fulldeps/sanitizer-staticlib-link/program.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#[link(name = "library")]
extern {
fn overflow();
}

fn main() {
unsafe {
overflow();
}
}

0 comments on commit 857a55b

Please sign in to comment.