search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
99.19k stars 12.81k forks source link

Tracking issue for sanitizer support #39699

Open japaric opened 7 years ago

japaric commented 7 years ago

Currently we have:

kennytm commented 7 years ago

Just for record. Currently only 4 sanitizers are enabled (asan, lsan, msan, tsan), and only in x86_64-unknown-linux-gnu (#38699) and x86_64-apple-darwin (#41352).

As of the LLVM 4.0 merge (rust-lang/compiler-rt@c8a8767c5), compiler-rt actually supports much more targets than rustc do, and also some additional sanitizers (e.g. esan) can be enabled in the future.

Architecture x86 x86_64 ppc64 arm32 arm64 mips32 mips64 s390x
asan ✓ ✓ ✓ ✓ ✓ ✓ ✓ ✓
dfsan ✓ ✓ ✓
lsan ✓ ✓ ✓
msan ✓ ✓ ✓ ✓
tsan ✓ ✓ ✓ ✓
ubsan ✓ ✓ ✓ ✓ ✓ ✓ ✓ ✓
esan ✓ ✓
cfi ✓ ✓ ✓
profile ✓ ✓ ✓ ✓ ✓ ✓ ✓ ✓
safestack ✓ ✓ ✓ ✓ ✓
scudo ✓ ✓ ✓
xray ✓ ✓ ✓
Platform linux android darwin & ios windows-msvc freebsd
asan ✓ ✓ ✓ ✓ ✓
dfsan ✓
lsan ✓ ✓
msan ✓
tsan ✓ ✓ ✓
ubsan ✓ ✓ ✓ ✓
esan ✓
cfi ✓
profile ✓ ✓ ✓ ✓
safestack ✓ ✓ ✓
scudo ✓
xray ✓

(not all of these are sanitizers, some of them are just tools or libraries that depend on the common sanitizer runtime)

cuviper commented 7 years ago

Is there any plan for stabilization here? Even if it remains x86_64-only for now, with only a few of the available sanitizers, it will still be quite useful to have. I have users that want this (rhbz1447423), but now that -Z is forbidden I want to wait for properly-supported sanitizer options.

Firstyear commented 7 years ago

Hi,

If I understand this issue, you would like to block certain -Z features from coming to stable rust. Sanitisers seems to be one of these. We have a very good use case for them though. When you have a C + Rust with FFI, and the C code is linked to libasan, the rust component will fail to link as it's missing libasan. For us it's important to get sanitisers into rust stable as we have an extensive C code based (that is well sanitised), and having this option available to us will help to determine if our Rust + C integration is behaving correctly.

I hope this helps explain our use case, as for us this is a blocker to our project adopting Rust today.

Thanks you!

cuviper commented 7 years ago

FWIW, it's not just certain -Z features, but -Z as a whole being blocked as unstable now.

Firstyear commented 7 years ago

42711 PR for dylib asan support.

Firstyear commented 7 years ago

Hi,

I don't see any activity here. We really need this for our quality assurance workflow to be stabilised. What is outstanding and required for this to be made available in stable rustc?

Thanks!

Firstyear commented 6 years ago

Hi, sorry to bump this again, but @kennytm @aturon @japaric, would you be able to look at this and comment on why this is still blocked from being in "not-nightly"? We really need this support for our project to be able to adopt rust 100%.

Thanks,

elichai commented 5 years ago

Any updates on this? Using UBSan is an important feature, though I think it will require recompiling libstd with UBSan enabled

tmiasko commented 4 years ago
rbtcollins commented 4 years ago

I'm interested in the XRay LLVM support; is this bug a sensible place to talk about that, or should I file a new bug, or an internals discussion ?

tmiasko commented 4 years ago

The guide to rustc development now contains an overview of sanitizer implementation in rustc. It should be a good starting point for work on additional sanitizers or even XRay since there is a lot of common ground there.

XiangpengHao commented 3 years ago

Hi, is there a way to access sanitizer interfaces, e.g., the __asan_poison_memory_region

A use case is when designing custom allocators based on mmap, those functions help to make the sanitizers more accurate and helpful.

Related: https://github.com/google/sanitizers/wiki/AddressSanitizerManualPoisoning

tmiasko commented 3 years ago

@XiangpengHao when building with sanitizer enabled it should be matter of declaring it with extern "C" { ... } and simply calling it. For example https://github.com/rust-lang/rust/blob/f412fb56b8d11c168e7ee49ee74e79c4ab2e5637/src/test/ui/sanitize/thread.rs#L32-L35.

XiangpengHao commented 3 years ago

Thanks, that saves my day!

repnop commented 3 years ago

curious question: is there any way that the sanitizers could eventually be enabled in a #![no_std] context for currently unsupported targets / is that possible to do with (assumedly) a lot of heavy lifting? I have an OS kernel I've been developing which involves a lot of unsafe code and it would be nice to have at least a sub-selection of the sanitizers available in that environment for debugging and helping check that my unsafe code is correct :) I'd be perfectly fine with filling in any missing functions that would fail to link, etc, to support this in my project if the base instrumentation for the code is there.

thomcc commented 3 years ago

So, if you use #[cfg(sanitize = "thread")] (or, pick your fav sanitizer) on stable/beta, it produces a feature gate error rather than just evaluating to false.

I think we should considering removing the feature gate from just the #[cfg(sanitize = "...")] used to test if a particular sanitizer is enabled, and allow use of that cfg, even on stable, where (until the rest of the sanitizer stuff mentioned here stabilizes) it will just evaluate to false.

  1. It evaluating to false is completely correct behavior for rust versions/configurations that don't support compiling with sanitizers, because obviously that means the sanitizer cannot be enabled.

  2. If we do this and later decide against stabilizing sanitizer support, even removing it from rustc. everything would be fine, and we wouldn't have to keep any code for supporting this around at all.

    The #[cfg(sanitize = "...")] would continue to accurately evaluate to false, breaking no code, and continuing to provide perfectly correct answers forever.

  3. It's already pseudo-stable. You can currently emulate it using a build.rs which examines env::var("CARGO_CFG_SANITIZE"), and the fields it has to the crate (behind, say, a cargo:rustc-cfg=__tsan_enabled or similar).

    (Note: This seems somewhat different to me than code that checks for nightly rustc at compile time, it handles nightly and stable identically, and only operates using env vars provided by cargo)

    There's code in the wild that does this, and it works without caveats beyond "it's not great that this is required", "it's also not great that it works", and "it's undesirable to add a build.rs just for this purpose".

  4. The feature is harmless, low impact, seems to have an obvious design, and has been around for a while. It seems doubtful that we will regret doing this now, so yeah.

So yeah, I don't see a downside to doing that. Again, I'm speaking only about the cfg, not the rest of the feature tracked in this issue. (I'm not even really sure if I'm asking for it to be stabilized, or for an overzealous feature gate check to be removed).

P.S. I'm unsure of how to move forward with this, given that this tracking issue tracks so many parts. Thanks.

dcsommer commented 3 years ago

What are the steps or blockers for enabling ASan on rustc targets beyond x86_64-unknown-linux-gnu and x86_64-apple-darwin? In particular, I would like to enable ASan for armv7-linux-androideabi, aarch64-linux-android, i686-linux-android, and x86_64-linux-android. The rustc-dev-guide doesn't have info for how to go about adding new targets.

Kixunil commented 3 years ago

I'm getting "ThreadSanitizer: unexpected memory mapping" on Debian 11 with latest nightly. I tried changing relocation model but it doesn't help. Anyone has an idea?

heisen-li commented 3 years ago

I have a problem. Why is there no specific number of lines displayed here? References：https://github.com/rust-lang/rust/issues/90977

Excuse me, I've taken care of it.

kamulos commented 2 years ago

Is it possible to get a quick overview of the status of this? The ASAN on Linux seems to work really well and is an invaluable tool in a mixed Rust and C application. Is it possible to stabilize this in steps?

After the incredible recent push of @joshtriplett to stabilize commonly used features like instrument-coverage this will be the last thing forcing a nightly compiler in the test pipeline of my company.

pnkfelix commented 2 years ago

visiting for T-compiler backlog bonanza.

@rustbot label: +S-tracking-needs-summary

(We cannot tell what the current status of this thing is. Its clear from comments like the one immediately above that there are people interested in this and are using it.)

landaire commented 2 years ago

Wanted to voice my support for this for usage in fazi and also suggest that this should expand to whole packages. e.g. I would not like to instrument any code in Fazi, but any callers of Fazi should be instrumented.

Additionally, I think sancov should be included in this. The reason I found this issue is because my fuzzing library gets compiled with sancov hooks and deadlocks while handling said hooks.

nagisa commented 2 years ago

On the naming of no_sanitize – as I was reviewing the shadow stack PR it seemed to me much more natural that you’d be able to say which functions you want to sanitize as well. Is this really the best design for an attribute in the context of knowledge that a) not every sanitizer requires linking in a support library; and b) it might make sense to sanitize specific functions (does it?) at some point in the future?

