Enable TrapUnreachable in LLVM. #45920

sunfishcode · 2017-11-10T20:00:30Z

This patch enables LLVM's TrapUnreachable flag, which tells it to translate unreachable instructions into hardware trap instructions, rather than allowing control flow to "fall through" into whatever code happens to follow it in memory.

This follows up on #28728 (comment). For example, for @zackw's testcase here, the output function contains a ud2 instead of no code, so it won't "fall through" into whatever happens to be next in memory.

(I'm also working on the problem of LLVM optimizing away infinite loops, but the patch here is useful independently.)

I tested this patch on a few different codebases, and the code size increase ranged from 0.0% to 0.1%.

Enable LLVM's TrapUnreachable flag, which tells it to translate `unreachable` instructions into hardware trap instructions, rather than allowing control flow to "fall through" into whatever code happens to follow it in memory.

zackw · 2017-11-10T20:49:15Z

src/rustllvm/PassWrapper.cpp

+  // Tell LLVM to translate `unreachable` into an explicit trap instruction.
+  // This limits the extent of possible undefined behavior in some cases, as it
+  // prevents control flow from "falling through" into whatever code happens to
+  // be layed out next in memory.


Typo: "layed" should be "laid"

zackw · 2017-11-10T20:51:01Z

As the person who originally asked for this, thanks for making sure it got done.

With rustc now emitting "ud2" on unreachable code, a "return nothing" sequence may take the same number of lines as an "unreachable" sequence in assembly code. This test is testing that intrinsics::unreachable() works by testing that it reduces the number of lines in the assembly code. Fix it by adding a return value, which requires an extra instruction in the reachable case, which provides the test what it's looking for.

Zoxc · 2017-11-11T02:30:20Z

I'd like to see this option as a part of TargetOptions and have it be enabled by default.

Zoxc · 2017-11-11T02:32:58Z

src/test/run-make/intrinsic-unreachable/exit-unreachable.rs

    unsafe {
        // Pretend this asm is an exit() syscall.
        asm!("" :: "r"(n) :: "volatile");
        intrinsics::unreachable()
    }
+    42


Could you add a comment that the 42 is there just so there's some code for LLVM to remove, making the test that the assembly output of exit-unreachable.rs is shorter than exit-ret.rs succeed?

Comment added.

The return value in these tests is just being used to generate extra code so that it can be detected in the test script, which is just counting lines in the assembly output.

Zoxc · 2017-11-12T00:21:37Z

@bors r+

bors · 2017-11-12T00:21:38Z

@Zoxc: 🔑 Insufficient privileges: Not in reviewers

Zoxc · 2017-11-12T00:22:39Z

I guess I don't have bors permissions yet.

@alexcrichton When will that happen? =P

Mark-Simulacrum · 2017-11-12T00:23:58Z

@bors r=Zoxc

bors · 2017-11-12T00:23:58Z

📌 Commit 7b6b764 has been approved by Zoxc

bors · 2017-11-15T03:57:24Z

⌛ Testing commit 7b6b764 with merge 1f893306a350c62927740ec18e4e75c57c9dc517...

bors · 2017-11-15T04:48:13Z

💔 Test failed - status-travis

kennytm · 2017-11-15T05:32:44Z

Failed to build rustdoc at stage2 in debug build (i686-gnu-nopt) with SIGILL.

[00:46:24] error: process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustc -vV` (signal: 4, SIGILL: illegal instruction)
[00:46:24] 
[00:46:24] 
[00:46:24] command did not execute successfully: "/checkout/obj/build/i686-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "i686-unknown-linux-gnu" "--release" "--locked" "--color" "always" "--manifest-path" "/checkout/src/tools/rustdoc/Cargo.toml"
[00:46:24] expected success, got: exit code: 101

zackw · 2017-11-15T07:13:45Z

Yikes, this looks like one of those unreachable traps actually happened.

sunfishcode · 2017-11-15T20:28:12Z

Looks like it's just 32-bit x86. I've been able to reproduce the crash now; here's a backtrace:

(gdb) where
#0  0xf7ea6dd0 in __rust_probestack () from /home/rustacean/rust/build/i686-unknown-linux-gnu/stage2/lib/librustc_driver-57160753fe603ca7.so
#1  0xf7db46be in std::sys_common::backtrace::__rust_begin_short_backtrace () from /home/rustacean/rust/build/i686-unknown-linux-gnu/stage2/lib/librustc_driver-57160753fe603ca7.so
#2  0xf7c67488 in __rust_maybe_catch_panic () from /home/rustacean/rust/build/i686-unknown-linux-gnu/stage2/lib/libstd-650e8e2b6b1595ab.so
#3  0xf7e36752 in <F as alloc::boxed::FnBox<A>>::call_box () from /home/rustacean/rust/build/i686-unknown-linux-gnu/stage2/lib/librustc_driver-57160753fe603ca7.so
#4  0xf7c0a635 in std::sys::imp::thread::Thread::new::thread_start () from /home/rustacean/rust/build/i686-unknown-linux-gnu/stage2/lib/libstd-650e8e2b6b1595ab.so
#5  0xf532b2f5 in start_thread (arg=0xf1dffb40) at pthread_create.c:456
#6  0xf7ac76de in clone () at ../sysdeps/unix/sysv/linux/i386/clone.S:113

__rust_probestack is a function that contains an inline asm that does a "ret", which is fairly suspicious. I'll investigate further.

kennytm · 2017-11-15T20:51:46Z

cc @alexcrichton (#42816) this PR seems to have conflict with the x86 stack probe.

sunfishcode · 2017-11-15T21:36:24Z

I've now filed rust-lang/compiler-builtins#207 which makes __rust_probestack more resistant to instruction reordering by the compiler, which fixes the crash.

sunfishcode · 2017-11-15T23:16:42Z

rust-lang/compiler-builtins#207 is now merged. Can we do a new bors run with an updated compiler-builtins?

alexcrichton · 2017-11-15T23:44:00Z

@sunfishcode oh that's actually just a submodule in this repository at src/libcompiler_builtins, to test against an updated version you'll need to update the submodule revision as part of this PR as well

sunfishcode · 2017-11-16T01:33:33Z

Ok, I've now updated the submodule.

kennytm · 2017-11-16T05:22:09Z

@bors r=Zoxc

bors · 2017-11-16T05:22:10Z

📌 Commit ac48348 has been approved by Zoxc

bors · 2017-11-16T06:10:42Z

⌛ Testing commit ac48348 with merge 1410d56...

@zackw

Enable TrapUnreachable in LLVM. This patch enables LLVM's TrapUnreachable flag, which tells it to translate `unreachable` instructions into hardware trap instructions, rather than allowing control flow to "fall through" into whatever code happens to follow it in memory. This follows up on #28728 (comment). For example, for @zackw's testcase [here](#42009 (comment)), the output function contains a `ud2` instead of no code, so it won't "fall through" into whatever happens to be next in memory. (I'm also working on the problem of LLVM optimizing away infinite loops, but the patch here is useful independently.) I tested this patch on a few different codebases, and the code size increase ranged from 0.0% to 0.1%.

bors · 2017-11-16T08:31:55Z

☀️ Test successful - status-appveyor, status-travis
Approved by: Zoxc
Pushing 1410d56 to master...

The object file in the rlib has been renamed again, and the name now includes now gibberish. The assembly now has an extra instruction due to rust-lang/rust#45920 but despite this the final executable is actually smaller than before. (There doesn't appear to be a way to disable the unreachable() trap.)

The object file in the rlib has been renamed again, and the name now includes now gibberish. The assembly now has an extra instruction due to rust-lang/rust#45920 which increases the executable size with two bytes. (There doesn't appear to be a way to disable the unreachable() trap.)

Enable TrapUnreachable in LLVM.

d9f0e88

Enable LLVM's TrapUnreachable flag, which tells it to translate `unreachable` instructions into hardware trap instructions, rather than allowing control flow to "fall through" into whatever code happens to follow it in memory.

sunfishcode mentioned this pull request Nov 10, 2017

LLVM loop optimization can make safe programs crash #28728

Closed

kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 10, 2017

zackw reviewed Nov 10, 2017

View reviewed changes

sunfishcode added 2 commits November 10, 2017 12:52

Fix a spello.

89652d6

Zoxc reviewed Nov 11, 2017

View reviewed changes

Zoxc self-assigned this Nov 11, 2017

sunfishcode added 2 commits November 11, 2017 06:41

Add comments to the intrinsic-unreachable test programs.

365c159

The return value in these tests is just being used to generate extra code so that it can be detected in the test script, which is just counting lines in the assembly output.

Control LLVM's TrapUnreachable feature through rustc's TargetOptions.

7b6b764

kennytm added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 12, 2017

kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 15, 2017

sunfishcode mentioned this pull request Nov 15, 2017

Add "volatile" and "memory" clobber to asm! that doesn't fall through. rust-lang/compiler-builtins#207

Merged

Update the compiler-builtins to latest master.

ac48348

Zoxc mentioned this pull request Nov 16, 2017

Add an error for inline assembly which are non-volatile without any outputs #46026

Closed

kennytm added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 16, 2017

bors merged commit ac48348 into rust-lang:master Nov 16, 2017

bors mentioned this pull request Nov 16, 2017

std: Add a new wasm32-unknown-unknown target #45905

Merged

sunfishcode deleted the trap-on-unreachable branch November 16, 2017 13:50

alexcrichton mentioned this pull request Nov 17, 2017

Make inline assembly volatile if it has no outputs. Fixes #46026 #46030

Merged

bluss added the relnotes Marks issues that should be documented in the release notes of the next release. label Nov 18, 2017

bluss mentioned this pull request Nov 18, 2017

Bug: Spawn a thread with a empty loop, program will coredump when compile with optimizations #46076

Closed

sfackler mentioned this pull request Apr 18, 2018

Tracking issue for mem::unreachable #43751

Closed

sfackler mentioned this pull request May 23, 2018

Making a value for uninhabited type is compiled to SIGILL on Nightly but only in Release mode #50976

Closed

the8472 mentioned this pull request Aug 2, 2018

rustc emits unnecessary abort / trap instructions after calls to divergent functions #52998

Closed

tormol mentioned this pull request Sep 7, 2018

Upgrade to rustc 1.30-nightly (2018-08-30) kmcallister/tiny-rust-demo#2

Open

eira-fransham mentioned this pull request Apr 24, 2019

Reviving tail-call elimination rust-lang/rfcs#2691

Open

Amanieu mentioned this pull request Aug 7, 2020

Constrained Naked Functions rust-lang/rfcs#2972

Merged

15 tasks

huitseeker mentioned this pull request Aug 26, 2020

[safety-rules, consensus, ...] Remove a few panics diem/diem#5617

Merged

majaha mentioned this pull request Sep 12, 2023

Ensure NoTrapAfterNoreturn is false for the wasm backend llvm/llvm-project#65876

Merged

duk-37 mentioned this pull request Sep 23, 2024

[WIP] [CodeGen] Enable TrapUnreachable by default for all targets. llvm/llvm-project#109732

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable TrapUnreachable in LLVM. #45920

Enable TrapUnreachable in LLVM. #45920

sunfishcode commented Nov 10, 2017

zackw Nov 10, 2017

sunfishcode Nov 10, 2017

zackw commented Nov 10, 2017

Zoxc commented Nov 11, 2017

Zoxc Nov 11, 2017

sunfishcode Nov 11, 2017

Zoxc commented Nov 12, 2017

bors commented Nov 12, 2017

Zoxc commented Nov 12, 2017

Mark-Simulacrum commented Nov 12, 2017

bors commented Nov 12, 2017

bors commented Nov 15, 2017

bors commented Nov 15, 2017

kennytm commented Nov 15, 2017 •

edited

Loading

zackw commented Nov 15, 2017 via email

sunfishcode commented Nov 15, 2017

kennytm commented Nov 15, 2017

sunfishcode commented Nov 15, 2017

sunfishcode commented Nov 15, 2017

alexcrichton commented Nov 15, 2017

sunfishcode commented Nov 16, 2017

kennytm commented Nov 16, 2017

bors commented Nov 16, 2017

bors commented Nov 16, 2017

bors commented Nov 16, 2017

Enable TrapUnreachable in LLVM. #45920

Enable TrapUnreachable in LLVM. #45920

Conversation

sunfishcode commented Nov 10, 2017

zackw Nov 10, 2017

Choose a reason for hiding this comment

sunfishcode Nov 10, 2017

Choose a reason for hiding this comment

zackw commented Nov 10, 2017

Zoxc commented Nov 11, 2017

Zoxc Nov 11, 2017

Choose a reason for hiding this comment

sunfishcode Nov 11, 2017

Choose a reason for hiding this comment

Zoxc commented Nov 12, 2017

bors commented Nov 12, 2017

Zoxc commented Nov 12, 2017

Mark-Simulacrum commented Nov 12, 2017

bors commented Nov 12, 2017

bors commented Nov 15, 2017

bors commented Nov 15, 2017

kennytm commented Nov 15, 2017 • edited Loading

zackw commented Nov 15, 2017 via email

sunfishcode commented Nov 15, 2017

kennytm commented Nov 15, 2017

sunfishcode commented Nov 15, 2017

sunfishcode commented Nov 15, 2017

alexcrichton commented Nov 15, 2017

sunfishcode commented Nov 16, 2017

kennytm commented Nov 16, 2017

bors commented Nov 16, 2017

bors commented Nov 16, 2017

bors commented Nov 16, 2017

kennytm commented Nov 15, 2017 •

edited

Loading