Re: [PATCH v1 5/5] rust: Add warn_on and warn_on_once

[Alice Ryhl]

On Wed, Dec 11, 2024 at 1:41 AM FUJITA Tomonori
<fujita.tomonori@xxxxxxxxx> wrote:
>
> On Tue, 10 Dec 2024 10:05:07 +0100
> Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote:
>
> > On Tue, Dec 10, 2024 at 1:19 AM FUJITA Tomonori
> > <fujita.tomonori@xxxxxxxxx> wrote:
> >>
> >> Add warn_on and warn_on_once macros. Wrapping the C's WARN_* and BUG_*
> >> macros doesn't work so this uses the assembly code exported by the C
> >> side via ARCH_WARN_ASM macro. Like the static branch code, this
> >> generates the assembly code for rust dynamically by using the C
> >> preprocessor.
> >>
> >> file()! macro doesn't work for the Rust inline assembly in the same
> >> way as __FILE__ for the C inline assembly. So the code to handle a
> >> file name is different from the C assembly code (similar to the
> >> arm64/loongarch assembly).
> >>
> >> ASM_REACHABLE definition works in the same way to get objtool's
> >> reachable asm code. The architectures which use objtool (x86 and
> >> loongarch) needs it.
> >>
> >> Signed-off-by: FUJITA Tomonori <fujita.tomonori@xxxxxxxxx>
> >
> >> +#[macro_export]
> >> +#[doc(hidden)]
> >> +#[cfg(all(CONFIG_BUG, not(CONFIG_UML)))]
> >> +#[cfg(any(target_arch = "x86_64", target_arch = "riscv64"))]
> >
> >> +#[macro_export]
> >> +#[doc(hidden)]
> >> +#[cfg(all(CONFIG_BUG, not(CONFIG_UML)))]
> >> +#[cfg(any(target_arch = "aarch64", target_arch = "loongarch64"))]
> >
> > What's the reason for this arch-specific code? The file!()/line!()
> > invocations? Could they be passed as an argument to the asm instead so
> > that we don't need target_arch cfgs? I understand that they don't work
> > exactly the same way, but maybe it could still work?
>
> Because of "error: named argument never used" in Rust inline assembly:
>
> All the archs define ARCH_WARN_ASM macro in the same way:
>
> #define ARCH_WARN_ASM(file, line, flags, size)
>
> However, only x86 and risc-v asm code use the size argument. Without
> the cfgs, I'll get the following on arm64/loongarch:
>
> error: named argument never used
>   --> /home/fujita/git/linux-rust/drivers/block/rnull.rs:54:9
>    |
> 54 |         warn_on!(true);
>    |         ^^^^^^^^^^^^^^ named argument never used
>    |
>    = help: if this argument is intentionally unused, consider using it in an asm comment: `"/* {size} */"`
>    = note: this error originates in the macro `$crate::warn_flags` which comes from the expansion of the macro `warn_on` (in Nightly builds, run with -Z macro-backtrace for more info)
>
>
> Any way to make the compiler to ignore this?

The error message suggests adding an asm comment. Does that not work?

You could even add the comment unconditionally on the Rust side. It's
not like the comment hurts on the platforms that *do* use the size
parameter.

> >> +#[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_char,
> >> +                line!() as i32,
> >> +                $flags as u32,
> >> +                ::core::ptr::null() as *const ::core::ffi::c_char,
> >
> > I wonder if this could be written to utilize Location::caller()
> > instead so that `#[track_caller]` works?
>
> You meant that we could make warn_flags() function instead of macro
> with Location::caller()?
>
> If so, we need to add cfgs to warn_on and warn_on_once because both macro
> and function of warn_flags are necessary?

Well, I'm not sure! I don't know if it's feasible at all, since using
Location::caller() would mean that the file/line is not a compile-time
constant. But if we can, then I think #[track_caller] support would be
nice.

Alice