melver commented 2 years ago

Are there any technical blockers that prevent this from "graduating" to stable?

Rust is being used in more mixed language codebases, and with the merging of Rust support in the Linux kernel, I'd like to enable some of the kernel sanitizers (KCSAN, KASAN) for Rust code there eventually. This would improve the C bindings (if there are bugs) but also prove Rust's safety proposition quantitively (i.e. data showing fewer or no bugs found in Rust code).

Thanks.

landaire commented 1 year ago

Is there a separate issue tracking ASAN enlightenment for container types like Vec<T>? I haven't tested this, but I believe that today if you were to allocate a Vec whose len() < capacity() and pass its data pointer across an FFI boundary you wouldn't detect OOB accesses in the range of [ptr + len, ptr + cap]. I didn't see any calls to __asan_poison_memory_region or __sanitizer_annotate_contiguous_container so I suspect this is true.

LLVM's libc++ ASAN enlightenment for std::vector<T> can be found here: https://github.com/llvm/llvm-project/blob/b7a2ff296352acacdc413d6f3f912e50f90ebb31/libcxx/include/vector#L740-L750

This issue isn't really unique to vectors either -- any container that over-allocates memory to reduce total number of allocations would be affected.

kamulos commented 1 year ago

The bad thing in my environment is the following. At my job I am developing a mixed Rust and C program built with Cargo. We already had a few ASAN findings slip into the main branch because during development ASAN is not used. I want to enforce, that every developer has ASAN active all the time and currently I only see the following options:

  1. Force everyone to use the nightly compiler. This might be possible, when pinning a certain nightly version that works well. But this sounds really wrong on many levels.

  2. Only sanitize C code (CFLAGS="${CFLAGS} -fsanitize=address,undefined" RUSTFLAGS="${RUSTFLAGS} -Clink-arg=-fsanitize=address,undefined").: this might just be the way I will proceed. Obviously the unsafe Rust code here will be omitted, but in the past we did not have any bugs there. Still it feels wrong to leave out parts of the code from the instrumentation.

Thomasdezeeuw commented 1 year ago

@kamulos I think this is getting a little off-topic, but can you set up a CI with a nightly compiler? I did this recently for Mio (using Github Action, but it should work anywhere) if you want a concrete example: https://github.com/tokio-rs/mio/blob/7ed74bf478230a0cfa7543901f6be6df8bb3602e/.github/workflows/ci.yml#L115-L128, https://github.com/tokio-rs/mio/blob/7ed74bf478230a0cfa7543901f6be6df8bb3602e/Makefile#L20-L24.

kamulos commented 1 year ago

@Thomasdezeeuw absolutely, we have that, but it only runs a very basic set of tests and unfortunately some findings slipped through that, and were only later caught in the full test run which also is done with the sanitized debug binary (built with a nightly compiler). So a big part of our testing is done on a binary built with a nightly compiler which is not really that great.

For any C or C++ program I like to enable the sanitizer all the time (especially during development) and only leave out the instrumentation for the release build. Having this feature unstable makes this a bit awkward.

This just seems strange, because it is such an important feature, that (for me) works flawlessly.

tgross35 commented 1 year ago

Do we control the output or does the plugin?

==167109==WARNING: MemorySanitizer: use-of-uninitialized-value
test alloc::const_unchecked_layout ... Uninitialized bytes in __interceptor_memrchr at offset 0 inside [0x704000000000, 5)
    #0 0x556657568548 in core::slice::cmp::_$LT$impl$u20$core..cmp..PartialEq$LT$$u5b$B$u5d$$GT$$u20$for$u20$$u5b$A$u5d$$GT$::eq::h662ebfb49ff1fbfe /rustc/da6b55cc5eaf76ed6acb7dc2f7d611e32af7c9a7/library/core/src/slice/cmp.rs:26:9
