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

MemorySanitizer detects an use of unitialized value in the test runner (`rustc --test`) #39610

Open
japaric opened this Issue Feb 7, 2017 · 10 comments

Comments

Projects
None yet
6 participants
@japaric
Copy link
Member

japaric commented Feb 7, 2017

STR

$ cargo new --lib test-runner && cd $_

$ edit src/lib.rs && cat $_
#[test]
fn foo() {}
$ RUSTFLAGS="-Z sanitizer=memory" cargo test --target x86_64-unknown-linux-gnu
     Running target/x86_64-unknown-linux-gnu/debug/deps/test_runner-d861d6557762b235
Uninitialized bytes in __interceptor_memchr at offset 13 inside [0x70400000ef60, 23)
==6915==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x55aec536a8b5 in std::ffi::c_str::CString::_new::h1600b539eb5d8b8c ($PWD/target/x86_64-unknown-linux-gnu/debug/deps/test_runner-d861d6557762b235+0xc58b5)
    #1 0x55aec537399a in std::sys::imp::fs::stat::h72120555244bec39 ($PWD/target/x86_64-unknown-linux-gnu/debug/deps/test_runner-d861d6557762b235+0xce99a)
    #2 0x55aec5355b18 in std::fs::metadata::h4ae9b0fd118f3836 ($PWD/target/x86_64-unknown-linux-gnu/debug/deps/test_runner-d861d6557762b235+0xb0b18)
    #3 0x55aec535bea8 in term::terminfo::searcher::get_dbpath_for_term::hc53288f466988180 ($PWD/target/x86_64-unknown-linux-gnu/debug/deps/test_runner-d861d6557762b235+0xb6ea8)
    #4 0x55aec535b3f1 in term::terminfo::TermInfo::from_name::hb95f189f4c99eccf ($PWD/target/x86_64-unknown-linux-gnu/debug/deps/test_runner-d861d6557762b235+0xb63f1)
    #5 0x55aec535b1a2 in term::terminfo::TermInfo::from_env::h45b8e5476a2a09d7 ($PWD/target/x86_64-unknown-linux-gnu/debug/deps/test_runner-d861d6557762b235+0xb61a2)
    #6 0x55aec5365c70 in term::stdout::h84d7912730b73cf4 ($PWD/target/x86_64-unknown-linux-gnu/debug/deps/test_runner-d861d6557762b235+0xc0c70)
    #7 0x55aec52d28bd in _$LT$test..ConsoleTestState$LT$T$GT$$GT$::new::h937954646ef1f1d9 ($PWD/target/x86_64-unknown-linux-gnu/debug/deps/test_runner-d861d6557762b235+0x2d8bd)
    #8 0x55aec52d481a in test::run_tests_console::h7b41f829f623d5c0 ($PWD/target/x86_64-unknown-linux-gnu/debug/deps/test_runner-d861d6557762b235+0x2f81a)
    #9 0x55aec52cfdb8 in test::test_main::hae140f91361b0544 ($PWD/target/x86_64-unknown-linux-gnu/debug/deps/test_runner-d861d6557762b235+0x2adb8)
    #10 0x55aec52d06ce in test::test_main_static::h9b2aae5d6f64eac6 ($PWD/target/x86_64-unknown-linux-gnu/debug/deps/test_runner-d861d6557762b235+0x2b6ce)
    #11 0x55aec52be9a3 in test_runner::__test::main::h164d7dfa966cbb3f $PWD/src/lib.rs:1
    #12 0x55aec537d5d6 in __rust_maybe_catch_panic ($PWD/target/x86_64-unknown-linux-gnu/debug/deps/test_runner-d861d6557762b235+0xd85d6)
    #13 0x55aec5376bc9 in std::rt::lang_start::h6954771f55df116b ($PWD/target/x86_64-unknown-linux-gnu/debug/deps/test_runner-d861d6557762b235+0xd1bc9)
    #14 0x55aec52bea19 in main ($PWD/target/x86_64-unknown-linux-gnu/debug/deps/test_runner-d861d6557762b235+0x19a19)
    #15 0x7fc24cf2f290 in __libc_start_main (/usr/lib/libc.so.6+0x20290)
    #16 0x55aec52be839 in _start ($PWD/target/x86_64-unknown-linux-gnu/debug/deps/test_runner-d861d6557762b235+0x19839)

SUMMARY: MemorySanitizer: use-of-uninitialized-value ($PWD/target/x86_64-unknown-linux-gnu/debug/deps/test_runner-d861d6557762b235+0xc58b5) in std::ffi::c_str::CString::_new::h1600b539eb5d8b8c
Exiting
error: test failed

Meta

TODO Sanitizers are not yet in tree. (cf. #38699)


cc @alexcrichton @brson

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Feb 7, 2017

Naively I'd say this is a false positive, but I could be wrong! This stack trace looks familiar, though, as I think I've seen it in terms of valgrind warnings in the past as well. (although I think those may have been false positives as well)

@dimbleby

This comment has been minimized.

Copy link

dimbleby commented Feb 17, 2017

Apparently:

If you want MemorySanitizer to work properly and not produce any false positives, you must ensure that all the code in your program and in libraries it uses is instrumented (i.e. built with -fsanitize=memory). In particular, you would need to link against MSan-instrumented C++ standard library.

I'm guessing that we're just using the regular libc?

Also, here is much the same thing happening in another project.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Feb 17, 2017

Thanks for the info @dimbleby! Right now I believe the standard library is not compiled with -fsanitize=memory, which could be (and likely is) the cause of these problems.

@japaric WDYT about modifying -Clto to work here? There's already custom passes with -C lto + -C panic=abort and I think we could easily modify LTO to apply the necessary LLVM attribute everywhere as well.

After that I think we could generate a nicer error up front saying "not everything is compiled with the memory sanitizer" than perhaps deferring this to runtime.

@dimbleby

This comment has been minimized.

Copy link

dimbleby commented Feb 18, 2017

Not sure whether we're at cross purposes, but just to make sure: I wasn't talking about the Rust standard library but the regular C libc - which is, I suppose, where the memchr() in the stack is ultimately coming from.

Having said that, it sounds as though both would need to be instrumented for the memory sanitizer to give reliable results...

@japaric

This comment has been minimized.

Copy link
Member Author

japaric commented Feb 18, 2017

you must ensure that all the code in your program and in libraries it uses is instrumented

I think this is the root of the problem because if you run the sanitizer using Xargo, to force intrumentation of the whole std facade, then there's no error.

@dimbleby I don't think one needs to compile libc with -fsanitize=memory as the sanitizer runtime that we link already instruments most of libc functions (memcpy, memchr, etc.) by overriding those symbols. The example you linked doesn't recompile libc either. We should, though, compile other C libraries that get linked to the final executable with -fsanitize=memory. I think that could be handled with a CARGO_SANITIZER=memory env variable that build scripts can read to decide whether to compile code with -fsanitize or not.

@alexcrichton

Right now I believe the standard library is not compiled with -fsanitize=memory

We can't ship a std facade compiled with -Z sanitizer because if a single function is compiled with -Z sanitizer then you have to link the sanitizer runtime or your executable won't link due to missing symbols.

There's already custom passes with -C lto + -C panic=abort

Interesting. I didn't know about that. I'll take a look. Do you know if that also forces the recompilation of functions that are already contained in e.g. libstd.rlib in the form of object files?

After that I think we could generate a nicer error up front saying "not everything is compiled with the memory sanitizer" than perhaps deferring this to runtime.

Do you mean making it so that a crate won't compile with -Z sanitizer if -C lto is not used?

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Feb 21, 2017

We can't ship a std facade compiled with -Z sanitizer because if a single function is compiled with -Z sanitizer then you have to link the sanitizer runtime or your executable won't link due to missing symbols.

Excellent point! Although unless we could ship two libstd builds and select between them dynamically 😉

(yeah let's not do that)

Interesting. I didn't know about that. I'll take a look. Do you know if that also forces the recompilation of functions that are already contained in e.g. libstd.rlib in the form of object files?

Yeah the "recompilation" here is that we build a massive wad of LLVM IR representing the entire world of Rust, we then optimize it all again, and then emit object code again. That re-codegens the entire world, but doesn't "recompile" the entire world (depending on your definition)

Do you mean making it so that a crate won't compile with -Z sanitizer if -C lto is not used?

Yeah that's what I'm thinking. Unless all libraries are compiled with the right sanitizer then this is basically guaranteed to return false positives and errors, right?

@danburkert

This comment has been minimized.

Copy link
Contributor

danburkert commented Jul 2, 2017

Is there a way to pass -fsanitize-blacklist to work around this until a proper fix lands?

@gnzlbg gnzlbg referenced this issue Feb 1, 2018

Open

Add travis ci #7

@gnzlbg

This comment has been minimized.

Copy link
Contributor

gnzlbg commented Feb 2, 2018

Excellent point! Although unless we could ship two libstd builds and select between them dynamically 😉

(yeah let's not do that)

What we could do is add targets for sanitized builds. I want to just cargo test --target x86_64-unknown-linux-gnu-msan and have all my dependencies compiled with -fsanitize=memory (and origins and what not) and linked against an appropriately compiled std library. rustup should be able to ship this target.

@gnzlbg

This comment has been minimized.

Copy link
Contributor

gnzlbg commented Feb 28, 2018

@alexcrichton how hard would it be to add a new target: x86_64-unknown-linux-gnu-msan that ships its own std-lib compiled with -fsanitize=memory -fsanitize-memory-track-origins=2 -fno-omit-frame-pointer -g ?

The memory sanitizer is Linux-only, and probably x86_64 only as well, so this is really all that we would need (maybe a x86_64-unknown-linux-musl-msan if there is desire for this in the future).

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Feb 28, 2018

It unfortunately wouldn't be the easiest thing but it also woudln't necessarily be too hard. It's definitely pretty hard to "do on a whim", so we'd want to commit to the design before doing so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.