ICE when using an object-safe trait with methods marked as extern

[tuxmark5]

This snippet causes rustc to ICE:

trait Foo {
    extern fn method(&self);
}

struct Bar;
impl Foo for Bar { 
    extern fn method(&self) { }
}

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

rustc version: rustc 1.28.0-nightly (2a1c4ee 2018-06-25)

Error message:

error: internal compiler error: librustc_codegen_llvm/abi.rs:304: FnType::new_vtable: non-pair self ArgType { layout: TyLayout { ty: &Foo, details: LayoutDetails { variants: Single { index: 0 }, fields: Arbitrary { offsets: [Size { raw: 0 }, Size { raw: 8 }], memory_index: [0, 1] }, abi: ScalarPair(Scalar { value: Pointer, valid_range: 1..=18446744073709551615 }, Scalar { value: Pointer, valid_range: 1..=18446744073709551615 }), align: Align { abi_pow2: 3, pref_pow2: 3 }, size: Size { raw: 16 } } }, pad: None, mode: Direct(ArgAttributes { regular: (empty), pointee_size: Size { raw: 0 }, pointee_align: None }) }

[kennytm]

This is a regression introduced since 1.24.

[nikomatsakis]

Can we bisect this down to a nightly?

[nikomatsakis]

Perhaps with https://github.com/rust-lang-nursery/cargo-bisect-rustc ?

[nikomatsakis]

Marking as P-high, regression, but not assigning anyone till we have some better idea of the cause.

[pietroalbini]

The regression happened in nightly-2017-11-21. In that range there is #45225, which is a refactoring by @eddyb on the file where the ICE is happening.

By the way, we shouldn't waste time bisecting regressions, we already have @nikomatsakis...

<nmatsakis> would be useful to know which PR
<nmatsakis> my money is on some massive eddyb refactoring ;)

[nikomatsakis]

@eddyb do you think you can take a look at this bug?

[eddyb]

Yeah this is #45225, looks like the pre-existing assumption of having the first argument split (because it's a pointer to a dyn Trait, i.e. a pair of pointer and vtable) stopped holding on non-Rust ABIs.
(technically a bug-fix, now we treat Rust "scalar pair" types like plain C structs, across FFI)

I'm surprised nobody hit this until now and that we have no tests for non-Rust ABIs in trait objects!

[eddyb]

Do we want to backport the fix or anything?

[pietroalbini]

Well, this is pretty old, so backporting to stable is probably too much. Nominating for beta backport though. cc @rust-lang/compiler

[arielb1]

Please no backporting codegen changes to beta. They are likely to introduce more bugs. There's no sufficiently good reason to backport this.