std: Make abs() panic on overflow in debug mode by alexcrichton · Pull Request #25441 · rust-lang/rust

alexcrichton · 2015-05-15T16:24:26Z

Debug overflow checks for arithmetic negation landed in #24500, at which time
the abs method on signed integers was changed to using wrapping_neg to
ensure that the function never panicked. This implied that abs of INT_MIN
would return INT_MIN, another negative value. When this change was back-ported
to beta, however, in #24708, the wrapping_neg function had not yet been
backported, so the implementation was changed in #24785 to !self + 1. This
change had the unintended side effect of enabling debug overflow checks for the
abs function. Consequently, the current state of affairs is that the beta
branch checks for overflow in debug mode for abs and the nightly branch does
not.

This commit alters the behavior of nightly to have abs always check for
overflow in debug mode. This change is more consistent with the way the standard
library treats overflow as well, and it is also not a breaking change as it's
what the beta branch currently does (albeit if by accident).

cc #25378

rust-highfive · 2015-05-15T16:24:36Z

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

alexcrichton · 2015-05-15T16:25:07Z

r? @aturon

aturon · 2015-05-18T23:05:41Z

src/libcore/num/mod.rs

Can you leave a comment saying that this has the correct overflow semantics only if kept as inline given how std is distributed? I hope we can do something more robust in the future.

Otherwise r=me.

Debug overflow checks for arithmetic negation landed in rust-lang#24500, at which time the `abs` method on signed integers was changed to using `wrapping_neg` to ensure that the function never panicked. This implied that `abs` of `INT_MIN` would return `INT_MIN`, another negative value. When this change was back-ported to beta, however, in rust-lang#24708, the `wrapping_neg` function had not yet been backported, so the implementation was changed in rust-lang#24785 to `!self + 1`. This change had the unintended side effect of enabling debug overflow checks for the `abs` function. Consequently, the current state of affairs is that the beta branch checks for overflow in debug mode for `abs` and the nightly branch does not. This commit alters the behavior of nightly to have `abs` always check for overflow in debug mode. This change is more consistent with the way the standard library treats overflow as well, and it is also not a breaking change as it's what the beta branch currently does (albeit if by accident). cc rust-lang#25378

alexcrichton · 2015-05-19T00:53:08Z

@bors: r=aturon 5f39ceb

bors · 2015-05-19T01:08:55Z

⌛ Testing commit 5f39ceb with merge 8dbc369...

Debug overflow checks for arithmetic negation landed in #24500, at which time the `abs` method on signed integers was changed to using `wrapping_neg` to ensure that the function never panicked. This implied that `abs` of `INT_MIN` would return `INT_MIN`, another negative value. When this change was back-ported to beta, however, in #24708, the `wrapping_neg` function had not yet been backported, so the implementation was changed in #24785 to `!self + 1`. This change had the unintended side effect of enabling debug overflow checks for the `abs` function. Consequently, the current state of affairs is that the beta branch checks for overflow in debug mode for `abs` and the nightly branch does not. This commit alters the behavior of nightly to have `abs` always check for overflow in debug mode. This change is more consistent with the way the standard library treats overflow as well, and it is also not a breaking change as it's what the beta branch currently does (albeit if by accident). cc #25378

bors · 2015-05-19T03:17:32Z

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt

petrochenkov · 2015-05-19T14:13:00Z

Hey, it broke the compiler
Edit: I'm fixing it

$ RUST_BACKTRACE=1 make
cfg: version 1.2.0-dev (b301e02f3 2015-05-19) (built 2015-05-19)
cfg: build triple x86_64-pc-windows-gnu
cfg: host triples x86_64-pc-windows-gnu
cfg: target triples x86_64-pc-windows-gnu
cfg: enabling debug assertions (CFG_ENABLE_DEBUG_ASSERTIONS)
cfg: enabling debuginfo (CFG_ENABLE_DEBUGINFO)
cfg: host for x86_64-pc-windows-gnu is x86_64
cfg: os for x86_64-pc-windows-gnu is pc-windows-gnu
cfg: good valgrind for x86_64-pc-windows-gnu is
cfg: using CC=gcc (CFG_CC)
cfg: disabling valgrind run-pass tests
cfg: no xelatex found, disabling LaTeX docs
cfg: no pandoc found, omitting PDF and EPUB docs
rustc: x86_64-pc-windows-gnu/stage1/bin/rustlib/x86_64-pc-windows-gnu/lib/libcore
error: internal compiler error: unexpected panic
note: the compiler unexpectedly panicked. this is a bug.
note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
note: run with `RUST_BACKTRACE=1` for a backtrace
thread 'rustc' panicked at 'attempted to negate with overflow', C:/msys64/home/we/rust/src/libcore\num/mod.rs:581

