Commit ae6df65d authored by Miguel Ojeda's avatar Miguel Ojeda
Browse files

rust: upgrade to Rust 1.72.1

This is the third upgrade to the Rust toolchain, from 1.71.1 to 1.72.1
(i.e. the latest) [1].

See the upgrade policy [2] and the comments on the first upgrade in
commit 3ed03f4d ("rust: upgrade to Rust 1.68.2").

# Unstable features

No unstable features (that we use) were stabilized.

Therefore, the only unstable feature allowed to be used outside
the `kernel` crate is still `new_uninit`, though other code to be
upstreamed may increase the list.

Please see [3] for details.

# Other improvements

Previously, the compiler could incorrectly generate a `.eh_frame`
section under `-Cpanic=abort`. We were hitting this bug when debug
assertions were enabled (`CONFIG_RUST_DEBUG_ASSERTIONS=y`) [4]:

      LD      .tmp_vmlinux.kallsyms1
    ld.lld: error: <internal>:(.eh_frame) is being placed in '.eh_frame'

Gary fixed the issue in Rust 1.72.0 [5].

# Required changes

For the upgrade, the following changes are required:

  - A call to `Box::from_raw` in `rust/kernel/sync/arc.rs` now requires
    an explicit `drop()` call. See previous patch for details.

# `alloc` upgrade and reviewing

The vast majority of changes are due to our `alloc` fork being upgraded
at once.

There are two kinds of changes to be aware of: the ones coming from
upstream, which we should follow as closely as possible, and the updates
needed in our added fallible APIs to keep them matching the newer
infallible APIs coming from upstream.

Instead of taking a look at the diff of this patch, an alternative
approach is reviewing a diff of the changes between upstream `alloc` and
the kernel's. This allows to easily inspect the kernel additions only,
especially to check if the fallible methods we already have still match
the infallible ones in the new version coming from upstream.

Another approach is reviewing the changes introduced in the additions in
the kernel fork between the two versions. This is useful to spot
potentially unintended changes to our additions.

To apply these approaches, one may follow steps similar to the following
to generate a pair of patches that show the differences between upstream
Rust and the kernel (for the subset of `alloc` we use) before and after
applying this patch:

    # Get the difference with respect to the old version.
    git -C rust checkout $(linux/scripts/min-tool-version.sh rustc)
    git -C linux ls-tree -r --name-only HEAD -- rust/alloc |
        cut -d/ -f3- |
        grep -Fv README.md |
        xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH
    git -C linux diff --patch-with-stat --summary -R > old.patch
    git -C linux restore rust/alloc

    # Apply this patch.
    git -C linux am rust-upgrade.patch

    # Get the difference with respect to the new version.
    git -C rust checkout $(linux/scripts/min-tool-version.sh rustc)
    git -C linux ls-tree -r --name-only HEAD -- rust/alloc |
        cut -d/ -f3- |
        grep -Fv README.md |
        xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH
    git -C linux diff --patch-with-stat --summary -R > new.patch
    git -C linux restore rust/alloc

Now one may check the `new.patch` to take a look at the additions (first
approach) or at the difference between those two patches (second
approach). For the latter, a side-by-side tool is recommended.

Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1721-2023-09-19 [1]
Link: https://rust-for-linux.com/rust-version-policy [2]
Link: https://github.com/Rust-for-Linux/linux/issues/2 [3]
Closes: https://github.com/Rust-for-Linux/linux/issues/1012 [4]
Link: https://github.com/rust-lang/rust/pull/112403

 [5]
Reviewed-by: default avatarMartin Rodriguez Reboredo <yakoyoku@gmail.com>
Reviewed-by: default avatarGary Guo <gary@garyguo.net>
Reviewed-by: default avatarAlice Ryhl <aliceryhl@google.com>
Reviewed-by: default avatarBjörn Roy Baron <bjorn3_gh@protonmail.com>
Link: https://lore.kernel.org/r/20230823160244.188033-3-ojeda@kernel.org


[ Used 1.72.1 instead of .0 (no changes in `alloc`) and reworded
  to mention that we hit the `.eh_frame` bug under debug assertions. ]
Signed-off-by: default avatarMiguel Ojeda <ojeda@kernel.org>
parent 828176d0
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -31,7 +31,7 @@ you probably needn't concern yourself with pcmciautils.
====================== ===============  ========================================
GNU C                  5.1              gcc --version
Clang/LLVM (optional)  11.0.0           clang --version
Rust (optional)        1.71.1           rustc --version
Rust (optional)        1.72.1           rustc --version
bindgen (optional)     0.65.1           bindgen --version
GNU make               3.82             make --version
bash                   4.2              bash --version
+5 −4
Original line number Diff line number Diff line
@@ -6,8 +6,10 @@

#[cfg(not(test))]
use core::intrinsics;
#[cfg(all(bootstrap, not(test)))]
use core::intrinsics::{min_align_of_val, size_of_val};

#[cfg(all(bootstrap, not(test)))]
use core::ptr::Unique;
#[cfg(not(test))]
use core::ptr::{self, NonNull};
@@ -40,7 +42,6 @@
    #[rustc_nounwind]
    fn __rust_alloc_zeroed(size: usize, align: usize) -> *mut u8;

    #[cfg(not(bootstrap))]
    static __rust_no_alloc_shim_is_unstable: u8;
}

@@ -98,7 +99,6 @@ pub unsafe fn alloc(layout: Layout) -> *mut u8 {
    unsafe {
        // Make sure we don't accidentally allow omitting the allocator shim in
        // stable code until it is actually stabilized.
        #[cfg(not(bootstrap))]
        core::ptr::read_volatile(&__rust_no_alloc_shim_is_unstable);

        __rust_alloc(layout.size(), layout.align())
@@ -339,14 +339,15 @@ unsafe fn exchange_malloc(size: usize, align: usize) -> *mut u8 {
    }
}

#[cfg_attr(not(test), lang = "box_free")]
#[cfg(all(bootstrap, not(test)))]
#[lang = "box_free"]
#[inline]
// This signature has to be the same as `Box`, otherwise an ICE will happen.
// When an additional parameter to `Box` is added (like `A: Allocator`), this has to be added here as
// well.
// For example if `Box` is changed to  `struct Box<T: ?Sized, A: Allocator>(Unique<T>, A)`,
// this function has to be changed to `fn box_free<T: ?Sized, A: Allocator>(Unique<T>, A)` as well.
pub(crate) unsafe fn box_free<T: ?Sized, A: Allocator>(ptr: Unique<T>, alloc: A) {
unsafe fn box_free<T: ?Sized, A: Allocator>(ptr: Unique<T>, alloc: A) {
    unsafe {
        let size = size_of_val(ptr.as_ref());
        let align = min_align_of_val(ptr.as_ref());
+9 −1
Original line number Diff line number Diff line
@@ -1215,8 +1215,16 @@ pub const fn into_pin(boxed: Self) -> Pin<Self>

#[stable(feature = "rust1", since = "1.0.0")]
unsafe impl<#[may_dangle] T: ?Sized, A: Allocator> Drop for Box<T, A> {
    #[inline]
    fn drop(&mut self) {
        // FIXME: Do nothing, drop is currently performed by compiler.
        // the T in the Box is dropped by the compiler before the destructor is run

        let ptr = self.0;

        unsafe {
            let layout = Layout::for_value_raw(ptr.as_ptr());
            self.1.deallocate(From::from(ptr.cast()), layout)
        }
    }
}

+5 −5
Original line number Diff line number Diff line
@@ -58,6 +58,11 @@
//! [`Rc`]: rc
//! [`RefCell`]: core::cell

// To run alloc tests without x.py without ending up with two copies of alloc, Miri needs to be
// able to "empty" this crate. See <https://github.com/rust-lang/miri-test-libstd/issues/4>.
// rustc itself never sets the feature, so this line has no affect there.
#![cfg(any(not(feature = "miri-test-libstd"), test, doctest))]
//
#![allow(unused_attributes)]
#![stable(feature = "alloc", since = "1.36.0")]
#![doc(
@@ -77,11 +82,6 @@
))]
#![no_std]
#![needs_allocator]
// To run alloc tests without x.py without ending up with two copies of alloc, Miri needs to be
// able to "empty" this crate. See <https://github.com/rust-lang/miri-test-libstd/issues/4>.
// rustc itself never sets the feature, so this line has no affect there.
#![cfg(any(not(feature = "miri-test-libstd"), test, doctest))]
//
// Lints:
#![deny(unsafe_op_in_unsafe_fn)]
#![deny(fuzzy_provenance_casts)]
+115 −0
Original line number Diff line number Diff line
// SPDX-License-Identifier: Apache-2.0 OR MIT

