Skip to content

Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited) #136985

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

Merged
merged 6 commits into from
Feb 21, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Add test that uninhabited repr(transparent) type has same function re…
…turn ABI as wrapped type.

Fix codegen of uninhabited PassMode::Indirect return types.

Add codegen test for uninhabited PassMode::Indirect return types.

Enable optimizations for uninhabited return type codegen test
  • Loading branch information
zachs18 committed Feb 20, 2025
commit 58ebf6afdd01d32f9f6d22d2e788c0dc10bc65a5
28 changes: 7 additions & 21 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ use rustc_abi::{BackendRepr, ExternAbi, HasDataLayout, Reg, WrappingRange};
use rustc_ast as ast;
use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece};
use rustc_hir::lang_items::LangItem;
use rustc_middle::mir::{
self, AssertKind, BasicBlock, InlineAsmMacro, SwitchTargets, UnwindTerminateReason,
};
use rustc_middle::mir::{self, AssertKind, InlineAsmMacro, SwitchTargets, UnwindTerminateReason};
use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf, ValidityRequirement};
use rustc_middle::ty::print::{with_no_trimmed_paths, with_no_visible_paths};
use rustc_middle::ty::{self, Instance, Ty};
Expand Down Expand Up @@ -942,7 +940,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
&fn_abi.ret,
&mut llargs,
Some(intrinsic),
target,
);
let dest = match ret_dest {
_ if fn_abi.ret.is_indirect() => llargs[0],
Expand Down Expand Up @@ -998,19 +995,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
};

let mut llargs = Vec::with_capacity(arg_count);
let destination = target.as_ref().map(|&target| {
(
self.make_return_dest(
bx,
destination,
&fn_abi.ret,
&mut llargs,
None,
Some(target),
),
target,
)
});

// We still need to call `make_return_dest` even if there's no `target`, since
// `fn_abi.ret` could be `PassMode::Indirect`, even if it is uninhabited,
// and `make_return_dest` adds the return-place indirect pointer to `llargs`.
let return_dest = self.make_return_dest(bx, destination, &fn_abi.ret, &mut llargs, None);
let destination = target.map(|target| (return_dest, target));

// Split the rust-call tupled arguments off.
let (first_args, untuple) = if abi == ExternAbi::RustCall && !args.is_empty() {
Expand Down Expand Up @@ -1813,11 +1803,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
fn_ret: &ArgAbi<'tcx, Ty<'tcx>>,
llargs: &mut Vec<Bx::Value>,
intrinsic: Option<ty::IntrinsicDef>,
target: Option<BasicBlock>,
) -> ReturnDest<'tcx, Bx::Value> {
if target.is_none() {
return ReturnDest::Nothing;
}
// If the return is ignored, we can just return a do-nothing `ReturnDest`.
if fn_ret.is_ignore() {
return ReturnDest::Nothing;
Expand Down
44 changes: 44 additions & 0 deletions tests/codegen/uninhabited-transparent-return-abi.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
//@ compile-flags: -Copt-level=3

// See https://github.com/rust-lang/rust/issues/135802

#![crate_type = "lib"]

enum Void {}

// Should be ABI-compatible with T, but wasn't prior to the PR adding this test.
#[repr(transparent)]
struct NoReturn<T>(T, Void);

// Returned by invisible reference (in most ABIs)
#[allow(dead_code)]
struct Large(u64, u64, u64);

extern "Rust" {
fn opaque() -> NoReturn<Large>;
fn opaque_with_arg(rsi: u32) -> NoReturn<Large>;
}

// CHECK-LABEL: @test_uninhabited_ret_by_ref
#[no_mangle]
pub fn test_uninhabited_ret_by_ref() {
// CHECK: %_1 = alloca [24 x i8], align {{8|4}}
// CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 24, ptr nonnull %_1)
// CHECK-NEXT: call void @opaque(ptr noalias nocapture noundef nonnull sret([24 x i8]) align {{8|4}} dereferenceable(24) %_1) #2
// CHECK-NEXT: unreachable
unsafe {
opaque();
}
}

// CHECK-LABEL: @test_uninhabited_ret_by_ref_with_arg
#[no_mangle]
pub fn test_uninhabited_ret_by_ref_with_arg(rsi: u32) {
// CHECK: %_2 = alloca [24 x i8], align {{8|4}}
// CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 24, ptr nonnull %_2)
// CHECK-NEXT: call void @opaque_with_arg(ptr noalias nocapture noundef nonnull sret([24 x i8]) align {{8|4}} dereferenceable(24) %_2, i32 noundef %rsi) #2
// CHECK-NEXT: unreachable
unsafe {
opaque_with_arg(rsi);
}
}
33 changes: 33 additions & 0 deletions tests/ui/uninhabited/uninhabited-transparent-return-abi.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//@ run-pass
//@ needs-unwind
// See https://github.com/rust-lang/rust/issues/135802

enum Void {}

// Should be ABI-compatible with T, but wasn't prior to the PR adding this test.
#[repr(transparent)]
struct NoReturn<T>(T, Void);

// Returned by invisible reference (in most ABIs)
#[allow(dead_code)]
struct Large(u64, u64, u64);

// Prior to the PR adding this test, this function had a different ABI than
// `fn() -> Large` (on `x86_64-unknown-linux-gnu` at least), so calling it as `fn() -> Large`
// would pass the return place pointer in rdi and `correct` in rsi, but the function
// would expect `correct` in rdi.
fn never(correct: &mut bool) -> NoReturn<Large> {
*correct = true;
panic!("catch this")
}

fn main() {
let mut correct = false;
let never: fn(&mut bool) -> NoReturn<Large> = never;
// Safety: `NoReturn<Large>` is a `repr(transparent)` wrapper around `Large`,
// so they should be ABI-compatible.
let never: fn(&mut bool) -> Large = unsafe { std::mem::transmute(never) };
let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| never(&mut correct)));
assert!(result.is_err(), "function should have panicked");
assert!(correct, "function should have stored `true` into `correct`");
}