Skip to content

miri: checked_pow: overflowing shift by 64 in unchecked_shl #120537

@morrisonlevi

Description

@morrisonlevi

Code

I tried this code:

fn main() {
    match 2u64.checked_pow(64) {
        Some(n) => println!("2^64: {n}"),
        None => {}
    };
}

I expected that a checked_* operation would specifically not cause UB on overflow.

Instead, this happened (playground):

MIRIFLAGS=-Zmiri-backtrace=full cargo +nightly miri run
Preparing a sysroot for Miri (target: aarch64-apple-darwin)... done
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
     Running `/Users/levi.morrison/.rustup/toolchains/nightly-aarch64-apple-darwin/bin/cargo-miri runner target/miri/aarch64-apple-darwin/debug/miri`
error: Undefined Behavior: overflowing shift by 64 in `unchecked_shl`
    --> /Users/levi.morrison/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/num/mod.rs:1181:5
     |
1181 | /     uint_impl! {
1182 | |         Self = u64,
1183 | |         ActualT = u64,
1184 | |         SignedT = i64,
...    |
1198 | |         bound_condition = "",
1199 | |     }
     | |_____^ overflowing shift by 64 in `unchecked_shl`
     |
     = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
     = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

<snip> 

error: aborting due to 1 previous error

Version it worked on

It most recently worked on: cargo 1.77.0-nightly (84976cd69 2024-01-12)

Version with regression

cargo 1.77.0-nightly (7bb7b5395 2024-01-20)

I didn't bisect this, but I believe it started happening with GH PR #119911.

Backtrace

Backtrace

   = note: BACKTRACE:
     = note: inside `core::num::<impl u64>::checked_pow` at /Users/levi.morrison/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/num/uint_macros.rs:1382:31: 1385:18
note: inside `main`
    --> src/main.rs:2:11
     |
2    |     match 2u64.checked_pow(64) {
     |           ^^^^^^^^^^^^^^^^^^^^
     = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /Users/levi.morrison/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5: 250:71
     = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at /Users/levi.morrison/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:155:18: 155:21
     = note: inside closure at /Users/levi.morrison/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:166:18: 166:82
     = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at /Users/levi.morrison/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:284:13: 284:31
     = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /Users/levi.morrison/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:554:40: 554:43
     = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /Users/levi.morrison/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:518:19: 518:81
     = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /Users/levi.morrison/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panic.rs:142:14: 142:33
     = note: inside closure at /Users/levi.morrison/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:148:48: 148:73
     = note: inside `std::panicking::r#try::do_call::<{closure@std::rt::lang_start_internal::{closure#2}}, isize>` at /Users/levi.morrison/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:554:40: 554:43
     = note: inside `std::panicking::r#try::<isize, {closure@std::rt::lang_start_internal::{closure#2}}>` at /Users/levi.morrison/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:518:19: 518:81
     = note: inside `std::panic::catch_unwind::<{closure@std::rt::lang_start_internal::{closure#2}}, isize>` at /Users/levi.morrison/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panic.rs:142:14: 142:33
     = note: inside `std::rt::lang_start_internal` at /Users/levi.morrison/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:148:20: 148:98
     = note: inside `std::rt::lang_start::<()>` at /Users/levi.morrison/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:165:17: 170:6
     = note: this error originates in the macro `uint_impl` (in Nightly builds, run with -Z macro-backtrace for more info)

I believe GH PR #119911 is the cause.

Activity

added
I-prioritizeIssue: Indicates that prioritization has been requested for this issue.
needs-triageThis issue may need triage. Remove it if it has been sufficiently triaged.
on Jan 31, 2024
Noratrieb

Noratrieb commented on Feb 1, 2024

@Noratrieb
Member

@NCGThompson @oli-obk I feel like int macro changes would have best been split out into a separate PR^^

added
I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness
T-libsRelevant to the library team, which will review and decide on the PR/issue.
and removed
needs-triageThis issue may need triage. Remove it if it has been sufficiently triaged.
on Feb 1, 2024
apiraino

apiraino commented on Feb 1, 2024

@apiraino
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high +T-compiler

added
P-highHigh priority
T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
and removed
I-prioritizeIssue: Indicates that prioritization has been requested for this issue.
on Feb 1, 2024
NCGThompson

NCGThompson commented on Feb 1, 2024

@NCGThompson
Contributor
    // SAFETY: We just checked this is a power of two. and above zero.
    let power_used = unsafe { intrinsics::cttz_nonzero(self) as u32 };
    if exp > Self::BITS / power_used { return None; } // Division of constants is free

I think the issue here is I needed to use >= rather than >.

@rustbot claim

6 remaining items

cuviper

cuviper commented on Feb 1, 2024

@cuviper
Member

For signed types, note that 2.pow(BITS - 1) is still an overflow, but (-2).pow(BITS - 1) is its MIN.

NCGThompson

NCGThompson commented on Feb 1, 2024

@NCGThompson
Contributor

For signed types, note that 2.pow(BITS - 1) is still an overflow, but (-2).pow(BITS - 1) is its MIN.

Right. Since overflow on BITS - 1 is dependent on the sign of the base, we use a separate check for that. The optimizer folds the two checks into a single comparison.

cuviper

cuviper commented on Feb 1, 2024

@cuviper
Member

Oh, you mean this? Then OK, but that could use a comment.

let sign = self.is_negative() && exp & 1 != 0;
if !sign && res == Self::MIN {
None

added a commit that references this issue on Feb 3, 2024

Rollup merge of rust-lang#120562 - oli-obk:revert_stuff, r=cuviper

977945d
added a commit that references this issue on Feb 4, 2024
4312e36
cuviper

cuviper commented on Feb 4, 2024

@cuviper
Member

The revert landed after the branch move, so let's re-open until that's backported.

reopened this on Feb 4, 2024
added this to the 1.77.0 milestone on Feb 4, 2024
added and removed
regression-untriagedUntriaged performance or correctness regression.
on Feb 5, 2024
linked a pull request that will close this issue[beta] backports #121069on Feb 14, 2024
cuviper

cuviper commented on Feb 28, 2024

@cuviper
Member

Backport was done in #121069.

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-bugCategory: This is a bug.I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessP-highHigh priorityT-compilerRelevant to the compiler 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.regression-from-stable-to-betaPerformance or correctness regression from stable to beta.

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Participants

      @cuviper@morrisonlevi@oli-obk@apiraino@NCGThompson

      Issue actions

        miri: checked_pow: overflowing shift by 64 in `unchecked_shl` · Issue #120537 · rust-lang/rust