From 84ed87a8414959ae3715532d70edb3eb5a9fbdda Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 3 Apr 2025 17:20:45 +0200 Subject: [PATCH 1/7] Fix 2024 edition doctest panic output (cherry picked from commit a91e97c06c778b84663cfe4f2871b868d275a137) --- src/librustdoc/doctest/runner.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/doctest/runner.rs b/src/librustdoc/doctest/runner.rs index f891505d2a609..09a3224a3c3fe 100644 --- a/src/librustdoc/doctest/runner.rs +++ b/src/librustdoc/doctest/runner.rs @@ -113,6 +113,7 @@ impl DocTestRunner { mod __doctest_mod {{ use std::sync::OnceLock; use std::path::PathBuf; + use std::process::ExitCode; pub static BINARY_PATH: OnceLock = OnceLock::new(); pub const RUN_OPTION: &str = \"RUSTDOC_DOCTEST_RUN_NB_TEST\"; @@ -123,16 +124,17 @@ mod __doctest_mod {{ }} #[allow(unused)] - pub fn doctest_runner(bin: &std::path::Path, test_nb: usize) -> Result<(), String> {{ + pub fn doctest_runner(bin: &std::path::Path, test_nb: usize) -> ExitCode {{ let out = std::process::Command::new(bin) .env(self::RUN_OPTION, test_nb.to_string()) .args(std::env::args().skip(1).collect::>()) .output() .expect(\"failed to run command\"); if !out.status.success() {{ - Err(String::from_utf8_lossy(&out.stderr).to_string()) + eprintln!(\"{{}}\", String::from_utf8_lossy(&out.stderr)); + ExitCode::FAILURE }} else {{ - Ok(()) + ExitCode::SUCCESS }} }} }} From 3d4b571c3d7509e4140f0a4d1a784313ebe97387 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 3 Apr 2025 17:21:03 +0200 Subject: [PATCH 2/7] Add regression test for #137970 (cherry picked from commit fff2484700bb7d1fa320a666109d1868767dfa6a) --- .../edition-2024-doctest-error-output.rs | 12 +++++++ .../edition-2024-doctest-error-output.stdout | 32 +++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 tests/rustdoc-ui/edition-2024-doctest-error-output.rs create mode 100644 tests/rustdoc-ui/edition-2024-doctest-error-output.stdout diff --git a/tests/rustdoc-ui/edition-2024-doctest-error-output.rs b/tests/rustdoc-ui/edition-2024-doctest-error-output.rs new file mode 100644 index 0000000000000..cabe5d1ae6eae --- /dev/null +++ b/tests/rustdoc-ui/edition-2024-doctest-error-output.rs @@ -0,0 +1,12 @@ +// This is a regression test for . +// The output must look nice and not like a `Debug` display of a `String`. + +//@ edition: 2024 +//@ compile-flags: --test +//@ normalize-stdout: "tests/rustdoc-ui/doctest" -> "$$DIR" +//@ normalize-stdout: "finished in \d+\.\d+s" -> "finished in $$TIME" +//@ failure-status: 101 + +//! ```rust +//! assert_eq!(2 + 2, 5); +//! ``` diff --git a/tests/rustdoc-ui/edition-2024-doctest-error-output.stdout b/tests/rustdoc-ui/edition-2024-doctest-error-output.stdout new file mode 100644 index 0000000000000..5c4de44c21ab3 --- /dev/null +++ b/tests/rustdoc-ui/edition-2024-doctest-error-output.stdout @@ -0,0 +1,32 @@ + +running 1 test +test tests/rustdoc-ui/edition-2024-doctest-error-output.rs - (line 10) ... FAILED + +failures: + +---- tests/rustdoc-ui/edition-2024-doctest-error-output.rs - (line 10) stdout ---- + +thread 'main' panicked at /tmp/rustdoctestSZq9aS/doctest_bundle_2024.rs:6:1: +assertion `left == right` failed + left: 4 + right: 5 +stack backtrace: + 0: __rustc::rust_begin_unwind + 1: core::panicking::panic_fmt + 2: core::panicking::assert_failed_inner + 3: core::panicking::assert_failed + 4: doctest_bundle_2024::__doctest_0::main + 5: doctest_bundle_2024::__doctest_0::__main_fn + 6: doctest_runner_2024::__doctest_0::TEST::{{closure}} + 7: core::ops::function::FnOnce::call_once + 8: doctest_runner_2024::main + 9: core::ops::function::FnOnce::call_once +note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. + + + +failures: + tests/rustdoc-ui/edition-2024-doctest-error-output.rs - (line 10) + +test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME + From 10e682e67a466e7db0527aa557c50376aae69b3f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 3 Apr 2025 17:58:59 +0200 Subject: [PATCH 3/7] Use `eprint!` instead of `eprintln!` (cherry picked from commit f9927ee042f7d7418fbb64082272ce1feff4d895) --- src/librustdoc/doctest/runner.rs | 2 +- .../edition-2024-error-output.rs} | 2 ++ .../doctest/edition-2024-error-output.stdout | 20 ++++++++++++ .../edition-2024-doctest-error-output.stdout | 32 ------------------- 4 files changed, 23 insertions(+), 33 deletions(-) rename tests/rustdoc-ui/{edition-2024-doctest-error-output.rs => doctest/edition-2024-error-output.rs} (80%) create mode 100644 tests/rustdoc-ui/doctest/edition-2024-error-output.stdout delete mode 100644 tests/rustdoc-ui/edition-2024-doctest-error-output.stdout diff --git a/src/librustdoc/doctest/runner.rs b/src/librustdoc/doctest/runner.rs index 09a3224a3c3fe..5fd577e353155 100644 --- a/src/librustdoc/doctest/runner.rs +++ b/src/librustdoc/doctest/runner.rs @@ -131,7 +131,7 @@ mod __doctest_mod {{ .output() .expect(\"failed to run command\"); if !out.status.success() {{ - eprintln!(\"{{}}\", String::from_utf8_lossy(&out.stderr)); + eprint!(\"{{}}\", String::from_utf8_lossy(&out.stderr)); ExitCode::FAILURE }} else {{ ExitCode::SUCCESS diff --git a/tests/rustdoc-ui/edition-2024-doctest-error-output.rs b/tests/rustdoc-ui/doctest/edition-2024-error-output.rs similarity index 80% rename from tests/rustdoc-ui/edition-2024-doctest-error-output.rs rename to tests/rustdoc-ui/doctest/edition-2024-error-output.rs index cabe5d1ae6eae..82a85debcd191 100644 --- a/tests/rustdoc-ui/edition-2024-doctest-error-output.rs +++ b/tests/rustdoc-ui/doctest/edition-2024-error-output.rs @@ -4,7 +4,9 @@ //@ edition: 2024 //@ compile-flags: --test //@ normalize-stdout: "tests/rustdoc-ui/doctest" -> "$$DIR" +//@ normalize-stdout: "panicked at .+rs:" -> "panicked at $$TMP:" //@ normalize-stdout: "finished in \d+\.\d+s" -> "finished in $$TIME" +//@ rustc-env:RUST_BACKTRACE=0 //@ failure-status: 101 //! ```rust diff --git a/tests/rustdoc-ui/doctest/edition-2024-error-output.stdout b/tests/rustdoc-ui/doctest/edition-2024-error-output.stdout new file mode 100644 index 0000000000000..8f056a5f703ed --- /dev/null +++ b/tests/rustdoc-ui/doctest/edition-2024-error-output.stdout @@ -0,0 +1,20 @@ + +running 1 test +test $DIR/edition-2024-error-output.rs - (line 12) ... FAILED + +failures: + +---- $DIR/edition-2024-error-output.rs - (line 12) stdout ---- + +thread 'main' panicked at $TMP:6:1: +assertion `left == right` failed + left: 4 + right: 5 +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + + +failures: + $DIR/edition-2024-error-output.rs - (line 12) + +test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME + diff --git a/tests/rustdoc-ui/edition-2024-doctest-error-output.stdout b/tests/rustdoc-ui/edition-2024-doctest-error-output.stdout deleted file mode 100644 index 5c4de44c21ab3..0000000000000 --- a/tests/rustdoc-ui/edition-2024-doctest-error-output.stdout +++ /dev/null @@ -1,32 +0,0 @@ - -running 1 test -test tests/rustdoc-ui/edition-2024-doctest-error-output.rs - (line 10) ... FAILED - -failures: - ----- tests/rustdoc-ui/edition-2024-doctest-error-output.rs - (line 10) stdout ---- - -thread 'main' panicked at /tmp/rustdoctestSZq9aS/doctest_bundle_2024.rs:6:1: -assertion `left == right` failed - left: 4 - right: 5 -stack backtrace: - 0: __rustc::rust_begin_unwind - 1: core::panicking::panic_fmt - 2: core::panicking::assert_failed_inner - 3: core::panicking::assert_failed - 4: doctest_bundle_2024::__doctest_0::main - 5: doctest_bundle_2024::__doctest_0::__main_fn - 6: doctest_runner_2024::__doctest_0::TEST::{{closure}} - 7: core::ops::function::FnOnce::call_once - 8: doctest_runner_2024::main - 9: core::ops::function::FnOnce::call_once -note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. - - - -failures: - tests/rustdoc-ui/edition-2024-doctest-error-output.rs - (line 10) - -test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME - From d1cd5ee9f64147e89ecf07b970d8bfce2a202a91 Mon Sep 17 00:00:00 2001 From: mejrs <59372212+mejrs@users.noreply.github.com> Date: Fri, 4 Apr 2025 21:43:03 +0200 Subject: [PATCH 4/7] make `Arguments::as_statically_known_str` doc(hidden) (cherry picked from commit cfcc47ea540750dac30010edb42c4fa21efd577a) --- library/core/src/fmt/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 30fd2d7815f51..0c8e1495c62d4 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -743,6 +743,7 @@ impl<'a> Arguments<'a> { #[unstable(feature = "fmt_internals", reason = "internal to standard library", issue = "none")] #[must_use] #[inline] + #[doc(hidden)] pub fn as_statically_known_str(&self) -> Option<&'static str> { let s = self.as_str(); if core::intrinsics::is_val_statically_known(s.is_some()) { s } else { None } From 4782bbc5680a2e3004bd5389c73d5fb66b665ded Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 15 Apr 2025 21:00:11 +0300 Subject: [PATCH 5/7] Revert "Deduplicate template parameter creation" This reverts commit 6adc2c1fd6ecde7bf83c8b8fbc71f402ced87054. (cherry picked from commit 38f7060a73acd5ec6ed7d4820dccbf2aa584fc68) --- .../src/debuginfo/metadata.rs | 36 +++++++------------ .../src/debuginfo/metadata/enums/mod.rs | 1 - .../rustc_codegen_llvm/src/debuginfo/mod.rs | 34 ++++++++++++++++-- 3 files changed, 45 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index 2eaaf127e41ea..97a3b62046635 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -1314,31 +1314,21 @@ fn build_generic_type_param_di_nodes<'ll, 'tcx>( ty: Ty<'tcx>, ) -> SmallVec> { if let ty::Adt(def, args) = *ty.kind() { - let generics = cx.tcx.generics_of(def.did()); - return get_template_parameters(cx, generics, args); - } - - return smallvec![]; -} - -pub(super) fn get_template_parameters<'ll, 'tcx>( - cx: &CodegenCx<'ll, 'tcx>, - generics: &ty::Generics, - args: ty::GenericArgsRef<'tcx>, -) -> SmallVec> { - if args.types().next().is_some() { - let names = get_parameter_names(cx, generics); - let template_params: SmallVec<_> = iter::zip(args, names) - .filter_map(|(kind, name)| { - kind.as_type().map(|ty| { - let actual_type = cx.tcx.normalize_erasing_regions(cx.typing_env(), ty); - let actual_type_di_node = type_di_node(cx, actual_type); - Some(cx.create_template_type_parameter(name.as_str(), actual_type_di_node)) + if args.types().next().is_some() { + let generics = cx.tcx.generics_of(def.did()); + let names = get_parameter_names(cx, generics); + let template_params: SmallVec<_> = iter::zip(args, names) + .filter_map(|(kind, name)| { + kind.as_type().map(|ty| { + let actual_type = cx.tcx.normalize_erasing_regions(cx.typing_env(), ty); + let actual_type_di_node = type_di_node(cx, actual_type); + Some(cx.create_template_type_parameter(name.as_str(), actual_type_di_node)) + }) }) - }) - .collect(); + .collect(); - return template_params; + return template_params; + } } return smallvec![]; diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/mod.rs index 6792c307fdc45..7c701926d2c5e 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/mod.rs @@ -363,7 +363,6 @@ fn build_coroutine_variant_struct_type_di_node<'ll, 'tcx>( state_specific_fields.into_iter().chain(common_fields).collect() }, - // FIXME: this is a no-op. `build_generic_type_param_di_nodes` only works for Adts. |cx| build_generic_type_param_di_nodes(cx, coroutine_type_and_layout.ty), ) .di_node diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index ae7d080db66f7..0f94a1dbb0d59 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -2,8 +2,8 @@ use std::cell::{OnceCell, RefCell}; use std::ops::Range; -use std::ptr; use std::sync::Arc; +use std::{iter, ptr}; use libc::c_uint; use metadata::create_subroutine_type; @@ -486,10 +486,40 @@ impl<'ll, 'tcx> DebugInfoCodegenMethods<'tcx> for CodegenCx<'ll, 'tcx> { generics: &ty::Generics, args: GenericArgsRef<'tcx>, ) -> &'ll DIArray { - let template_params = metadata::get_template_parameters(cx, generics, args); + if args.types().next().is_none() { + return create_DIArray(DIB(cx), &[]); + } + + // Again, only create type information if full debuginfo is enabled + let template_params: Vec<_> = if cx.sess().opts.debuginfo == DebugInfo::Full { + let names = get_parameter_names(cx, generics); + iter::zip(args, names) + .filter_map(|(kind, name)| { + kind.as_type().map(|ty| { + let actual_type = cx.tcx.normalize_erasing_regions(cx.typing_env(), ty); + let actual_type_metadata = type_di_node(cx, actual_type); + Some(cx.create_template_type_parameter( + name.as_str(), + actual_type_metadata, + )) + }) + }) + .collect() + } else { + vec![] + }; + create_DIArray(DIB(cx), &template_params) } + fn get_parameter_names(cx: &CodegenCx<'_, '_>, generics: &ty::Generics) -> Vec { + let mut names = generics.parent.map_or_else(Vec::new, |def_id| { + get_parameter_names(cx, cx.tcx.generics_of(def_id)) + }); + names.extend(generics.own_params.iter().map(|param| param.name)); + names + } + /// Returns a scope, plus `true` if that's a type scope for "class" methods, /// otherwise `false` for plain namespace scopes. fn get_containing_scope<'ll, 'tcx>( From e71ad4a2c9da7100c092e095c80b6436d3193740 Mon Sep 17 00:00:00 2001 From: Petros Angelatos Date: Thu, 10 Apr 2025 11:09:31 +0300 Subject: [PATCH 6/7] sync::mpsc: add miri reproducer of double free Signed-off-by: Petros Angelatos (cherry picked from commit 9eb6a5446a4e35f48ad22a5b70a74a8badb9fa0d) --- library/std/src/sync/mpmc/list.rs | 5 +++ .../miri/tests/pass/issues/issue-139553.rs | 45 +++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 src/tools/miri/tests/pass/issues/issue-139553.rs diff --git a/library/std/src/sync/mpmc/list.rs b/library/std/src/sync/mpmc/list.rs index d88914f529142..2dd8f41226d63 100644 --- a/library/std/src/sync/mpmc/list.rs +++ b/library/std/src/sync/mpmc/list.rs @@ -213,6 +213,11 @@ impl Channel { .compare_exchange(block, new, Ordering::Release, Ordering::Relaxed) .is_ok() { + // This yield point leaves the channel in a half-initialized state where the + // tail.block pointer is set but the head.block is not. This is used to + // facilitate the test in src/tools/miri/tests/pass/issues/issue-139553.rs + #[cfg(miri)] + crate::thread::yield_now(); self.head.block.store(new, Ordering::Release); block = new; } else { diff --git a/src/tools/miri/tests/pass/issues/issue-139553.rs b/src/tools/miri/tests/pass/issues/issue-139553.rs new file mode 100644 index 0000000000000..97cb9e1a8057c --- /dev/null +++ b/src/tools/miri/tests/pass/issues/issue-139553.rs @@ -0,0 +1,45 @@ +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-compare-exchange-weak-failure-rate=0 +use std::sync::mpsc::channel; +use std::thread; + +/// This test aims to trigger a race condition that causes a double free in the unbounded channel +/// implementation. The test relies on a particular thread scheduling to happen as annotated by the +/// comments below. +fn main() { + let (s1, r) = channel::(); + let s2 = s1.clone(); + + let t1 = thread::spawn(move || { + // 1. The first action executed is an attempt to send the first value in the channel. This + // will begin to initialize the channel but will stop at a critical momement as + // indicated by the `yield_now()` call in the `start_send` method of the implementation. + let _ = s1.send(42); + // 4. The sender is re-scheduled and it finishes the initialization of the channel by + // setting head.block to the same value as tail.block. It then proceeds to publish its + // value but observes that the channel has already disconnected (due to the concurrent + // call of `discard_all_messages`) and aborts the send. + }); + std::thread::yield_now(); + + // 2. A second sender attempts to send a value while the channel is in a half-initialized + // state. Here, half-initialized means that the `tail.block` pointer points to a valid block + // but `head.block` is still null. This condition is ensured by the yield of step 1. When + // this call returns the channel state has tail.index != head.index, tail.block != NULL, and + // head.block = NULL. + s2.send(42).unwrap(); + // 3. This thread continues with dropping the one and only receiver. When all receivers are + // gone `discard_all_messages` will attempt to drop all currently sent values and + // de-allocate all the blocks. If `tail.block != NULL` but `head.block = NULL` the + // implementation waits for the initializing sender to finish by spinning/yielding. + drop(r); + // 5. This thread is rescheduled and `discard_all_messages` observes the head.block pointer set + // by step 4 and proceeds with deallocation. In the problematic version of the code + // `head.block` is simply read via an `Acquire` load and not swapped with NULL. After this + // call returns the channel state has tail.index = head.index, tail.block = NULL, and + // head.block != NULL. + t1.join().unwrap(); + // 6. The last sender (s1) is dropped here which also attempts to cleanup any data in the + // channel. It observes `tail.index = head.index` and so it doesn't attempt to cleanup any + // messages but it also observes that `head.block != NULL` and attempts to deallocate it. + // This is however already deallocated by `discard_all_messages`, leading to a double free. +} From f31cf5cb7c8f2e7bb60c1806459895134e4d7268 Mon Sep 17 00:00:00 2001 From: Petros Angelatos Date: Tue, 8 Apr 2025 22:37:25 +0300 Subject: [PATCH 7/7] sync::mpsc: prevent double free on `Drop` This PR is fixing a regression introduced by #121646 that can lead to a double free when dropping the channel. The details of the bug can be found in the corresponding crossbeam PR https://github.com/crossbeam-rs/crossbeam/pull/1187 Signed-off-by: Petros Angelatos (cherry picked from commit b9e2ac5c7b1d6bb3b6f5fdfe0819eaf7e95bf7ff) --- library/std/src/sync/mpmc/list.rs | 8 +++++++- src/tools/miri/tests/pass/issues/issue-139553.rs | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/library/std/src/sync/mpmc/list.rs b/library/std/src/sync/mpmc/list.rs index 2dd8f41226d63..1c6acb29e375f 100644 --- a/library/std/src/sync/mpmc/list.rs +++ b/library/std/src/sync/mpmc/list.rs @@ -569,9 +569,15 @@ impl Channel { // In that case, just wait until it gets initialized. while block.is_null() { backoff.spin_heavy(); - block = self.head.block.load(Ordering::Acquire); + block = self.head.block.swap(ptr::null_mut(), Ordering::AcqRel); } } + // After this point `head.block` is not modified again and it will be deallocated if it's + // non-null. The `Drop` code of the channel, which runs after this function, also attempts + // to deallocate `head.block` if it's non-null. Therefore this function must maintain the + // invariant that if a deallocation of head.block is attemped then it must also be set to + // NULL. Failing to do so will lead to the Drop code attempting a double free. For this + // reason both reads above do an atomic swap instead of a simple atomic load. unsafe { // Drop all messages between head and tail and deallocate the heap-allocated blocks. diff --git a/src/tools/miri/tests/pass/issues/issue-139553.rs b/src/tools/miri/tests/pass/issues/issue-139553.rs index 97cb9e1a8057c..119d589d1eada 100644 --- a/src/tools/miri/tests/pass/issues/issue-139553.rs +++ b/src/tools/miri/tests/pass/issues/issue-139553.rs @@ -38,7 +38,7 @@ fn main() { // call returns the channel state has tail.index = head.index, tail.block = NULL, and // head.block != NULL. t1.join().unwrap(); - // 6. The last sender (s1) is dropped here which also attempts to cleanup any data in the + // 6. The last sender (s2) is dropped here which also attempts to cleanup any data in the // channel. It observes `tail.index = head.index` and so it doesn't attempt to cleanup any // messages but it also observes that `head.block != NULL` and attempts to deallocate it. // This is however already deallocated by `discard_all_messages`, leading to a double free.