Skip to content

Many functions with rustc_inherit_overflow_checks should also have track_caller #119682

Closed
@NCGThompson

Description

@NCGThompson
Contributor

In general, if a library function panics due to invalid input, its best if the error message point to the function call and explain why the input was invalid. An overflow in a core library function with the #[rustc_inherit_overflow_checks] attribute is probably due to invalid input.

Assuming a function could only panic due to incorrect input, the only potential drawback to #[track_caller] I can see is the panic message not adequately explaining why the input was invalid. Until we get #111466, we can only pass the text of the overflow error. For div_euclid*, this is perfect. For next_multiple_of, this is sufficient. For advance_by, I'm not sure.

Adding the attribute is trivial. I'm only making this issue to ask if/what functions I should add the attribute to and if I need to.

See also: #102024 #114841

* Edit: The division function might be a confusing example since it doesn't care about #[rustc_inherit_overflow_checks]. However, it should have #[track_caller] like the others.

Activity

added
needs-triageThis issue may need triage. Remove it if it has been sufficiently triaged.
on Jan 7, 2024
added
T-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.
T-libsRelevant to the library team, which will review and decide on the PR/issue.
C-discussionCategory: Discussion or questions that doesn't represent real issues.
and removed
needs-triageThis issue may need triage. Remove it if it has been sufficiently triaged.
on Jan 7, 2024
dtolnay

dtolnay commented on Apr 5, 2024

@dtolnay
Member

Fixed by #119726.

zheland

zheland commented on Dec 2, 2024

@zheland

It doesn't seem like #119726 fixes all such missed #[track_caller].
The #119726 doesn't add #[track_caller] at least to next_multiple_of and advance_by that is mentioned in this issue.

There are panicking functions with #[rustc_inherit_overflow_checks] that miss #[track_caller]:

  • library/core/src/num/uint_macros.rs: pow, next_multiple_of, next_power_of_two.
  • library/core/src/num/int_macros.rs: pow, next_multiple_of, abs.
  • library/core/src/ops/arith.rs: neg.
  • library/core/src/ops/bit.rs: shl, shr, shl_assign, shr_assign.

Functions for which #[rustc_inherit_overflow_checks] seems redundant:

  • library/core/src/num/uint_macros.rs: is_multiple_of.
  • library/core/src/slice/iter/macros.rs: position.

Additionally #[rustc_inherit_overflow_checks] without #[track_caller] is often used in count-like functions iterators functions in:

  • library/core/src/iter/range.rs
  • library/core/src/iter/adapters/{chain.rs, cycle.rs, enumerate.rs, flatten.rs, peekable.rs, skip.rs, take.rs}
  • library/core/src/iter/traits/{accum.rs, iterator.rs}

Possibly all functions that require #[rustc_inherit_overflow_checks] should have #[track_caller] and there are still some functions with redundant #[rustc_inherit_overflow_checks].

Example:

#![feature(int_roundings)]
#![allow(unused_must_use)]

pub fn main() {
    let imin = core::hint::black_box(i32::MIN);
    let imax = core::hint::black_box(i32::MAX);
    let umax = core::hint::black_box(u32::MAX);
    let v32 = core::hint::black_box(32);
    std::thread::scope(|s| {
        s.spawn(|| umax.pow(12)).join();
        s.spawn(|| umax.next_multiple_of(7)).join();
        s.spawn(|| umax.next_power_of_two()).join();

        s.spawn(|| imax.pow(12)).join();
        s.spawn(|| imax.next_multiple_of(7)).join(); // requires int_roundings
        s.spawn(|| imin.abs()).join();

        s.spawn(|| std::ops::Neg::neg(imin)).join();

        s.spawn(|| std::ops::Shl::shl(umax, v32)).join();
        s.spawn(|| std::ops::Shl::shl(imax, v32)).join();

        s.spawn(|| std::ops::Shr::shr(umax, v32)).join();
        s.spawn(|| std::ops::Shr::shr(imax, v32)).join();

        s.spawn(move || std::ops::ShlAssign::shl_assign(&mut (umax + 0), v32))
            .join();
        s.spawn(move || std::ops::ShlAssign::shl_assign(&mut (imax + 0), v32))
            .join();

        s.spawn(move || std::ops::ShrAssign::shr_assign(&mut (umax + 0), v32))
            .join();
        s.spawn(move || std::ops::ShrAssign::shr_assign(&mut (imax + 0), v32))
            .join();

        s.spawn(|| {
            let mut iter = core::iter::repeat(0).enumerate();
            iter.nth(usize::MAX);
            iter.nth(usize::MAX);
        })
        .join();
    });
}

With:

cargo +nightly run

Results in:

thread '<unnamed>' panicked at /home/zheland/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/num/mod.rs:1199:5:
attempt to multiply with overflow
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread '<unnamed>' panicked at /home/zheland/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/num/mod.rs:1199:5:
attempt to add with overflow
thread '<unnamed>' panicked at /home/zheland/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/num/mod.rs:1199:5:
attempt to add with overflow
thread '<unnamed>' panicked at /home/zheland/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/num/mod.rs:383:5:
attempt to multiply with overflow
thread '<unnamed>' panicked at /home/zheland/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/num/mod.rs:383:5:
attempt to add with overflow
thread '<unnamed>' panicked at /home/zheland/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/num/mod.rs:383:5:
attempt to negate with overflow
thread '<unnamed>' panicked at /home/zheland/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/arith.rs:712:1:
attempt to negate with overflow
thread '<unnamed>' panicked at /home/zheland/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/bit.rs:496:1:
attempt to shift left with overflow
thread '<unnamed>' panicked at /home/zheland/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/bit.rs:496:1:
attempt to shift left with overflow
thread '<unnamed>' panicked at /home/zheland/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/bit.rs:614:1:
attempt to shift right with overflow
thread '<unnamed>' panicked at /home/zheland/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/bit.rs:614:1:
attempt to shift right with overflow
thread '<unnamed>' panicked at /home/zheland/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/bit.rs:950:1:
attempt to shift left with overflow
thread '<unnamed>' panicked at /home/zheland/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/bit.rs:950:1:
attempt to shift left with overflow
thread '<unnamed>' panicked at /home/zheland/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/bit.rs:1032:1:
attempt to shift right with overflow
thread '<unnamed>' panicked at /home/zheland/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/bit.rs:1032:1:
attempt to shift right with overflow
thread '<unnamed>' panicked at /home/zheland/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/iter/adapters/enumerate.rs:63:22:
attempt to add with overflow

Expected something like:

...
thread '<unnamed>' panicked at src/main.rs:18:20:
attempt to negate with overflow
...
thread '<unnamed>' panicked at src/main.rs:31:22:
attempt to shift left with overflow
...

Version (last nightly):

$ rustc +nightly -V
rustc 1.86.0-nightly (824759493 2025-01-09)
zheland

zheland commented on Jan 11, 2025

@zheland

@dtolnay, could you please review my previous comment. I think that issue is not fixed and should be reopened.
And since it looks like a simple fix and I've already started getting into it, I can double-check the details and try to prepare a PR for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-discussionCategory: Discussion or questions that doesn't represent real issues.T-libsRelevant to the library team, which will review and decide on the PR/issue.T-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @dtolnay@zheland@saethlin@NCGThompson@rustbot

        Issue actions

          Many functions with `rustc_inherit_overflow_checks` should also have `track_caller` · Issue #119682 · rust-lang/rust