From a17fb64fcecdc42c5e25dd1745e517db881f14e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Steinbrink?= Date: Fri, 29 Jan 2016 11:11:24 +0100 Subject: [PATCH] Workaround LLVM optimizer bug by not marking &mut pointers as noalias 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 #29485 --- src/librustc_trans/trans/attributes.rs | 2 +- src/test/auxiliary/issue-29485.rs | 26 ++++++++++++++++++++++++++ src/test/codegen/function-arguments.rs | 9 ++++++--- src/test/run-pass/issue-29485.rs | 25 +++++++++++++++++++++++++ 4 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 src/test/auxiliary/issue-29485.rs create mode 100644 src/test/run-pass/issue-29485.rs diff --git a/src/librustc_trans/trans/attributes.rs b/src/librustc_trans/trans/attributes.rs index 28dfa4e07e668..95c3941354f22 100644 --- a/src/librustc_trans/trans/attributes.rs +++ b/src/librustc_trans/trans/attributes.rs @@ -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); } diff --git a/src/test/auxiliary/issue-29485.rs b/src/test/auxiliary/issue-29485.rs new file mode 100644 index 0000000000000..825c449702154 --- /dev/null +++ b/src/test/auxiliary/issue-29485.rs @@ -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 or the MIT license +// , 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; +} diff --git a/src/test/codegen/function-arguments.rs b/src/test/codegen/function-arguments.rs index 90ced88324e7f..c373cdb76c5c7 100644 --- a/src/test/codegen/function-arguments.rs +++ b/src/test/codegen/function-arguments.rs @@ -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) { } @@ -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]) { } diff --git a/src/test/run-pass/issue-29485.rs b/src/test/run-pass/issue-29485.rs new file mode 100644 index 0000000000000..9e8f58694377a --- /dev/null +++ b/src/test/run-pass/issue-29485.rs @@ -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 or the MIT license +// , 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!(); +}