Skip to content

Deprecate most implicit calls to BUG() ? #283

Closed
@TheSven73

Description

@TheSven73

Linus Torvalds / the kernel community has a strong dislike of BUG()/BUG_ON() calls, but the Rust core implicitly calls them from various places, such as assert macros, unwrap()/expect(), or overflow checks.

If I interpret the documentation correctly, then:

  • BUG()/BUG_ON() should “never” be called, as it’s deprecated. There are however some very limited cases where it would be appropriate. But it should never make it in without a carefully considered, valid reason - the exception not the norm.
  • For "expected to be unreachable" or “impossible error conditions” use WARN()/WARN_ON(). Note that these functions return, and the kernel should continue “as gracefully as possible” on their return.
  • For “reachable but undesirable” situations, use pr_warn(). This is because some system owners set the “panic on WARN()” sysctl, which kills the system on a call to WARN(). We do not want this to happen for “reachable but undesirable” situations.

How would this translate to Rust? Bold suggestion:

  • For exceptional calls to BUG(), we create a Rust kernel_bug! macro.
  • Rust assert!, build_assert! and friends map to “expected to be unreachable” or “impossible” error conditions, so they must map to WARN(). Since WARN() returns and assert! does not, we should prohibit these standard macros outside of the Rust core. Replace with new assertion macros which do return.
  • Rust unwrap(), expect() calls also map to “impossible” error conditions. Since there is no way to recover gracefully from these, they should be disabled outside of the Rust core.
  • In fact, Rust kernel code should “never” map “impossible error condition” to a BUG() call. With some very limited exceptions such as when it’s not possible or desirable to recover gracefully, eg. when the Rust core triggers one of its internal assertions. Or when an overflow is detected, as suggested previously by @kees?
  • For “reachable but undesirable situations” we can simply use pr_warn! as suggested.

I’m aware that my bold suggestion may generate some strong objections. But it’s good to discuss this and find out what the consensus or strategy is. We may refer back to any consensus reached here if the issue comes up again in the future?

Follow-up from this discussion.

Metadata

Metadata

Assignees

No one assigned

    Labels

    • libRelated to the `rust/` library.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions