Auto merge of #139988 - cuviper:beta-next, r=cuviper

[beta] backports

- Fix 2024 edition doctest panic output #139328
- make `Arguments::as_statically_known_str` doc(hidden) #139389
- Revert "Deduplicate template parameter creation" #139878
- sync::mpsc: prevent double free on `Drop` #139553

r? cuviper

Author: bors

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<Option<&'ll DIType>> {
     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<Option<&'ll DIType>> {
-    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![];

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

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<Symbol> {
+            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>(

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 }

library/std/src/sync/mpmc/list.rs

@@ -213,6 +213,11 @@ impl<T> Channel<T> {
                     .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 {
@@ -564,9 +569,15 @@ impl<T> Channel<T> {
             // 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.

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<PathBuf> = 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::<Vec<_>>())
             .output()
             .expect(\"failed to run command\");
         if !out.status.success() {{
-            Err(String::from_utf8_lossy(&out.stderr).to_string())
+            eprint!(\"{{}}\", String::from_utf8_lossy(&out.stderr));
+            ExitCode::FAILURE
         }} else {{
-            Ok(())
+            ExitCode::SUCCESS
         }}
     }}
 }}

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::<u64>();
+    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 (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.
+}

tests/rustdoc-ui/doctest/edition-2024-error-output.rs

@@ -0,0 +1,14 @@
+// This is a regression test for <https://github.com/rust-lang/rust/issues/137970>.
+// 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: "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
+//! assert_eq!(2 + 2, 5);
+//! ```

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
+