Re: [PATCH v4 4/4] rust: Add warn_on macro
From: Alice Ryhl <aliceryhl@google.com>
Date: 2025-04-08 13:55:52
Also in:
linux-riscv, lkml, loongarch, rust-for-linux
On Wed, Mar 5, 2025 at 12:10 PM FUJITA Tomonori [off-list ref] wrote:
Add warn_on macro, uses the BUG/WARN feature (lib/bug.c) via assembly for x86_64/arm64/riscv. The current Rust code simply wraps BUG() macro but doesn't provide the proper debug information. The BUG/WARN feature can only be used from assembly. This uses the assembly code exported by the C side via ARCH_WARN_ASM macro. To avoid duplicating the assembly code, this approach follows the same strategy as the static branch code: it generates the assembly code for Rust using the C preprocessor at compile time. Similarly, ARCH_WARN_REACHABLE is also used at compile time to generate the assembly code; objtool's reachable anotation code. It's used for only architectures that use objtool. For now, Loongarch just uses a wrapper for WARN macro. UML doesn't use the assembly BUG/WARN feature; just wrapping generic BUG/WARN functions implemented in C works. Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
Overall looks reasonable. A few nits below:
+#[macro_export]
+#[doc(hidden)]
+#[cfg(all(CONFIG_BUG, not(CONFIG_UML), not(CONFIG_LOONGARCH)))]
+#[cfg(CONFIG_DEBUG_BUGVERBOSE)]
+macro_rules! warn_flags {
+ ($flags:expr) => {
+ const FLAGS: u32 = $crate::bindings::BUGFLAG_WARNING | $flags;
+ const _FILE: &[u8] = file!().as_bytes();
+ // Plus one for null-terminator.
+ static FILE: [u8; _FILE.len() + 1] = {
+ let mut bytes = [0; _FILE.len() + 1];
+ let mut i = 0;
+ while i < _FILE.len() {
+ bytes[i] = _FILE[i];
+ i += 1;
+ }
+ bytes
+ };
+ // SAFETY: Just an FFI call.
+ unsafe {
+ $crate::asm!(concat!(
+ "/* {size} */",
+ include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_warn_asm.rs")),
+ include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_reachable_asm.rs")));
+ file = sym FILE,
+ line = const line!(),
+ flags = const FLAGS,
+ size = const ::core::mem::size_of::<$crate::bindings::bug_entry>(),The indentation here is odd. Shouldn't the arguments be indented one more?
+ );
+ }
+ }
+}
+
+#[macro_export]
+#[doc(hidden)]
+#[cfg(all(CONFIG_BUG, not(CONFIG_UML), not(CONFIG_LOONGARCH)))]
+#[cfg(not(CONFIG_DEBUG_BUGVERBOSE))]
+macro_rules! warn_flags {
+ ($flags:expr) => {
+ const FLAGS: u32 = $crate::bindings::BUGFLAG_WARNING | $flags;
+ // SAFETY: Just an FFI call.
+ unsafe {
+ $crate::asm!(
+ concat!(
+ "/* {size} */",
+ include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_warn_asm.rs")),
+ include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_reachable_asm.rs")));
+ flags = const FLAGS,
+ size = const ::core::mem::size_of::<$crate::bindings::bug_entry>(),Same thing here.
+ );
+ }
+ }
+}
+
+#[macro_export]
+#[doc(hidden)]
+#[cfg(all(CONFIG_BUG, CONFIG_UML))]
+macro_rules! warn_flags {
+ ($flags:expr) => {
+ // SAFETY: Just an FFI call.
+ unsafe {
+ $crate::bindings::warn_slowpath_fmt(
+ $crate::c_str!(::core::file!()).as_ptr() as *const ::core::ffi::c_uchar,
+ line!() as i32,
+ $flags as u32,
+ ::core::ptr::null() as *const ::core::ffi::c_uchar,Why unsigned char? I think this should be ::kernel::ffi::c_char to utilize the custom integer type definitions in kernel rather than the faulty ones in core. (Maybe $crate rather than kernel) Alice