Skip to content

Commit 0cd3587

Browse files
committed
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.
1 parent 2fa5340 commit 0cd3587

File tree

3 files changed

+158
-4
lines changed

3 files changed

+158
-4
lines changed

src/librustc_trans/intrinsic.rs

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,38 @@ fn get_simple_intrinsic(ccx: &CrateContext, name: &str) -> Option<ValueRef> {
8383
Some(ccx.get_intrinsic(&llvm_name))
8484
}
8585

86+
fn warn_if_size_is_weird<'a, 'tcx>(bcx: &Builder<'a, 'tcx>,
87+
tp_ty: Ty<'tcx>,
88+
count: ValueRef,
89+
span: Span,
90+
name: &str) {
91+
let ccx = bcx.ccx;
92+
let lltp_ty = type_of::type_of(ccx, tp_ty);
93+
let ty_size = machine::llsize_of(ccx, lltp_ty);
94+
let total = const_to_uint( bcx.mul(ty_size, count) );
95+
96+
if total > 0 {
97+
return;
98+
}
99+
100+
let text = format!("suspicious monomorphization of `{}` intrinsic", name);
101+
let note = match name
102+
{
103+
"volatile_load" | "volatile_store" =>
104+
format!("'{}' was specialized with zero-sized type '{}'",
105+
name, tp_ty),
106+
_ => format!("'{}' was specialized with type '{}', number of \
107+
elements is {}",
108+
name, tp_ty,
109+
const_to_uint(count))
110+
};
111+
112+
let sess = bcx.sess();
113+
sess.struct_span_warn(span, &text)
114+
.note(&note)
115+
.emit();
116+
}
117+
86118
/// Remember to add all intrinsics here, in librustc_typeck/check/mod.rs,
87119
/// and in libcore/intrinsics.rs; if you need access to any llvm intrinsics,
88120
/// add them to librustc_trans/trans/context.rs
@@ -217,17 +249,24 @@ pub fn trans_intrinsic_call<'a, 'tcx>(bcx: &Builder<'a, 'tcx>,
217249
}
218250

