Skip to content

Commit ed98853

Browse files
bors[bot]cuviper
andauthored
Merge #1010
1010: Fix ownership invalidation of saved scope panics r=cuviper a=cuviper Miri complained when we tried to reconstruct a panic `Box` from an `AtomicPtr`, because a later call to `mem::forget` invalidated that pointer, even though we just meant not to drop it. `ManuallyDrop` is a way to accomplish that *before* we take the pointer. ``` test scope::test::panic_propagate_nested_scope_spawn - should panic ... error: Undefined Behavior: trying to retag from <547680> for Unique permission at alloc207942[0x0], but that tag does not exist in the borrow stack for this location --> ~/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1014:9 | 1014 | Box(unsafe { Unique::new_unchecked(raw) }, alloc) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | | trying to retag from <547680> for Unique permission at alloc207942[0x0], but that tag does not exist in the borrow stack for this location | this error occurs as part of retag at alloc207942[0x0..0x10] | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows 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 help: <547680> was created by a SharedReadWrite retag at offsets [0x0..0x10] --> rayon-core/src/scope/mod.rs:732:36 | 732 | .compare_exchange(nil, &mut *err, Ordering::Release, Ordering::Relaxed) | ^^^^^^^^^ help: <547680> was later invalidated at offsets [0x0..0x10] by a Unique retag --> rayon-core/src/scope/mod.rs:735:25 | 735 | mem::forget(err); // ownership now transferred into self.panic | ^^^ ``` Co-authored-by: Josh Stone <[email protected]>
2 parents 0cc5912 + 063b406 commit ed98853

File tree

1 file changed

+15
-9
lines changed

1 file changed

+15
-9
lines changed

rayon-core/src/scope/mod.rs

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use crate::unwind;
1313
use std::any::Any;
1414
use std::fmt;
1515
use std::marker::PhantomData;
16-
use std::mem;
16+
use std::mem::ManuallyDrop;
1717
use std::ptr;
1818
use std::sync::atomic::{AtomicPtr, Ordering};
1919
use std::sync::Arc;
@@ -725,14 +725,20 @@ impl<'scope> ScopeBase<'scope> {
725725

726726
fn job_panicked(&self, err: Box<dyn Any + Send + 'static>) {
727727
// capture the first error we see, free the rest
728-
let nil = ptr::null_mut();
729-
let mut err = Box::new(err); // box up the fat ptr
730-
if self
731-
.panic
732-
.compare_exchange(nil, &mut *err, Ordering::Release, Ordering::Relaxed)
733-
.is_ok()
734-
{
735-
mem::forget(err); // ownership now transferred into self.panic
728+
if self.panic.load(Ordering::Relaxed).is_null() {
729+
let nil = ptr::null_mut();
730+
let mut err = ManuallyDrop::new(Box::new(err)); // box up the fat ptr
731+
let err_ptr: *mut Box<dyn Any + Send + 'static> = &mut **err;
732+
if self
733+
.panic
734+
.compare_exchange(nil, err_ptr, Ordering::Release, Ordering::Relaxed)
735+
.is_ok()
736+
{
737+
// ownership now transferred into self.panic
738+
} else {
739+
// another panic raced in ahead of us, so drop ours
740+
let _: Box<Box<_>> = ManuallyDrop::into_inner(err);
741+
}
736742
}
737743
}
738744

0 commit comments

Comments
 (0)