Add more test to be tested with miri in CI #6885

tiif · 2024-10-07T17:14:33Z

Motivation

Open up more test to be tested in Miri as mentioned in #6812

Solution

tiif · 2024-10-07T17:37:27Z

Currently all tests passed except for https://github.com/tokio-rs/tokio/actions/runs/11220730350/job/31189595252?pr=6885, seems like a toolchain installation problem?

Darksonn · 2024-10-07T18:20:05Z

I'll try rerunning that particular check.

tiif · 2024-10-08T02:38:14Z

Yup cargo miri test --features full --lib --tests works. So I added it in ci.yml in the latest commit.

tiif · 2024-10-08T02:39:17Z

.github/workflows/ci.yml

        working-directory: tokio
        env:
-          MIRIFLAGS: -Zmiri-disable-isolation -Zmiri-strict-provenance -Zmiri-retag-fields
+          MIRIFLAGS: -Zmiri-disable-isolation -Zmiri-strict-provenance -Zmiri-retag-fields -Zmiri-tree-borrows


I also added tree-borrows check because I ran the test suite with it, if this is not desired I can remove them.

That is fine. We may want to consider whether it makes sense to run with both?

If both means tree borrows and stacked borrows, I think -Zmiri-tree-borrows will replace stacked borrows with tree borrows rule. On second thought, since tree borrows seems to be more experimental, let me remove this and keep it to stacked borrow.

tiif · 2024-10-08T02:52:12Z

The CI logs said error: unsupported operation: returning ready events from epoll_wait is not yet implemented. This unsupported error has already been removed, is the CI using older version of miri?

EDIT: Yup I saw rustup toolchain install nightly-2024-05-05 --component miri --profile minimal --no-self-update. Is it possible to update it to at least 2024-09-19?

rustc 1.83.0-nightly (506f22b46 2024-09-19)
binary: rustc
commit-hash: 506f22b4663f3e756e1e6a4f66c6309fdc00819c
commit-date: 2024-09-19
host: x86_64-unknown-linux-gnu
release: 1.83.0-nightly
LLVM version: 19.1.0

Darksonn · 2024-10-08T06:04:59Z

Sure, feel free to change the miri version.

tiif · 2024-10-10T01:23:58Z

.github/workflows/ci.yml

+          rustup toolchain install nightly-2024-09-19 --component miri --profile minimal --no-self-update
+          rustup override set nightly
+          cargo miri test --features full --lib --tests --no-fail-fast


I am trying to keep the change minimal, so I don't really want to change env.rust_nightly, but I am not sure if this will work.

EDIT: This won't work

We'll probably want to get them back in sync as a follow-up PR, then.

Do you mean rust_miri_nightly and rust_nightly?

tiif · 2024-10-10T02:16:00Z

As expected, with the amount of new tests tested with miri, it slows down the CI significantly. It goes from 1 minute to 37 minutes.

Darksonn

Overal looks good. It would be nice to know why tests are disabled - I see you commented the reason on only a few of them.

Darksonn · 2024-10-10T09:03:25Z

tokio/tests/rt_handle_block_on.rs

This both disables miri for the entire file and for specific tests.

Right, thanks for catching this.

Darksonn · 2024-10-10T09:04:01Z

tokio-util/tests/spawn_pinned.rs

+// Blocked on https://github.com/rust-lang/miri/issues/3911
+#![cfg(not(miri))]


Was this fixed?

Yes, but I don't think the latest fix is synced to rustc yet.

Darksonn · 2024-10-10T09:04:56Z

.github/workflows/ci.yml

        working-directory: tokio
        env:
          MIRIFLAGS: -Zmiri-disable-isolation -Zmiri-strict-provenance -Zmiri-retag-fields
-


Deleting this line seems like a spurious change.

tiif · 2024-10-10T15:07:20Z

Overal looks good. It would be nice to know why tests are disabled - I see you commented the reason on only a few of them.

I checked the error messages, most of them are either too slow, or having unsupported error. I also filed issues for other errors. If you don't mind, I can annotate them in future PRs. I planned to collect and count all the unsupported error, so I can do this together too.

Besides, I am thinking of bumping the miri nightly version again in future to include all the fixes. So it might be better to keep rust_miri_nightly and rust_nightly separated as there will be more frequent version bump needed for miri?

tiif · 2024-10-10T16:33:57Z

.github/workflows/ci.yml

+      - name: Install cargo-nextest
+        uses: taiki-e/install-action@v2
+        with:
+          tool: cargo-nextest


