Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#[no_core] loses overflow checks with optimizations #38136

Closed
hanna-kruppe opened this issue Dec 3, 2016 · 1 comment
Closed

#[no_core] loses overflow checks with optimizations #38136

hanna-kruppe opened this issue Dec 3, 2016 · 1 comment

Comments

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented Dec 3, 2016

The following file, when compiled without optimizations, correctly generates an overflow check in add_one:

#![feature(no_core, lang_items, rustc_attrs)]
#![no_core]
#![crate_type="lib"]

#[lang="sized"]
trait Sized {}

#[lang="add"]
trait Add<RHS=Self> {
    type Output;
    fn add(self, rhs: RHS) -> Self::Output;
}

impl Add for i32 {
    type Output = i32;

    #[inline]
    #[rustc_inherit_overflow_checks]
    fn add(self, other: i32) -> i32 { self + other }
}

#[cold] #[inline(never)] // this is the slow path, always
#[lang = "panic"]
pub fn panic(_expr_file_line: &(&'static str, &'static str, u32)) -> ! {
	loop {}
}

#[lang="copy"]
trait Copy {}

pub fn add_one(x: i32) -> i32 {
  x + 1
}

However, compiling with optimizations plus -C debug-assertions=on or -Z force-overflow-checks=on generates code without the overflow check.

However, if I move add_one to a separate no_core crate like this, the issue disappears and the overflow check is generated:

#![feature(no_core)]
#![no_core]
#![crate_type="lib"]
extern crate minimal;

pub fn add_one(x: i32) -> i32 {
  x + 1
}
@hanna-kruppe
Copy link
Contributor Author

hanna-kruppe commented Dec 3, 2016

@eddyb noticed that my panic implementation (loop {}) is UB according to LLVM (see #28728), so it optimizes the overflow check out on the assumption that it can never be triggered (since that would be UB). Replacing it with e.g. intrinsics::abort() fixes the issue.

hanna-kruppe pushed a commit to hanna-kruppe/rust that referenced this issue Jan 4, 2017
Due to rust-lang#28728 loop {} is very risky and can lead to fun debugging experiences like in rust-lang#38136. Besides, aborting is probably better behavior than an infinite loop.
steveklabnik added a commit to steveklabnik/rust that referenced this issue Jan 4, 2017
book: use abort() over loop {} for panic

Due to rust-lang#28728 `loop {}` is very risky and can lead to fun debugging experiences such as rust-lang#38136. Besides, aborting is probably better behavior than an infinite loop.

r? @steveklabnik
frewsxcv pushed a commit to frewsxcv/rust that referenced this issue Jan 9, 2017
Due to rust-lang#28728 loop {} is very risky and can lead to fun debugging experiences like in rust-lang#38136. Besides, aborting is probably better behavior than an infinite loop.
bors added a commit that referenced this issue Jan 10, 2017
book: use abort() over loop {} for panic

Due to #28728 `loop {}` is very risky and can lead to fun debugging experiences such as #38136. Besides, aborting is probably better behavior than an infinite loop.

r? @steveklabnik
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant