Skip to contentSkip to conversation

rust: upgrade to Rust 1.71.0

PATCH Applied Miguel Ojeda 6 participants 10 messages Jul 29, 2023 – Aug 14, 2023

Subsystems (4)
lore
Share & tools
lore

Miguel Ojeda

author maintainer
#0 lore raw

PATCH 1/2 rust: upgrade to Rust 1.71.0

This is the second upgrade to the Rust toolchain, from 1.68.2 to 1.71.0 (i.e. the latest).

See the upgrade policy [1] and the comments on the first upgrade in commit 3ed03f4da06e ("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 [2] for details.

# Required changes

For the upgrade, this patch requires the following changes:

- Removal of the __rust_* allocator functions, together with the addition of the __rust_no_alloc_shim_is_unstable static. See [3] for details.

- Some more compiler builtins added due to <f{32,64}>::midpoint() that got added in Rust 1.71 [4].

# 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.

---
Based on top of rust-next, since we want to be on top of commit 7c74839b2acc ("rust: alloc: Add realloc and alloc_zeroed to the GlobalAlloc impl"), in order to be able to simply remove the __rust_* functions here.

 Documentation/process/changes.rst |   2 +-
 rust/Makefile                     |   4 +-
 rust/alloc/alloc.rs               |  20 +++--
 rust/alloc/boxed.rs               | 131 +++++++++++++++---------------
 rust/alloc/lib.rs                 |  50 ++++++------
 rust/alloc/raw_vec.rs             |  18 ++--
 rust/alloc/slice.rs               |  43 +++++++---
 rust/alloc/vec/drain.rs           |   8 +-
 rust/alloc/vec/drain_filter.rs    |   8 +-
 rust/alloc/vec/into_iter.rs       |  35 +++++---
 rust/alloc/vec/mod.rs             |  84 +++++--------------
 rust/compiler_builtins.rs         |   7 ++
 rust/kernel/allocator.rs          |  37 +--------
 scripts/min-tool-version.sh       |   2 +-
 14 files changed, 209 insertions(+), 240 deletions(-)
Documentation/process/changes.rst +1 -1
index 5561dae94f85..535bdd94a222 100644
@@ -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.68.2           rustc --version
+Rust (optional)        1.71.0           rustc --version
 bindgen (optional)     0.56.0           bindgen --version
 GNU make               3.82             make --version
 bash                   4.2              bash --version
rust/Makefile +2 -2
index 7c9d9f11aec5..e617930e5ef3 100644
@@ -377,8 +377,8 @@ rust-analyzer:
 		$(RUST_LIB_SRC) > $(objtree)/rust-project.json
 
 redirect-intrinsics = \
-	__eqsf2 __gesf2 __lesf2 __nesf2 __unordsf2 \
-	__unorddf2 \
+	__addsf3 __eqsf2 __gesf2 __lesf2 __ltsf2 __mulsf3 __nesf2 __unordsf2 \
+	__adddf3 __ledf2 __ltdf2 __muldf3 __unorddf2 \
 	__muloti4 __multi3 \
 	__udivmodti4 __udivti3 __umodti3
 
rust/alloc/alloc.rs +12 -8
index acf22d45e6f2..0b6bf5b6da43 100644
@@ -16,8 +16,6 @@
 #[doc(inline)]
 pub use core::alloc::*;
 
-use core::marker::Destruct;
-
 #[cfg(test)]
 mod tests;
 
@@ -41,6 +39,9 @@
     #[rustc_allocator_zeroed]
     #[rustc_nounwind]
     fn __rust_alloc_zeroed(size: usize, align: usize) -> *mut u8;
+
+    #[cfg(not(bootstrap))]
+    static __rust_no_alloc_shim_is_unstable: u8;
 }
 
 /// The global memory allocator.
@@ -94,7 +95,14 @@
 #[must_use = "losing the pointer will leak memory"]
 #[inline]
 pub unsafe fn alloc(layout: Layout) -> *mut u8 {
-    unsafe { __rust_alloc(layout.size(), layout.align()) }
+    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())
+    }
 }
 
 /// Deallocate memory with the global allocator.
@@ -333,16 +341,12 @@ unsafe fn exchange_malloc(size: usize, align: usize) -> *mut u8 {
 
 #[cfg_attr(not(test), lang = "box_free")]
 #[inline]
-#[rustc_const_unstable(feature = "const_box", issue = "92521")]
 // 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) const unsafe fn box_free<T: ?Sized, A: ~const Allocator + ~const Destruct>(
-    ptr: Unique<T>,
-    alloc: A,
-) {
+pub(crate) 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());
rust/alloc/boxed.rs +64 -67

326 lines not on this page · raw message

rust/alloc/lib.rs +26 -24

122 lines not on this page · raw message

rust/alloc/raw_vec.rs +12 -6
index 5db87eac53b7..65d5ce15828e 100644
@@ -6,7 +6,6 @@
 use core::cmp;
 use core::intrinsics;
 use core::mem::{self, ManuallyDrop, MaybeUninit, SizedTypeProperties};
-use core::ops::Drop;
 use core::ptr::{self, NonNull, Unique};
 use core::slice;
 
@@ -274,10 +273,15 @@ fn current_memory(&self) -> Option<(NonNull<u8>, Layout)> {
         if T::IS_ZST || self.cap == 0 {
             None
         } else {
-            // We have an allocated chunk of memory, so we can bypass runtime
-            // checks to get our current layout.
+            // We could use Layout::array here which ensures the absence of isize and usize overflows
+            // and could hypothetically handle differences between stride and size, but this memory
+            // has already been allocated so we know it can't overflow and currently rust does not
+            // support such types. So we can do better by skipping some checks and avoid an unwrap.
+            let _: () = const { assert!(mem::size_of::<T>() % mem::align_of::<T>() == 0) };
             unsafe {
-                let layout = Layout::array::<T>(self.cap).unwrap_unchecked();
+                let align = mem::align_of::<T>();
+                let size = mem::size_of::<T>().unchecked_mul(self.cap);
+                let layout = Layout::from_size_align_unchecked(size, align);
                 Some((self.ptr.cast().into(), layout))
             }
         }
@@ -465,11 +469,13 @@ fn shrink(&mut self, cap: usize) -> Result<(), TryReserveError> {
         assert!(cap <= self.capacity(), "Tried to shrink to a larger capacity");
 
         let (ptr, layout) = if let Some(mem) = self.current_memory() { mem } else { return Ok(()) };
-
+        // See current_memory() why this assert is here
+        let _: () = const { assert!(mem::size_of::<T>() % mem::align_of::<T>() == 0) };
         let ptr = unsafe {
             // `Layout::array` cannot overflow here because it would have
             // overflowed earlier when capacity was larger.
-            let new_layout = Layout::array::<T>(cap).unwrap_unchecked();
+            let new_size = mem::size_of::<T>().unchecked_mul(cap);
+            let new_layout = Layout::from_size_align_unchecked(new_size, layout.align());
             self.alloc
                 .shrink(ptr, layout, new_layout)
                 .map_err(|_| AllocError { layout: new_layout, non_exhaustive: () })?
rust/alloc/slice.rs +33 -10

55 lines not on this page · raw message

rust/alloc/vec/drain.rs +3 -5
index d503d2f478ce..78177a9e2ad0 100644
@@ -18,7 +18,7 @@
 ///
 /// ```
 /// let mut v = vec![0, 1, 2];
-/// let iter: std::vec::Drain<_> = v.drain(..);
+/// let iter: std::vec::Drain<'_, _> = v.drain(..);
 /// ```
 #[stable(feature = "drain", since = "1.6.0")]
 pub struct Drain<
@@ -114,9 +114,7 @@ pub fn keep_rest(self) {
             let unyielded_ptr = this.iter.as_slice().as_ptr();
 
             // ZSTs have no identity, so we don't need to move them around.
-            let needs_move = mem::size_of::<T>() != 0;
-
-            if needs_move {
+            if !T::IS_ZST {
                 let start_ptr = source_vec.as_mut_ptr().add(start);
 
                 // memmove back unyielded elements
@@ -199,7 +197,7 @@ fn drop(&mut self) {
             }
         }
 
-        let iter = mem::replace(&mut self.iter, (&mut []).iter());
+        let iter = mem::take(&mut self.iter);
         let drop_len = iter.len();
 
         let mut vec = self.vec;
rust/alloc/vec/drain_filter.rs +3 -5
index 4b019220657d..09efff090e42 100644
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0 OR MIT
 
 use crate::alloc::{Allocator, Global};
-use core::mem::{self, ManuallyDrop};
+use core::mem::{ManuallyDrop, SizedTypeProperties};
 use core::ptr;
 use core::slice;
 
@@ -18,7 +18,7 @@
 /// #![feature(drain_filter)]
 ///
 /// let mut v = vec![0, 1, 2];
-/// let iter: std::vec::DrainFilter<_, _> = v.drain_filter(|x| *x % 2 == 0);
+/// let iter: std::vec::DrainFilter<'_, _, _> = v.drain_filter(|x| *x % 2 == 0);
 /// ```
 #[unstable(feature = "drain_filter", reason = "recently added", issue = "43244")]
 #[derive(Debug)]
@@ -98,9 +98,7 @@ pub fn keep_rest(self) {
 
         unsafe {
             // ZSTs have no identity, so we don't need to move them around.
-            let needs_move = mem::size_of::<T>() != 0;
-
-            if needs_move && this.idx < this.old_len && this.del > 0 {
+            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);
rust/alloc/vec/into_iter.rs +24 -11
index 34a2a70d6ded..aac0ec16aef1 100644
@@ -13,6 +13,7 @@
 };
 use core::marker::PhantomData;
 use core::mem::{self, ManuallyDrop, MaybeUninit, SizedTypeProperties};
+use core::num::NonZeroUsize;
 #[cfg(not(no_global_oom_handling))]
 use core::ops::Deref;
 use core::ptr::{self, NonNull};
@@ -109,7 +110,7 @@ fn as_raw_mut_slice(&mut self) -> *mut [T] {
     /// ```
     /// # let mut into_iter = Vec::<u8>::with_capacity(10).into_iter();
     /// let mut into_iter = std::mem::replace(&mut into_iter, Vec::new().into_iter());
-    /// (&mut into_iter).for_each(core::mem::drop);
+    /// (&mut into_iter).for_each(drop);
     /// std::mem::forget(into_iter);
     /// ```
     ///
@@ -215,7 +216,7 @@ fn size_hint(&self) -> (usize, Option<usize>) {
     }
 
     #[inline]
-    fn advance_by(&mut self, n: usize) -> Result<(), usize> {
+    fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> {
         let step_size = self.len().min(n);
         let to_drop = ptr::slice_from_raw_parts_mut(self.ptr as *mut T, step_size);
         if T::IS_ZST {
@@ -229,10 +230,7 @@ fn advance_by(&mut self, n: usize) -> Result<(), usize> {
         unsafe {
             ptr::drop_in_place(to_drop);
         }
-        if step_size < n {
-            return Err(step_size);
-        }
-        Ok(())
+        NonZeroUsize::new(n - step_size).map_or(Ok(()), Err)
     }
 
     #[inline]
@@ -315,7 +313,7 @@ fn next_back(&mut self) -> Option<T> {
     }
 
     #[inline]
-    fn advance_back_by(&mut self, n: usize) -> Result<(), usize> {
+    fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> {
         let step_size = self.len().min(n);
         if T::IS_ZST {
             // SAFETY: same as for advance_by()
@@ -329,10 +327,7 @@ fn advance_back_by(&mut self, n: usize) -> Result<(), usize> {
         unsafe {
             ptr::drop_in_place(to_drop);
         }
-        if step_size < n {
-            return Err(step_size);
-        }
-        Ok(())
+        NonZeroUsize::new(n - step_size).map_or(Ok(()), Err)
     }
 }
 
@@ -349,6 +344,24 @@ impl<T, A: Allocator> FusedIterator for IntoIter<T, A> {}
 #[unstable(feature = "trusted_len", issue = "37572")]
 unsafe impl<T, A: Allocator> TrustedLen for IntoIter<T, A> {}
 
+#[stable(feature = "default_iters", since = "1.70.0")]
+impl<T, A> Default for IntoIter<T, A>
+where
+    A: Allocator + Default,
+{
+    /// Creates an empty `vec::IntoIter`.
+    ///
+    /// ```
+    /// # use std::vec;
+    /// let iter: vec::IntoIter<u8> = Default::default();
+    /// assert_eq!(iter.len(), 0);
+    /// assert_eq!(iter.as_slice(), &[]);
+    /// ```
+    fn default() -> Self {
+        super::Vec::new_in(Default::default()).into_iter()
+    }
+}
+
 #[doc(hidden)]
 #[unstable(issue = "none", feature = "std_internals")]
 #[rustc_unsafe_specialization_marker]
rust/alloc/vec/mod.rs +19 -65

207 lines not on this page · raw message

rust/compiler_builtins.rs +7 -0
index 43378357ece9..fb8ac3f211de 100644
@@ -37,14 +37,21 @@ pub extern "C" fn $ident() {
 );
 
 define_panicking_intrinsics!("`f32` should not be used", {
+    __addsf3,
     __eqsf2,
     __gesf2,
     __lesf2,
+    __ltsf2,
+    __mulsf3,
     __nesf2,
     __unordsf2,
 });
 
 define_panicking_intrinsics!("`f64` should not be used", {
+    __adddf3,
+    __ledf2,
+    __ltdf2,
+    __muldf3,
     __unorddf2,
 });
 
rust/kernel/allocator.rs +2 -35
index 99b58b52e8c8..9ad5d4419678 100644
@@ -88,39 +88,6 @@ unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 {
 #[global_allocator]
 static ALLOCATOR: KernelAllocator = KernelAllocator;
 
-// `rustc` only generates these for some crate types. Even then, we would need
-// to extract the object file that has them from the archive. For the moment,
-// let's generate them ourselves instead.
-//
-// Note that `#[no_mangle]` implies exported too, nowadays.
+// See <https://github.com/rust-lang/rust/pull/86844>.
 #[no_mangle]
-fn __rust_alloc(size: usize, _align: usize) -> *mut u8 {
-    unsafe { bindings::krealloc(core::ptr::null(), size, bindings::GFP_KERNEL) as *mut u8 }
-}
-
-#[no_mangle]
-fn __rust_dealloc(ptr: *mut u8, _size: usize, _align: usize) {
-    unsafe { bindings::kfree(ptr as *const core::ffi::c_void) };
-}
-
-#[no_mangle]
-fn __rust_realloc(ptr: *mut u8, _old_size: usize, _align: usize, new_size: usize) -> *mut u8 {
-    unsafe {
-        bindings::krealloc(
-            ptr as *const core::ffi::c_void,
-            new_size,
-            bindings::GFP_KERNEL,
-        ) as *mut u8
-    }
-}
-
-#[no_mangle]
-fn __rust_alloc_zeroed(size: usize, _align: usize) -> *mut u8 {
-    unsafe {
-        bindings::krealloc(
-            core::ptr::null(),
-            size,
-            bindings::GFP_KERNEL | bindings::__GFP_ZERO,
-        ) as *mut u8
-    }
-}
+static __rust_no_alloc_shim_is_unstable: u8 = 0;
scripts/min-tool-version.sh +1 -1
index 2ade63149466..73715794e7f8 100755
@@ -31,7 +31,7 @@ llvm)
 	fi
 	;;
 rustc)
-	echo 1.68.2
+	echo 1.71.0
 	;;
 bindgen)
 	echo 0.56.0

base-commit: 7c74839b2acc9b5a5d16d3a1737741dd64e1804b

Signature · 1 line
2.41.0
author maintainer
#1 lore raw

PATCH 2/2 rust: enable `no_mangle_with_rust_abi` Clippy lint

Introduced in Rust 1.69.0 [1], this lint prevents forgetting to set the C ABI when using #[no_mangle] (or thinking it is implied).

For instance, it would have prevented the issue [2] fixed by commit c682e4c37d2b ("rust: kernel: Mark rust_fmt_argument as extern "C"").

    error: `#[no_mangle]` set on a function with the default (`Rust`) ABI
      --> rust/kernel/print.rs:21:1
       |
    21 | / unsafe fn rust_fmt_argument(
    22 | |     buf: *mut c_char,
    23 | |     end: *mut c_char,
    24 | |     ptr: *const c_void,
    25 | | ) -> *mut c_char {
       | |________________^
       |
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#no_mangle_with_rust_abi
       = note: requested on the command line with `-D clippy::no-mangle-with-rust-abi`
    help: set an ABI
       |
    21 | unsafe extern "C" fn rust_fmt_argument(
       |        ++++++++++
    help: or explicitly set the default
       |
    21 | unsafe extern "Rust" fn rust_fmt_argument(
       |        +++++++++++++

Thus enable it.

In rare cases, we may need to use the Rust ABI even with #[no_mangle] (e.g. one case, before 1.71.0, would have been the __rust_* functions). In those cases, we would need to #[allow(...)] the lint, since using extern "Rust" explicitly (as the compiler suggests) currently gets overwritten by rustfmt [3].

---
 Makefile | 1 +
 1 file changed, 1 insertion(+)
Makefile +1 -0
index 658ec2b8aa74..93bf3eca3ead 100644
@@ -467,6 +467,7 @@ export rust_common_flags := --edition=2021 \
 			    -Dclippy::let_unit_value -Dclippy::mut_mut \
 			    -Dclippy::needless_bitwise_bool \
 			    -Dclippy::needless_continue \
+			    -Dclippy::no_mangle_with_rust_abi \
 			    -Wclippy::dbg_macro
 
 KBUILD_HOSTCFLAGS   := $(KBUILD_USERHOSTCFLAGS) $(HOST_LFS_CFLAGS) $(HOSTCFLAGS)
Signature · 1 line
2.41.0
Reviewed-by on v1 2/2
#2 lore raw

Re: [PATCH 2/2] rust: enable `no_mangle_with_rust_abi` Clippy lint

Quoted 35 lines · Miguel Ojeda

Introduced in Rust 1.69.0 [1], this lint prevents forgetting to set the C ABI when using #[no_mangle] (or thinking it is implied).

For instance, it would have prevented the issue [2] fixed by commit c682e4c37d2b ("rust: kernel: Mark rust_fmt_argument as extern "C"").

     error: `#[no_mangle]` set on a function with the default (`Rust`) ABI
       --> rust/kernel/print.rs:21:1
        |
     21 | / unsafe fn rust_fmt_argument(
     22 | |     buf: *mut c_char,
     23 | |     end: *mut c_char,
     24 | |     ptr: *const c_void,
     25 | | ) -> *mut c_char {
        | |________________^
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#no_mangle_with_rust_abi
        = note: requested on the command line with `-D clippy::no-mangle-with-rust-abi`
     help: set an ABI
        |
     21 | unsafe extern "C" fn rust_fmt_argument(
        |        ++++++++++
     help: or explicitly set the default
        |
     21 | unsafe extern "Rust" fn rust_fmt_argument(
        |        +++++++++++++

Thus enable it.

In rare cases, we may need to use the Rust ABI even with #[no_mangle] (e.g. one case, before 1.71.0, would have been the __rust_* functions). In those cases, we would need to #[allow(...)] the lint, since using extern "Rust" explicitly (as the compiler suggests) currently gets overwritten by rustfmt [3].

Quoted 6 lines

---
[...]

reviewer Reviewed-by on v1 2/2
#3 lore raw

Re: [PATCH 2/2] rust: enable `no_mangle_with_rust_abi` Clippy lint

In reply to Miguel Ojeda
Quoted 36 lines · Miguel Ojeda

Introduced in Rust 1.69.0 [1], this lint prevents forgetting to set the C ABI when using #[no_mangle] (or thinking it is implied).

For instance, it would have prevented the issue [2] fixed by commit c682e4c37d2b ("rust: kernel: Mark rust_fmt_argument as extern "C"").

    error: `#[no_mangle]` set on a function with the default (`Rust`) ABI
      --> rust/kernel/print.rs:21:1
       |
    21 | / unsafe fn rust_fmt_argument(
    22 | |     buf: *mut c_char,
    23 | |     end: *mut c_char,
    24 | |     ptr: *const c_void,
    25 | | ) -> *mut c_char {
       | |________________^
       |
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#no_mangle_with_rust_abi
       = note: requested on the command line with `-D clippy::no-mangle-with-rust-abi`
    help: set an ABI
       |
    21 | unsafe extern "C" fn rust_fmt_argument(
       |        ++++++++++
    help: or explicitly set the default
       |
    21 | unsafe extern "Rust" fn rust_fmt_argument(
       |        +++++++++++++

Thus enable it.

In rare cases, we may need to use the Rust ABI even with #[no_mangle] (e.g. one case, before 1.71.0, would have been the __rust_* functions). In those cases, we would need to #[allow(...)] the lint, since using extern "Rust" explicitly (as the compiler suggests) currently gets overwritten by rustfmt [3].

Quoted 6 lines

---
[...]

reviewer Reviewed-by on v1 2/2
#4 lore raw

Re: [PATCH 2/2] rust: enable `no_mangle_with_rust_abi` Clippy lint

In reply to Miguel Ojeda
Quoted 35 lines · On Sun, 30 Jul 2023 00:03:17 +0200 Miguel Ojeda <[email protected]> wrote:

Introduced in Rust 1.69.0 [1], this lint prevents forgetting to set the C ABI when using #[no_mangle] (or thinking it is implied).

For instance, it would have prevented the issue [2] fixed by commit c682e4c37d2b ("rust: kernel: Mark rust_fmt_argument as extern "C"").

    error: `#[no_mangle]` set on a function with the default (`Rust`) ABI
      --> rust/kernel/print.rs:21:1  
       |
    21 | / unsafe fn rust_fmt_argument(
    22 | |     buf: *mut c_char,
    23 | |     end: *mut c_char,
    24 | |     ptr: *const c_void,
    25 | | ) -> *mut c_char {
       | |________________^
       |
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#no_mangle_with_rust_abi
       = note: requested on the command line with `-D clippy::no-mangle-with-rust-abi`
    help: set an ABI
       |
    21 | unsafe extern "C" fn rust_fmt_argument(
       |        ++++++++++
    help: or explicitly set the default
       |
    21 | unsafe extern "Rust" fn rust_fmt_argument(
       |        +++++++++++++

Thus enable it.

In rare cases, we may need to use the Rust ABI even with #[no_mangle] (e.g. one case, before 1.71.0, would have been the __rust_* functions). In those cases, we would need to #[allow(...)] the lint, since using extern "Rust" explicitly (as the compiler suggests) currently gets overwritten by rustfmt [3].

Quoted 4 lines
Quoted 16 lines
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)
Makefile +1 -0
index 658ec2b8aa74..93bf3eca3ead 100644
@@ -467,6 +467,7 @@ export rust_common_flags := --edition=2021 \
 			    -Dclippy::let_unit_value -Dclippy::mut_mut \
 			    -Dclippy::needless_bitwise_bool \
 			    -Dclippy::needless_continue \
+			    -Dclippy::no_mangle_with_rust_abi \
 			    -Wclippy::dbg_macro
 
 KBUILD_HOSTCFLAGS   := $(KBUILD_USERHOSTCFLAGS) $(HOST_LFS_CFLAGS) $(HOSTCFLAGS)
#5 lore raw
In reply to Miguel Ojeda
kernel test robot noticed the following build errors:

Hi Miguel,

kernel test robot noticed the following build errors:

[auto build test ERROR on 7c74839b2acc9b5a5d16d3a1737741dd64e1804b]

url:    https://github.com/intel-lab-lkp/linux/commits/Miguel-Ojeda/rust-enable-no_mangle_with_rust_abi-Clippy-lint/20230730-060434
base:   7c74839b2acc9b5a5d16d3a1737741dd64e1804b
patch link:    https://lore.kernel.org/r/20230729220317.416771-1-ojeda%40kernel.org
patch subject: [PATCH 1/2] rust: upgrade to Rust 1.71.0
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20230730/[email protected]/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce: (https://download.01.org/0day-ci/archive/20230730/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at_include/linux/compiler_types_h_146_2)" is not a valid Ident', /opt/cross/rustc-1.71.0-bindgen-0.56.0/cargo/registry/src/index.crates.io-6f17d22bba15001f/proc-macro2-1.0.24/src/fallback.rs:693:9
   stack backtrace:
      0: rust_begin_unwind
                at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:593:5
      1: core::panicking::panic_fmt
                at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/panicking.rs:67:14
      2: proc_macro2::fallback::Ident::_new
      3: proc_macro2::Ident::new
      4: bindgen::ir::context::BindgenContext::rust_ident
      5: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
      6: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
      7: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
      8: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
      9: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
     10: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
     11: <bindgen::ir::module::Module as bindgen::codegen::CodeGenerator>::codegen
     12: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
     13: bindgen::ir::context::BindgenContext::gen
     14: bindgen::Builder::generate
     15: bindgen::main
   note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
   make[3]: *** [rust/Makefile:316: rust/uapi/uapi_generated.rs] Error 1
   make[3]: *** Deleting file 'rust/uapi/uapi_generated.rs'
>> thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at_include/linux/compiler_types_h_146_2)" is not a valid Ident', /opt/cross/rustc-1.71.0-bindgen-0.56.0/cargo/registry/src/index.crates.io-6f17d22bba15001f/proc-macro2-1.0.24/src/fallback.rs:693:9
   stack backtrace:
      0: rust_begin_unwind
                at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:593:5
      1: core::panicking::panic_fmt
                at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/panicking.rs:67:14
      2: proc_macro2::fallback::Ident::_new
      3: proc_macro2::Ident::new
      4: bindgen::ir::context::BindgenContext::rust_ident
      5: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
      6: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
      7: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
      8: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
      9: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
     10: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
     11: <bindgen::ir::module::Module as bindgen::codegen::CodeGenerator>::codegen
     12: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
     13: bindgen::ir::context::BindgenContext::gen
     14: bindgen::Builder::generate
     15: bindgen::main
   note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
   make[3]: *** [rust/Makefile:310: rust/bindings/bindings_generated.rs] Error 1
   make[3]: *** Deleting file 'rust/bindings/bindings_generated.rs'
   make[3]: Target 'rust/' not remade because of errors.
   make[2]: *** [Makefile:1281: prepare] Error 2
   make[1]: *** [Makefile:234: __sub-make] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:234: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.
Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Reviewed-by on v1 1/2
#6 lore raw
In reply to Miguel Ojeda
Quoted 82 lines · Miguel Ojeda

This is the second upgrade to the Rust toolchain, from 1.68.2 to 1.71.0 (i.e. the latest).

See the upgrade policy [1] and the comments on the first upgrade in commit 3ed03f4da06e ("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 [2] for details.

# Required changes

For the upgrade, this patch requires the following changes:

- Removal of the __rust_* allocator functions, together with the addition of the __rust_no_alloc_shim_is_unstable static. See [3] for details.

- Some more compiler builtins added due to <f{32,64}>::midpoint() that got added in Rust 1.71 [4].

# 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.

Quoted 7 lines

---
Based on top of rust-next, since we want to be on top of commit 7c74839b2acc ("rust: alloc: Add realloc and alloc_zeroed to the GlobalAlloc impl"), in order to be able to simply remove the __rust_* functions here.

[...]

reviewer Reviewed-by on v1 1/2
#7 lore raw
In reply to Miguel Ojeda
Quoted 78 lines · Miguel Ojeda

This is the second upgrade to the Rust toolchain, from 1.68.2 to 1.71.0 (i.e. the latest).

See the upgrade policy [1] and the comments on the first upgrade in commit 3ed03f4da06e ("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 [2] for details.

# Required changes

For the upgrade, this patch requires the following changes:

- Removal of the __rust_* allocator functions, together with the addition of the __rust_no_alloc_shim_is_unstable static. See [3] for details.

- Some more compiler builtins added due to <f{32,64}>::midpoint() that got added in Rust 1.71 [4].

# 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.

Quoted 6 lines

A few highlights since 1.68.2 that may eventually be of use:

- CStr::is_empty and CStr::from_bytes_until_nul help smooth FFI a bit
- NonNull::slice_from_raw_parts for slice pointers
- AtomicX::as_ptr makes interop with C more clear (doesn't seem like we use these types anywhere yet)
- const ptr::read and ptr::read_unaligned
- {Option, Result}::is_some_and
- core::cell::OnceCell one-time initialization
- repr(C) enums now use c_int rather than always i32 (no difference for currently supported targets)

#8 lore raw
In reply to Miguel Ojeda
Quoted 3 lines · Miguel Ojeda

This is the second upgrade to the Rust toolchain, from 1.68.2 to 1.71.0 (i.e. the latest).

When I apply it, I will update to 1.71.1 instead, which got released yesterday, since I assume that is what most people would want to use. There should be no difference for this patch apart from the version number, since alloc did not change upstream.

Cheers,
Miguel

Applied
#9 lore raw
In reply to Miguel Ojeda
Quoted 78 lines · Miguel Ojeda

This is the second upgrade to the Rust toolchain, from 1.68.2 to 1.71.0 (i.e. the latest).

See the upgrade policy [1] and the comments on the first upgrade in commit 3ed03f4da06e ("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 [2] for details.

# Required changes

For the upgrade, this patch requires the following changes:

- Removal of the __rust_* allocator functions, together with the addition of the __rust_no_alloc_shim_is_unstable static. See [3] for details.

- Some more compiler builtins added due to <f{32,64}>::midpoint() that got added in Rust 1.71 [4].

# 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.

Quoted 5 lines

Applied to rust-next -- thanks everyone!

As mentioned earlier, I have moved to 1.71.1 instead that got released meanwhile, since it does not require any changes w.r.t. this one except the obvious ones and it is likely the one that people would like to use/have (so less warnings).

Cheers,
Miguel