Skip to content

[beta] Don't sanity check function pointers in vtables #53425

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.

Sign up for GitHub

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 2 commits into from
Aug 26, 2018
Merged

[beta] Don't sanity check function pointers in vtables #53425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bddfece
Don't sanity check function pointers in vtables
oli-obk Aug 16, 2018
690c075
Update test
oli-obk Aug 25, 2018
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
18 changes: 2 additions & 16 deletions src/librustc/ty/layout.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1621,25 +1621,11 @@ impl<'a, 'tcx, C> TyLayoutMethods<'tcx, C> for Ty<'tcx>
match tcx.struct_tail(pointee).sty {
ty::TySlice(_) |
ty::TyStr => tcx.types.usize,
ty::TyDynamic(data, _) => {
let trait_def_id = data.principal().unwrap().def_id();
let num_fns: u64 = crate::traits::supertrait_def_ids(tcx, trait_def_id)
.map(|trait_def_id| {
tcx.associated_items(trait_def_id)
.filter(|item| item.kind == ty::AssociatedKind::Method)
.count() as u64
})
.sum();
ty::TyDynamic(_, _) => {
tcx.mk_imm_ref(
tcx.types.re_static,
tcx.mk_array(tcx.types.usize, 3 + num_fns),
)
/* FIXME use actual fn pointers
tcx.mk_tup(&[
tcx.mk_array(tcx.types.usize, 3),
tcx.mk_array(Option<fn()>),
])
*/
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, this is a bit surprising to me, but I'm gonna trust you all here =)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So...the issue is that functions with where bounds also contributed to the count, even if they were excluded because they aren't object safe and thus they shouldn't be in the vtable. the previous code was simply wrong, and fixing it fully would blow anything we could beta backport

Copy link
Member

Choose a reason for hiding this comment

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

The same patch should also land on master though as the layout is still wrong there?

}
_ => bug!("TyLayout::field_type({:?}): not applicable", this)
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/codegen/function-arguments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,13 @@ pub fn unsafe_slice(_: &[UnsafeInner]) {
pub fn str(_: &[u8]) {
}

// CHECK: @trait_borrow({}* nonnull %arg0.0, [4 x [[USIZE]]]* noalias readonly dereferenceable({{.*}}) %arg0.1)
// CHECK: @trait_borrow({}* nonnull %arg0.0, [3 x [[USIZE]]]* noalias readonly dereferenceable({{.*}}) %arg0.1)
// FIXME #25759 This should also have `nocapture`
#[no_mangle]
pub fn trait_borrow(_: &Drop) {
}

// CHECK: @trait_box({}* noalias nonnull, [4 x [[USIZE]]]* noalias readonly dereferenceable({{.*}}))
// CHECK: @trait_box({}* noalias nonnull, [3 x [[USIZE]]]* noalias readonly dereferenceable({{.*}}))
#[no_mangle]
pub fn trait_box(_: Box<Drop>) {
}
Expand Down
21 changes: 21 additions & 0 deletions src/test/ui/consts/const-eval/issue-53401.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// compile-pass

pub const STATIC_TRAIT: &Test = &();

fn main() {}

pub trait Test {
fn test() where Self: Sized {}
}

impl Test for () {}