From f38dba69b18a8ce593272ea3c2369c4274d8837b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 23 Apr 2024 08:42:52 +0200 Subject: [PATCH 1/3] weak memory outdated loads: show where the load was from --- src/tools/miri/src/concurrency/weak_memory.rs | 4 +++- src/tools/miri/src/diagnostics.rs | 11 +++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/tools/miri/src/concurrency/weak_memory.rs b/src/tools/miri/src/concurrency/weak_memory.rs index f544393cfe658..574962c48d43e 100644 --- a/src/tools/miri/src/concurrency/weak_memory.rs +++ b/src/tools/miri/src/concurrency/weak_memory.rs @@ -520,7 +520,9 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>: validate, )?; if global.track_outdated_loads && recency == LoadRecency::Outdated { - this.emit_diagnostic(NonHaltingDiagnostic::WeakMemoryOutdatedLoad); + this.emit_diagnostic(NonHaltingDiagnostic::WeakMemoryOutdatedLoad { + ptr: place.ptr(), + }); } return Ok(loaded); diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index 0c0ac4c6036d9..9fa786332e3cd 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -125,7 +125,9 @@ pub enum NonHaltingDiagnostic { Int2Ptr { details: bool, }, - WeakMemoryOutdatedLoad, + WeakMemoryOutdatedLoad { + ptr: Pointer>, + }, } /// Level of Miri specific diagnostics @@ -583,7 +585,8 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { | AccessedAlloc(..) | FreedAlloc(..) | ProgressReport { .. } - | WeakMemoryOutdatedLoad => ("tracking was triggered".to_string(), DiagLevel::Note), + | WeakMemoryOutdatedLoad { .. } => + ("tracking was triggered".to_string(), DiagLevel::Note), }; let msg = match &e { @@ -610,8 +613,8 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { ProgressReport { .. } => format!("progress report: current operation being executed is here"), Int2Ptr { .. } => format!("integer-to-pointer cast"), - WeakMemoryOutdatedLoad => - format!("weak memory emulation: outdated value returned from load"), + WeakMemoryOutdatedLoad { ptr } => + format!("weak memory emulation: outdated value returned from load at {ptr}"), }; let notes = match &e { From ea9cff254f2e363a30fcd6f887347f05a76a0f70 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 23 Apr 2024 08:43:06 +0200 Subject: [PATCH 2/3] add a test for the TLS memory leak --- src/tools/miri/tests/many-seeds/tls-leak.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 src/tools/miri/tests/many-seeds/tls-leak.rs diff --git a/src/tools/miri/tests/many-seeds/tls-leak.rs b/src/tools/miri/tests/many-seeds/tls-leak.rs new file mode 100644 index 0000000000000..70a506958d1e3 --- /dev/null +++ b/src/tools/miri/tests/many-seeds/tls-leak.rs @@ -0,0 +1,13 @@ +//! Regression test for . +use std::thread; + +pub(crate) fn with_thread_local() { + thread_local! { static X: Box = Box::new(0); } + X.with(|_x| {}) +} + +fn main() { + let j2 = thread::spawn(with_thread_local); + with_thread_local(); + j2.join().unwrap(); +} From 247e82cb8391765cef976d0a23d7bdc3e509a978 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 23 Apr 2024 08:44:49 +0200 Subject: [PATCH 3/3] run many-seeds tests at least a few times on all tier 1 targets --- src/tools/miri/ci/ci.sh | 18 +++++++++++------- src/tools/miri/tests/many-seeds/tls-leak.rs | 21 +++++++++++++++++---- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index f8ba612750ee9..ad0c795315e39 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -128,16 +128,18 @@ function run_tests_minimal { ## Main Testing Logic ## # In particular, fully cover all tier 1 targets. +# We also want to run the many-seeds tests on all tier 1 targets. case $HOST_TARGET in x86_64-unknown-linux-gnu) # Host GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests # Extra tier 1 - MIRI_TEST_TARGET=i686-unknown-linux-gnu run_tests - MIRI_TEST_TARGET=aarch64-unknown-linux-gnu run_tests - MIRI_TEST_TARGET=x86_64-apple-darwin run_tests - MIRI_TEST_TARGET=i686-pc-windows-gnu run_tests - MIRI_TEST_TARGET=x86_64-pc-windows-gnu run_tests + # With reduced many-seed count to avoid spending too much time on that. + # (All OSes are run with 64 seeds at least once though via the macOS runner.) + MANY_SEEDS=16 MIRI_TEST_TARGET=i686-unknown-linux-gnu run_tests + MANY_SEEDS=16 MIRI_TEST_TARGET=aarch64-unknown-linux-gnu run_tests + MANY_SEEDS=16 MIRI_TEST_TARGET=x86_64-apple-darwin run_tests + MANY_SEEDS=16 MIRI_TEST_TARGET=x86_64-pc-windows-gnu run_tests # Extra tier 2 MIRI_TEST_TARGET=aarch64-apple-darwin run_tests MIRI_TEST_TARGET=arm-unknown-linux-gnueabi run_tests @@ -155,13 +157,15 @@ case $HOST_TARGET in # Host (tier 2) GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests # Extra tier 1 - MIRI_TEST_TARGET=x86_64-pc-windows-msvc CARGO_MIRI_ENV=1 run_tests + MANY_SEEDS=64 MIRI_TEST_TARGET=i686-pc-windows-gnu run_tests + MANY_SEEDS=64 MIRI_TEST_TARGET=x86_64-pc-windows-msvc CARGO_MIRI_ENV=1 run_tests # Extra tier 2 MIRI_TEST_TARGET=s390x-unknown-linux-gnu run_tests # big-endian architecture ;; i686-pc-windows-msvc) # Host - # Only smoke-test `many-seeds`; 64 runs take 15min here! + # Only smoke-test `many-seeds`; 64 runs of just the scoped-thread-leak test take 15min here! + # See . GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=1 TEST_BENCH=1 run_tests # Extra tier 1 # We really want to ensure a Linux target works on a Windows host, diff --git a/src/tools/miri/tests/many-seeds/tls-leak.rs b/src/tools/miri/tests/many-seeds/tls-leak.rs index 70a506958d1e3..3b2436334395f 100644 --- a/src/tools/miri/tests/many-seeds/tls-leak.rs +++ b/src/tools/miri/tests/many-seeds/tls-leak.rs @@ -1,13 +1,26 @@ //! Regression test for . use std::thread; -pub(crate) fn with_thread_local() { +fn with_thread_local1() { thread_local! { static X: Box = Box::new(0); } X.with(|_x| {}) } +fn with_thread_local2() { + thread_local! { static Y: Box = Box::new(0); } + Y.with(|_y| {}) +} + fn main() { - let j2 = thread::spawn(with_thread_local); - with_thread_local(); - j2.join().unwrap(); + // Here we have two threads racing on initializing the thread-local and adding it to the global + // dtor list (on targets that have such a list, i.e., targets without target_thread_local). + let t = thread::spawn(with_thread_local1); + with_thread_local1(); + t.join().unwrap(); + + // Here we have one thread running the destructors racing with another thread initializing a + // thread-local. The second thread adds a destructor that could be picked up by the first. + let t = thread::spawn(|| { /* immediately just run destructors */ }); + with_thread_local2(); // initialize thread-local + t.join().unwrap(); }