Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add llvm.sideeffect to potential infinite loops and recursions #59546

Merged
merged 3 commits into from
Oct 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1472,6 +1472,9 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
"which mangling version to use for symbol names"),
binary_dep_depinfo: bool = (false, parse_bool, [TRACKED],
"include artifacts (sysroot, crate dependencies) used during compilation in dep-info"),
insert_sideeffect: bool = (false, parse_bool, [TRACKED],
"fix undefined behavior when a thread doesn't eventually make progress \
(such as entering an empty infinite loop) by inserting llvm.sideeffect"),
}

pub fn default_lib_output() -> CrateType {
Expand Down
1 change: 1 addition & 0 deletions src/librustc_codegen_llvm/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,7 @@ impl CodegenCx<'b, 'tcx> {
ifn!("llvm.trap", fn() -> void);
ifn!("llvm.debugtrap", fn() -> void);
ifn!("llvm.frameaddress", fn(t_i32) -> i8p);
ifn!("llvm.sideeffect", fn() -> void);

ifn!("llvm.powi.f32", fn(t_f32, t_i32) -> t_f32);
ifn!("llvm.powi.v2f32", fn(t_v2f32, t_i32) -> t_v2f32);
Expand Down
10 changes: 10 additions & 0 deletions src/librustc_codegen_llvm/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,13 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> {
self.call(expect, &[cond, self.const_bool(expected)], None)
}

fn sideeffect(&mut self) {
if self.tcx.sess.opts.debugging_opts.insert_sideeffect {
let fnname = self.get_intrinsic(&("llvm.sideeffect"));
self.call(fnname, &[], None);
}
}

fn va_start(&mut self, va_list: &'ll Value) -> &'ll Value {
let intrinsic = self.cx().get_intrinsic("llvm.va_start");
self.call(intrinsic, &[va_list], None)
Expand Down Expand Up @@ -810,6 +817,7 @@ fn codegen_msvc_try(
) {
let llfn = get_rust_try_fn(bx, &mut |mut bx| {
bx.set_personality_fn(bx.eh_personality());
bx.sideeffect();

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

bx.sideeffect();

let mut then = bx.build_sibling_block("then");
let mut catch = bx.build_sibling_block("catch");

Expand Down
41 changes: 40 additions & 1 deletion src/librustc_codegen_ssa/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,26 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'a, 'tcx> {
}
}
}

// Generate sideeffect intrinsic if jumping to any of the targets can form
// a loop.
fn maybe_sideeffect<'b, 'tcx2: 'b, Bx: BuilderMethods<'b, 'tcx2>>(
&self,
mir: &'b mir::Body<'tcx>,
bx: &mut Bx,
targets: &[mir::BasicBlock],
) {
if bx.tcx().sess.opts.debugging_opts.insert_sideeffect {
if targets.iter().any(|target| {
*target <= *self.bb
&& target
.start_location()
.is_predecessor_of(self.bb.start_location(), mir)
}) {
bx.sideeffect();
}
}
}
}

/// Codegen implementations for some terminator variants.
Expand Down Expand Up @@ -196,6 +216,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let lltrue = helper.llblock(self, targets[0]);
let llfalse = helper.llblock(self, targets[1]);
if switch_ty == bx.tcx().types.bool {
helper.maybe_sideeffect(self.mir, &mut bx, targets.as_slice());
// Don't generate trivial icmps when switching on bool
if let [0] = values[..] {
bx.cond_br(discr.immediate(), llfalse, lltrue);
Expand All @@ -209,9 +230,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
);
let llval = bx.const_uint_big(switch_llty, values[0]);
let cmp = bx.icmp(IntPredicate::IntEQ, discr.immediate(), llval);
helper.maybe_sideeffect(self.mir, &mut bx, targets.as_slice());
bx.cond_br(cmp, lltrue, llfalse);
}
} else {
helper.maybe_sideeffect(self.mir, &mut bx, targets.as_slice());
let (otherwise, targets) = targets.split_last().unwrap();
bx.switch(
discr.immediate(),
Expand Down Expand Up @@ -310,6 +333,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {

if let ty::InstanceDef::DropGlue(_, None) = drop_fn.def {
// we don't actually need to drop anything.
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
helper.funclet_br(self, &mut bx, target);
return
}
Expand Down Expand Up @@ -340,6 +364,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
FnType::of_instance(&bx, drop_fn))
}
};
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
helper.do_call(self, &mut bx, fn_ty, drop_fn, args,
Some((ReturnDest::Nothing, target)),
unwind);
Expand Down Expand Up @@ -375,6 +400,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {

// Don't codegen the panic block if success if known.
if const_cond == Some(expected) {
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
helper.funclet_br(self, &mut bx, target);
return;
}
Expand All @@ -385,6 +411,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// Create the failure block and the conditional branch to it.
let lltarget = helper.llblock(self, target);
let panic_block = self.new_block("panic");
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
if expected {
bx.cond_br(cond, lltarget, panic_block.llbb());
} else {
Expand Down Expand Up @@ -488,6 +515,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
if let Some(destination_ref) = destination.as_ref() {
let &(ref dest, target) = destination_ref;
self.codegen_transmute(&mut bx, &args[0], dest);
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
helper.funclet_br(self, &mut bx, target);
} else {
// If we are trying to transmute to an uninhabited type,
Expand Down Expand Up @@ -518,6 +546,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
Some(ty::InstanceDef::DropGlue(_, None)) => {
// Empty drop glue; a no-op.
let &(_, target) = destination.as_ref().unwrap();
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
helper.funclet_br(self, &mut bx, target);
return;
}
Expand Down Expand Up @@ -554,6 +583,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let fn_ty = FnType::of_instance(&bx, instance);
let llfn = bx.get_fn(instance);

if let Some((_, target)) = destination.as_ref() {
helper.maybe_sideeffect(self.mir, &mut bx, &[*target]);
}
// Codegen the actual panic invoke/call.
helper.do_call(
self,
Expand All @@ -566,7 +598,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
);
} else {
// a NOP
helper.funclet_br(self, &mut bx, destination.as_ref().unwrap().1)
let target = destination.as_ref().unwrap().1;
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
helper.funclet_br(self, &mut bx, target);
}
return;
}
Expand Down Expand Up @@ -675,6 +709,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
}

if let Some((_, target)) = *destination {
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
helper.funclet_br(self, &mut bx, target);
} else {
bx.unreachable();
Expand Down Expand Up @@ -786,6 +821,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
_ => span_bug!(span, "no llfn for call"),
};

if let Some((_, target)) = destination.as_ref() {
helper.maybe_sideeffect(self.mir, &mut bx, &[*target]);
}
helper.do_call(self, &mut bx, fn_ty, fn_ptr, &llargs,
destination.as_ref().map(|&(_, target)| (ret_dest, target)),
cleanup);
Expand Down Expand Up @@ -835,6 +873,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
}

mir::TerminatorKind::Goto { target } => {
helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
helper.funclet_br(self, &mut bx, target);
}

Expand Down
2 changes: 2 additions & 0 deletions src/librustc_codegen_ssa/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,8 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
bx.set_personality_fn(cx.eh_personality());
}

bx.sideeffect();

let cleanup_kinds = analyze::cleanup_kinds(&mir);
// Allocate a `Block` for every basic block, except
// the start block, if nothing loops back to it.
Expand Down
1 change: 1 addition & 0 deletions src/librustc_codegen_ssa/traits/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub trait IntrinsicCallMethods<'tcx>: BackendTypes {
fn abort(&mut self);
fn assume(&mut self, val: Self::Value);
fn expect(&mut self, cond: Self::Value, expected: bool) -> Self::Value;
fn sideeffect(&mut self);
/// Trait method used to inject `va_start` on the "spoofed" `VaListImpl` in
/// Rust defined C-variadic functions.
fn va_start(&mut self, val: Self::Value) -> Self::Value;
Expand Down
17 changes: 17 additions & 0 deletions src/test/codegen/non-terminate/infinite-loop-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// compile-flags: -C opt-level=3 -Z insert-sideeffect

#![crate_type = "lib"]

fn infinite_loop() -> u8 {
loop {}
}

// CHECK-LABEL: @test
#[no_mangle]
fn test() -> u8 {
// CHECK-NOT: unreachable
// CHECK: br label %{{.+}}
// CHECK-NOT: unreachable
let x = infinite_loop();
x
}
19 changes: 19 additions & 0 deletions src/test/codegen/non-terminate/infinite-loop-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// compile-flags: -C opt-level=3 -Z insert-sideeffect

#![crate_type = "lib"]

fn infinite_loop() -> u8 {
let i = 2;
while i > 1 {}
1
}

// CHECK-LABEL: @test
#[no_mangle]
fn test() -> u8 {
// CHECK-NOT: unreachable
// CHECK: br label %{{.+}}
// CHECK-NOT: unreachable
let x = infinite_loop();
x
}
14 changes: 14 additions & 0 deletions src/test/codegen/non-terminate/infinite-recursion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// compile-flags: -C opt-level=3 -Z insert-sideeffect

#![crate_type = "lib"]

#![allow(unconditional_recursion)]

// CHECK-LABEL: @infinite_recursion
#[no_mangle]
fn infinite_recursion() -> u8 {
// CHECK-NOT: ret i8 undef
// CHECK: br label %{{.+}}
// CHECK-NOT: ret i8 undef
infinite_recursion()
}