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 sanitizers on CI #591

Merged
merged 1 commit into from
Feb 15, 2021
Merged

Run sanitizers on CI #591

merged 1 commit into from
Feb 15, 2021

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Nov 3, 2020

This adds AddressSanitizer/MemorySanitizer/ThreadSanitizer tests to CI.

See #589 (tsan: data race in deque), #644 (tsan: data race in atomic), and #614 (asan: memory leak in skiplist) for issues reported by sanitizers.

Closes #154
Closes #589
Closes #644

Refs:

@taiki-e taiki-e marked this pull request as draft November 3, 2020 15:39
@taiki-e taiki-e changed the title Run asan/tsan on CI [wip] Run asan/tsan on CI Nov 3, 2020
@taiki-e taiki-e force-pushed the san branch 2 times, most recently from 33f39c7 to dbdd891 Compare November 3, 2020 16:27
@taiki-e taiki-e changed the title [wip] Run asan/tsan on CI [wip] Run sanitizers on CI Nov 3, 2020
@taiki-e taiki-e force-pushed the san branch 2 times, most recently from 04621b1 to 74e3e01 Compare November 3, 2020 17:00
@taiki-e taiki-e changed the title [wip] Run sanitizers on CI [wip] Run asan/tsan on CI Nov 3, 2020
@taiki-e taiki-e force-pushed the san branch 6 times, most recently from 6e9753d to c6957f3 Compare November 3, 2020 20:16
@taiki-e taiki-e changed the title [wip] Run asan/tsan on CI [wip] Run sanitizers on CI Nov 3, 2020
@taiki-e taiki-e force-pushed the san branch 2 times, most recently from 0bee96d to f1b6d98 Compare November 4, 2020 07:57
@taiki-e taiki-e changed the title [wip] Run sanitizers on CI Run sanitizers on CI Nov 16, 2020
@taiki-e taiki-e added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author label Nov 29, 2020
@taiki-e taiki-e marked this pull request as ready for review December 9, 2020 14:59
@taiki-e taiki-e marked this pull request as draft December 10, 2020 01:51
@taiki-e taiki-e removed the S-blocked Status: Blocked on something else label Jan 4, 2021
@taiki-e taiki-e force-pushed the san branch 5 times, most recently from fdf1af2 to 784d002 Compare January 5, 2021 06:02
@taiki-e taiki-e removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author label Jan 5, 2021
@taiki-e taiki-e marked this pull request as ready for review January 5, 2021 06:23
@taiki-e taiki-e force-pushed the san branch 4 times, most recently from f30f693 to 523d7df Compare January 5, 2021 06:50
Comment on lines +202 to 204
#[cfg(not(crossbeam_sanitize))] // TODO: assertions failed due to `cfg(crossbeam_sanitize)` reduce `internal::MAX_OBJECTS`
#[test]
fn incremental() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why this test fails when we reduce internal::MAX_OBJECTS, but it also fails when I run a normal test with --cfg crossbeam_sanitize. (And, except for cfg(test) code, cfg(crossbeam_sanitize) only changes internal::MAX_OBJECTS.)

$ RUSTFLAGS='--cfg crossbeam_sanitize' cargo test --lib
...

---- collector::tests::incremental stdout ----
thread 'collector::tests::incremental' panicked at 'assertion failed: curr - last <= 1024', crossbeam-epoch/src/collector.rs:227:13
stack backtrace:
   0: rust_begin_unwind
             at /rustc/80184183ba0a53aa4f491753de9502acd3d6920c/library/std/src/panicking.rs:493:5
   1: core::panicking::panic_fmt
             at /rustc/80184183ba0a53aa4f491753de9502acd3d6920c/library/core/src/panicking.rs:92:14
   2: core::panicking::panic
             at /rustc/80184183ba0a53aa4f491753de9502acd3d6920c/library/core/src/panicking.rs:50:5
   3: crossbeam_epoch::collector::tests::incremental
             at ./src/collector.rs:227:13
   4: crossbeam_epoch::collector::tests::incremental::{{closure}}
             at ./src/collector.rs:204:5
   5: core::ops::function::FnOnce::call_once
             at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
   6: core::ops::function::FnOnce::call_once
             at /rustc/80184183ba0a53aa4f491753de9502acd3d6920c/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.


failures:
    collector::tests::incremental

test result: FAILED. 35 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 2.01s

error: test failed, to rerun pass '--lib'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth reporting in a separate issue. @taiki-e would you please?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. I've filed #662.

@taiki-e
Copy link
Member Author

taiki-e commented Jan 5, 2021

This should be ready for review now.

See #589 (tsan: data race in deque), #644 (tsan: data race in atomic), and #614 (asan: memory leak in skiplist) for issues reported by sanitizers.

@taiki-e
Copy link
Member Author

taiki-e commented Feb 13, 2021

@jeehoonkang Are there any changes that you'd like me to make?

Copy link
Contributor

@jeehoonkang jeehoonkang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@taiki-e LGTM. Please feel free to let bors merge it.

Comment on lines +202 to 204
#[cfg(not(crossbeam_sanitize))] // TODO: assertions failed due to `cfg(crossbeam_sanitize)` reduce `internal::MAX_OBJECTS`
#[test]
fn incremental() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth reporting in a separate issue. @taiki-e would you please?

@taiki-e
Copy link
Member Author

taiki-e commented Feb 15, 2021

bors r=jeehoonkang

@bors
Copy link
Contributor

bors bot commented Feb 15, 2021

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants