Build-time assertion

[nbdd0121]

Add link_time_panic and link_time_assert!. link_time_panic is defined as a const but #[inline(never)] function, so it panics when used in const context, but will not be inlined. This crate is not used when linking, so if optimiser cannot optimize away a callsite a link error is generated.

[bjorn3]

This crate is not used when linking, so if optimiser cannot optimize away a callsite a link error is generated.

Usage of this macro will very likely break compiling without optimizations. In addition certain (future) versions of rustc/LLVM may not be able to optimize the call sites away.

[bjorn3]

You could downgrade this crate to the 2015 edition and silence the warning for panic!(msg). Past editions are forever supported by rustc. Rust-analyzer barely has support for it, but this crate is simple enough that I don't think rust-analyzer has any problems with it. Maybe it resolves panic!() to the 2021 edition version and gives an error, but it doesn't show errors for closed files anyway, so it isn't very distracting.

[nbdd0121]

Currently all crates have editions specified by a global rustc_flags, and --edition flag couldn't be specified more than once.

[alex]

It does look like the .o is linked into kernel.o, is the idea that it's linked there, but not into the final .ko?

[nbdd0121]

kernel.o has link_time_panic.o as a Make dependency, but it will not link against it.

[ojeda]

Isn't this static_assert!?

static_assert! cannot be used in your relax_bound in the other PR, though, because of the painful error[E0401]: can't use generic parameters from outer function. However, I would simply make it a panic! since we already have const_panic.

[nbdd0121]

Isn't this static_assert!?

static_assert! cannot be used in your relax_bound in the other PR, though, because of the painful error[E0401]: can't use generic parameters from outer function. However, I would simply make it a panic! since we already have const_panic.

static_assert!'s argument is in const context, i.e. it cannot refer to generic parameters. Using const panic! however cannot guarantee that it will not leak through into the final binary. More about const context vs const fn here: https://doc.rust-lang.org/reference/const_eval.html. link_time_panic can be used anywhere panic! can be used, const context, const fn and even non-const fns, so it is more versatile. When used in const contexts, it behaves just like a panic!.

The reason to have link_time_panic arises from Linus Torvalds's opinion that anything that could be statically checked in compile time should not be checked in the runtime, so that a bug is catched as early as possible. The idea of having a link error to replace a runtime error came from @wedsonaf's email to Linus Walleij in the mailing list, https://lore.kernel.org/rust-for-linux/YIbQ3dHOpyD%[email protected]/. But instead of a non-existing extern symbol I put it in a never-linked crate, and make it a const fn, so it could be used in const contexts and const fns.

Usage of this macro will very likely break compiling without optimizations.

I am not worried about turning off optimisations. Using non-existing symbol and rely on dead code elimination to remove it is a common hack widely used in the kernel's C code already AFAIK (trying to compile the kernel with O0 will give you link errors), so I think it's okay to do the same thing in the Rust side as well. Plus, you'll never want to turn off optimisation for kernel or any large project in the first place, as it will be painfully slow and large.

In addition certain (future) versions of rustc/LLVM may not be able to optimize the call sites away.

I'll consider it as a P-critical I-slow regression for rustc/LLVM if it cannot fold away constants, if it ever happens, so I'm not particularly worried about it. Even if it happens, we can simply link link_time_panic.o temporarily while the regression is being fixed. Even if link_time_panic.o is indeed being linked it gives a way to tell apart which panic is really not supposed to happen when analysing the binary.

[bjorn3]

I am not worried about turning off optimisations. Using non-existing symbol and rely on dead code elimination to remove it is a common hack widely used in the kernel's C code already AFAIK (trying to compile the kernel with O0 will give you link errors), so I think it's okay to do the same thing in the Rust side as well. Plus, you'll never want to turn off optimisation for kernel or any large project in the first place, as it will be painfully slow and large.

You may want to for better debuginfo. It can be hard to wade through heaps of inline functions. These are much more common in Rust than C due to favoring zero-cost abstractions over no abstractions at all. In addition you don't necessarily have to disable optimizations for everything. Disabling it just for the component you are debugging makes sense.

I'll consider it as a P-critical I-slow regression for rustc/LLVM if it cannot fold away constants, if it ever happens, so I'm not particularly worried about it.

The conditional that should be folded away may not be trivial in which case it is pretty much luck if it is being folded away at all. Any small change to a completely unrelated optimization pass may prevent folding away the conditional. It may even be caused by another optimization getting smarter and optimizing the program better overall. Or maybe a function in the standard library or elsewhere stops being marked as #[inline] because inlining it has been shown to reduce performance. In that case LTO would be required for there being any possibility of folding the conditional away. It may also be that enabling two rust kernel modules at the same time when using LTO would cause LLVM to consider inlining a particular function that would be favorable to inline when only one of those kernel modules is compiled to no longer be favorable when both kernel modules are compiled, as said function for example now gets called twice instead of once.

Even if it happens, we can simply link link_time_panic.o temporarily while the regression is being fixed.

I guess so.

Using non-existing symbol and rely on dead code elimination to remove it is a common hack widely used in the kernel's C code already AFAIK (trying to compile the kernel with O0 will give you link errors), so I think it's okay to do the same thing in the Rust side as well.

If it is already commonly used by C in the kernel I guess it is fine to do the same for rust. 🤷

[ojeda]

static_assert!'s argument is in const context, i.e. it cannot refer to generic parameters.

Currently, yes. But conceptually, this is intended for the same purpose as static_assert!. I do not care how it is implemented, but I do not understand why you want to have two different implementations.

The new name is also wrong, because it leaks implementation details.

Using const panic! however cannot guarantee that it will not leak through into the final binary.

const_panic or otherwise, it does not change how this works. The approach of this PR uses a simple conditional that is expected to be optimized out, the same as I suggested.

The difference is that a simple panic!() would still work for non-optimized builds, while yours would fail to build. This is important, because we "support" -O0 and -O1 so far (not for production, just under the hacking menu, but nevertheless, it is there).

We can discuss removing support for those builds, that is fine and it would not be the first time, but it is not the same discussion.

More about const context vs const fn here: https://doc.rust-lang.org/reference/const_eval.html. link_time_panic can be used anywhere panic! can be used, const context, const fn and even non-const fns, so it is more versatile. When used in const contexts, it behaves just like a panic!.

Yes, that is understood -- it just uses a simple conditional. But it is not what I was talking about.

The reason to have link_time_panic arises from Linus Torvalds's opinion that anything that could be statically checked in compile time should not be checked in the runtime, so that a bug is catched as early as possible.

Not sure who you are trying to convince here -- I added static_assert! to the project...

In fact, it is not really an "opinion", it is almost an engineering fact at this point. And it is not news either, in C++ testing conditions like this for similar cases (like templates with NTTPs) is everyday business.

The idea of having a link error to replace a runtime error came from @wedsonaf's email to Linus Walleij in the mailing list, https://lore.kernel.org/rust-for-linux/YIbQ3dHOpyD%[email protected]/. But instead of a non-existing extern symbol I put it in a never-linked crate, and make it a const fn, so it could be used in const contexts and const fns.

I am not really sure what your point is with this paragraph either, but the link error suggestion was mentioned by Torvalds many days before that: https://lore.kernel.org/rust-for-linux/CAHk-=wgJvJJtd2mpYpx5+zn_hPrSOqGqi-Pxb7e+h+anhsLnQg@mail.gmail.com/ (and it is not a new idea).

[ojeda]

I am not worried about turning off optimisations. Using non-existing symbol and rely on dead code elimination to remove it is a common hack widely used in the kernel's C code already AFAIK (trying to compile the kernel with O0 will give you link errors), so I think it's okay to do the same thing in the Rust side as well.

Yes, but that change needs to be discussed, because as I mentioned above, this is currently "supported".

Plus, you'll never want to turn off optimisation for kernel or any large project in the first place, as it will be painfully slow and large.

Not true: under QEMU, a pass over -O0 samples take a couple tenths of a second. From boot to reboot, it takes less than 2 seconds in my machine. Remember that we do not need to compile the entire kernel like that. And there are use cases for compiling without optimizations (or less optimizations).

[nbdd0121]

Currently, yes. But conceptually, this is intended for the same purpose as static_assert!. I do not care how it is implemented, but I do not understand why you want to have two different implementations.

It is not. It you take a look at the snippet I mentioned in the mailing list:

impl<const SIZE: usize> IoMemBlock<SIZE> {
    pub fn write<T>(&self, value: T, offset: usize) {
        if let Some(end) = offset.checked_add(size_of::<T>()) {
            if end <= SIZE {
                // SAFETY: We just checked above that offset was within bounds.
                let ptr = unsafe { self.ptr.add(offset) } as *mut T;
                // SAFETY: We just checked that the offset+size was within bounds.
                unsafe { ptr.write_volatile(value) };
                return;
            }
        }
        // SAFETY: Unimplemented function to cause compilation error.
        unsafe { bad_write() };
    }
}

You could use link_time_assert! in this scenario:

impl<const SIZE: usize> IoMemBlock<SIZE> {
    pub fn write<T>(&self, value: T, offset: usize) {
        if let Some(end) = offset.checked_add(size_of::<T>()) {
            if end <= SIZE {
                // SAFETY: We just checked above that offset was within bounds.
                let ptr = unsafe { self.ptr.add(offset) } as *mut T;
                // SAFETY: We just checked that the offset+size was within bounds.
                unsafe { ptr.write_volatile(value) };
                return;
            }
        }
        link_time_panic("write out of bound");
    }
}

Or you could panic!, but you cannot use a static_assert! here because this is not const. It relies on knowledge that LLVM knows but not rustc.

static_assert! works only in compilation time, panic! works in compilation time in const context, but in runtime if not. There is currently no middle ground. link_time_assert! works in compilation time in const context and rely on LLVM+linker to prevent the panic from being carried to runtime.

The new name is also wrong, because it leaks implementation details.

I think it's good to be explicit about implementation detail here. You don't really have to hide it because everything is tweakable in the kernel, it's not like this is going to be used by external library.

The difference is that a simple panic!() would still work for non-optimized builds, while yours would fail to build. This is important, because we "support" -O0 and -O1 so far (not for production, just under the hacking menu, but nevertheless, it is there).

We can discuss removing support for those builds, that is fine and it would not be the first time, but it is not the same discussion.

I don't think it is necessary to support -O0, kernel C code does not support -O0 anyway. O1 can fold away constants but don't inline non-#[inline(always)] functions, so I think it is good enough. Ideally we'd have something similar to -Og in GCC, but 🤷.

Not sure who you are trying to convince here -- I added static_assert! to the project...

In fact, it is not really an "opinion", it is almost an engineering fact at this point. And it is not news either, in C++ testing conditions like this for similar cases (like templates with NTTPs) is everyday business.

link_time_assert! cannot replace static_assert!. link_time_assert! will cause an error in link time even if condition can be evaluated in const context. link_time_assert! creates a compile time error only if itself is evaluated in const context.

fn foo() {
    static_assert!(1 > 1); // Compile-time error
    link_time_assert!(1 > 1); // Link-time error
    assert!(1 > 1); // Run-time error
}

const fn bar1<const N: usize>() {
    // static_assert!(N > 1); is not allowed
    link_time_assert!(N > 1);
}

const fn bar2<const N: usize>() {
    assert!(N > 1);
}

const A1: () = bar1::<1>(); // Compile-time error
const A2: () = bar2::<1>(); // Compile-time error

fn call_bar() {
    bar(1); // Link-time error
    bar(2); // Run-time error
}

Obviously the goal is to catch error as early as possible. So static_assert if preferred if possible, link_time_assert second, and assert is only used as the last resort.

The idea of having a link error to replace a runtime error came from @wedsonaf's email to Linus Walleij in the mailing list, https://lore.kernel.org/rust-for-linux/YIbQ3dHOpyD%[email protected]/. But instead of a non-existing extern symbol I put it in a never-linked crate, and make it a const fn, so it could be used in const contexts and const fns.

I am not really sure what your point is with this paragraph either, but the link error suggestion was mentioned by Torvalds many days before that: https://lore.kernel.org/rust-for-linux/CAHk-=wgJvJJtd2mpYpx5+zn_hPrSOqGqi-Pxb7e+h+anhsLnQg@mail.gmail.com/ (and it is not a new idea).

I quoted this email precisely because it's a use case where only link_time_panic can offer.

[ojeda]

Currently, yes. But conceptually, this is intended for the same purpose as static_assert!. I do not care how it is implemented, but I do not understand why you want to have two different implementations.

It is not. It you take a look at the snippet I mentioned in the mailing list:

It is. Let me quote your docs:

/// Asserts that a boolean expression is `true` at compile time.

and the current ones:

/// Static assert (i.e. compile-time assert).

Please tell me what is the difference.

What we want is to fail the build under a given compile-time condition. Both macros aim to do that. The fact that the current implementation does not work in some contexts does not change that.

[...]

You could use link_time_assert! in this scenario:

[...]

Or you could panic!, but you cannot use a static_assert! here because this is not const. It relies on knowledge that LLVM knows but not rustc.

static_assert! works only in compilation time, panic! works in compilation time in const context, but in runtime if not. There is currently no middle ground. link_time_assert! works in compilation time in const context and rely on LLVM+linker to prevent the panic from being carried to runtime.

This is (again) an explanation on why static_assert! cannot be used in all contexts. And, again, it does not explain why we would want both.

The new name is also wrong, because it leaks implementation details.

I think it's good to be explicit about implementation detail here.

Why would we want to be explicit? It is rarely a good idea to expose implementation details for no reason.

You don't really have to hide it because everything is tweakable in the kernel

Sure, but that is not an argument for exposing implementation details, just that we could potentially do it.

it's not like this is going to be used by external library.

rust/ code is pretty much the definition of a "library" in our context -- it is going to be used by everyone else in the kernel, same as include/ bits are.

The difference is that a simple panic!() would still work for non-optimized builds, while yours would fail to build. This is important, because we "support" -O0 and -O1 so far (not for production, just under the hacking menu, but nevertheless, it is there).
We can discuss removing support for those builds, that is fine and it would not be the first time, but it is not the same discussion.

I don't think it is necessary to support -O0, kernel C code does not support -O0 anyway. O1 can fold away constants but don't inline non-#[inline(always)] functions, so I think it is good enough. Ideally we'd have something similar to -Og in GCC, but .

It is not necessary, but that is not an argument for removing it.

Supporting this implementation is an argument, but before restricting what we can do optimization-flags-wise just for this, I would prefer having conditional compilation on this, so that for optimized builds the link error approach is used, and for others, the panic()! one. That would get us the best of both worlds.

Not sure who you are trying to convince here -- I added static_assert! to the project...
In fact, it is not really an "opinion", it is almost an engineering fact at this point. And it is not news either, in C++ testing conditions like this for similar cases (like templates with NTTPs) is everyday business.

link_time_assert! cannot replace static_assert!. link_time_assert! will cause an error in link time even if condition can be evaluated in const context. link_time_assert! creates a compile time error only if itself is evaluated in const context.

I am not sure why you are replying with this here: in the part you quote, I was saying that you do not need to convince me about compile-time errors being better than runtime ones -- I have been convinced for many years and I am an avid user of static_assert in C and C++ (and I would like Rust to have built-in support for it too).

Anyway: the code you wrote does not show it cannot replace it. In fact, it shows the opposite: what you cannot do is replace link_time_assert! with static_assert!.

Obviously the goal is to catch error as early as possible. So static_assert if preferred if possible, link_time_assert second, and assert is only used as the last resort.

This is the argument I was asking for.

So, if I understand your point of view, you are saying it is better to have two macros so that we can have compile-time errors instead of link-time errors (in a subset of cases), no?

If yes: what are the advantages? Saving a few seconds building invalid code? Not worth it at all. Better diagnostics for invalid code (for the subset of cases where both macros would work)? Worth it, but not sure if worth enough to keep two macros.

A few questions:

• Is there an actual case where link_time_panic! cannot prevent the build from finishing where static_assert! could?

• Is it possible to build the macro in a way that chooses the right one (e.g. to detect the context where it is called)? I assume not, but given C++ has introduced things like is_constant_evaluated...

• Is Rust going to lift the restriction on using generic parameters in const contexts?

