-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[Merged by Bors] - unpin nightly and disable weak memory emulation #4988
[Merged by Bors] - unpin nightly and disable weak memory emulation #4988
Conversation
|
If this is a new problem that started when updating Miri, it is probably caused by rust-lang/rust#97684: Miri got better at detecting when provenance should be lost. If this hypothesis is correct, you can add EDIT: No the regression range doesn't work out here; see my next comment. |
This looks very much like once_cell is relying on integer-to-pointer casts. Maybe it could use the strict provenance APIs instead. :) Then you can keep the In rust-lang/miri#2133 we are tracking the progress towards making raw ptr tagging the default and making it work with integer-pointer casts. |
I'm confused, it was successful in Bevy repo: https://github.com/bevyengine/bevy/runs/6844120020?check_suite_focus=true |
You have isolation disabled, so factors like how many environment variables exist can affect Miri's RNG and therefore its scheduler. Not sure if that is the problem here but it's possible. If this is the reason then running it multiple times with different seeds will lead to failure in both repos:
|
I tried running these tests locally to see if our recent improved support for permissive provenance helps, but it seems like the test |
Interestingly, this seems to be non-deterministic -- running the exact same command again sometimes terminates quickly and sometimes doesn't. Miri with I made Miri print a stacktrace every now and then, and the infinite loop seems to be somewhere inside here:
It is mostly here
and sometimes here
So, it is probably unable to leave this loop: bevy/crates/bevy_tasks/src/task_pool.rs Lines 207 to 214 in 012ae07
|
Interestingly, the loop disappears with I think this is a bug somewhere in bevy, but it might also be a bug in Miri. Anyway for now I would recommend passing |
It's still failing randomly for me, with yesterday's nightly, even with It's always failing with seed 1:
It passes with |
Trying many different seeds, I do not get a hang that way. I do get a failure in once_cell though, but that is another problem:
That failure is expected since you are passing |
Starting with the next nightly (not released yet), you can pass To summarize, there are two problems:
|
ef74578
to
3a14a5c
Compare
Co-authored-by: Ralf Jung <post@ralfj.de>
Co-authored-by: Ralf Jung <post@ralfj.de>
These are all the outdated atomic reads in an infinite loop run. diagnostics
These lines in if new_head & MARK_BIT == 0 {
crate::full_fence();
let tail = self.tail.index.load(Ordering::Relaxed); This is the same pattern as crossbeam-rs/crossbeam#838 ( It is never right to put an Acquire/Release fence before an atomic load (fence must come after the load to cause any synchronisation). Putting a SeqCst fence before an atomic load can sometimes be useful but there has to be a paired SeqCst fence or store on the same location, but this does not provide synchronisation. I don't know if this is the cause of this bevy issue (I'll expriment further tomorrow unless someone else beats me to it), but this fence is probably cc @taiki-e |
@alice-i-cecile so what needs to happen to land this? Other than linking to the issue tracking the infinite loop problem, this looks all good to me. |
Co-authored-by: Ralf Jung <post@ralfj.de>
It needs to be approved by at least 2 persons. You can be one of them 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a bevy contributor, but sure, I approve. ;)
By virtue of your assistance you are now ;) |
bors r+ |
# Objective - Follow suggestion from #4984 (comment) ## Solution - Unpin nightly, disable weak memory emulation --- This failed the miri job in my branch with the following error: ``` error: Undefined Behavior: attempting a read access using <untagged> at alloc198028[0x0], but that tag does not exist in the borrow stack for this location --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/once_cell-1.12.0/src/imp_std.rs:177:28 | 177 | let next = (*waiter).next; | ^^^^^^^^^^^^^^ | | | attempting a read access using <untagged> at alloc198028[0x0], but that tag does not exist in the borrow stack for this location | this error occurs as part of an access at alloc198028[0x0..0x8] | = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information ``` @BoxyUwU could you take a look? I guess it's related to the issue mentioned in rust-lang/miri#2223
Bevy contributor is not strictly defined as "having committed to the repo". Reviewing PRs or opening issues make you a contributor too 🙂 |
# Objective - Follow suggestion from bevyengine#4984 (comment) ## Solution - Unpin nightly, disable weak memory emulation --- This failed the miri job in my branch with the following error: ``` error: Undefined Behavior: attempting a read access using <untagged> at alloc198028[0x0], but that tag does not exist in the borrow stack for this location --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/once_cell-1.12.0/src/imp_std.rs:177:28 | 177 | let next = (*waiter).next; | ^^^^^^^^^^^^^^ | | | attempting a read access using <untagged> at alloc198028[0x0], but that tag does not exist in the borrow stack for this location | this error occurs as part of an access at alloc198028[0x0..0x8] | = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information ``` @BoxyUwU could you take a look? I guess it's related to the issue mentioned in rust-lang/miri#2223
# Objective - Follow suggestion from bevyengine#4984 (comment) ## Solution - Unpin nightly, disable weak memory emulation --- This failed the miri job in my branch with the following error: ``` error: Undefined Behavior: attempting a read access using <untagged> at alloc198028[0x0], but that tag does not exist in the borrow stack for this location --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/once_cell-1.12.0/src/imp_std.rs:177:28 | 177 | let next = (*waiter).next; | ^^^^^^^^^^^^^^ | | | attempting a read access using <untagged> at alloc198028[0x0], but that tag does not exist in the borrow stack for this location | this error occurs as part of an access at alloc198028[0x0..0x8] | = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information ``` @BoxyUwU could you take a look? I guess it's related to the issue mentioned in rust-lang/miri#2223
# Objective - Follow suggestion from bevyengine#4984 (comment) ## Solution - Unpin nightly, disable weak memory emulation --- This failed the miri job in my branch with the following error: ``` error: Undefined Behavior: attempting a read access using <untagged> at alloc198028[0x0], but that tag does not exist in the borrow stack for this location --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/once_cell-1.12.0/src/imp_std.rs:177:28 | 177 | let next = (*waiter).next; | ^^^^^^^^^^^^^^ | | | attempting a read access using <untagged> at alloc198028[0x0], but that tag does not exist in the borrow stack for this location | this error occurs as part of an access at alloc198028[0x0..0x8] | = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information ``` @BoxyUwU could you take a look? I guess it's related to the issue mentioned in rust-lang/miri#2223
Objective
Solution
This failed the miri job in my branch with the following error:
@BoxyUwU could you take a look? I guess it's related to the issue mentioned in rust-lang/miri#2223