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 Miri on CI #11

Merged
merged 1 commit into from
Mar 18, 2020
Merged

run Miri on CI #11

merged 1 commit into from
Mar 18, 2020

Conversation

RalfJung
Copy link
Contributor

After discussions like #7, I figured running Miri to check the test suite for UB might be a good idea. However, I am not sure how extensive the test suite is so far. Miri can only find UB if the test suite actually hits the problematic code paths (think of it like ubsan or other sanitizers).

It seems like cargo test is not even run on CI?

@maciejhirsz
Copy link
Owner

Cheers! Tests are being run for stable, beta, and nightly.

There are two additional builds that don't run tests:

  • Making sure const_fn feature compiles on nightly (this can probably be a full test).
  • Check if no_std compiles for thumbv6m-none-eabi.

Adding more tests, in particular longer tests that do allocations and deallocations multiple times, is definitely on the agenda.

@maciejhirsz maciejhirsz merged commit 94cbbde into maciejhirsz:master Mar 18, 2020
@RalfJung
Copy link
Contributor Author

RalfJung commented Mar 18, 2020

Oh d'oh, there's a default script that Travis uses when you give no script: yourself...

I was looking for the cargo test and didn't see it.^^ Sorry.

@RalfJung RalfJung deleted the miri branch March 18, 2020 13:32
cargo miri setup

# FIXME: investigate memory leaks
cargo miri test --all-features -- -Zmiri-ignore-leaks
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't do any further investigation for why Miri sees a memory leak.
This might or might not be rust-lang/miri#940.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One of the leaked allocations is allocated here:

note: inside call to `std::string::String::push_str` at src/lean.rs:87:13
   --> src/lean.rs:87:13
    |
87  |             owned.push_str("Hello?.. ");
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside call to `lean::tests::stress_test_owned` at src/lean.rs:75:5
   --> src/lean.rs:75:5
    |
75  | /     fn stress_test_owned() {
76  | |         let mut expected = String::from("Hello... ");
77  | |         let mut cow: Cow<str> = Cow::borrowed("Hello... ");
78  | |
...   |
92  | |         assert_eq!(expected, cow.into_owned());
93  | |     }
    | |_____^

Doesn't look like there is anything being put in a static here, so this is likely a genuine leak?

@RalfJung RalfJung mentioned this pull request Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants