From: Alexey Tarasov Date: Sat, 12 Aug 2017 08:42:44 +0000 (+1000) Subject: Fixes issue 39827: ICE in volatile_store intrinsic X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=0cd358742dfcf3fa6b0d8a318cfb5cc748dc905d;p=rust.git Fixes issue 39827: ICE in volatile_store intrinsic - adds handling of zero-sized types for volatile_store. - adds type size checks and warnigns for other volatile intrinsics. - adds a test to check warnings emitting. 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. Additionally warnings are added for all volatile operations which are specialised with zero-sized arguments. In fact, those operations are omitted in LLVM backend if no memory affected at all, e.g. number of elements is zero or type is zero-sized. This was not explicitly documented before and could lead to potential issues if developer expects volatile behaviour, but type has degraded to zero-sized. --- diff --git a/src/librustc_trans/intrinsic.rs b/src/librustc_trans/intrinsic.rs index 9956c28e641..5ebd9bed5c8 100644 --- a/src/librustc_trans/intrinsic.rs +++ b/src/librustc_trans/intrinsic.rs @@ -83,6 +83,38 @@ fn get_simple_intrinsic(ccx: &CrateContext, name: &str) -> Option { Some(ccx.get_intrinsic(&llvm_name)) } +fn warn_if_size_is_weird<'a, 'tcx>(bcx: &Builder<'a, 'tcx>, + tp_ty: Ty<'tcx>, + count: ValueRef, + span: Span, + name: &str) { + let ccx = bcx.ccx; + let lltp_ty = type_of::type_of(ccx, tp_ty); + let ty_size = machine::llsize_of(ccx, lltp_ty); + let total = const_to_uint( bcx.mul(ty_size, count) ); + + if total > 0 { + return; + } + + let text = format!("suspicious monomorphization of `{}` intrinsic", name); + let note = match name + { + "volatile_load" | "volatile_store" => + format!("'{}' was specialized with zero-sized type '{}'", + name, tp_ty), + _ => format!("'{}' was specialized with type '{}', number of \ + elements is {}", + name, tp_ty, + const_to_uint(count)) + }; + + let sess = bcx.sess(); + sess.struct_span_warn(span, &text) + .note(¬e) + .emit(); +} + /// Remember to add all intrinsics here, in librustc_typeck/check/mod.rs, /// and in libcore/intrinsics.rs; if you need access to any llvm intrinsics, /// add them to librustc_trans/trans/context.rs @@ -217,17 +249,24 @@ pub fn trans_intrinsic_call<'a, 'tcx>(bcx: &Builder<'a, 'tcx>, } "volatile_copy_nonoverlapping_memory" => { - copy_intrinsic(bcx, false, true, substs.type_at(0), llargs[0], llargs[1], llargs[2]) + let tp_ty = substs.type_at(0); + warn_if_size_is_weird(bcx, tp_ty, llargs[2], span, name); + copy_intrinsic(bcx, false, true, tp_ty, llargs[0], llargs[1], llargs[2]) } "volatile_copy_memory" => { - copy_intrinsic(bcx, true, true, substs.type_at(0), llargs[0], llargs[1], llargs[2]) + let tp_ty = substs.type_at(0); + warn_if_size_is_weird(bcx, tp_ty, llargs[2], span, name); + copy_intrinsic(bcx, true, true, tp_ty, llargs[0], llargs[1], llargs[2]) } "volatile_set_memory" => { - memset_intrinsic(bcx, true, substs.type_at(0), llargs[0], llargs[1], llargs[2]) + let tp_ty = substs.type_at(0); + warn_if_size_is_weird(bcx, tp_ty, llargs[2], span, name); + memset_intrinsic(bcx, true, tp_ty, llargs[0], llargs[1], llargs[2]) } "volatile_load" => { let tp_ty = substs.type_at(0); let mut ptr = llargs[0]; + warn_if_size_is_weird(bcx, tp_ty, C_uint(ccx,1usize), span, name); if let Some(ty) = fn_ty.ret.cast { ptr = bcx.pointercast(ptr, ty.ptr_to()); } @@ -239,6 +278,7 @@ pub fn trans_intrinsic_call<'a, 'tcx>(bcx: &Builder<'a, 'tcx>, }, "volatile_store" => { let tp_ty = substs.type_at(0); + warn_if_size_is_weird(bcx, tp_ty, C_uint(ccx,1usize), span, name); if type_is_fat_ptr(bcx.ccx, tp_ty) { bcx.volatile_store(llargs[1], get_dataptr(bcx, llargs[0])); bcx.volatile_store(llargs[2], get_meta(bcx, llargs[0])); @@ -246,7 +286,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); diff --git a/src/test/ui/issue-39827.rs b/src/test/ui/issue-39827.rs new file mode 100644 index 00000000000..86a3f67b40a --- /dev/null +++ b/src/test/ui/issue-39827.rs @@ -0,0 +1,37 @@ +// 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 or the MIT license +// , 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 }; + +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); + } +} diff --git a/src/test/ui/issue-39827.stderr b/src/test/ui/issue-39827.stderr new file mode 100644 index 00000000000..228309872f9 --- /dev/null +++ b/src/test/ui/issue-39827.stderr @@ -0,0 +1,73 @@ +warning: suspicious monomorphization of `volatile_copy_memory` intrinsic + --> $DIR/issue-39827.rs:26:9 + | +26 | volatile_copy_memory(&mut dst_pair, &dst_pair, COUNT_0); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: 'volatile_copy_memory' was specialized with type '(i32, i32)', number of elements is 0 + +warning: suspicious monomorphization of `volatile_copy_nonoverlapping_memory` intrinsic + --> $DIR/issue-39827.rs:27:9 + | +27 | volatile_copy_nonoverlapping_memory(&mut dst_pair, &src_pair, 0); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: 'volatile_copy_nonoverlapping_memory' was specialized with type '(i32, i32)', number of elements is 0 + +warning: suspicious monomorphization of `volatile_copy_memory` intrinsic + --> $DIR/issue-39827.rs:28:9 + | +28 | volatile_copy_memory(&mut dst_empty, &dst_empty, 100); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: 'volatile_copy_memory' was specialized with type '()', number of elements is 100 + +warning: suspicious monomorphization of `volatile_copy_nonoverlapping_memory` intrinsic + --> $DIR/issue-39827.rs:29:9 + | +29 | / volatile_copy_nonoverlapping_memory(&mut dst_empty, &src_empty, +30 | | COUNT_100); + | |______________________________________________________^ + | + = note: 'volatile_copy_nonoverlapping_memory' was specialized with type '()', number of elements is 100 + +warning: suspicious monomorphization of `volatile_set_memory` intrinsic + --> $DIR/issue-39827.rs:31:9 + | +31 | volatile_set_memory(&mut dst_empty, 0, COUNT_100); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: 'volatile_set_memory' was specialized with type '()', number of elements is 100 + +warning: suspicious monomorphization of `volatile_set_memory` intrinsic + --> $DIR/issue-39827.rs:32:9 + | +32 | volatile_set_memory(&mut dst_pair, 0, COUNT_0); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: 'volatile_set_memory' was specialized with type '(i32, i32)', number of elements is 0 + +warning: suspicious monomorphization of `volatile_store` intrinsic + --> $DIR/issue-39827.rs:33:9 + | +33 | volatile_store(&mut dst_empty, ()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: 'volatile_store' was specialized with zero-sized type '()' + +warning: suspicious monomorphization of `volatile_store` intrinsic + --> $DIR/issue-39827.rs:34:9 + | +34 | volatile_store(&mut dst_empty, src_empty); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: 'volatile_store' was specialized with zero-sized type '()' + +warning: suspicious monomorphization of `volatile_load` intrinsic + --> $DIR/issue-39827.rs:35:9 + | +35 | volatile_load(&src_empty); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: 'volatile_load' was specialized with zero-sized type '()' +