mirror of
https://github.com/rust-lang/rust.git
synced 2025-04-28 11:07:42 +00:00
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.
This commit is contained in:
parent
2fa5340318
commit
0cd358742d
@ -83,6 +83,38 @@ fn get_simple_intrinsic(ccx: &CrateContext, name: &str) -> Option<ValueRef> {
|
||||
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);
|
||||
|
37
src/test/ui/issue-39827.rs
Normal file
37
src/test/ui/issue-39827.rs
Normal file
@ -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 <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 };
|
||||
|
||||
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);
|
||||
}
|
||||
}
|
73
src/test/ui/issue-39827.stderr
Normal file
73
src/test/ui/issue-39827.stderr
Normal file
@ -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 '()'
|
||||
|
Loading…
Reference in New Issue
Block a user