Skip to content

Commit 58b5491

Browse files
borsbors
committed
Auto merge of #59546 - sfanxiang:interminable-ub, r=nagisa
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 **UPDATE:** [Mentoring instructions here](#59546 (comment)) to unstall this PR
2 parents 9c588c1 + e9acfa3 commit 58b5491

File tree

9 files changed

+107
-1
lines changed

9 files changed

+107
-1
lines changed

‎src/librustc/session/config.rs

+3
Original file line numberDiff line numberDiff line change
@@ -1467,6 +1467,9 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
14671467
"which mangling version to use for symbol names"),
14681468
binary_dep_depinfo: bool = (false, parse_bool, [TRACKED],
14691469
"include artifacts (sysroot, crate dependencies) used during compilation in dep-info"),
1470+
insert_sideeffect: bool = (false, parse_bool, [TRACKED],
1471+
"fix undefined behavior when a thread doesn't eventually make progress \
1472+
(such as entering an empty infinite loop) by inserting llvm.sideeffect"),
14701473
}
14711474

14721475
pub fn default_lib_output() -> CrateType {

‎src/librustc_codegen_llvm/context.rs

+1
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

+10
Original file line numberDiff line numberDiff line change
@@ -724,6 +724,13 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> {
724724
self.call(expect, &[cond, self.const_bool(expected)], None)
725725
}
726726

727+
fn sideeffect(&mut self) {
728+
if self.tcx.sess.opts.debugging_opts.insert_sideeffect {
729+
let fnname = self.get_intrinsic(&("llvm.sideeffect"));
730+
self.call(fnname, &[], None);
731+
}
732+
}
733+
727734
fn va_start(&mut self, va_list: &'ll Value) -> &'ll Value {
728735
let intrinsic = self.cx().get_intrinsic("llvm.va_start");
729736
self.call(intrinsic, &[va_list], None)
@@ -810,6 +817,7 @@ fn codegen_msvc_try(
810817
) {
811818
let llfn = get_rust_try_fn(bx, &mut |mut bx| {
812819
bx.set_personality_fn(bx.eh_personality());
820+
bx.sideeffect();
813821

814822
let mut normal = bx.build_sibling_block("normal");
815823
let mut catchswitch = bx.build_sibling_block("catchswitch");
@@ -933,6 +941,8 @@ fn codegen_gnu_try(
933941
// expected to be `*mut *mut u8` for this to actually work, but that's
934942
// managed by the standard library.
935943

944+
bx.sideeffect();
945+
936946
let mut then = bx.build_sibling_block("then");
937947
let mut catch = bx.build_sibling_block("catch");
938948

‎src/librustc_codegen_ssa/mir/block.rs

+40-1
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,26 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'a, 'tcx> {
149149
}
150150
}
151151
}
152+
153+
// Generate sideeffect intrinsic if jumping to any of the targets can form
154+
// a loop.
155+
fn maybe_sideeffect<'b, 'tcx2: 'b, Bx: BuilderMethods<'b, 'tcx2>>(
156+
&self,
157+
mir: &'b mir::Body<'tcx>,
158+
bx: &mut Bx,
159+
targets: &[mir::BasicBlock],
160+
) {
161+
if bx.tcx().sess.opts.debugging_opts.insert_sideeffect {
162+
if targets.iter().any(|target| {
163+
*target <= *self.bb
164+
&& target
165+
.start_location()
166+
.is_predecessor_of(self.bb.start_location(), mir)
167+
}) {
168+
bx.sideeffect();
169+
}
170+
}
171+
}
152172
}
153173

154174
/// Codegen implementations for some terminator variants.
@@ -197,6 +217,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
197217
let lltrue = helper.llblock(self, targets[0]);
198218
let llfalse = helper.llblock(self, targets[1]);
199219
if switch_ty == bx.tcx().types.bool {
220+
helper.maybe_sideeffect(self.mir, &mut bx, targets.as_slice());
200221
// Don't generate trivial icmps when switching on bool
201222
if let [0] = values[..] {
202223
bx.cond_br(discr.immediate(), llfalse, lltrue);
@@ -210,9 +231,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
210231
);
211232
let llval = bx.const_uint_big(switch_llty, values[0]);
212233
let cmp = bx.icmp(IntPredicate::IntEQ, discr.immediate(), llval);
234+
helper.maybe_sideeffect(self.mir, &mut bx, targets.as_slice());
213235
bx.cond_br(cmp, lltrue, llfalse);
214236
}
215237
} else {
238+
helper.maybe_sideeffect(self.mir, &mut bx, targets.as_slice());
216239
let (otherwise, targets) = targets.split_last().unwrap();
217240
bx.switch(
218241
discr.immediate(),
@@ -308,6 +331,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
308331

309332
if let ty::InstanceDef::DropGlue(_, None) = drop_fn.def {
310333
// we don't actually need to drop anything.
334+
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
311335
helper.funclet_br(self, &mut bx, target);
312336
return
313337
}
@@ -338,6 +362,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
338362
FnType::of_instance(&bx, drop_fn))
339363
}
340364
};
365+
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
341366
helper.do_call(self, &mut bx, fn_ty, drop_fn, args,
342367
Some((ReturnDest::Nothing, target)),
343368
unwind);
@@ -373,6 +398,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
373398

374399
// Don't codegen the panic block if success if known.
375400
if const_cond == Some(expected) {
401+
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
376402
helper.funclet_br(self, &mut bx, target);
377403
return;
378404
}
@@ -383,6 +409,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
383409
// Create the failure block and the conditional branch to it.
384410
let lltarget = helper.llblock(self, target);
385411
let panic_block = self.new_block("panic");
412+
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
386413
if expected {
387414
bx.cond_br(cond, lltarget, panic_block.llbb());
388415
} else {
@@ -486,6 +513,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
486513
if let Some(destination_ref) = destination.as_ref() {
487514
let &(ref dest, target) = destination_ref;
488515
self.codegen_transmute(&mut bx, &args[0], dest);
516+
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
489517
helper.funclet_br(self, &mut bx, target);
490518
} else {
491519
// If we are trying to transmute to an uninhabited type,
@@ -513,6 +541,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
513541
Some(ty::InstanceDef::DropGlue(_, None)) => {
514542
// Empty drop glue; a no-op.
515543
let &(_, target) = destination.as_ref().unwrap();
544+
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
516545
helper.funclet_br(self, &mut bx, target);
517546
return;
518547
}
@@ -549,6 +578,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
549578
let fn_ty = FnType::of_instance(&bx, instance);
550579
let llfn = bx.get_fn(instance);
551580

581+
if let Some((_, target)) = destination.as_ref() {
582+
helper.maybe_sideeffect(self.mir, &mut bx, &[*target]);
583+
}
552584
// Codegen the actual panic invoke/call.
553585
helper.do_call(
554586
self,
@@ -561,7 +593,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
561593
);
562594
} else {
563595
// a NOP
564-
helper.funclet_br(self, &mut bx, destination.as_ref().unwrap().1)
596+
let target = destination.as_ref().unwrap().1;
597+
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
598+
helper.funclet_br(self, &mut bx, target);
565599
}
566600
return;
567601
}
@@ -670,6 +704,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
670704
}
671705

672706
if let Some((_, target)) = *destination {
707+
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
673708
helper.funclet_br(self, &mut bx, target);
674709
} else {
675710
bx.unreachable();
@@ -762,6 +797,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
762797
_ => span_bug!(span, "no llfn for call"),
763798
};
764799

800+
if let Some((_, target)) = destination.as_ref() {
801+
helper.maybe_sideeffect(self.mir, &mut bx, &[*target]);
802+
}
765803
helper.do_call(self, &mut bx, fn_ty, fn_ptr, &llargs,
766804
destination.as_ref().map(|&(_, target)| (ret_dest, target)),
767805
cleanup);
@@ -811,6 +849,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
811849
}
812850

813851
mir::TerminatorKind::Goto { target } => {
852+
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
814853
helper.funclet_br(self, &mut bx, target);
815854
}
816855

‎src/librustc_codegen_ssa/mir/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,8 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
200200
bx.set_personality_fn(cx.eh_personality());
201201
}
202202

203+
bx.sideeffect();
204+
203205
let cleanup_kinds = analyze::cleanup_kinds(&mir);
204206
// Allocate a `Block` for every basic block, except
205207
// the start block, if nothing loops back to it.

‎src/librustc_codegen_ssa/traits/intrinsic.rs

+1
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/non-terminate/infinite-loop-1.rs

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// compile-flags: -C opt-level=3 -Z insert-sideeffect
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

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// compile-flags: -C opt-level=3 -Z insert-sideeffect
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

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// compile-flags: -C opt-level=3 -Z insert-sideeffect
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+
}

0 commit comments

Comments
 (0)