...
_$LT$alloc..boxed..Box$LT$F$C$A$GT$$u20$as$u20$core..ops..function..FnOnce$LT$Args$GT$$GT$::call_once::h6d7a22d2c39a1595 /rustc/da6b55cc5eaf76ed6acb7dc2f7d611e32af7c9a7/library/alloc/src/boxed.rs:2007:9
    #27 0x556657af7fd4 in std::sys::unix::thread::Thread::new::thread_start::h9e696b653623791e /rustc/da6b55cc5eaf76ed6acb7dc2f7d611e32af7c9a7/library/std/src/sys/unix/thread.rs:108:17
    #28 0x7f8efe358ac2  (/lib/x86_64-linux-gnu/libc.so.6+0x94ac2) (BuildId: a43bfc8428df6623cd498c9c0caeb91aec9be4f9)
    #29 0x7f8efe3eaa3f  (/lib/x86_64-linux-gnu/libc.so.6+0x126a3f) (BuildId: a43bfc8428df6623cd498c9c0caeb91aec9be4f9)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /rustc/da6b55cc5eaf76ed6acb7dc2f7d611e32af7c9a7/library/core/src/slice/cmp.rs:26:9 in core::slice::cmp::_$LT$impl$u20$core..cmp..PartialEq$LT$$u5b$B$u5d$$GT$$u20$for$u20$$u5b$A$u5d$$GT$::eq::h662ebfb49ff1fbfe
Exiting

If we are able to control it, some demangling would be nice

kamulos commented 1 year ago

If we are able to control it, some demangling would be nice

You need the llvm-symbolizer tool, but there is also a python script that can do it after printing out. Here is some more information: https://github.com/google/sanitizers/wiki/AddressSanitizerCallStack

But in general it would be awesome if Rust could do that internally somehow. For me it is sometimes a problem, that the container/machine running my program needs the llvm-symbolizer which will pull in the whole llvm machinery. I even think usually the version of the symbolizer needs to be the same as the one used to compile, but I am not entirely sure about that.

tgross35 commented 1 year ago

I do have llvm-symbolizer in path but I'm wondering if I need to manually add it to the ASAN args. I'm trying to get the sanitizers running on the rust library as some dogfooding for this feature (plus the sanity check of course), if anyone knows how to improve the result output then feel free to chime in at https://github.com/rust-lang/miri-test-libstd/pull/21 🙂

edit: it's not in path on CI but it is in path for me on local and I'm getting the same results. Weird...

tmiasko commented 1 year ago

llvm-symbolizer supports rustc -Csymbol-mangling-version=v0.

tgross35 commented 1 year ago

v0 is the default anyway right?

The output without symbolizer isn't useful, https://github.com/rust-lang/miri-test-libstd/actions/runs/6429779032/job/17459492252?pr=21#step:4:70, with symbolizer it is https://github.com/rust-lang/miri-test-libstd/actions/runs/6434877064/job/17474985110?pr=21#step:4:69. Which is significantly more useful, but doesn't seem to handle everything

  0.000050       #1 0x5597d044d120 in _$LT$std..path..Component$u20$as$u20$core..cmp..PartialEq$GT$::eq::ha7b21883c1cda59a /rustc/e0d7ed1f453fb54578cc96dfea859b0e7be15016/library/std/src/path.rs
  0.000007       #2 0x5597d04495f9 in core::cmp::impls::_$LT$impl$u20$core..cmp..PartialEq$LT$$RF$B$GT$$u20$for$u20$$RF$A$GT$::eq::h83a11786eb14d2a9 /rustc/e0d7ed1f453fb54578cc96dfea859b0e7be15016/library/core/src/cmp.rs:1573:13
tmiasko commented 1 year ago

v0 is the default anyway right?

The legacy format is still the default one.

The legacy format also happens to be embedded inside Itanium, so it can be partially demangled as you can see above, but generics aren't intelligible without dedicated support.

cmazakas commented 1 month ago

Can I ask a potentially dumb question? Why is there no UndefinedSanitizer here? C and C++ both have ubsan but I don't see it here.