Add intrinsics::unreachable #16970

Merged
merged 2 commits into from Oct 5, 2014

Projects

None yet

8 participants

@kmcallister
Contributor

I'm not sure how to add an automated test for this.

@thestinger
Contributor

A simple run-pass test making use of it would be nice, even though it can't be directly tested.

@alexcrichton
Member

What happens in a program like this?

use std::intrinsics;
fn main() {
    unsafe { intrinsics::unreachable(); }
}
@thestinger
Contributor

That has undefined behaviour, it's the point of the intrinsic. It's the same thing as __builtin_unreachable with gcc and clang.

@kmcallister
Contributor

My use case is something like

pub fn exit(n: uint) -> ! {
    static NR_EXIT: uint = 60;
    unsafe {
        asm!("syscall"
            :: "{rax}"(NR_EXIT), "{rdi}"(n));
        intrinsics::unreachable()
    }
}

which compiles on x86-64 Linux to just

0000000000000000 <_ZN4exit20he7def17266e9100bdaaE>:
   0:   b8 3c 00 00 00          mov    $0x3c,%eax
   5:   0f 05                   syscall 

Without the intrinsic I need something like loop { } to fulfill the bottom return type, which wastes 2 bytes in the object code, plus more for alignment.

@kmcallister
Contributor

Not sure why the Travis build failed. The log shows rustc dying with SIGBUS.

@Kimundi
Member
Kimundi commented Sep 4, 2014

Travis always fails its llvm 3.4 build atm. The 3.3 one succeeded though, which is the important one.

@huonw
Member
huonw commented Sep 4, 2014

Does this avoid crashing horribly? (iirc there have been crashes due to LLVM assertions about unreachability previously)

fn foo() {
    unreachable();
    println!("foo");
}
@kmcallister
Contributor

It doesn't crash rustc, no.

@kmcallister
Contributor

@huonw, @thestinger: Any more thoughts about this?

@bors bors added a commit that referenced this pull request Sep 15, 2014
@bors bors auto merge of #16970 : kmcallister/rust/llvm-unreachable, r=thestinger
I'm not sure how to add an automated test for this.
a2e0ca0
@kmcallister
Contributor

Any way I can get the contents of /c/bot/slave/auto-win64-64-nopt-t/build/src/test/run-make/intrinsic-unreachable/ without my own Windows machine? It may have failed just because of using test and wc in the Makefile; what kind of environment do these Makefiles use on Windows?

@alexcrichton
Member

Not at this point sadly, the bots clean out all previous runs when they make a new build. You could update the test, however, to print it out and we can push to try.

@vadimcn
Contributor
vadimcn commented Sep 20, 2014

@kmcallister: Please note that on Win64, the 'unreachable' instruction actually emits code.

@kmcallister
Contributor

Aha!

@ghost
ghost commented Sep 24, 2014

FWIW, matching an empty enum will produce an unreachable instruction:

use std::any::Void;
use std::intrinsics::transmute;

fn main() {
    let x: Void = unsafe { transmute(()) };
    match x {}
}
%"enum.core::any::Void<[]>[#3]" = type {}

; Function Attrs: uwtable
define internal void @_ZN4main20h31af98ebf3d86da3gaaE() unnamed_addr #0 {
entry-block:
  %x = alloca %"enum.core::any::Void<[]>[#3]"
  unreachable

so the unreachable intrinsic could be implemented in the library itself. Or not implemented at all if the above hack is deemed sufficient.

@tbu- tbu- commented on an outdated diff Oct 2, 2014
src/test/run-pass/intrinsic-unreachable.rs
@@ -0,0 +1,24 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use std::intrinsics;
+
+// See also src/test/run-make/intrinsic-unreachable.
+
+fn f(x: uint) -> uint {
@tbu-
tbu- Oct 2, 2014 Contributor

I'm not sure whether tests should feature idiomatic code, but in this case I think it'd be better if you'd mark this function unsafe.

kmcallister added some commits Sep 3, 2014
@kmcallister kmcallister Add intrinsics::unreachable 401aeaf
@kmcallister kmcallister Add tests for intrinsics::unreachable
675aa76
@kmcallister
Contributor

Rebased and addressed @tbu-'s comment. r? @thestinger

@thestinger

r+

Contributor
bors replied Oct 5, 2014

saw approval from thestinger
at kmcallister@675aa76

Contributor
bors replied Oct 5, 2014

merging kmcallister/rust/llvm-unreachable = 675aa76 into auto

Contributor
bors replied Oct 5, 2014

kmcallister/rust/llvm-unreachable = 675aa76 merged ok, testing candidate = 5660db2

Contributor
bors replied Oct 5, 2014

fast-forwarding master to auto = 5660db2

@bors bors added a commit that referenced this pull request Oct 5, 2014
@bors bors auto merge of #16970 : kmcallister/rust/llvm-unreachable, r=thestinger
I'm not sure how to add an automated test for this.
5660db2
@bors bors closed this Oct 5, 2014
@bors bors merged commit 675aa76 into rust-lang:master Oct 5, 2014

2 checks passed

continuous-integration/travis-ci The Travis CI build passed
Details
default all tests passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment