Skip to content

Commit

Permalink
Auto merge of #43836 - taleks:issue-39827, r=arielb1
Browse files Browse the repository at this point in the history
Fix for issue #39827

*Cause of the issue*

While preparing for `trans_intrinsic_call()` invoke arguments are processed with `trans_argument()` method which excludes zero-sized types from argument list (to be more correct - all arguments for which `ArgKind` is `Ignore` are filtered out). As result `volatile_store()` intrinsic gets one argument instead of expected address and value.

*How it is fixed*

Modification of the `trans_argument()` method may cause side effects, therefore change was implemented in `volatile_store()` intrinsic building code itself. Now it checks function signature and if it was specialised with zero-sized type, then emits `C_nil()` instead of accessing non-existing second argument.
  • Loading branch information
bors committed Aug 13, 2017
2 parents ab40a7c + faf6b84 commit f3cf206
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 4 deletions.
9 changes: 6 additions & 3 deletions src/libcore/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1044,20 +1044,23 @@ extern "rust-intrinsic" {
/// a size of `count` * `size_of::<T>()` and an alignment of
/// `min_align_of::<T>()`
///
/// The volatile parameter is set to `true`, so it will not be optimized out.
/// The volatile parameter is set to `true`, so it will not be optimized out
/// unless size is equal to zero.
pub fn volatile_copy_nonoverlapping_memory<T>(dst: *mut T, src: *const T,
count: usize);
/// Equivalent to the appropriate `llvm.memmove.p0i8.0i8.*` intrinsic, with
/// a size of `count` * `size_of::<T>()` and an alignment of
/// `min_align_of::<T>()`
///
/// The volatile parameter is set to `true`, so it will not be optimized out.
/// The volatile parameter is set to `true`, so it will not be optimized out
/// unless size is equal to zero..
pub fn volatile_copy_memory<T>(dst: *mut T, src: *const T, count: usize);
/// Equivalent to the appropriate `llvm.memset.p0i8.*` intrinsic, with a
/// size of `count` * `size_of::<T>()` and an alignment of
/// `min_align_of::<T>()`.
///
/// The volatile parameter is set to `true`, so it will not be optimized out.
/// The volatile parameter is set to `true`, so it will not be optimized out
/// unless size is equal to zero.
pub fn volatile_set_memory<T>(dst: *mut T, val: u8, count: usize);

/// Perform a volatile load from the `src` pointer.
Expand Down
10 changes: 10 additions & 0 deletions src/libcore/ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,11 @@ pub unsafe fn write_unaligned<T>(dst: *mut T, src: T) {
/// over time. That being said, the semantics will almost always end up pretty
/// similar to [C11's definition of volatile][c11].
///
/// The compiler shouldn't change the relative order or number of volatile
/// memory operations. However, volatile memory operations on zero-sized types
/// (e.g. if a zero-sized type is passed to `read_volatile`) are no-ops
/// and may be ignored.
///
/// [c11]: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf
///
/// # Safety
Expand Down Expand Up @@ -427,6 +432,11 @@ pub unsafe fn read_volatile<T>(src: *const T) -> T {
/// over time. That being said, the semantics will almost always end up pretty
/// similar to [C11's definition of volatile][c11].
///
/// The compiler shouldn't change the relative order or number of volatile
/// memory operations. However, volatile memory operations on zero-sized types
/// (e.g. if a zero-sized type is passed to `write_volatile`) are no-ops
/// and may be ignored.
///
/// [c11]: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf
///
/// # Safety
Expand Down
6 changes: 5 additions & 1 deletion src/librustc_trans/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,11 @@ pub fn trans_intrinsic_call<'a, 'tcx>(bcx: &Builder<'a, 'tcx>,
let val = if fn_ty.args[1].is_indirect() {
bcx.load(llargs[1], None)
} else {
from_immediate(bcx, llargs[1])
if !type_is_zero_size(ccx, tp_ty) {
from_immediate(bcx, llargs[1])
} else {
C_nil(ccx)
}
};
let ptr = bcx.pointercast(llargs[0], val_ty(val).ptr_to());
let store = bcx.volatile_store(val, ptr);
Expand Down
42 changes: 42 additions & 0 deletions src/test/run-pass/issue-39827.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// 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.
#![feature(core_intrinsics)]

use std::intrinsics::{ volatile_copy_memory, volatile_store, volatile_load,
volatile_copy_nonoverlapping_memory,
volatile_set_memory };

//
// This test ensures that volatile intrinsics can be specialised with
// zero-sized types and, in case of copy/set functions, can accept
// number of elements equal to zero.
//
fn main () {
let mut dst_pair = (1, 2);
let src_pair = (3, 4);
let mut dst_empty = ();
let src_empty = ();

const COUNT_0: usize = 0;
const COUNT_100: usize = 100;

unsafe {
volatile_copy_memory(&mut dst_pair, &dst_pair, COUNT_0);
volatile_copy_nonoverlapping_memory(&mut dst_pair, &src_pair, 0);
volatile_copy_memory(&mut dst_empty, &dst_empty, 100);
volatile_copy_nonoverlapping_memory(&mut dst_empty, &src_empty,
COUNT_100);
volatile_set_memory(&mut dst_empty, 0, COUNT_100);
volatile_set_memory(&mut dst_pair, 0, COUNT_0);
volatile_store(&mut dst_empty, ());
volatile_store(&mut dst_empty, src_empty);
volatile_load(&src_empty);
}
}

0 comments on commit f3cf206

Please sign in to comment.