`build_assert` relies on the compiler to optimize out its error path,
lest build fails with the dreaded error:

    ERROR: modpost: "rust_build_error" [path/to/module.ko] undefined!

It has been observed that very trivial code performing I/O accesses
(sometimes even using an immediate value) would seemingly randomly fail
with this error whenever `CLIPPY=1` was set. The same behavior was also
observed until different, very similar conditions [1][2].

The cause appears to be that the failing function is eventually using
`build_assert` with its argument, but is only annotated with
`#[inline]`. This gives the compiler freedom to not inline the function,
which it notably did when Clippy was active, triggering the error.

The fix is to annotate functions passing their argument to
`build_assert` with `#[inline(always)]`, telling the compiler to be as
aggressive as possible with their inlining. This is also the correct
behavior as inlining is mandatory for correct behavior in these cases.

Add a paragraph instructing to annotate such functions with
`#[inline(always)]` in `build_assert`'s documentation, and split its
example to illustrate.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 rust/kernel/build_assert.rs | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/build_assert.rs b/rust/kernel/build_assert.rs
index 6331b15d7c4d..f8124dbc663f 100644
--- a/rust/kernel/build_assert.rs
+++ b/rust/kernel/build_assert.rs
@@ -61,8 +61,13 @@ macro_rules! build_error {
 ///     build_assert!(N > 1); // Build-time check
 ///     assert!(N > 1); // Run-time check
 /// }
+/// ```
 ///
-/// #[inline]
+/// When a condition depends on a function argument, the function must be annotated with
+/// `#[inline(always)]`. Without this attribute, the compiler may choose to not inline the
+/// function, preventing it from optimizing out the error path.
+/// ```
+/// #[inline(always)]
 /// fn bar(n: usize) {
 ///     // `static_assert!(n > 1);` is not allowed
 ///     build_assert!(n > 1); // Build-time check

-- 
2.52.0

On 11/27/25 18:11, Alexandre Courbot wrote:

> `build_assert` relies on the compiler to optimize out its error path,
> lest build fails with the dreaded error:
>
>     ERROR: modpost: "rust_build_error" [path/to/module.ko] undefined!
>
> It has been observed that very trivial code performing I/O accesses
> (sometimes even using an immediate value) would seemingly randomly fail
> with this error whenever `CLIPPY=1` was set. The same behavior was also
> observed until different, very similar conditions [1][2].
>
> The cause appears to be that the failing function is eventually using
> `build_assert` with its argument, but is only annotated with
> `#[inline]`. This gives the compiler freedom to not inline the function,
> which it notably did when Clippy was active, triggering the error.
>
> The fix is to annotate functions passing their argument to
> `build_assert` with `#[inline(always)]`, telling the compiler to be as
> aggressive as possible with their inlining. This is also the correct
> behavior as inlining is mandatory for correct behavior in these cases.
>
> Add a paragraph instructing to annotate such functions with
> `#[inline(always)]` in `build_assert`'s documentation, and split its
> example to illustrate.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  rust/kernel/build_assert.rs | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/build_assert.rs b/rust/kernel/build_assert.rs
> index 6331b15d7c4d..f8124dbc663f 100644
> --- a/rust/kernel/build_assert.rs
> +++ b/rust/kernel/build_assert.rs
> @@ -61,8 +61,13 @@ macro_rules! build_error {
>  ///     build_assert!(N > 1); // Build-time check
>  ///     assert!(N > 1); // Run-time check
>  /// }
> +/// ```
>  ///
> -/// #[inline]
> +/// When a condition depends on a function argument, the function must be annotated with
> +/// `#[inline(always)]`. Without this attribute, the compiler may choose to not inline the
> +/// function, preventing it from optimizing out the error path.
> +/// ```
> +/// #[inline(always)]

The compiler may still choose to not inline the function, even under
`#[inline(always)]`:

"#[inline(always)] suggests that inline expansion should always be
performed." [1] 

"Note: In every form the attribute is a hint. The compiler may ignore
it." [also 1]

1: https://doc.rust-lang.org/reference/attributes/codegen.html

Regards,
Edwin Peer

On 11/27/25 6:11 PM, Alexandre Courbot wrote:
> `build_assert` relies on the compiler to optimize out its error path,
> lest build fails with the dreaded error:
> 
>      ERROR: modpost: "rust_build_error" [path/to/module.ko] undefined!
> 
> It has been observed that very trivial code performing I/O accesses
> (sometimes even using an immediate value) would seemingly randomly fail
> with this error whenever `CLIPPY=1` was set. The same behavior was also
> observed until different, very similar conditions [1][2].
> 
> The cause appears to be that the failing function is eventually using
> `build_assert` with its argument, but is only annotated with
> `#[inline]`. This gives the compiler freedom to not inline the function,
> which it notably did when Clippy was active, triggering the error.
> 
> The fix is to annotate functions passing their argument to
> `build_assert` with `#[inline(always)]`, telling the compiler to be as
> aggressive as possible with their inlining. This is also the correct
> behavior as inlining is mandatory for correct behavior in these cases.

Very interesting. So by adding a partially faulty build_assert!() call,
these functions were actually wrong when they created! Maybe a Fixes:
tag is warranted.


thanks,
-- 
John Hubbard

> 
> Add a paragraph instructing to annotate such functions with
> `#[inline(always)]` in `build_assert`'s documentation, and split its
> example to illustrate.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>   rust/kernel/build_assert.rs | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/build_assert.rs b/rust/kernel/build_assert.rs
> index 6331b15d7c4d..f8124dbc663f 100644
> --- a/rust/kernel/build_assert.rs
> +++ b/rust/kernel/build_assert.rs
> @@ -61,8 +61,13 @@ macro_rules! build_error {
>   ///     build_assert!(N > 1); // Build-time check
>   ///     assert!(N > 1); // Run-time check
>   /// }
> +/// ```
>   ///
> -/// #[inline]
> +/// When a condition depends on a function argument, the function must be annotated with
> +/// `#[inline(always)]`. Without this attribute, the compiler may choose to not inline the
> +/// function, preventing it from optimizing out the error path.
> +/// ```
> +/// #[inline(always)]
>   /// fn bar(n: usize) {
>   ///     // `static_assert!(n > 1);` is not allowed
>   ///     build_assert!(n > 1); // Build-time check
>

On Sun, Nov 30, 2025 at 10:44 PM John Hubbard <jhubbard@nvidia.com> wrote:
>
> Very interesting. So by adding a partially faulty build_assert!() call,
> these functions were actually wrong when they created! Maybe a Fixes:
> tag is warranted.

To clarify: it is the lack of optimization in certain configs (-Os,
CLIPPY=1...) as well as possibly certain code patterns that may
trigger it, not that the calls were faulty (note that `always` doesn't
guarantee it either anyway).

Daniel suggested Fixes in #0 -- if any of these trigger a build error
(like the `Bounded` one), then yeah. Cc: stable@ too.

Thanks!

Cheers,
Miguel

On 11/30/25 1:56 PM, Miguel Ojeda wrote:
> On Sun, Nov 30, 2025 at 10:44 PM John Hubbard <jhubbard@nvidia.com> wrote:
>>
>> Very interesting. So by adding a partially faulty build_assert!() call,
>> these functions were actually wrong when they created! Maybe a Fixes:
>> tag is warranted.
> 
> To clarify: it is the lack of optimization in certain configs (-Os,
> CLIPPY=1...) as well as possibly certain code patterns that may
> trigger it, not that the calls were faulty (note that `always` doesn't

It seems pretty clear that if one writes a *build* assertion about
a function argument, then that is just conceptually wrong unless it
is inlined. Because it can only really be a run-time assertion.

This is what Alex pointed out, and looking at the code I agree.

Thoughts?


> guarantee it either anyway).

Yes, understood. So maybe "Fixes" is too strong. It's more like
"Mitigates:".  :)


thanks,
-- 
John Hubbard

> 
> Daniel suggested Fixes in #0 -- if any of these trigger a build error
> (like the `Bounded` one), then yeah. Cc: stable@ too.
> 
> Thanks!
> 
> Cheers,
> Miguel

On Sun, Nov 30, 2025 at 11:01 PM John Hubbard <jhubbard@nvidia.com> wrote:
>
> It seems pretty clear that if one writes a *build* assertion about
> a function argument, then that is just conceptually wrong unless it
> is inlined. Because it can only really be a run-time assertion.
>
> This is what Alex pointed out, and looking at the code I agree.

No, the function here was already inline.

What Alexandre wrote, which is correct, is that the fix is about
asking for *more* inlining.

The build assertion itself is fine. What is "wrong" is that the
inlining wasn't enough.

Nevertheless, it is (or at least some of these are) definitely a "fix"
in the sense that it did fix cases we hit where the inlining wasn't
enough, like Clippy ones which may change codegen (which in turn is
why we say it cannot be used in "production" kernel builds:
https://github.com/rust-lang/rust-clippy/pull/8037 -- back then it
disabled MIR optimizations).

Cheers,
Miguel

On 11/30/25 2:42 PM, Miguel Ojeda wrote:
> On Sun, Nov 30, 2025 at 11:01 PM John Hubbard <jhubbard@nvidia.com> wrote:
>>
>> It seems pretty clear that if one writes a *build* assertion about
>> a function argument, then that is just conceptually wrong unless it
>> is inlined. Because it can only really be a run-time assertion.
>>
>> This is what Alex pointed out, and looking at the code I agree.
> 
> No, the function here was already inline.

More precisely, it was already *hinted* to be inline.

> 
> What Alexandre wrote, which is correct, is that the fix is about
> asking for *more* inlining.
> 
> The build assertion itself is fine. What is "wrong" is that the
> inlining wasn't enough.

I'm having a difficult time with that statement, because if you
write:

fn bar(n: usize) {
    build_assert!(n > 1);
    ...
}

Then that is conceptually wrong, because it must be a runtime check.

The only way it can be a compile-time check is if you have some
way to *guarantee* that the function is inlined into code that has
a const n.

Absent such guarantees (and we have "nearly none", right?), we have
been writing "partly wrong" code in all such cases.

Why? Are the guarantees stronger than they look? Or other reasoning?

> 
> Nevertheless, it is (or at least some of these are) definitely a "fix"
> in the sense that it did fix cases we hit where the inlining wasn't
> enough, like Clippy ones which may change codegen (which in turn is
> why we say it cannot be used in "production" kernel builds:
> https://github.com/rust-lang/rust-clippy/pull/8037 -- back then it
> disabled MIR optimizations).
> 

Sorry for the fussy detailed questioning here. I'm trying to bottom
out here because CLIPPY=1 is a very solid requirement before posting
patches. 


thanks,
-- 
John Hubbard

On Mon, Dec 1, 2025 at 1:52 AM John Hubbard <jhubbard@nvidia.com> wrote:
>
> More precisely, it was already *hinted* to be inline.

By inline I mean it is marked `#[inline]`, which may or may not get
inlined, but it also has other implications, e.g. codegen can get
delayed even if there are no callers and is concrete.

> Then that is conceptually wrong, because it must be a runtime check.

No, it is not true it must be a runtime check -- it depends: you can
use such a function in some cases just fine.

That is the point of `build_assert!`, after all.

> Sorry for the fussy detailed questioning here. I'm trying to bottom
> out here because CLIPPY=1 is a very solid requirement before posting
> patches.

No worries, but I don't follow what you mean here. `CLIPPY=1` is still
required to be clean etc.

Cheers,
Miguel

On 11/30/25 7:44 PM, Miguel Ojeda wrote:
> On Mon, Dec 1, 2025 at 1:52 AM John Hubbard <jhubbard@nvidia.com> wrote:
>>
>> More precisely, it was already *hinted* to be inline.
> 
> By inline I mean it is marked `#[inline]`, which may or may not get
> inlined, but it also has other implications, e.g. codegen can get
> delayed even if there are no callers and is concrete.
> 
>> Then that is conceptually wrong, because it must be a runtime check.
> 
> No, it is not true it must be a runtime check -- it depends: you can
> use such a function in some cases just fine.
> 
> That is the point of `build_assert!`, after all.

This seems strange: something called build_assert!() should not be
put in places where it might not be possible to verify at build-time.
It's built right into the name.

> 
>> Sorry for the fussy detailed questioning here. I'm trying to bottom
>> out here because CLIPPY=1 is a very solid requirement before posting
>> patches.
> 
> No worries, but I don't follow what you mean here. `CLIPPY=1` is still
> required to be clean etc.
> 

CLIPPY=1 is effectively part of the developers' tool chain. It is also
one of the ways to break the build_assert!() call sites.

So now we have to go around and annotate the functions that *contain*
uses of build_assert!(). Otherwise we end up with an unreliable tool
chain for developers. This is not where we should want to be, in the
long run.

Is there proc macro magic we can come up with? Or rustc or clippy
changes? So that this is becomes a better foundation upon which to
build?

thanks,
-- 
John Hubbard

On Mon, Dec 1, 2025 at 5:36 AM John Hubbard <jhubbard@nvidia.com> wrote:
>
> This seems strange: something called build_assert!() should not be
> put in places where it might not be possible to verify at build-time.
> It's built right into the name.

The build prefix means the assert is done at build time, not that it
can always be verified. Even other asserts could also "fail" in a
certain sense (diverging, conditional compilation...).

Detecting "unreasonable" uses that are likely to fail would be nice, of course.

> So now we have to go around and annotate the functions that *contain*
> uses of build_assert!().

Only for those that need it.

> Otherwise we end up with an unreliable tool
> chain for developers. This is not where we should want to be, in the
> long run.

It is not a black or white issue. Conditional compilation also breaks
if someone misuses it, and that alone doesn't mean we stop using it.
It is a tradeoff at the end of the day.

Nevertheless, also note that the C side also relies on optimizations.

> Is there proc macro magic we can come up with? Or rustc or clippy
> changes? So that this is becomes a better foundation upon which to
> build?

Converting more code to macros has their own set of tradeoffs, but it
depends on what you mean. Do you have something in mind?

And yes, I have had it in our usual lists for a long time and we
mentioned it to upstream Rust and so on. We are well aware that
`build_assert!` isn't ideal, and in many cases it is best to avoid it
when there is a better approach.

Now, if a company has the means to improve the situation, e.g. by
sponsoring someone upstream to work on features like this, then by all
means, please go ahead! That would be very welcome, and we have some
contacts that could be interested in working on things like that, so
please feel free to ping.

Cheers,
Miguel

On 12/1/25 8:43 AM, Miguel Ojeda wrote:
> On Mon, Dec 1, 2025 at 5:36 AM John Hubbard <jhubbard@nvidia.com> wrote:
...
>> Is there proc macro magic we can come up with? Or rustc or clippy
>> changes? So that this is becomes a better foundation upon which to
>> build?
> 
> Converting more code to macros has their own set of tradeoffs, but it
> depends on what you mean. Do you have something in mind?

Mainly just: is there a way to automatically "derive" (generate) an
always-inline directive for any function that attempts to call
build_assert!() on any of its arguments? And in fact, *force* the
always-inline, if it is not forced hard enough today.

Something along those lines.

> 
> And yes, I have had it in our usual lists for a long time and we
> mentioned it to upstream Rust and so on. We are well aware that
> `build_assert!` isn't ideal, and in many cases it is best to avoid it
> when there is a better approach.
> 
> Now, if a company has the means to improve the situation, e.g. by
> sponsoring someone upstream to work on features like this, then by all
> means, please go ahead! That would be very welcome, and we have some
> contacts that could be interested in working on things like that, so
> please feel free to ping.
> 

I will bring this up (along with the KSYM_NAME_LEN hashed symbol project)
to our internal Rust groups. Both of these seem like nice, self-contained
projects that someone could really get into.

thanks,
-- 
John Hubbard