From fb575c0ea9fdda046285d527a9063255e08507d4 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 2 Apr 2019 11:30:36 -0700 Subject: [PATCH] Never return uninhabited values at all Functions with uninhabited return values are already marked `noreturn`, but we were still generating return instructions for this. When running with `C passes=lint`, LLVM prints: Unusual: Return statement in function with noreturn attribute The LLVM manual makes a stronger statement about `noreturn` though: > This produces undefined behavior at runtime if the function ever does dynamically return. We now mark such return values with a new `IgnoreMode::Uninhabited`, and emit an `abort` anywhere that would have returned. --- src/librustc_codegen_llvm/abi.rs | 9 +++++-- src/librustc_codegen_ssa/mir/block.rs | 6 +++++ src/librustc_codegen_ssa/mir/mod.rs | 3 ++- src/librustc_target/abi/call/mod.rs | 2 ++ src/test/codegen/noreturn-uninhabited.rs | 32 ++++++++++++++++++++++++ 5 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 src/test/codegen/noreturn-uninhabited.rs diff --git a/src/librustc_codegen_llvm/abi.rs b/src/librustc_codegen_llvm/abi.rs index 3a0d9e1334cf6..6a80051569f6a 100644 --- a/src/librustc_codegen_llvm/abi.rs +++ b/src/librustc_codegen_llvm/abi.rs @@ -518,7 +518,11 @@ impl<'tcx> FnTypeExt<'tcx> for FnType<'tcx, Ty<'tcx>> { let arg_of = |ty: Ty<'tcx>, arg_idx: Option| { let is_return = arg_idx.is_none(); let mut arg = mk_arg_type(ty, arg_idx); - if arg.layout.is_zst() { + if is_return && arg.layout.abi.is_uninhabited() { + // Functions with uninhabited return values are marked `noreturn`, + // so we don't actually need to do anything with the value. + arg.mode = PassMode::Ignore(IgnoreMode::Uninhabited); + } else if arg.layout.is_zst() { // For some forsaken reason, x86_64-pc-windows-gnu // doesn't ignore zero-sized struct arguments. // The same is true for s390x-unknown-linux-gnu @@ -677,7 +681,8 @@ impl<'tcx> FnTypeExt<'tcx> for FnType<'tcx, Ty<'tcx>> { ); let llreturn_ty = match self.ret.mode { - PassMode::Ignore(IgnoreMode::Zst) => cx.type_void(), + PassMode::Ignore(IgnoreMode::Zst) + | PassMode::Ignore(IgnoreMode::Uninhabited) => cx.type_void(), PassMode::Ignore(IgnoreMode::CVarArgs) => bug!("`va_list` should never be a return type"), PassMode::Direct(_) | PassMode::Pair(..) => { diff --git a/src/librustc_codegen_ssa/mir/block.rs b/src/librustc_codegen_ssa/mir/block.rs index 53e8f7ed88b4e..76357078c4514 100644 --- a/src/librustc_codegen_ssa/mir/block.rs +++ b/src/librustc_codegen_ssa/mir/block.rs @@ -244,6 +244,12 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { return; } + PassMode::Ignore(IgnoreMode::Uninhabited) => { + bx.abort(); + bx.unreachable(); + return; + } + PassMode::Ignore(IgnoreMode::CVarArgs) => { bug!("C-variadic arguments should never be the return type"); } diff --git a/src/librustc_codegen_ssa/mir/mod.rs b/src/librustc_codegen_ssa/mir/mod.rs index 91aa9bcc7808b..0b3476c44e2ce 100644 --- a/src/librustc_codegen_ssa/mir/mod.rs +++ b/src/librustc_codegen_ssa/mir/mod.rs @@ -521,7 +521,8 @@ fn arg_local_refs<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>>( PassMode::Ignore(IgnoreMode::Zst) => { return local(OperandRef::new_zst(bx, arg.layout)); } - PassMode::Ignore(IgnoreMode::CVarArgs) => {} + PassMode::Ignore(IgnoreMode::CVarArgs) + | PassMode::Ignore(IgnoreMode::Uninhabited) => {} PassMode::Direct(_) => { let llarg = bx.get_param(llarg_idx); bx.set_value_name(llarg, &name); diff --git a/src/librustc_target/abi/call/mod.rs b/src/librustc_target/abi/call/mod.rs index fbbd120f934be..8cbe99e3d8c76 100644 --- a/src/librustc_target/abi/call/mod.rs +++ b/src/librustc_target/abi/call/mod.rs @@ -29,6 +29,8 @@ pub enum IgnoreMode { CVarArgs, /// A zero-sized type. Zst, + /// An uninhabited type. + Uninhabited, } #[derive(Clone, Copy, PartialEq, Eq, Debug)] diff --git a/src/test/codegen/noreturn-uninhabited.rs b/src/test/codegen/noreturn-uninhabited.rs new file mode 100644 index 0000000000000..1b65da9f2877a --- /dev/null +++ b/src/test/codegen/noreturn-uninhabited.rs @@ -0,0 +1,32 @@ +// compile-flags: -g -C no-prepopulate-passes +// ignore-tidy-linelength + +#![crate_type = "lib"] + +#[derive(Clone, Copy)] +pub enum EmptyEnum {} + +#[no_mangle] +pub fn empty(x: &EmptyEnum) -> EmptyEnum { + // CHECK: @empty({{.*}}) unnamed_addr #0 + // CHECK-NOT: ret void + // CHECK: call void @llvm.trap() + // CHECK: unreachable + *x +} + +pub struct Foo(String, EmptyEnum); + +#[no_mangle] +pub fn foo(x: String, y: &EmptyEnum) -> Foo { + // CHECK: @foo({{.*}}) unnamed_addr #0 + // CHECK-NOT: ret %Foo + // CHECK: call void @llvm.trap() + // CHECK: unreachable + Foo(x, *y) +} + +// CHECK: attributes #0 = {{{.*}} noreturn {{.*}}} + +// CHECK: DISubprogram(name: "empty", {{.*}} DIFlagNoReturn +// CHECK: DISubprogram(name: "foo", {{.*}} DIFlagNoReturn