Skip to content

Commit

Permalink
std: Ensure OOM is classified as nounwind
Browse files Browse the repository at this point in the history
OOM can't unwind today, and historically it's been optimized as if it can't
unwind. This accidentally regressed with recent changes to the OOM handler, so
this commit adds in a codegen test to assert that everything gets optimized away
after the OOM function is approrpiately classified as nounwind

Closes #50925
  • Loading branch information
alexcrichton committed May 24, 2018
1 parent a76bff8 commit f674537
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 16 deletions.
1 change: 1 addition & 0 deletions src/liballoc/alloc.rs
Expand Up @@ -133,6 +133,7 @@ pub(crate) unsafe fn box_free<T: ?Sized>(ptr: Unique<T>) {
}
}

#[rustc_allocator_nounwind]
pub fn oom() -> ! {
extern {
#[lang = "oom"]
Expand Down
29 changes: 24 additions & 5 deletions src/librustc_codegen_llvm/attributes.rs
Expand Up @@ -20,7 +20,9 @@ use rustc::ty::TyCtxt;
use rustc::ty::maps::Providers;
use rustc_data_structures::sync::Lrc;
use rustc_data_structures::fx::FxHashMap;
use rustc_target::spec::PanicStrategy;

use attributes;
use llvm::{self, Attribute, ValueRef};
use llvm::AttributePlace::Function;
use llvm_util;
Expand Down Expand Up @@ -135,11 +137,28 @@ pub fn from_fn_attrs(cx: &CodegenCx, llfn: ValueRef, id: DefId) {
Attribute::NoAlias.apply_llfn(
llvm::AttributePlace::ReturnValue, llfn);
}
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::UNWIND) {
unwind(llfn, true);
}
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_ALLOCATOR_NOUNWIND) {
unwind(llfn, false);

let can_unwind = if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::UNWIND) {
Some(true)
} else if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_ALLOCATOR_NOUNWIND) {
Some(false)

// Perhaps questionable, but we assume that anything defined
// *in Rust code* may unwind. Foreign items like `extern "C" {
// fn foo(); }` are assumed not to unwind **unless** they have
// a `#[unwind]` attribute.
} else if !cx.tcx.is_foreign_item(id) {
Some(true)
} else {
None
};

match can_unwind {
Some(false) => attributes::unwind(llfn, false),
Some(true) if cx.tcx.sess.panic_strategy() == PanicStrategy::Unwind => {
attributes::unwind(llfn, true);
}
Some(true) | None => {}
}

let features = llvm_target_features(cx.tcx.sess)
Expand Down
11 changes: 0 additions & 11 deletions src/librustc_codegen_llvm/callee.rs
Expand Up @@ -26,7 +26,6 @@ use rustc::hir::def_id::DefId;
use rustc::ty::{self, TypeFoldable};
use rustc::ty::layout::LayoutOf;
use rustc::ty::subst::Substs;
use rustc_target::spec::PanicStrategy;

/// Codegens a reference to a fn/method item, monomorphizing and
/// inlining as it goes.
Expand Down Expand Up @@ -102,16 +101,6 @@ pub fn get_fn<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,

let instance_def_id = instance.def_id();

// Perhaps questionable, but we assume that anything defined
// *in Rust code* may unwind. Foreign items like `extern "C" {
// fn foo(); }` are assumed not to unwind **unless** they have
// a `#[unwind]` attribute.
if tcx.sess.panic_strategy() == PanicStrategy::Unwind {
if !tcx.is_foreign_item(instance_def_id) {
attributes::unwind(llfn, true);
}
}

// Apply an appropriate linkage/visibility value to our item that we
// just declared.
//
Expand Down
21 changes: 21 additions & 0 deletions src/test/codegen/vec-iter-collect-len.rs
@@ -0,0 +1,21 @@
// Copyright 2017 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.

// no-system-llvm
// compile-flags: -O
#![crate_type="lib"]

#[no_mangle]
pub fn get_len() -> usize {
// CHECK-LABEL: @get_len
// CHECK-NOT: call
// CHECK-NOT: invoke
[1, 2, 3].iter().collect::<Vec<_>>().len()
}

0 comments on commit f674537

Please sign in to comment.