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

Fix: ices on virtual-function-elimination about principal trait #130734

Merged
merged 1 commit into from
Sep 25, 2024
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
9 changes: 2 additions & 7 deletions compiler/rustc_codegen_ssa/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use crate::back::write::{
submit_post_lto_module_to_llvm, submit_pre_lto_module_to_llvm, ComputedLtoType, OngoingCodegen,
};
use crate::common::{self, IntPredicate, RealPredicate, TypeKind};
use crate::meth::load_vtable;
use crate::mir::operand::OperandValue;
use crate::mir::place::PlaceRef;
use crate::traits::*;
Expand Down Expand Up @@ -135,14 +136,8 @@ fn unsized_info<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(

if let Some(entry_idx) = vptr_entry_idx {
let ptr_size = bx.data_layout().pointer_size;
let ptr_align = bx.data_layout().pointer_align.abi;
let vtable_byte_offset = u64::try_from(entry_idx).unwrap() * ptr_size.bytes();
let gep = bx.inbounds_ptradd(old_info, bx.const_usize(vtable_byte_offset));
let new_vptr = bx.load(bx.type_ptr(), gep, ptr_align);
bx.nonnull_metadata(new_vptr);
// VTable loads are invariant.
bx.set_invariant_load(new_vptr);
new_vptr
load_vtable(bx, old_info, bx.type_ptr(), vtable_byte_offset, source, true)
} else {
old_info
}
Expand Down
67 changes: 39 additions & 28 deletions compiler/rustc_codegen_ssa/src/meth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,9 @@ impl<'a, 'tcx> VirtualIndex {

let llty = bx.fn_ptr_backend_type(fn_abi);
let ptr_size = bx.data_layout().pointer_size;
let ptr_align = bx.data_layout().pointer_align.abi;
let vtable_byte_offset = self.0 * ptr_size.bytes();

if bx.cx().sess().opts.unstable_opts.virtual_function_elimination
&& bx.cx().sess().lto() == Lto::Fat
{
let typeid = bx
.typeid_metadata(typeid_for_trait_ref(bx.tcx(), expect_dyn_trait_in_self(ty)))
.unwrap();
let func = bx.type_checked_load(llvtable, vtable_byte_offset, typeid);
func
} else {
let gep = bx.inbounds_ptradd(llvtable, bx.const_usize(vtable_byte_offset));
let ptr = bx.load(llty, gep, ptr_align);
// VTable loads are invariant.
bx.set_invariant_load(ptr);
if nonnull {
bx.nonnull_metadata(ptr);
}
ptr
}
load_vtable(bx, llvtable, llty, vtable_byte_offset, ty, nonnull)
}

pub(crate) fn get_optional_fn<Bx: BuilderMethods<'a, 'tcx>>(
Expand All @@ -75,31 +57,27 @@ impl<'a, 'tcx> VirtualIndex {
self,
bx: &mut Bx,
llvtable: Bx::Value,
ty: Ty<'tcx>,
) -> Bx::Value {
// Load the data pointer from the object.
debug!("get_int({:?}, {:?})", llvtable, self);

let llty = bx.type_isize();
let ptr_size = bx.data_layout().pointer_size;
let ptr_align = bx.data_layout().pointer_align.abi;
let vtable_byte_offset = self.0 * ptr_size.bytes();

let gep = bx.inbounds_ptradd(llvtable, bx.const_usize(vtable_byte_offset));
let ptr = bx.load(llty, gep, ptr_align);
// VTable loads are invariant.
bx.set_invariant_load(ptr);
ptr
load_vtable(bx, llvtable, llty, vtable_byte_offset, ty, false)
}
}

/// This takes a valid `self` receiver type and extracts the principal trait
/// ref of the type.
fn expect_dyn_trait_in_self(ty: Ty<'_>) -> ty::PolyExistentialTraitRef<'_> {
/// ref of the type. Return `None` if there is no principal trait.
fn dyn_trait_in_self(ty: Ty<'_>) -> Option<ty::PolyExistentialTraitRef<'_>> {
for arg in ty.peel_refs().walk() {
if let GenericArgKind::Type(ty) = arg.unpack()
&& let ty::Dynamic(data, _, _) = ty.kind()
{
return data.principal().expect("expected principal trait object");
return data.principal();
}
}

Expand Down Expand Up @@ -138,3 +116,36 @@ pub(crate) fn get_vtable<'tcx, Cx: CodegenMethods<'tcx>>(
cx.vtables().borrow_mut().insert((ty, trait_ref), vtable);
vtable
}

/// Call this function whenever you need to load a vtable.
pub(crate) fn load_vtable<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
bx: &mut Bx,
llvtable: Bx::Value,
llty: Bx::Type,
vtable_byte_offset: u64,
ty: Ty<'tcx>,
nonnull: bool,
) -> Bx::Value {
let ptr_align = bx.data_layout().pointer_align.abi;

if bx.cx().sess().opts.unstable_opts.virtual_function_elimination
&& bx.cx().sess().lto() == Lto::Fat
{
if let Some(trait_ref) = dyn_trait_in_self(ty) {
let typeid = bx.typeid_metadata(typeid_for_trait_ref(bx.tcx(), trait_ref)).unwrap();
let func = bx.type_checked_load(llvtable, vtable_byte_offset, typeid);
return func;
} else if nonnull {
bug!("load nonnull value from a vtable without a principal trait")
}
}

let gep = bx.inbounds_ptradd(llvtable, bx.const_usize(vtable_byte_offset));
let ptr = bx.load(llty, gep, ptr_align);
// VTable loads are invariant.
bx.set_invariant_load(ptr);
if nonnull {
bx.nonnull_metadata(ptr);
}
ptr
}
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
sym::vtable_align => ty::COMMON_VTABLE_ENTRIES_ALIGN,
_ => bug!(),
};
let value = meth::VirtualIndex::from_index(idx).get_usize(bx, vtable);
let value = meth::VirtualIndex::from_index(idx).get_usize(bx, vtable, callee_ty);
match name {
// Size is always <= isize::MAX.
sym::vtable_size => {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/size_of_val.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ pub fn size_and_align_of_dst<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
// Load size/align from vtable.
let vtable = info.unwrap();
let size = meth::VirtualIndex::from_index(ty::COMMON_VTABLE_ENTRIES_SIZE)
.get_usize(bx, vtable);
.get_usize(bx, vtable, t);
let align = meth::VirtualIndex::from_index(ty::COMMON_VTABLE_ENTRIES_ALIGN)
.get_usize(bx, vtable);
.get_usize(bx, vtable, t);

// Size is always <= isize::MAX.
let size_bound = bx.data_layout().ptr_sized_integer().signed_max() as u128;
Expand Down
6 changes: 0 additions & 6 deletions tests/crashes/123955.rs

This file was deleted.

7 changes: 0 additions & 7 deletions tests/crashes/124092.rs

This file was deleted.

17 changes: 17 additions & 0 deletions tests/ui/codegen/virtual-function-elimination.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//@ build-pass
//@ compile-flags: -Zvirtual-function-elimination=true -Clto=true
//@ only-x86_64
//@ no-prefer-dynamic
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this argument needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

./x test needs this argument to perform LTO.
Below is the error message without no-prefer-dynamic:

running 1 tests

[ui] tests/ui/codegen/virtual-function-elimination.rs ... F


failures:

---- [ui] tests/ui/codegen/virtual-function-elimination.rs stdout ----

error: test compilation failed although it shouldn't!
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/home/ray/Desktop/rust/build/x86_64-unknown-linux-gnu/stage1/bin/rustc" "/home/ray/Desktop/rust/tests/ui/codegen/virtual-function-elimination.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/home/ray/.cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/home/ray/Desktop/rust/vendor" "--sysroot" "/home/ray/Desktop/rust/build/x86_64-unknown-linux-gnu/stage1" "--target=x86_64-unknown-linux-gnu" "--check-cfg" "cfg(FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "-C" "prefer-dynamic" "--out-dir" "/home/ray/Desktop/rust/build/x86_64-unknown-linux-gnu/test/ui/codegen/virtual-function-elimination" "-A" "unused" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=/home/ray/Desktop/rust/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/home/ray/Desktop/rust/build/x86_64-unknown-linux-gnu/test/ui/codegen/virtual-function-elimination/auxiliary" "-Zvirtual-function-elimination=true" "-Clto=true"
stdout: none
--- stderr -------------------------------
error: cannot prefer dynamic linking when performing LTO
   |
   = note: only 'staticlib', 'bin', and 'cdylib' outputs are supported with LTO

error: aborting due to 1 previous error
------------------------------------------


// issue #123955
pub fn test0() {
_ = Box::new(()) as Box<dyn Send>;
}

// issue #124092
const X: for<'b> fn(&'b ()) = |&()| ();
pub fn test1() {
let _dyn_debug = Box::new(X) as Box<fn(&'static ())> as Box<dyn Send>;
}

fn main() {}
Loading