Skip to content

Commit

Permalink
Workaround LLVM optimizer bug by not marking &mut pointers as noalias
Browse files Browse the repository at this point in the history
LLVM's memory dependence analysis doesn't properly account for calls
that could unwind and thus effectively act as a branching point. This
can lead to stores that are only visible when the call unwinds being
removed, possibly leading to calls to drop() functions with b0rked
memory contents.

As there is no fix for this in LLVM yet and we want to keep
compatibility to current LLVM versions anyways, we have to workaround
this bug by omitting the noalias attribute on &mut function arguments.
Benchmarks suggest that the performance loss by this change is very
small.

Thanks to @RalfJung for pushing me towards not removing too many
noalias annotations and @alexcrichton for helping out with the test for
this bug.

Fixes rust-lang#29485
  • Loading branch information
dotdash committed Feb 10, 2016
1 parent 052b3fd commit a17fb64
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/librustc_trans/trans/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ pub fn from_fn_type<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, fn_type: ty::Ty<'tcx
// on memory dependencies rather than pointer equality
let interior_unsafe = mt.ty.type_contents(ccx.tcx()).interior_unsafe();

if mt.mutbl == hir::MutMutable || !interior_unsafe {
if mt.mutbl != hir::MutMutable && !interior_unsafe {
attrs.arg(idx, llvm::Attribute::NoAlias);
}

Expand Down
26 changes: 26 additions & 0 deletions src/test/auxiliary/issue-29485.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2016 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.

#![crate_name="a"]
#![crate_type = "lib"]

pub struct X(pub u8);

impl Drop for X {
fn drop(&mut self) {
assert_eq!(self.0, 1)
}
}

pub fn f(x: &mut X, g: fn()) {
x.0 = 1;
g();
x.0 = 0;
}
9 changes: 6 additions & 3 deletions src/test/codegen/function-arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,16 @@ pub fn named_borrow<'r>(_: &'r i32) {
pub fn unsafe_borrow(_: &UnsafeInner) {
}

// CHECK: @mutable_unsafe_borrow(%UnsafeInner* noalias dereferenceable(2))
// CHECK: @mutable_unsafe_borrow(%UnsafeInner* dereferenceable(2))
// ... unless this is a mutable borrow, those never alias
// ... except that there's this LLVM bug that forces us to not use noalias, see #29485
#[no_mangle]
pub fn mutable_unsafe_borrow(_: &mut UnsafeInner) {
}

// CHECK: @mutable_borrow(i32* noalias dereferenceable(4))
// CHECK: @mutable_borrow(i32* dereferenceable(4))
// FIXME #25759 This should also have `nocapture`
// ... there's this LLVM bug that forces us to not use noalias, see #29485
#[no_mangle]
pub fn mutable_borrow(_: &mut i32) {
}
Expand Down Expand Up @@ -100,8 +102,9 @@ fn helper(_: usize) {
fn slice(_: &[u8]) {
}

// CHECK: @mutable_slice(i8* noalias nonnull, [[USIZE]])
// CHECK: @mutable_slice(i8* nonnull, [[USIZE]])
// FIXME #25759 This should also have `nocapture`
// ... there's this LLVM bug that forces us to not use noalias, see #29485
#[no_mangle]
fn mutable_slice(_: &mut [u8]) {
}
Expand Down
25 changes: 25 additions & 0 deletions src/test/run-pass/issue-29485.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2016 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.

// aux-build:issue-29485.rs

#[feature(recover)]

extern crate a;

fn main() {
let _ = std::thread::spawn(move || {
a::f(&mut a::X(0), g);
}).join();
}

fn g() {
panic!();
}

0 comments on commit a17fb64

Please sign in to comment.