use crate::alloc::{Allocator, Global};
use core::mem::{ManuallyDrop, SizedTypeProperties};
use core::ptr;
use core::slice;

@@ -9,20 +8,21 @@

/// An iterator which uses a closure to determine if an element should be removed.
///
/// This struct is created by [`Vec::drain_filter`].
/// This struct is created by [`Vec::extract_if`].
/// See its documentation for more.
///
/// # Example
///
/// ```
/// #![feature(drain_filter)]
/// #![feature(extract_if)]
///
/// let mut v = vec![0, 1, 2];
/// let iter: std::vec::DrainFilter<'_, _, _> = v.drain_filter(|x| *x % 2 == 0);
/// let iter: std::vec::ExtractIf<'_, _, _> = v.extract_if(|x| *x % 2 == 0);
/// ```
#[unstable(feature = "drain_filter", reason = "recently added", issue = "43244")]
#[unstable(feature = "extract_if", reason = "recently added", issue = "43244")]
#[derive(Debug)]
pub struct DrainFilter<
#[must_use = "iterators are lazy and do nothing unless consumed"]
pub struct ExtractIf<
    'a,
    T,
    F,
@@ -39,15 +39,9 @@ pub struct DrainFilter<
    pub(super) old_len: usize,
    /// The filter test predicate.
    pub(super) pred: F,
    /// A flag that indicates a panic has occurred in the filter test predicate.
    /// This is used as a hint in the drop implementation to prevent consumption
    /// of the remainder of the `DrainFilter`. Any unprocessed items will be
    /// backshifted in the `vec`, but no further items will be dropped or
    /// tested by the filter predicate.
    pub(super) panic_flag: bool,
}

impl<T, F, A: Allocator> DrainFilter<'_, T, F, A>
impl<T, F, A: Allocator> ExtractIf<'_, T, F, A>
where
    F: FnMut(&mut T) -> bool,
{
@@ -57,63 +51,10 @@ impl<T, F, A: Allocator> DrainFilter<'_, T, F, A>
    pub fn allocator(&self) -> &A {
        self.vec.allocator()
    }

    /// Keep unyielded elements in the source `Vec`.
    ///
    /// # Examples
    ///
    /// ```
    /// #![feature(drain_filter)]
    /// #![feature(drain_keep_rest)]
    ///
    /// let mut vec = vec!['a', 'b', 'c'];
    /// let mut drain = vec.drain_filter(|_| true);
    ///
    /// assert_eq!(drain.next().unwrap(), 'a');
    ///
    /// // This call keeps 'b' and 'c' in the vec.
    /// drain.keep_rest();
    ///
    /// // If we wouldn't call `keep_rest()`,
    /// // `vec` would be empty.
    /// assert_eq!(vec, ['b', 'c']);
    /// ```
    #[unstable(feature = "drain_keep_rest", issue = "101122")]
    pub fn keep_rest(self) {
        // At this moment layout looks like this:
        //
        //  _____________________/-- old_len
        // /                     \
        // [kept] [yielded] [tail]
        //        \_______/ ^-- idx
        //                \-- del
        //
        // Normally `Drop` impl would drop [tail] (via .for_each(drop), ie still calling `pred`)
        //
        // 1. Move [tail] after [kept]
        // 2. Update length of the original vec to `old_len - del`
        //    a. In case of ZST, this is the only thing we want to do
        // 3. Do *not* drop self, as everything is put in a consistent state already, there is nothing to do
        let mut this = ManuallyDrop::new(self);

        unsafe {
            // ZSTs have no identity, so we don't need to move them around.
            if !T::IS_ZST && this.idx < this.old_len && this.del > 0 {
                let ptr = this.vec.as_mut_ptr();
                let src = ptr.add(this.idx);
                let dst = src.sub(this.del);
                let tail_len = this.old_len - this.idx;
                src.copy_to(dst, tail_len);
            }

            let new_len = this.old_len - this.del;
            this.vec.set_len(new_len);
        }
    }
}

