Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 1091eee

Browse filesBrowse files
alexcrichtonalexcrichton
committed
rustc: Switch extern functions to abort by default on panic
This was intended to land way back in 1.24, but it was backed out due to breakage which has long since been fixed. An unstable `#[unwind]` attribute can be used to tweak the behavior here, but this is currently simply switching rustc's internal default to abort-by-default if an `extern` function panics, making our codegen sound primarily (as currently you can produce UB with safe code) Closes #52652
1 parent bd47d68 commit 1091eee
Copy full SHA for 1091eee

File tree

Expand file treeCollapse file tree

10 files changed

+61
-46
lines changed
Filter options
Expand file treeCollapse file tree

10 files changed

+61
-46
lines changed

‎src/librustc_codegen_llvm/attributes.rs

Copy file name to clipboardExpand all lines: src/librustc_codegen_llvm/attributes.rs
+34-23Lines changed: 34 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,15 @@ use rustc::hir::{CodegenFnAttrFlags, CodegenFnAttrs};
1515
use rustc::hir::def_id::{DefId, LOCAL_CRATE};
1616
use rustc::session::Session;
1717
use rustc::session::config::Sanitizer;
18-
use rustc::ty::TyCtxt;
18+
use rustc::ty::{self, TyCtxt, PolyFnSig};
1919
use rustc::ty::layout::HasTyCtxt;
2020
use rustc::ty::query::Providers;
2121
use rustc_data_structures::sync::Lrc;
2222
use rustc_data_structures::fx::FxHashMap;
2323
use rustc_target::spec::PanicStrategy;
2424
use rustc_codegen_ssa::traits::*;
2525

26+
use abi::Abi;
2627
use attributes;
2728
use llvm::{self, Attribute};
2829
use llvm::AttributePlace::Function;
@@ -60,7 +61,7 @@ pub fn emit_uwtable(val: &'ll Value, emit: bool) {
6061

6162
/// Tell LLVM whether the function can or cannot unwind.
6263
#[inline]
63-
pub fn unwind(val: &'ll Value, can_unwind: bool) {
64+
fn unwind(val: &'ll Value, can_unwind: bool) {
6465
Attribute::NoUnwind.toggle_llfn(Function, val, !can_unwind);
6566
}
6667

@@ -150,9 +151,10 @@ pub fn non_lazy_bind(sess: &Session, llfn: &'ll Value) {
150151
/// Composite function which sets LLVM attributes for function depending on its AST (`#[attribute]`)
151152
/// attributes.
152153
pub fn from_fn_attrs(
153-
cx: &CodegenCx<'ll, '_>,
154+
cx: &CodegenCx<'ll, 'tcx>,
154155
llfn: &'ll Value,
155156
id: Option<DefId>,
157+
sig: PolyFnSig<'tcx>,
156158
) {
157159
let codegen_fn_attrs = id.map(|id| cx.tcx.codegen_fn_attrs(id))
158160
.unwrap_or_else(|| CodegenFnAttrs::new());
@@ -194,28 +196,37 @@ pub fn from_fn_attrs(
194196
llvm::AttributePlace::ReturnValue, llfn);
195197
}
196198

197-
let can_unwind = if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::UNWIND) {
198-
Some(true)
199+
unwind(llfn, if cx.tcx.sess.panic_strategy() != PanicStrategy::Unwind {
200+
// In panic=abort mode we assume nothing can unwind anywhere, so
201+
// optimize based on this!
202+
false
203+
} else if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::UNWIND) {
204+
// If a specific #[unwind] attribute is present, use that
205+
true
199206
} else if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_ALLOCATOR_NOUNWIND) {
200-
Some(false)
201-
202-
// Perhaps questionable, but we assume that anything defined
203-
// *in Rust code* may unwind. Foreign items like `extern "C" {
204-
// fn foo(); }` are assumed not to unwind **unless** they have
205-
// a `#[unwind]` attribute.
206-
} else if id.map(|id| !cx.tcx.is_foreign_item(id)).unwrap_or(false) {
207-
Some(true)
208-
} else {
209-
None
210-
};
211-
212-
match can_unwind {
213-
Some(false) => attributes::unwind(llfn, false),
214-
Some(true) if cx.tcx.sess.panic_strategy() == PanicStrategy::Unwind => {
215-
attributes::unwind(llfn, true);
207+
// Special attribute for allocator functions, which can't unwind
208+
false
209+
} else if let Some(id) = id {
210+
let sig = cx.tcx.normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), &sig);
211+
if cx.tcx.is_foreign_item(id) {
212+
// Foreign items like `extern "C" { fn foo(); }` are assumed not to
213+
// unwind
214+
false
215+
} else if sig.abi != Abi::Rust && sig.abi != Abi::RustCall {
216+
// Any items defined in Rust that *don't* have the `extern` ABI are
217+
// defined to not unwind. We insert shims to abort if an unwind
218+
// happens to enforce this.
219+
false
220+
} else {
221+
// Anything else defined in Rust is assumed that it can possibly
222+
// unwind
223+
true
216224
}
217-
Some(true) | None => {}
218-
}
225+
} else {
226+
// assume this can possibly unwind, avoiding the application of a
227+
// `nounwind` attribute below.
228+
true
229+
});
219230

220231
// Always annotate functions with the target-cpu they are compiled for.
221232
// Without this, ThinLTO won't inline Rust functions into Clang generated

‎src/librustc_codegen_llvm/callee.rs

