Skip to content

Commit f71e0da

Browse files
sfanxiangsfanxiang
committed
Add llvm.sideeffect to potential infinite loops and recursions
LLVM assumes that a thread will eventually cause side effect. This is not true in Rust if a loop or recursion does nothing in its body, causing undefined behavior even in common cases like `loop {}`. Inserting llvm.sideeffect fixes the undefined behavior. As a micro-optimization, only insert llvm.sideeffect when jumping back in blocks or calling a function. A patch for LLVM is expected to allow empty non-terminate code by default and fix this issue from LLVM side. #28728
1 parent a37fe2d commit f71e0da

18 files changed

+127
-18
lines changed

‎src/librustc_codegen_llvm/context.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,7 @@ impl CodegenCx<'b, 'tcx> {
537537
ifn!("llvm.trap", fn() -> void);
538538
ifn!("llvm.debugtrap", fn() -> void);
539539
ifn!("llvm.frameaddress", fn(t_i32) -> i8p);
540+
ifn!("llvm.sideeffect", fn() -> void);
540541

541542
ifn!("llvm.powi.f32", fn(t_f32, t_i32) -> t_f32);
542543
ifn!("llvm.powi.v2f32", fn(t_v2f32, t_i32) -> t_v2f32);

‎src/librustc_codegen_llvm/intrinsic.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> {
124124
self.call(expect, &[args[0].immediate(), self.const_bool(false)], None)
125125
}
126126
"try" => {
127+
self.sideeffect();
127128
try_intrinsic(self,
128129
args[0].immediate(),
129130
args[1].immediate(),
@@ -724,6 +725,11 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> {
724725
self.call(expect, &[cond, self.const_bool(expected)], None)
725726
}
726727

728+
fn sideeffect(&mut self) {
729+
let fnname = self.get_intrinsic(&("llvm.sideeffect"));
730+
self.call(fnname, &[], None);
731+
}
732+
727733
fn va_start(&mut self, va_list: &'ll Value) -> &'ll Value {
728734
let intrinsic = self.cx().get_intrinsic("llvm.va_start");
729735
self.call(intrinsic, &[va_list], None)

‎src/librustc_codegen_ssa/mir/block.rs

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,24 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'a, 'tcx> {
148148
}
149149
}
150150
}
151+
152+
// Generate sideeffect intrinsic if jumping to any of the targets can form
153+
// a loop.
154+
fn maybe_sideeffect<'b, 'tcx2: 'b, Bx: BuilderMethods<'b, 'tcx2>>(
155+
&self,
156+
mir: &'b mir::Body<'tcx>,
157+
bx: &mut Bx,
158+
targets: &[mir::BasicBlock],
159+
) {
160+
if targets.iter().any(|target| {
161+
*target <= *self.bb
162+
&& target
163+
.start_location()
164+
.is_predecessor_of(self.bb.start_location(), mir)
165+
}) {
166+
bx.sideeffect();
167+
}
168+
}
151169
}
152170

153171
/// Codegen implementations for some terminator variants.
@@ -196,6 +214,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
196214
let lltrue = helper.llblock(self, targets[0]);
197215
let llfalse = helper.llblock(self, targets[1]);
198216
if switch_ty == bx.tcx().types.bool {
217+
helper.maybe_sideeffect(self.mir, &mut bx, targets.as_slice());
199218
// Don't generate trivial icmps when switching on bool
200219
if let [0] = values[..] {
201220
bx.cond_br(discr.immediate(), llfalse, lltrue);
@@ -209,9 +228,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
209228
);
210229
let llval = bx.const_uint_big(switch_llty, values[0]);
211230
let cmp = bx.icmp(IntPredicate::IntEQ, discr.immediate(), llval);
231+
helper.maybe_sideeffect(self.mir, &mut bx, targets.as_slice());
212232
bx.cond_br(cmp, lltrue, llfalse);
213233
}
214234
} else {
235+
helper.maybe_sideeffect(self.mir, &mut bx, targets.as_slice());
215236
let (otherwise, targets) = targets.split_last().unwrap();
216237
bx.switch(
217238
discr.immediate(),
@@ -310,6 +331,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
310331

311332
if let ty::InstanceDef::DropGlue(_, None) = drop_fn.def {
312333
// we don't actually need to drop anything.
334+
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
313335
helper.funclet_br(self, &mut bx, target);
314336
return
315337
}
@@ -340,6 +362,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
340362
FnType::of_instance(&bx, drop_fn))
341363
}
342364
};
365+
bx.sideeffect();
343366
helper.do_call(self, &mut bx, fn_ty, drop_fn, args,
344367
Some((ReturnDest::Nothing, target)),
345368
unwind);
@@ -375,6 +398,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
375398

376399
// Don't codegen the panic block if success if known.
377400
if const_cond == Some(expected) {
401+
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
378402
helper.funclet_br(self, &mut bx, target);
379403
return;
380404
}
@@ -385,6 +409,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
385409
// Create the failure block and the conditional branch to it.
386410
let lltarget = helper.llblock(self, target);
387411
let panic_block = self.new_block("panic");
412+
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
388413
if expected {
389414
bx.cond_br(cond, lltarget, panic_block.llbb());
390415
} else {
@@ -437,6 +462,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
437462
let fn_ty = FnType::of_instance(&bx, instance);
438463
let llfn = bx.get_fn(instance);
439464

465+
bx.sideeffect();
440466
// Codegen the actual panic invoke/call.
441467
helper.do_call(self, &mut bx, fn_ty, llfn, &args, None, cleanup);
442468
}
@@ -488,6 +514,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
488514
if let Some(destination_ref) = destination.as_ref() {
489515
let &(ref dest, target) = destination_ref;
490516
self.codegen_transmute(&mut bx, &args[0], dest);
517+
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
491518
helper.funclet_br(self, &mut bx, target);
492519
} else {
493520
// If we are trying to transmute to an uninhabited type,
@@ -518,6 +545,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
518545
Some(ty::InstanceDef::DropGlue(_, None)) => {
519546
// Empty drop glue; a no-op.
520547
let &(_, target) = destination.as_ref().unwrap();
548+
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
521549
helper.funclet_br(self, &mut bx, target);
522550
return;
523551
}
@@ -554,6 +582,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
554582
let fn_ty = FnType::of_instance(&bx, instance);
555583
let llfn = bx.get_fn(instance);
556584

585+
bx.sideeffect();
557586
// Codegen the actual panic invoke/call.
558587
helper.do_call(
559588
self,
@@ -566,7 +595,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
566595
);
567596
} else {
568597
// a NOP
569-
helper.funclet_br(self, &mut bx, destination.as_ref().unwrap().1)
598+
let target = destination.as_ref().unwrap().1;
599+
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
600+
helper.funclet_br(self, &mut bx, target);
570601
}
571602
return;
572603
}
@@ -675,6 +706,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
675706
}
676707

677708
if let Some((_, target)) = *destination {
709+
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
678710
helper.funclet_br(self, &mut bx, target);
679711
} else {
680712
bx.unreachable();
@@ -786,6 +818,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
786818
_ => span_bug!(span, "no llfn for call"),
787819
};
788820

821+
bx.sideeffect();
789822
helper.do_call(self, &mut bx, fn_ty, fn_ptr, &llargs,
790823
destination.as_ref().map(|&(_, target)| (ret_dest, target)),
791824
cleanup);
@@ -835,6 +868,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
835868
}
836869

837870
mir::TerminatorKind::Goto { target } => {
871+
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
838872
helper.funclet_br(self, &mut bx, target);
839873
}
840874

‎src/librustc_codegen_ssa/mir/rvalue.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
486486
};
487487
let instance = ty::Instance::mono(bx.tcx(), def_id);
488488
let r = bx.cx().get_fn(instance);
489+
bx.sideeffect();
489490
let call = bx.call(r, &[llsize, llalign], None);
490491
let val = bx.pointercast(call, llty_ptr);
491492

‎src/librustc_codegen_ssa/traits/intrinsic.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ pub trait IntrinsicCallMethods<'tcx>: BackendTypes {
2020
fn abort(&mut self);
2121
fn assume(&mut self, val: Self::Value);
2222
fn expect(&mut self, cond: Self::Value, expected: bool) -> Self::Value;
23+
fn sideeffect(&mut self);
2324
/// Trait method used to inject `va_start` on the "spoofed" `VaListImpl` in
2425
/// Rust defined C-variadic functions.
2526
fn va_start(&mut self, val: Self::Value) -> Self::Value;

‎src/test/codegen/alloc-optimisation.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
pub fn alloc_test(data: u32) {
88
// CHECK-LABEL: @alloc_test
99
// CHECK-NEXT: start:
10-
// CHECK-NEXT: ret void
10+
// CHECK-NOT: alloc
11+
// CHECK: ret void
1112
let x = Box::new(data);
1213
drop(x);
1314
}

‎src/test/codegen/dealloc-no-unwind.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ impl Drop for A {
1717
pub fn a(a: Box<i32>) {
1818
// CHECK-LABEL: define void @a
1919
// CHECK: call void @__rust_dealloc
20-
// CHECK-NEXT: call void @foo
20+
// CHECK: call void @foo
2121
let _a = A;
2222
drop(a);
2323
}

‎src/test/codegen/issue-34947-pow-i32.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,11 @@
66
#[no_mangle]
77
pub fn issue_34947(x: i32) -> i32 {
88
// CHECK: mul
9-
// CHECK-NEXT: mul
10-
// CHECK-NEXT: mul
11-
// CHECK-NEXT: ret
9+
// CHECK-NOT: br label
10+
// CHECK: mul
11+
// CHECK-NOT: br label
12+
// CHECK: mul
13+
// CHECK-NOT: br label
14+
// CHECK: ret
1215
x.pow(5)
1316
}

‎src/test/codegen/issue-45222.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
// ignore-test
2+
3+
// FIXME:
4+
// LLVM can't optimize some loops with a large number of iterations because of
5+
// @llvm.sideeffect() (see also #59546)
6+
17
// compile-flags: -O
28
// ignore-debug: the debug assertions get in the way
39

‎src/test/codegen/naked-functions.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
// ignore-tidy-linelength
2-
31
// compile-flags: -C no-prepopulate-passes
42

53
#![crate_type = "lib"]
@@ -53,27 +51,27 @@ pub fn naked_with_args_and_return(a: isize) -> isize {
5351
#[naked]
5452
pub fn naked_recursive() {
5553
// CHECK-NEXT: {{.+}}:
56-
// CHECK-NEXT: call void @naked_empty()
54+
// CHECK: call void @naked_empty()
5755

5856
// FIXME(#39685) Avoid one block per call.
5957
// CHECK-NEXT: br label %bb1
6058
// CHECK: bb1:
6159

6260
naked_empty();
6361

64-
// CHECK-NEXT: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_return()
62+
// CHECK: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_return()
6563

6664
// FIXME(#39685) Avoid one block per call.
6765
// CHECK-NEXT: br label %bb2
6866
// CHECK: bb2:
6967

70-
// CHECK-NEXT: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_args_and_return(i{{[0-9]+}} %{{[0-9]+}})
68+
// CHECK: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_args_and_return(i{{[0-9]+}} %{{[0-9]+}})
7169

7270
// FIXME(#39685) Avoid one block per call.
7371
// CHECK-NEXT: br label %bb3
7472
// CHECK: bb3:
7573

76-
// CHECK-NEXT: call void @naked_with_args(i{{[0-9]+}} %{{[0-9]+}})
74+
// CHECK: call void @naked_with_args(i{{[0-9]+}} %{{[0-9]+}})
7775

7876
// FIXME(#39685) Avoid one block per call.
7977
// CHECK-NEXT: br label %bb4

‎src/test/codegen/non-terminate/infinite-loop-1.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// compile-flags: -C opt-level=3
2+
3+
#![crate_type = "lib"]
4+
5+
fn infinite_loop() -> u8 {
6+
loop {}
7+
}
8+
9+
// CHECK-LABEL: @test
10+
#[no_mangle]
11+
fn test() -> u8 {
12+
// CHECK-NOT: unreachable
13+
// CHECK: br label %{{.+}}
14+
// CHECK-NOT: unreachable
15+
let x = infinite_loop();
16+
x
17+
}

‎src/test/codegen/non-terminate/infinite-loop-2.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// compile-flags: -C opt-level=3
2+
3+
#![crate_type = "lib"]
4+
5+
fn infinite_loop() -> u8 {
6+
let i = 2;
7+
while i > 1 {}
8+
1
9+
}
10+
11+
// CHECK-LABEL: @test
12+
#[no_mangle]
13+
fn test() -> u8 {
14+
// CHECK-NOT: unreachable
15+
// CHECK: br label %{{.+}}
16+
// CHECK-NOT: unreachable
17+
let x = infinite_loop();
18+
x
19+
}

‎src/test/codegen/non-terminate/infinite-recursion.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// compile-flags: -C opt-level=3
2+
3+
#![crate_type = "lib"]
4+
5+
#![allow(unconditional_recursion)]
6+
7+
// CHECK-LABEL: @infinite_recursion
8+
#[no_mangle]
9+
fn infinite_recursion() -> u8 {
10+
// CHECK-NOT: ret i8 undef
11+
// CHECK: br label %{{.+}}
12+
// CHECK-NOT: ret i8 undef
13+
infinite_recursion()
14+
}

‎src/test/codegen/repeat-trusted-len.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ pub fn helper(_: usize) {
1414
// CHECK-LABEL: @repeat_take_collect
1515
#[no_mangle]
1616
pub fn repeat_take_collect() -> Vec<u8> {
17-
// CHECK: call void @llvm.memset.p0i8.[[USIZE]](i8* {{(nonnull )?}}align 1 %{{[0-9]+}}, i8 42, [[USIZE]] 100000, i1 false)
17+
// FIXME: At the time of writing LLVM transforms this loop into a single
18+
// `store` and then a `memset` with size = 99999. The correct check should be:
19+
// call void @llvm.memset.p0i8.[[USIZE]](i8* {{(nonnull )?}}align 1 %{{[a-z0-9.]+}}, i8 42, [[USIZE]] 100000, i1 false)
20+
21+
// CHECK: call void @llvm.memset.p0i8.[[USIZE]](i8* {{(nonnull )?}}align 1 %{{[a-z0-9.]+}}, i8 42, [[USIZE]] 99999, i1 false)
1822
iter::repeat(42).take(100000).collect()
1923
}

‎src/test/codegen/vec-clear.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
// ignore-test
2+
3+
// FIXME:
4+
// LLVM can't optimize some loops with unknown number of iterations because of
5+
// @llvm.sideeffect() (see also #59546)
6+
17
// ignore-debug: the debug assertions get in the way
28
// compile-flags: -O
39

‎src/test/codegen/vec-iter-collect-len.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55

66
#[no_mangle]
77
pub fn get_len() -> usize {
8-
// CHECK-LABEL: @get_len
9-
// CHECK-NOT: call
10-
// CHECK-NOT: invoke
8+
// CHECK-COUNT-1: {{^define}}
119
[1, 2, 3].iter().collect::<Vec<_>>().len()
1210
}

‎src/test/codegen/vec-optimizes-away.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,6 @@
88
pub fn sum_me() -> i32 {
99
// CHECK-LABEL: @sum_me
1010
// CHECK-NEXT: {{^.*:$}}
11-
// CHECK-NEXT: ret i32 6
11+
// CHECK: ret i32 6
1212
vec![1, 2, 3].iter().sum::<i32>()
1313
}

‎src/test/run-make-fulldeps/inline-always-many-cgu/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
all:
44
$(RUSTC) foo.rs --emit llvm-ir -C codegen-units=2
5-
if cat $(TMPDIR)/*.ll | $(CGREP) -e '\bcall\b'; then \
5+
if cat $(TMPDIR)/*.ll | grep -v 'call void @llvm.sideeffect()' | $(CGREP) -e '\bcall\b'; then \
66
echo "found call instruction when one wasn't expected"; \
77
exit 1; \
88
fi

0 commit comments

Comments
 (0)