stack backtrace:
   1:         0x6d2b3fa1 - sys::backtrace::write::h410e8b1c02e6a4cb8fs
   2:         0x6d2bca98 - rt::unwind::register::h1ca1862cef28bdc0xNv
   3:         0x6d284e14 - rt::unwind::begin_unwind_inner::h6525e25cf712ad9fHKv
   4:         0x6d2857f9 - rt::unwind::begin_unwind_fmt::ha2ec3cbf348c0730NJv
   5:         0x6d2bc6a0 - rust_begin_unwind
   6:         0x6d2da032 - panicking::panic_fmt::h3bec9bbe2b23bab6pZB
   7:         0x6d2d2e2f - panicking::panic::hcc1bf6d590d7a6bfWXB
   8:         0x71201f22 - builtin::TypeLimits.LintPass::check_expr::h3cbdea1f95212ea22ea
   9:           0xac16b0 - lint::context::Context<'a, 'tcx>.Visitor<'v>::visit_expr::ha294eff0288554d123s
  10:           0xac1ae6 - lint::context::Context<'a, 'tcx>.Visitor<'v>::visit_expr::ha294eff0288554d123s
  11:           0xac190b - lint::context::Context<'a, 'tcx>.Visitor<'v>::visit_expr::ha294eff0288554d123s
  12:           0xac1ae6 - lint::context::Context<'a, 'tcx>.Visitor<'v>::visit_expr::ha294eff0288554d123s
  13:           0xac18f6 - lint::context::Context<'a, 'tcx>.Visitor<'v>::visit_expr::ha294eff0288554d123s
  14:           0xabd984 - lint::context::Context<'a, 'tcx>.Visitor<'v>::visit_item::h97cd569f2a202a39k0s
  15:           0xac21c9 - lint::context::Context<'a, 'tcx>.Visitor<'v>::visit_mod::ha15baa921fa04facaft
  16:           0xabda37 - lint::context::Context<'a, 'tcx>.Visitor<'v>::visit_item::h97cd569f2a202a39k0s
  17:           0xac21c9 - lint::context::Context<'a, 'tcx>.Visitor<'v>::visit_mod::ha15baa921fa04facaft
  18:           0xacf212 - lint::context::check_crate::h008b1bd3f8cb5d749yt
  19:         0x65399d3d - driver::phase_3_run_analysis_passes::h4302356ee8fb62b5tGa
  20:         0x6538322f - driver::compile_input::h2cf0fc5773828de8Qba
  21:         0x6541babd - run_compiler::hce52d824a1c83d3675b
  22:         0x65419a70 - run::h315a1ba5be8f35fbN5b
  23:         0x6541921e - run::h315a1ba5be8f35fbN5b
  24:         0x6d2fc07c - rust_try
  25:         0x6d2fc059 - rust_try
  26:         0x654194dc - run::h315a1ba5be8f35fbN5b
  27:         0x6d2bac4d - sys::process::Command::cwd::ha96a7c671c94f9abquu
  28:     0x7ffbd1b113d2 - BaseThreadInitThunk

Reported here #25441 (comment)

alexcrichton · 2015-06-09T22:30:28Z

Accepting for a beta backport

rust-highfive assigned brson May 15, 2015

alexcrichton mentioned this pull request May 15, 2015

std::i32::MIN.abs() results in panicked at 'arithmetic operation overflowed' #25378

Closed

rust-highfive assigned aturon and unassigned brson May 15, 2015

aturon reviewed May 18, 2015
View reviewed changes

alexcrichton force-pushed the debug-panic-neg branch from 2a05f77 to 5f39ceb Compare May 19, 2015 00:53

bors merged commit 5f39ceb into rust-lang:master May 19, 2015

alexcrichton added beta-nominated Nominated for backporting to the compiler in the beta channel. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels May 19, 2015

alexcrichton deleted the debug-panic-neg branch May 19, 2015 05:56

alexcrichton restored the debug-panic-neg branch May 19, 2015 05:56

alexcrichton deleted the debug-panic-neg branch May 19, 2015 05:56

alexcrichton restored the debug-panic-neg branch May 19, 2015 05:56

alexcrichton deleted the debug-panic-neg branch May 19, 2015 05:56

petrochenkov mentioned this pull request May 19, 2015

Fix panic in lint for out of range literals #25612

Merged

bors added a commit that referenced this pull request May 21, 2015

Auto merge of #25612 - petrochenkov:lintabs, r=@alexcrichton

9ee4ead

Reported here #25441 (comment)

alexcrichton added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jun 9, 2015

alexcrichton mentioned this pull request Jun 10, 2015

Backport accepted PRs to beta #26161

Merged

alexcrichton removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 11, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

std: Make abs() panic on overflow in debug mode#25441

std: Make abs() panic on overflow in debug mode#25441
bors merged 1 commit intorust-lang:masterfrom
alexcrichton:debug-panic-neg

alexcrichton commented May 15, 2015

Uh oh!

rust-highfive commented May 15, 2015

Uh oh!

alexcrichton commented May 15, 2015

Uh oh!

aturon May 18, 2015

Uh oh!

alexcrichton commented May 19, 2015

Uh oh!

bors commented May 19, 2015

Uh oh!

bors commented May 19, 2015

Uh oh!

petrochenkov commented May 19, 2015

Uh oh!

alexcrichton commented Jun 9, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Comments

Conversation

alexcrichton commented May 15, 2015

Uh oh!

rust-highfive commented May 15, 2015

Uh oh!

alexcrichton commented May 15, 2015

Uh oh!

aturon May 18, 2015

Choose a reason for hiding this comment

Uh oh!

alexcrichton commented May 19, 2015

Uh oh!

bors commented May 19, 2015

Uh oh!

bors commented May 19, 2015

Uh oh!

petrochenkov commented May 19, 2015

Uh oh!

alexcrichton commented Jun 9, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants