Lint: enable empty_loop for no_std crates

[josephlr]

Right now, we have a style lint against the following code:

loop {}

Note that this is no longer undefined behavior (since rust-lang/rust#77972).

Currently, there is no way to have a no_std binary or library use the empty_loop lint.
Even adding #![deny(clippy::empty_loop)] to a crate does nothing. This is because #5086 (fixing #3746), made the lint essentially not exist for no_std crates.

Furthermore, even in no_std crates, using a "hot" deadloop is almost always not what you want (as it burns a bunch of CPU, and can cause crashes). Even on a no_std target, a user should either:

• Panic
• Call a platform-specific halt or pause intrinsic.

Note, core::sync::atomic::spin_loop_hint is not a good recommendation here, as pause isn't meant for dead-loops (it's designed for spin-locks). See rust-lang/rust#77924

In my opinion, we should:

• Reenable the empty_loop lint for no_std crates.
• Have a no_std specific help message.
• Disable this lint automatically in #[panic_handler]s
  • As an alternative, we could just disable this lint for no_std binaries (while keeping it on for no_std libraries).
  • The only uses of deadloops in the Rustonomicon are inside of #[panic_handler].

Our documentation for fixing this (on std and no_std) is part of #6162

CC (people involved with this change last time): @therealprof, @oli-obk, @eddyp, @Areredify, @phansch

[jonas-schievink]

core::sync::atomic::spin_loop_hint does not help here, it is a no-op on most embedded platforms and never causes side-effects

[josephlr]

core::sync::atomic::spin_loop_hint does not help here, it is a no-op on most embedded platforms and never causes side-effects

Agreed that this doesn't completely fix the issue. That hint currently only does something on:

• x86/x86_64 with SSE2
• ARM V6 and later (including Aarch64)

It would be nice if that hint did fix the issue. I'll see about filing an issue upstream to make it so core::sync::atomic::spin_loop_hint always does some sort of side-effect.

[jonas-schievink]

IMO we should just fix the LLVM bugs instead. spin_loop_hint is not meant to do anything here, it's not meant to have a side-effect, it's just a hint to schedule another hyperthread.

[josephlr]

@jonas-schievink I talk a bit in rust-lang/rust#77924 about why we might want spin_loop_hint to have a side-effect on all platforms. Basically, it makes our recommendations to users better, and makes the behavior more consistent across targets (as spin_loop_hint is a side-effect on x86 and (most) ARM targets).

[jonas-schievink]

It does not have side effects on any target. LLVM might treat it as such, but really it doesn't.

[josephlr]

It does not have side effects on any target. LLVM might treat it as such, but really it doesn't.

I guess I'm confused, for the current implementation on x86, the hint alls the pause instruction, which is a side-effect for the purposes of optimization and the like.

[josephlr]

I've updated the description for this issue to make it clear this is no longer about undefined behavior. Now, this issue just tracks re-enabling the style lint for no_std libraries. Even for no_std libraries, it's probably a good idea to for use to have a lint that says "hey, you should panic instead of deadlooping".

I think we should either:

1. Enable this lint for no_std libraries, disabling it in no_std binaries
2. Enable this lint for no_std libraries and binaries, but disable it inside #[panic_handler]

EDIT: Implementation of (2) is up in #6205

[eddyp]

I think we should either:

1. Enable this lint for `no_std` libraries, disabling it in `no_std` binaries

2. Enable this lint for `no_std` libraries and binaries, but disable it inside `#[panic_handler]`

In my view the second is the correct approach as on no_std you can customary use an empty loop {} only on unrecoverable errors. Main loops, even in an interrupt driven application, is expected to have a non-empty loop as it should contain at least some code to save power (e.g. WFI on ARMv7-M ISA).

[therealprof]

(e.g. WFI on ARMv7-M ISA).

WFI interacts badly with a debugger. At the moment there're legitimate reasons to want a truly empty loop, maybe there should be a libcore function providing such a functionality in a problem free manner?

[eddyp]

(e.g. WFI on ARMv7-M ISA).

WFI interacts badly with a debugger.

Is that something specific to Rust code? Or some specific boards/SoCs? Can you provide more details?

I am yet to encounter such issues with JTAG debuggers in C code I worked with, and have limited experience with non-play embedded Rust code that would contain the main loop (making power saving instructions needed).

At the moment there're legitimate reasons to want a truly empty loop, maybe there should be a libcore function providing such a functionality in a problem free manner?

BTW, the issue is closed now, so if you have issues, you might want to follow up, as option 2 was implemented:

abfa331#diff-43e58dc26b162ad6f137224e22f25d0a652e9c319ed6a7b7d74061110e5dea77