nextest decreased miri running time roughly from 37 minutes to 21 minutes.

That's nice. The loom jobs are already much longer than that. We could consider gating it similarly to what we did for loom tests.

Darksonn

This seems reasonable. We can merge it as-is if you think it is ready. We can always continue to improve it in follow-up PRs, e.g. for bumping miri versions and such.

tiif · 2024-10-11T02:44:08Z

Yup, this PR is ready. I added the tasks that should be done in future PR in #6812, so we might want to keep the issue open until they are resolved. Thanks for the review and mentoring!

tiif added 2 commits October 8, 2024 01:07

Ignore miri on test that failed

91b3fa7

rustfmt

c75732b

tiif changed the title ~~Add miri to CI~~ Add more test to be tested with miri to CI Oct 7, 2024

tiif changed the title ~~Add more test to be tested with miri to CI~~ Add more test to be tested with miri in CI Oct 7, 2024

Darksonn added the A-ci Area: The continuous integration setup label Oct 7, 2024

Change the run command iin ci.yml

842787d

tiif commented Oct 8, 2024

View reviewed changes

tiif added 3 commits October 10, 2024 09:15

Remove tree borrows

9324ca2

Try to configure the date

1731493

Fix typo

5565479

tiif commented Oct 10, 2024

View reviewed changes

Try to pin a specific miri version

879da01

tiif added 3 commits October 10, 2024 10:18

Ignore time pause

40c3c26

Fix rustfmt

146134d

Merge branch 'master' into ignoremiri

dcdbadb

tiif marked this pull request as ready for review October 10, 2024 03:43

Darksonn reviewed Oct 10, 2024

View reviewed changes

Fix spurious deleted line

4a12abc

tiif added 4 commits October 10, 2024 23:09

Ignore miri for entire file

f1c36c9

Add one more too slow

936edf7

Disable a few more slow test

92fcd41

Try to speed up the test using nextest

4f37518

tiif commented Oct 10, 2024

View reviewed changes

Darksonn approved these changes Oct 11, 2024

View reviewed changes

Darksonn merged commit 161b8c8 into tokio-rs:master Oct 11, 2024
81 checks passed

tiif deleted the ignoremiri branch October 11, 2024 07:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add more test to be tested with miri in CI #6885

Add more test to be tested with miri in CI #6885

tiif commented Oct 7, 2024 •

edited

Loading

tiif commented Oct 7, 2024

Darksonn commented Oct 7, 2024

tiif commented Oct 8, 2024 •

edited

Loading

tiif Oct 8, 2024

Darksonn Oct 8, 2024

tiif Oct 10, 2024 •

edited

Loading

tiif commented Oct 8, 2024 •

edited

Loading

Darksonn commented Oct 8, 2024

tiif Oct 10, 2024 •

edited

Loading

Darksonn Oct 10, 2024

tiif Oct 10, 2024

tiif commented Oct 10, 2024

Darksonn left a comment

Darksonn Oct 10, 2024

tiif Oct 10, 2024

Darksonn Oct 10, 2024

tiif Oct 10, 2024

Darksonn Oct 10, 2024

tiif commented Oct 10, 2024 •

edited

Loading

tiif Oct 10, 2024 •

edited

Loading

Darksonn Oct 11, 2024

Darksonn left a comment

tiif commented Oct 11, 2024 •

edited

Loading

		// Blocked on https://github.com/rust-lang/miri/issues/3911
		#![cfg(not(miri))]

Add more test to be tested with miri in CI #6885

Add more test to be tested with miri in CI #6885

Conversation

tiif commented Oct 7, 2024 • edited Loading

Motivation

Solution

tiif commented Oct 7, 2024

Darksonn commented Oct 7, 2024

tiif commented Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiif Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

tiif commented Oct 8, 2024 • edited Loading

Darksonn commented Oct 8, 2024

tiif Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiif commented Oct 10, 2024

Darksonn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiif commented Oct 10, 2024 • edited Loading

tiif Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Darksonn left a comment

Choose a reason for hiding this comment

tiif commented Oct 11, 2024 • edited Loading

tiif commented Oct 7, 2024 •

edited

Loading

tiif commented Oct 8, 2024 •

edited

Loading

tiif Oct 10, 2024 •

edited

Loading

tiif commented Oct 8, 2024 •

edited

Loading

tiif Oct 10, 2024 •

edited

Loading

tiif commented Oct 10, 2024 •

edited

Loading

tiif Oct 10, 2024 •

edited

Loading

tiif commented Oct 11, 2024 •

edited

Loading