Skip to content

Commit 0ea0e06

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 2671421 commit 0ea0e06

File tree

15 files changed

+109
-12
lines changed

15 files changed

+109
-12
lines changed

‎src/librustc_codegen_llvm/context.rs‎

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

540541
ifn!("llvm.powi.f32", fn(t_f32, t_i32) -> t_f32);
541542
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
@@ -121,6 +121,7 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> {
121121
self.call(expect, &[args[0].immediate(), self.const_bool(false)], None)
122122
}
123123
"try" => {
124+
self.sideeffect();
124125
try_intrinsic(self,
125126
args[0].immediate(),
126127
args[1].immediate(),
@@ -718,6 +719,11 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> {
718719
self.call(expect, &[cond, self.const_bool(expected)], None)
719720
}
720721

722+
fn sideeffect(&mut self) {
723+
let fnname = self.get_intrinsic(&("llvm.sideeffect"));
724+
self.call(fnname, &[], None);
725+
}
726+
721727
fn va_start(&mut self, list: &'ll Value) -> &'ll Value {
722728
let target = &self.cx.tcx.sess.target.target;
723729
let arch = &target.arch;

‎src/librustc_codegen_ssa/mir/block.rs‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,18 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'a, 'tcx> {
149149
}
150150
}
151151
}
152+
153+
// Generate sideeffect intrinsic if any of the targets' index is not
154+
// greater than that of the current block.
155+
fn maybe_sideeffect<'b, 'tcx2: 'b, Bx: BuilderMethods<'b, 'tcx2>>(
156+
&self,
157+
bx: &mut Bx,
158+
targets: &[mir::BasicBlock],
159+
) {
160+
if targets.iter().any(|target| target <= self.bb) {
161+
bx.sideeffect();
162+
}
163+
}
152164
}
153165

154166
/// Codegen implementations for some terminator variants.
@@ -197,6 +209,7 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
197209
let lltrue = helper.llblock(self, targets[0]);
198210
let llfalse = helper.llblock(self, targets[1]);
199211
if switch_ty == bx.tcx().types.bool {
212+
helper.maybe_sideeffect(&mut bx, targets.as_slice());
200213
// Don't generate trivial icmps when switching on bool
201214
if let [0] = values[..] {
202215
bx.cond_br(discr.immediate(), llfalse, lltrue);
@@ -210,9 +223,11 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
210223
);
211224
let llval = bx.const_uint_big(switch_llty, values[0]);
212225
let cmp = bx.icmp(IntPredicate::IntEQ, discr.immediate(), llval);
226+
helper.maybe_sideeffect(&mut bx, targets.as_slice());
213227
bx.cond_br(cmp, lltrue, llfalse);
214228
}
215229
} else {
230+
helper.maybe_sideeffect(&mut bx, targets.as_slice());
216231
let (otherwise, targets) = targets.split_last().unwrap();
217232
bx.switch(
218233
discr.immediate(),
@@ -307,6 +322,7 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
307322

308323
if let ty::InstanceDef::DropGlue(_, None) = drop_fn.def {
309324
// we don't actually need to drop anything.
325+
helper.maybe_sideeffect(&mut bx, &[target]);
310326
helper.funclet_br(self, &mut bx, target);
311327
return
312328
}
@@ -337,6 +353,7 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
337353
bx.fn_type_of_instance(&drop_fn))
338354
}
339355
};
356+
bx.sideeffect();
340357
helper.do_call(self, &mut bx, fn_ty, drop_fn, args,
341358
Some((ReturnDest::Nothing, target)),
342359
unwind);
@@ -372,6 +389,7 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
372389

373390
// Don't codegen the panic block if success if known.
374391
if const_cond == Some(expected) {
392+
helper.maybe_sideeffect(&mut bx, &[target]);
375393
helper.funclet_br(self, &mut bx, target);
376394
return;
377395
}
@@ -382,6 +400,7 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
382400
// Create the failure block and the conditional branch to it.
383401
let lltarget = helper.llblock(self, target);
384402
let panic_block = self.new_block("panic");
403+
helper.maybe_sideeffect(&mut bx, &[target]);
385404
if expected {
386405
bx.cond_br(cond, lltarget, panic_block.llbb());
387406
} else {
@@ -435,6 +454,7 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
435454
let fn_ty = bx.fn_type_of_instance(&instance);
436455
let llfn = bx.get_fn(instance);
437456

457+
bx.sideeffect();
438458
// Codegen the actual panic invoke/call.
439459
helper.do_call(self, &mut bx, fn_ty, llfn, &args, None, cleanup);
440460
}
@@ -486,6 +506,7 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
486506
if let Some(destination_ref) = destination.as_ref() {
487507
let &(ref dest, target) = destination_ref;
488508
self.codegen_transmute(&mut bx, &args[0], dest);
509+
helper.maybe_sideeffect(&mut bx, &[target]);
489510
helper.funclet_br(self, &mut bx, target);
490511
} else {
491512
// If we are trying to transmute to an uninhabited type,
@@ -516,6 +537,7 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
516537
Some(ty::InstanceDef::DropGlue(_, None)) => {
517538
// Empty drop glue; a no-op.
518539
let &(_, target) = destination.as_ref().unwrap();
540+
helper.maybe_sideeffect(&mut bx, &[target]);
519541
helper.funclet_br(self, &mut bx, target);
520542
return;
521543
}
@@ -552,6 +574,7 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
552574
let fn_ty = bx.fn_type_of_instance(&instance);
553575
let llfn = bx.get_fn(instance);
554576

577+
bx.sideeffect();
555578
// Codegen the actual panic invoke/call.
556579
helper.do_call(
557580
self,
@@ -564,7 +587,9 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
564587
);
565588
} else {
566589
// a NOP
567-
helper.funclet_br(self, &mut bx, destination.as_ref().unwrap().1)
590+
let target = destination.as_ref().unwrap().1;
591+
helper.maybe_sideeffect(&mut bx, &[target]);
592+
helper.funclet_br(self, &mut bx, target);
568593
}
569594
return;
570595
}
@@ -668,6 +693,7 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
668693
}
669694

670695
if let Some((_, target)) = *destination {
696+
helper.maybe_sideeffect(&mut bx, &[target]);
671697
helper.funclet_br(self, &mut bx, target);
672698
} else {
673699
bx.unreachable();
@@ -790,6 +816,7 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
790816
_ => span_bug!(span, "no llfn for call"),
791817
};
792818

819+
bx.sideeffect();
793820
helper.do_call(self, &mut bx, fn_ty, fn_ptr, &llargs,
794821
destination.as_ref().map(|&(_, target)| (ret_dest, target)),
795822
cleanup);
@@ -839,6 +866,7 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
839866
}
840867

841868
mir::TerminatorKind::Goto { target } => {
869+
helper.maybe_sideeffect(&mut bx, &[target]);
842870
helper.funclet_br(self, &mut bx, target);
843871
}
844872

‎src/librustc_codegen_ssa/mir/rvalue.rs‎

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

‎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" `VaList` 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: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// ignore-test LLVM can't prove that these loops terminate.
2+
13
// compile-flags: -O
24

35
#![crate_type = "lib"]

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,27 +53,27 @@ pub fn naked_with_args_and_return(a: isize) -> isize {
5353
#[naked]
5454
pub fn naked_recursive() {
5555
// CHECK-NEXT: {{.+}}:
56-
// CHECK-NEXT: call void @naked_empty()
56+
// CHECK: call void @naked_empty()
5757

5858
// FIXME(#39685) Avoid one block per call.
5959
// CHECK-NEXT: br label %bb1
6060
// CHECK: bb1:
6161

6262
naked_empty();
6363

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

6666
// FIXME(#39685) Avoid one block per call.
6767
// CHECK-NEXT: br label %bb2
6868
// CHECK: bb2:
6969

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

7272
// FIXME(#39685) Avoid one block per call.
7373
// CHECK-NEXT: br label %bb3
7474
// CHECK: bb3:
7575

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

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

0 commit comments

Comments
 (0)