Copy file name to clipboardExpand all lines: src/librustc_codegen_llvm/callee.rs
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ pub fn get_fn(
9494
if instance.def.is_inline(tcx) {
9595
attributes::inline(cx, llfn, attributes::InlineAttr::Hint);
9696
}
97-
attributes::from_fn_attrs(cx, llfn, Some(instance.def.def_id()));
97+
attributes::from_fn_attrs(cx, llfn, Some(instance.def.def_id()), sig);
9898

9999
let instance_def_id = instance.def_id();
100100

‎src/librustc_codegen_llvm/context.rs

Copy file name to clipboardExpand all lines: src/librustc_codegen_llvm/context.rs
-1Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,6 @@ impl MiscMethods<'tcx> for CodegenCx<'ll, 'tcx> {
409409
));
410410

411411
let llfn = self.declare_fn("rust_eh_unwind_resume", sig);
412-
attributes::unwind(llfn, true);
413412
attributes::apply_target_cpu_attr(self, llfn);
414413
unwresume.set(Some(llfn));
415414
llfn

‎src/librustc_codegen_llvm/declare.rs

Copy file name to clipboardExpand all lines: src/librustc_codegen_llvm/declare.rs
+1-10Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,7 @@ use rustc::ty::{self, PolyFnSig};
2626
use rustc::ty::layout::LayoutOf;
2727
use rustc::session::config::Sanitizer;
2828
use rustc_data_structures::small_c_str::SmallCStr;
29-
use rustc_target::spec::PanicStrategy;
30-
use abi::{Abi, FnType, FnTypeExt};
29+
use abi::{FnType, FnTypeExt};
3130
use attributes;
3231
use context::CodegenCx;
3332
use type_::Type;
@@ -86,10 +85,6 @@ fn declare_raw_fn(
8685
_ => {},
8786
}
8887

89-
if cx.tcx.sess.panic_strategy() != PanicStrategy::Unwind {
90-
attributes::unwind(llfn, false);
91-
}
92-
9388
attributes::non_lazy_bind(cx.sess(), llfn);
9489

9590
llfn
@@ -132,10 +127,6 @@ impl DeclareMethods<'tcx> for CodegenCx<'ll, 'tcx> {
132127
llvm::Attribute::NoReturn.apply_llfn(Function, llfn);
133128
}
134129

135-
if sig.abi != Abi::Rust && sig.abi != Abi::RustCall {
136-
attributes::unwind(llfn, false);
137-
}
138-
139130
fty.apply_attrs_llfn(llfn);
140131

141132
llfn

‎src/librustc_codegen_llvm/intrinsic.rs

Copy file name to clipboardExpand all lines: src/librustc_codegen_llvm/intrinsic.rs
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1081,7 +1081,7 @@ fn gen_fn<'ll, 'tcx>(
10811081
Abi::Rust
10821082
));
10831083
let llfn = cx.define_internal_fn(name, rust_fn_sig);
1084-
attributes::from_fn_attrs(cx, llfn, None);
1084+
attributes::from_fn_attrs(cx, llfn, None, rust_fn_sig);
10851085
let bx = Builder::new_block(cx, llfn, "entry-block");
10861086
codegen(bx);
10871087
llfn

‎src/librustc_codegen_llvm/mono_item.rs

Copy file name to clipboardExpand all lines: src/librustc_codegen_llvm/mono_item.rs
+6-1Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,12 @@ impl PreDefineMethods<'tcx> for CodegenCx<'ll, 'tcx> {
8282
if instance.def.is_inline(self.tcx) {
8383
attributes::inline(self, lldecl, attributes::InlineAttr::Hint);
8484
}
85-
attributes::from_fn_attrs(self, lldecl, Some(instance.def.def_id()));
85+
attributes::from_fn_attrs(
86+
self,
87+
lldecl,
88+
Some(instance.def.def_id()),
89+
mono_sig,
90+
);
8691

8792
self.instances.borrow_mut().insert(instance, lldecl);
8893
}

‎src/librustc_mir/build/mod.rs

Copy file name to clipboardExpand all lines: src/librustc_mir/build/mod.rs
+1-6Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -628,12 +628,7 @@ fn should_abort_on_panic<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
628628
// unwind anyway. Don't stop them.
629629
let attrs = &tcx.get_attrs(fn_def_id);
630630
match attr::find_unwind_attr(Some(tcx.sess.diagnostic()), attrs) {
631-
None => {
632-
// FIXME(rust-lang/rust#48251) -- Had to disable
633-
// abort-on-panic for backwards compatibility reasons.
634-
false
635-
}
636-
631+
None => true,
637632
Some(UnwindAttr::Allowed) => false,
638633
Some(UnwindAttr::Aborts) => true,
639634
}

‎src/test/codegen/nounwind-extern.rs

Copy file name to clipboard
+16Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// compile-flags: -O
12+
13+
#![crate_type = "lib"]
14+
15+
// CHECK: Function Attrs: norecurse nounwind
16+
pub extern fn foo() {}

‎src/test/run-pass/abort-on-c-abi.rs

Copy file name to clipboardExpand all lines: src/test/run-pass/abort-on-c-abi.rs
-3Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,11 @@
1515
// ignore-cloudabi no env and process
1616
// ignore-emscripten no processes
1717

18-
#![feature(unwind_attributes)]
19-
2018
use std::{env, panic};
2119
use std::io::prelude::*;
2220
use std::io;
2321
use std::process::{Command, Stdio};
2422

25-
#[unwind(aborts)]
2623
extern "C" fn panic_in_ffi() {
2724
panic!("Test");
2825
}

‎src/test/run-pass/extern/extern-call-deep.rs

Copy file name to clipboardExpand all lines: src/test/run-pass/extern/extern-call-deep.rs
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
// run-pass
1212
// ignore-wasm32-bare no libc to test ffi with
13+
// ignore-emscripten blows the JS stack
1314

1415
#![feature(rustc_private)]
1516

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.