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

Run tests with MSan #15

Open
joshlf opened this issue Jan 8, 2019 · 9 comments
Open

Run tests with MSan #15

joshlf opened this issue Jan 8, 2019 · 9 comments

Comments

@joshlf
Copy link
Member

joshlf commented Jan 8, 2019

UPDATE: ASan has been implemented in 0148297. We should still run tests with MSan, although it's less important than ASan.

=== old text ===

We should enable ASan and MSan when running cargo test in order to catch issues with our use of the BoringSSL API. ASan should help catch issues with reference counting, allocation, and freeing, while MSan should help catch issues with memory initialization.

@brycx
Copy link

brycx commented Jan 8, 2019

There seem to be problems when running cargo test under MSan and I've never managed to do it successfully myself. Could Miri account for some of the things MSan would be able to check for?

@davidben
Copy link

davidben commented Jan 8, 2019

For refcounts and allocation bits, ASan (with leak-checking enabled) should be sufficient. But MSan is also generally a good idea.

I'm not familiar with Miri, but given that it instruments Rust's MIR, I doubt it would do much. The interesting stuff is going to be at the Rust <-> C boundary, so you need to instrument both compatibly.

@joshlf
Copy link
Member Author

joshlf commented Jan 8, 2019

ASan is the bigger deal here. Getting ASan alone working would be huge.

@Shnatsel
Copy link

Shnatsel commented Jan 9, 2019

What are the work items here? Why does RUSTFLAGS="-Z sanitizer=address" cargo test --target x86_64-unknown-linux-gnu not suffice?

@Shnatsel
Copy link

Shnatsel commented Jan 9, 2019

rust-lang/rust#53945 will a problem for running tests in release mode but there is a workaround, and if you're running in debug mode you're fine.

@joshlf
Copy link
Member Author

joshlf commented Jan 9, 2019

@Shnatsel

What are the work items here? Why does RUSTFLAGS="-Z sanitizer=address" cargo test --target x86_64-unknown-linux-gnu not suffice?

If it's that simple, then that's great. I know nothing about sanitizers in Rust, so it's very possible that I just wasn't aware it was this straightforward.

rust-lang/rust#53945 will a problem for running tests in release mode but there is a workaround, and if you're running in debug mode you're fine.

Doing it just in debug mode for the time being should be fine.

@Shnatsel
Copy link

Shnatsel commented Jan 10, 2019

Yup, it's that straightforward.

Sometimes ASAN produces what I assume to be false positives, complaining about "ODR violation" which seems to be an error condition exclusive to C++. This is how to suppress it:

// suppress ASAN false positives
const ASAN_DEFAULT_OPTIONS: &'static [u8] = b"detect_odr_violation=1\0";
#[no_mangle]
pub extern "C" fn __asan_default_options() -> *const u8 {
    ASAN_DEFAULT_OPTIONS as *const [u8] as *const u8
}

https://github.com/japaric/rust-san contains more documentation and recipes, AFAIK it's the hub for sanitizer work in Rust.

@joshlf
Copy link
Member Author

joshlf commented Aug 26, 2019

Update: We should be able to have this run all the time by editing test.sh.

@joshlf
Copy link
Member Author

joshlf commented Aug 29, 2019

ASan has been implemented here: 0148297.

I'm going to leave this open in case somebody wants to figure out how to get MSan working.

@joshlf joshlf changed the title Run tests with ASan and MSan Run tests with MSan Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants