[tor-commits] [tor/maint-0.3.5] Fix segfaults related to sanitizers+jemalloc

[nickm at torproject.org]

commit 74c1e44746a7d9c810f095177af88b7dbaafd3e3
Author: Alex Crichton <alex at alexcrichton.com>
Date:   Mon Oct 1 22:54:20 2018 -0700

    Fix segfaults related to sanitizers+jemalloc
    
    It looks to be the case that Rust's standard allocator, jemalloc, is
    incompatible with sanitizers. The incompatibility, for whatever reason,
    seems to cause segfaults at runtime when jemalloc is linked with
    sanitizers.
    
    Without actually trying to figure out what's going on here this commit
    instead takes the hammer of "let's remove jemalloc when testing". The
    `tor_allocate` crate now by default switches to the system allocator
    (eventually this will want to be the tor allocator). Most crates then
    link to `tor_allocate` ot pick this up, but the `smartlist` crate had to
    manually switch to the system allocator in testing and the `external`
    crate had to be sure to link to `tor_allocate`.
    
    The final gotcha here is that this patch also switches to
    unconditionally passing `--target` to Cargo. For weird and arcane
    reasons passing `--target` with the host target of the compiler (which
    Cargo otherwise uses as the default) is different than not passing
    `--target` at all. This ensure that our custom `RUSTFLAGS` with
    sanitizer options doesn't make its way into build scripts, just the
    final testing artifacts.
---
 src/rust/Cargo.lock          |  1 +
 src/rust/external/Cargo.toml |  5 ++---
 src/rust/external/lib.rs     |  2 +-
 src/rust/smartlist/lib.rs    |  9 +++++++++
 src/rust/tor_allocate/lib.rs |  5 +++++
 src/test/test_rust.sh        | 10 +++++++++-
 6 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/src/rust/Cargo.lock b/src/rust/Cargo.lock
index 1d2a7359a..7d6a6635c 100644
--- a/src/rust/Cargo.lock
+++ b/src/rust/Cargo.lock
@@ -26,6 +26,7 @@ version = "0.0.1"
 dependencies = [
  "libc 0.2.39 (registry+https://github.com/rust-lang/crates.io-index)",
  "smartlist 0.0.1",
+ "tor_allocate 0.0.1",
 ]
 
 [[package]]
diff --git a/src/rust/external/Cargo.toml b/src/rust/external/Cargo.toml
index 4735144ee..d5c3a739e 100644
--- a/src/rust/external/Cargo.toml
+++ b/src/rust/external/Cargo.toml
@@ -5,9 +5,8 @@ name = "external"
 
 [dependencies]
 libc = "=0.2.39"
-
-[dependencies.smartlist]
-path = "../smartlist"
+smartlist = { path = "../smartlist" }
+tor_allocate = { path = "../tor_allocate" }
 
 [lib]
 name = "external"
diff --git a/src/rust/external/lib.rs b/src/rust/external/lib.rs
index b72a4f6e4..d68036fca 100644
--- a/src/rust/external/lib.rs
+++ b/src/rust/external/lib.rs
@@ -8,7 +8,7 @@
 //! module implementing this functionality repeatedly.
 
 extern crate libc;
-
+extern crate tor_allocate;
 extern crate smartlist;
 
 pub mod crypto_digest;
diff --git a/src/rust/smartlist/lib.rs b/src/rust/smartlist/lib.rs
index 2716842af..34d0b907e 100644
--- a/src/rust/smartlist/lib.rs
+++ b/src/rust/smartlist/lib.rs
@@ -6,3 +6,12 @@ extern crate libc;
 mod smartlist;
 
 pub use smartlist::*;
+
+// When testing we may be compiled with sanitizers which are incompatible with
+// Rust's default allocator, jemalloc (unsure why at this time). Most crates
+// link to `tor_allocate` which switches by default to a non-jemalloc allocator,
+// but we don't already depend on `tor_allocate` so make sure that while testing
+// we don't use jemalloc. (but rather malloc/free)
+#[global_allocator]
+#[cfg(test)]
+static A: std::alloc::System = std::alloc::System;
diff --git a/src/rust/tor_allocate/lib.rs b/src/rust/tor_allocate/lib.rs
index 5a355bc8d..1cfa0b517 100644
--- a/src/rust/tor_allocate/lib.rs
+++ b/src/rust/tor_allocate/lib.rs
@@ -11,5 +11,10 @@
 
 extern crate libc;
 
+use std::alloc::System;
+
 mod tor_allocate;
 pub use tor_allocate::*;
+
+#[global_allocator]
+static A: System = System;
diff --git a/src/test/test_rust.sh b/src/test/test_rust.sh
index a1a56af48..00b3e88d3 100755
--- a/src/test/test_rust.sh
+++ b/src/test/test_rust.sh
@@ -5,12 +5,20 @@ set -e
 
 export LSAN_OPTIONS=suppressions=${abs_top_srcdir:-../../..}/src/test/rust_supp.txt
 
+# When testing Cargo we pass a number of very specific linker flags down
+# through Cargo. We do not, however, want these flags to affect things like
+# build scripts, only the tests that we're compiling. To ensure this happens
+# we unconditionally pass `--target` into Cargo, ensuring that `RUSTFLAGS` in
+# the environment won't make their way into build scripts.
+rustc_host=$(rustc -vV | grep host | sed 's/host: //')
+
 for cargo_toml_dir in "${abs_top_srcdir:-../../..}"/src/rust/*; do
     if [ -e "${cargo_toml_dir}/Cargo.toml" ]; then
 	cd "${abs_top_builddir:-../../..}/src/rust" && \
 	    CARGO_TARGET_DIR="${abs_top_builddir:-../../..}/src/rust/target" \
 	    "${CARGO:-cargo}" test ${CARGO_ONLINE-"--frozen"} \
-	    --features "test_linking_hack" \
+            --features "test_linking_hack" \
+            --target $rustc_host \
 	    ${EXTRA_CARGO_OPTIONS} \
 	    --manifest-path "${cargo_toml_dir}/Cargo.toml" || exitcode=1
     fi