• Are there any plans in Rust to reintroduce static_assert back? (e.g. Feature Request static asserts rust-lang/rfcs#2790).

I quoted this email precisely because it's a use case where only link_time_panic can offer.

The use case is clear: as I mentioned in the first message, you have one in the other PR (relax_bounds)...

[nbdd0121]

Why would we want to be explicit? It is rarely a good idea to expose implementation details for no reason.

That's just my personal preference, I am okay with any reasonable name anyway, so feel free to give it a new name.

Supporting this implementation is an argument, but before restricting what we can do optimization-flags-wise just for this, I would prefer having conditional compilation on this, so that for optimized builds the link error approach is used, and for others, the panic()! one. That would get us the best of both worlds.

That sounds reasonable.

So, if I understand your point of view, you are saying it is better to have two macros so that we can have compile-time errors instead of link-time errors (in a subset of cases), no?

Okay, I see the root of confusion here. When I mention 'compile-time' in the discussion, I was referring strictly about the invocation of rustc, not including the linking part. I should be more clear about that.

If yes: what are the advantages? Saving a few seconds building invalid code? Not worth it at all. Better diagnostics for invalid code (for the subset of cases where both macros would work)? Worth it, but not sure if worth enough to keep two macros.

We might want to introduce similar things to cargo check (aka rustc -Z no-codegen) to speed up development process. static_assert! in the current form can catch that, but link_time_assert! would not. Also, as you mentioned, better diagnostics (static_assert! produced way better diagnostics with stack traces, but link_time_assert! can only show you the caller).

Is there an actual case where link_time_panic! cannot prevent the build from finishing where static_assert! could?

Yes. If link_time_assert! is itself dead code, then it would not prevent the build while static_assert! still would. I believe this case will be rare though. Also static_assert can appear anywhere an item can (e.g. inside a module), but link_time_assert! is an expression so it can appear in fewer places.

Is it possible to build the macro in a way that chooses the right one (e.g. to detect the context where it is called)? I assume not, but given C++ has introduced things like is_constant_evaluated...

No, at least not yet.

Is Rust going to lift the restriction on using generic parameters in const contexts?

I doubt this restriction will be lifted, at least not in the near future. Unlike C++ templates, generics are type-checked ahead of time, not when they're monomorphized.

The use case is clear: as I mentioned in the first message, you have one in the other PR (relax_bounds)...

It's different. The relax_bound use case can be expressed with C++'s static_assert as the condition , but that use case couldn't.

[nbdd0121]

Would it be possible to have LD warn about all usages of a function but still proceed to link it?

[ojeda]

(Gary and I talked about this offline because it seems we misunderstood each other a bit)

If link_time_assert! is itself dead code, then it would not prevent the build while static_assert! still would. I believe this case will be rare though. Also static_assert can appear anywhere an item can (e.g. inside a module), but link_time_assert! is an expression so it can appear in fewer places.

These two are very good points and I think it seals the deal taken together with the diagnostics.

I am okay with any reasonable name anyway, so feel free to give it a new name.

Initially I thought about static_assert_generics! or something like that, but Gary suggested build_assert/error! or similar, which sounds fine to me (i.e. what the person writing the code wants is to fail the build, somehow, linker error or otherwise).

[wedsonaf]

Initially I thought about static_assert_generics! or something like that, but Gary suggested build_assert/error! or similar, which sounds fine to me (i.e. what the person writing the code wants is to fail the build, somehow, linker error or otherwise).

FWIW, I like the build_ prefix as it gives as intuitive indication of what this is meant for.

[nbdd0121]

Tried adjusting CI configs and adding exports_build_assert_generated.h, but still getting Inconsistent kallsyms data which I could not reproduce locally.

[nbdd0121]

I've added a choice to Kconfig.debug to control the behaviour of build_panic and build_assert.

• RUST_BUILD_ASSERT_ALLOW will link build_assert.o, so the check will be run-time.
• RUST_BUILD_ASSERT_WARN is similar to ALLOW, but it will also add a .gnu.warning section so that GNU ld would generate a warning.
• RUST_BUILD_ASSERT_DENY will not link build_assert.o, so the linker will reject.

Meanwhile CI still gives me Inconsistent kallsyms data.

[ojeda]

Do we want to mention build_error (here and everywhere else)? Should a user ever call it?

[nbdd0121]

I think we do. It might be called after using checked math ops to unwrap Option.

[nbdd0121]

Several changes:

• Now depends on rust: Run rustdoc for the target triple #272.
• build_error crate now only have a single build_error function, the rest is moved to kernel crate.
• Make build_error a hidden function, and then add a build_error! to mimick panic.
• Re-export build_assert! in prelude
• Examples added.
• Make print.rs use build_assert!

[nbdd0121]

Seems to be a transient failure, retry CI

[ojeda]

Is this one related or is another cleanup?

[nbdd0121]

This is in a separate commit. I couldn't build on my machine without this change, but it is trivial enough that I think it might not worth a separate PR.

[ojeda]

• Make print.rs use build_assert!

This does not seem to be here, is it intentional or you wanted to move it to another PR?

[ojeda]

It looks pretty good!

[nbdd0121]

• Make print.rs use build_assert!

This does not seem to be here, is it intentional or you wanted to move it to another PR?

I am doing some other work on print.rs so I removed it from this PR.

[nbdd0121]

Thanks for the grammar fixes, changes incorporated and squashed

[ksquirrel]

Review of 5043400eeca3:

• ✔️ Commit 478bc95: Looks fine!
• ✔️ Commit b8b9cab: Looks fine!
• ✔️ Commit 5043400: Looks fine!

[ojeda]

Let's get this in!