Skip to content

Commit

Permalink
Test UB to illustrate limitation of TokenCell
Browse files Browse the repository at this point in the history
  • Loading branch information
ryoqun committed Apr 4, 2024
1 parent 50ddb5c commit b851631
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 1 deletion.
4 changes: 4 additions & 0 deletions ci/test-miri.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,7 @@ source ci/rust-version.sh nightly
# miri is very slow; so only run very few of selective tests!
cargo "+${rust_nightly}" miri test -p solana-program -- hash:: account_info::
cargo "+${rust_nightly}" miri test -p solana-unified-scheduler-logic

# run intentionally-#[ignored] ub triggering tests for each to make sure they fail
! cargo "+${rust_nightly}" miri test -p solana-unified-scheduler-logic -- \
--ignored --exact "utils::tests::test_ub_illegally_shared_token_cell"
64 changes: 63 additions & 1 deletion unified-scheduler-logic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,10 @@ mod utils {

#[cfg(test)]
mod tests {
use super::Token;
use {
super::{Token, TokenCell},
std::{sync::Arc, thread},
};

#[test]
#[should_panic(
Expand All @@ -299,6 +302,65 @@ mod utils {
let _ = Token::<usize>::assume_exclusive_mutating_thread();
}
}

// As documented above, it's illegal to share (= co-own) the same instance of TokenCell
// across threads. Unfortunately, we can't prevent this from happening with some
// type-safety magic to cause compile errors... So sanity-check here test fails due to a
// runtime error of the known UB, when run under miri.
#[test]
// cause (harmless) UB unless running under miri by conditionally #[ignore]-ing, confirming
// false-positive result to coversely show the merit of miri!
#[cfg_attr(miri, ignore)]
fn test_ub_illegally_shared_token_cell() {
#[derive(Debug)]
struct FakeQueue {
v: Vec<u8>,
}

let queue1 = Arc::new(TokenCell::new(FakeQueue {
v: Vec::with_capacity(20),
}));
let queue2 = queue1.clone();
#[cfg(not(miri))]
let queue3 = queue1.clone();

// Usually miri immediately detects the data race; but just repeat enough time to avoid
// being flaky
for _ in 0..10 {
let (queue1, queue2) = (queue1.clone(), queue2.clone());
let thread1 = thread::spawn(move || {
let mut token =
unsafe { Token::<FakeQueue>::assume_exclusive_mutating_thread() };
queue1.with_borrow_mut(&mut token, |queue| {
// this is UB
queue.v.push(3);
});
});
// immediately spawn next thread without joining thread1 to ensure there's a dace race
// definitely. Otherwise, joining here wouldn't cause UB.
let thread2 = thread::spawn(move || {
let mut token =
unsafe { Token::<FakeQueue>::assume_exclusive_mutating_thread() };
queue2.with_borrow_mut(&mut token, |queue| {
// this is UB
queue.v.push(4);
});
});

thread1.join().unwrap();
thread2.join().unwrap();
}

// it's in ub already, so we can't assert reliably, so dbg!(...) just for fun
#[cfg(not(miri))]
{
drop((queue1, queue2));
dbg!(Arc::into_inner(queue3).unwrap().0.into_inner());
}

// return successfully to indicate an unexpected outcome, because this test should
// abort by now
}
}
}

Expand Down

0 comments on commit b851631

Please sign in to comment.