Skip to content

Commit

Permalink
Auto merge of #52089 - eddyb:issue-51907, r=nagisa
Browse files Browse the repository at this point in the history
rustc_codegen_llvm: replace the first argument early in FnType::new_vtable.

Fixes #51907 by removing the vtable pointer before the `ArgType` is even created.
This allows any ABI to support trait object method calls, regardless of how it passes `*dyn Trait`.

r? @nikomatsakis
  • Loading branch information
bors committed Jul 12, 2018
2 parents 4ba5ff4 + ce94518 commit 3244d53
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 37 deletions.
80 changes: 43 additions & 37 deletions src/librustc_codegen_llvm/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,9 +261,12 @@ pub trait FnTypeExt<'a, 'tcx> {
fn new_vtable(cx: &CodegenCx<'a, 'tcx>,
sig: ty::FnSig<'tcx>,
extra_args: &[Ty<'tcx>]) -> Self;
fn unadjusted(cx: &CodegenCx<'a, 'tcx>,
sig: ty::FnSig<'tcx>,
extra_args: &[Ty<'tcx>]) -> Self;
fn new_internal(
cx: &CodegenCx<'a, 'tcx>,
sig: ty::FnSig<'tcx>,
extra_args: &[Ty<'tcx>],
mk_arg_type: impl Fn(Ty<'tcx>, Option<usize>) -> ArgType<'tcx, Ty<'tcx>>,
) -> Self;
fn adjust_for_abi(&mut self,
cx: &CodegenCx<'a, 'tcx>,
abi: Abi);
Expand All @@ -285,40 +288,40 @@ impl<'a, 'tcx> FnTypeExt<'a, 'tcx> for FnType<'tcx, Ty<'tcx>> {
fn new(cx: &CodegenCx<'a, 'tcx>,
sig: ty::FnSig<'tcx>,
extra_args: &[Ty<'tcx>]) -> Self {
let mut fn_ty = FnType::unadjusted(cx, sig, extra_args);
fn_ty.adjust_for_abi(cx, sig.abi);
fn_ty
FnType::new_internal(cx, sig, extra_args, |ty, _| {
ArgType::new(cx.layout_of(ty))
})
}

fn new_vtable(cx: &CodegenCx<'a, 'tcx>,
sig: ty::FnSig<'tcx>,
extra_args: &[Ty<'tcx>]) -> Self {
let mut fn_ty = FnType::unadjusted(cx, sig, extra_args);
// Don't pass the vtable, it's not an argument of the virtual fn.
{
let self_arg = &mut fn_ty.args[0];
match self_arg.mode {
PassMode::Pair(data_ptr, _) => {
self_arg.mode = PassMode::Direct(data_ptr);
}
_ => bug!("FnType::new_vtable: non-pair self {:?}", self_arg)
}

let pointee = self_arg.layout.ty.builtin_deref(true)
.unwrap_or_else(|| {
bug!("FnType::new_vtable: non-pointer self {:?}", self_arg)
}).ty;
let fat_ptr_ty = cx.tcx.mk_mut_ptr(pointee);
self_arg.layout = cx.layout_of(fat_ptr_ty).field(cx, 0);
}
fn_ty.adjust_for_abi(cx, sig.abi);
fn_ty
FnType::new_internal(cx, sig, extra_args, |ty, arg_idx| {
let mut layout = cx.layout_of(ty);
// Don't pass the vtable, it's not an argument of the virtual fn.
// Instead, pass just the (thin pointer) first field of `*dyn Trait`.
if arg_idx == Some(0) {
// FIXME(eddyb) `layout.field(cx, 0)` is not enough because e.g.
// `Box<dyn Trait>` has a few newtype wrappers around the raw
// pointer, so we'd have to "dig down" to find `*dyn Trait`.
let pointee = layout.ty.builtin_deref(true)
.unwrap_or_else(|| {
bug!("FnType::new_vtable: non-pointer self {:?}", layout)
}).ty;
let fat_ptr_ty = cx.tcx.mk_mut_ptr(pointee);
layout = cx.layout_of(fat_ptr_ty).field(cx, 0);
}
ArgType::new(layout)
})
}

fn unadjusted(cx: &CodegenCx<'a, 'tcx>,
sig: ty::FnSig<'tcx>,
extra_args: &[Ty<'tcx>]) -> Self {
debug!("FnType::unadjusted({:?}, {:?})", sig, extra_args);
fn new_internal(
cx: &CodegenCx<'a, 'tcx>,
sig: ty::FnSig<'tcx>,
extra_args: &[Ty<'tcx>],
mk_arg_type: impl Fn(Ty<'tcx>, Option<usize>) -> ArgType<'tcx, Ty<'tcx>>,
) -> Self {
debug!("FnType::new_internal({:?}, {:?})", sig, extra_args);

use self::Abi::*;
let conv = match cx.sess().target.target.adjust_abi(sig.abi) {
Expand Down Expand Up @@ -435,8 +438,9 @@ impl<'a, 'tcx> FnTypeExt<'a, 'tcx> for FnType<'tcx, Ty<'tcx>> {
}
};

let arg_of = |ty: Ty<'tcx>, is_return: bool| {
let mut arg = ArgType::new(cx.layout_of(ty));
let arg_of = |ty: Ty<'tcx>, arg_idx: Option<usize>| {
let is_return = arg_idx.is_none();
let mut arg = mk_arg_type(ty, arg_idx);
if arg.layout.is_zst() {
// For some forsaken reason, x86_64-pc-windows-gnu
// doesn't ignore zero-sized struct arguments.
Expand Down Expand Up @@ -479,14 +483,16 @@ impl<'a, 'tcx> FnTypeExt<'a, 'tcx> for FnType<'tcx, Ty<'tcx>> {
arg
};

FnType {
ret: arg_of(sig.output(), true),
args: inputs.iter().chain(extra_args.iter()).map(|ty| {
arg_of(ty, false)
let mut fn_ty = FnType {
ret: arg_of(sig.output(), None),
args: inputs.iter().chain(extra_args).enumerate().map(|(i, ty)| {
arg_of(ty, Some(i))
}).collect(),
variadic: sig.variadic,
conv,
}
};
fn_ty.adjust_for_abi(cx, sig.abi);
fn_ty
}

fn adjust_for_abi(&mut self,
Expand Down
26 changes: 26 additions & 0 deletions src/test/run-pass/issue-51907.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// 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.

trait Foo {
extern fn borrow(&self);
extern fn take(self: Box<Self>);
}

struct Bar;
impl Foo for Bar {
extern fn borrow(&self) {}
extern fn take(self: Box<Self>) {}
}

fn main() {
let foo: Box<dyn Foo> = Box::new(Bar);
foo.borrow();
foo.take()
}

0 comments on commit 3244d53

Please sign in to comment.