219251
"volatile_copy_nonoverlapping_memory" => {
220-
copy_intrinsic(bcx, false, true, substs.type_at(0), llargs[0], llargs[1], llargs[2])
252+
let tp_ty = substs.type_at(0);
253+
warn_if_size_is_weird(bcx, tp_ty, llargs[2], span, name);
254+
copy_intrinsic(bcx, false, true, tp_ty, llargs[0], llargs[1], llargs[2])
221255
}
222256
"volatile_copy_memory" => {
223-
copy_intrinsic(bcx, true, true, substs.type_at(0), llargs[0], llargs[1], llargs[2])
257+
let tp_ty = substs.type_at(0);
258+
warn_if_size_is_weird(bcx, tp_ty, llargs[2], span, name);
259+
copy_intrinsic(bcx, true, true, tp_ty, llargs[0], llargs[1], llargs[2])
224260
}
225261
"volatile_set_memory" => {
226-
memset_intrinsic(bcx, true, substs.type_at(0), llargs[0], llargs[1], llargs[2])
262+
let tp_ty = substs.type_at(0);
263+
warn_if_size_is_weird(bcx, tp_ty, llargs[2], span, name);
264+
memset_intrinsic(bcx, true, tp_ty, llargs[0], llargs[1], llargs[2])
227265
}
228266
"volatile_load" => {
229267
let tp_ty = substs.type_at(0);
230268
let mut ptr = llargs[0];
269+
warn_if_size_is_weird(bcx, tp_ty, C_uint(ccx,1usize), span, name);
231270
if let Some(ty) = fn_ty.ret.cast {
232271
ptr = bcx.pointercast(ptr, ty.ptr_to());
233272
}
@@ -239,14 +278,19 @@ pub fn trans_intrinsic_call<'a, 'tcx>(bcx: &Builder<'a, 'tcx>,
239278
},
240279
"volatile_store" => {
241280
let tp_ty = substs.type_at(0);
281+
warn_if_size_is_weird(bcx, tp_ty, C_uint(ccx,1usize), span, name);
242282
if type_is_fat_ptr(bcx.ccx, tp_ty) {
243283
bcx.volatile_store(llargs[1], get_dataptr(bcx, llargs[0]));
244284
bcx.volatile_store(llargs[2], get_meta(bcx, llargs[0]));
245285
} else {
246286
let val = if fn_ty.args[1].is_indirect() {
247287
bcx.load(llargs[1], None)
248288
} else {
249-
from_immediate(bcx, llargs[1])
289+
if !type_is_zero_size(ccx, tp_ty) {
290+
from_immediate(bcx, llargs[1])
291+
} else {
292+
C_nil(ccx)
293+
}
250294
};
251295
let ptr = bcx.pointercast(llargs[0], val_ty(val).ptr_to());
252296
let store = bcx.volatile_store(val, ptr);

src/test/ui/issue-39827.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
#![feature(core_intrinsics)]
11+
12+
use std::intrinsics::{ volatile_copy_memory, volatile_store, volatile_load,
13+
volatile_copy_nonoverlapping_memory,
14+
volatile_set_memory };
15+
16+
fn main () {
17+
let mut dst_pair = (1, 2);
18+
let src_pair = (3, 4);
19+
let mut dst_empty = ();
20+
let src_empty = ();
21+
22+
const COUNT_0: usize = 0;
23+
const COUNT_100: usize = 100;
24+
25+
unsafe {
26+
volatile_copy_memory(&mut dst_pair, &dst_pair, COUNT_0);
27+
volatile_copy_nonoverlapping_memory(&mut dst_pair, &src_pair, 0);
28+
volatile_copy_memory(&mut dst_empty, &dst_empty, 100);
29+
volatile_copy_nonoverlapping_memory(&mut dst_empty, &src_empty,
30+
COUNT_100);
31+
volatile_set_memory(&mut dst_empty, 0, COUNT_100);
32+
volatile_set_memory(&mut dst_pair, 0, COUNT_0);
33+
volatile_store(&mut dst_empty, ());
34+
volatile_store(&mut dst_empty, src_empty);
35+
volatile_load(&src_empty);
36+
}
37+
}

src/test/ui/issue-39827.stderr

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
warning: suspicious monomorphization of `volatile_copy_memory` intrinsic
2+
--> $DIR/issue-39827.rs:26:9
3+
|
4+
26 | volatile_copy_memory(&mut dst_pair, &dst_pair, COUNT_0);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: 'volatile_copy_memory' was specialized with type '(i32, i32)', number of elements is 0
8+
9+
warning: suspicious monomorphization of `volatile_copy_nonoverlapping_memory` intrinsic
10+
--> $DIR/issue-39827.rs:27:9
11+
|
12+
27 | volatile_copy_nonoverlapping_memory(&mut dst_pair, &src_pair, 0);
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
14+
|
15+
= note: 'volatile_copy_nonoverlapping_memory' was specialized with type '(i32, i32)', number of elements is 0
16+
17+
warning: suspicious monomorphization of `volatile_copy_memory` intrinsic
18+
--> $DIR/issue-39827.rs:28:9
19+
|
20+
28 | volatile_copy_memory(&mut dst_empty, &dst_empty, 100);
21+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
22+
|
23+
= note: 'volatile_copy_memory' was specialized with type '()', number of elements is 100
24+
25+
warning: suspicious monomorphization of `volatile_copy_nonoverlapping_memory` intrinsic
26+
--> $DIR/issue-39827.rs:29:9
27+
|
28+
29 | / volatile_copy_nonoverlapping_memory(&mut dst_empty, &src_empty,
29+
30 | | COUNT_100);
30+
| |______________________________________________________^
31+
|
32+
= note: 'volatile_copy_nonoverlapping_memory' was specialized with type '()', number of elements is 100
33+
34+
warning: suspicious monomorphization of `volatile_set_memory` intrinsic
35+
--> $DIR/issue-39827.rs:31:9
36+
|
37+
31 | volatile_set_memory(&mut dst_empty, 0, COUNT_100);
38+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
39+
|
40+
= note: 'volatile_set_memory' was specialized with type '()', number of elements is 100
41+
42+
warning: suspicious monomorphization of `volatile_set_memory` intrinsic
43+
--> $DIR/issue-39827.rs:32:9
44+
|
45+
32 | volatile_set_memory(&mut dst_pair, 0, COUNT_0);
46+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
47+
|
48+
= note: 'volatile_set_memory' was specialized with type '(i32, i32)', number of elements is 0
49+
50+
warning: suspicious monomorphization of `volatile_store` intrinsic
51+
--> $DIR/issue-39827.rs:33:9
52+
|
53+
33 | volatile_store(&mut dst_empty, ());
54+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
55+
|
56+
= note: 'volatile_store' was specialized with zero-sized type '()'
57+
58+
warning: suspicious monomorphization of `volatile_store` intrinsic
59+
--> $DIR/issue-39827.rs:34:9
60+
|
61+
34 | volatile_store(&mut dst_empty, src_empty);
62+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
63+
|
64+
= note: 'volatile_store' was specialized with zero-sized type '()'
65+
66+
warning: suspicious monomorphization of `volatile_load` intrinsic
67+
--> $DIR/issue-39827.rs:35:9
68+
|
69+
35 | volatile_load(&src_empty);
70+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
71+
|
72+
= note: 'volatile_load' was specialized with zero-sized type '()'
73+

0 commit comments

Comments
 (0)