Add a compiler check that std::mem::size_of isn’t called on unsized types?

[steffahn]

While re-discovering #57893 (in #80783) and in particular while poking around in the world of all the wonderful ICEs that you can get once you’ve convinced the compiler that a trait object is actually Sized, I noticed that calling size_of does actually still compile without any error. It returns 0, e.g. see this example:

trait IsEqual {
    type To: ?Sized;
}
impl<A: ?Sized> IsEqual for A {
    type To = A;
}

fn main() {
    argument_equal_to_sized_type::<dyn IsEqual<To = u64>>();
}

fn argument_equal_to_sized_type<T: ?Sized>()
where
    <T as IsEqual>::To: Sized,
{
    argument_sized::<T>();
}

fn argument_sized<T>() {
    println!("{}", std::any::type_name::<T>());
    println!("{}", std::mem::size_of::<T>());
}

dyn playground::IsEqual+To = u64
0

I wonder what that value of 0 is good for! Couldn’t it raise an ICE instead?

@rustbot modify labels: T-compiler, C-enhancement, A-dst

[bjorn3]

0 is the lower bound on the dereferencable size for references to unsized types as told to LLVM.

[RalfJung]

FWIW, given that this relies on the coherence bug, I think trying to fix this is a game of whack-a-mole... we already check that size_of is only called on sized types, simply by doing type-checking. The type-checker has a bug, but that bug should be fixed... I am not convinced it is worth putting up second lines of defense here as that's a lost cause, there are just too many things relying on the type checker.

[RalfJung]

OTOH, I guess if it's as easy as this check then it doesn't really hurt either.^^

[lcnr]

it looks like we already implement the check described here. Don't think that we need to add a test for this.

[RalfJung]

There's still a bug though, see #79047.

[lcnr]

yeah, but that bug isn't tracked by this issue, is it?

[RalfJung]

Not sure... the bug that the original code here leads to invalid MIR remains.

[lcnr]

while this is true, that bug is tracked as part of #57893

[steffahn]

For the record, I’m of the opinion that it’s correct that this issue is closed.

[steffahn]

Wait.. actually, I’m confused: Why exactly does it hit “SizeOf nullary MIR operator called for unsized type” now and didn’t before? Which one was the relevant change? That line that triggers the “SizeOf nullary MIR operator called for unsized type” message seems to be pretty old already..

[RalfJung]

while this is true, that bug is tracked as part of #57893

This has nothing to do with #57893. rustc is showing an error after all. I would not have accepted a Miri patch that just works around that coherence issue.

The problem is that rustc produces MIR despite their being a type error, and then runs CTFE on that broken MIR. This is tracked in #79047 for transmute; I guess we can also use that issue for SizeOf.

[RalfJung]

Btw, the PRs that fixed the ICE are #81185 and #81243.

[lcnr]

ah yeah, rustc trying to eval broken mir seems like a separate bug to me.