#[unstable(feature = "drain_filter", reason = "recently added", issue = "43244")]
impl<T, F, A: Allocator> Iterator for DrainFilter<'_, T, F, A>
#[unstable(feature = "extract_if", reason = "recently added", issue = "43244")]
impl<T, F, A: Allocator> Iterator for ExtractIf<'_, T, F, A>
where
    F: FnMut(&mut T) -> bool,
{
@@ -124,9 +65,7 @@ fn next(&mut self) -> Option<T> {
            while self.idx < self.old_len {
                let i = self.idx;
                let v = slice::from_raw_parts_mut(self.vec.as_mut_ptr(), self.old_len);
                self.panic_flag = true;
                let drained = (self.pred)(&mut v[i]);
                self.panic_flag = false;
                // Update the index *after* the predicate is called. If the index
                // is updated prior and the predicate panics, the element at this
                // index would be leaked.
@@ -150,50 +89,27 @@ fn size_hint(&self) -> (usize, Option<usize>) {
    }
}

#[unstable(feature = "drain_filter", reason = "recently added", issue = "43244")]
impl<T, F, A: Allocator> Drop for DrainFilter<'_, T, F, A>
where
    F: FnMut(&mut T) -> bool,
{
    fn drop(&mut self) {
        struct BackshiftOnDrop<'a, 'b, T, F, A: Allocator>
        where
            F: FnMut(&mut T) -> bool,
        {
            drain: &'b mut DrainFilter<'a, T, F, A>,
        }

        impl<'a, 'b, T, F, A: Allocator> Drop for BackshiftOnDrop<'a, 'b, T, F, A>
#[unstable(feature = "extract_if", reason = "recently added", issue = "43244")]
impl<T, F, A: Allocator> Drop for ExtractIf<'_, T, F, A>
where
    F: FnMut(&mut T) -> bool,
{
    fn drop(&mut self) {
        unsafe {
                    if self.drain.idx < self.drain.old_len && self.drain.del > 0 {
            if self.idx < self.old_len && self.del > 0 {
                // This is a pretty messed up state, and there isn't really an
                // obviously right thing to do. We don't want to keep trying
                // to execute `pred`, so we just backshift all the unprocessed
                // elements and tell the vec that they still exist. The backshift
                // is required to prevent a double-drop of the last successfully
                // drained item prior to a panic in the predicate.
                        let ptr = self.drain.vec.as_mut_ptr();
                        let src = ptr.add(self.drain.idx);
                        let dst = src.sub(self.drain.del);
                        let tail_len = self.drain.old_len - self.drain.idx;
                let ptr = self.vec.as_mut_ptr();
                let src = ptr.add(self.idx);
                let dst = src.sub(self.del);
                let tail_len = self.old_len - self.idx;
                src.copy_to(dst, tail_len);
            }
                    self.drain.vec.set_len(self.drain.old_len - self.drain.del);
                }
            }
        }

        let backshift = BackshiftOnDrop { drain: self };

        // Attempt to consume any remaining elements if the filter predicate
        // has not yet panicked. We'll backshift any remaining elements
        // whether we've already panicked or if the consumption here panics.
        if !backshift.drain.panic_flag {
            backshift.drain.for_each(drop);
            self.vec.set_len(self.old_len - self.del);
        }
    }
}
Loading