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

Unexpected tail in unsized_info_ty: usize for ty=process::Core<isize, isize, isize> #32377

Closed
plietar opened this issue Mar 20, 2016 · 16 comments · Fixed by #32939
Closed

Unexpected tail in unsized_info_ty: usize for ty=process::Core<isize, isize, isize> #32377

plietar opened this issue Mar 20, 2016 · 16 comments · Fixed by #32939
Assignees
Labels
P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@plietar
Copy link
Contributor

plietar commented Mar 20, 2016

Hi,
compiling the eventual crate with the latest rust nightly (2016-03-20) gives me the following ICE. The previous nightly (2016-03-17) worked fine.

$ RUST_BACKTRACE=1 multirust run nightly cargo build
Compiling eventual v0.1.5 (file:///private/tmp/eventual)
error: internal compiler error: unexpected panic
note: the compiler unexpectedly panicked. this is a bug.
note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
thread 'rustc' panicked at 'internal error: entered unreachable code: Unexpected tail in unsized_info_ty: usize for ty=process::Core<isize, isize, isize>', ../src/librustc_trans/trans/type_of.rs:135
stack backtrace:
   1:        0x11312af38 - sys::backtrace::tracing::imp::write::h83bc1946ac132a7d0Gu
   2:        0x113134475 - panicking::default_hook::_$u7b$$u7b$closure$u7d$$u7d$::closure.43767
   3:        0x113133fd7 - panicking::default_hook::had684d056a84b05cPgz
   4:        0x1130fb336 - sys_common::unwind::begin_unwind_inner::hfef527c9f2434064HGt
   5:        0x1130fcf4e - sys_common::unwind::begin_unwind_fmt::h02d40e945b8e457cNFt
   6:        0x10f01d7fa - trans::type_of::unsized_info_ty::h2e7677e19516a63dYwS
   7:        0x10eee2abe - trans::type_of::sizing_type_of::h7b00ca8fada013b0roS
   8:        0x10ef39726 - trans::base::trans_crate::h389b5fe1abf2545ch3k
   9:        0x10ec74e6e - driver::phase_4_translate_to_llvm::h9c2dd5a6e3ca512cK2a
  10:        0x10ec73774 - driver::compile_input::_$u7b$$u7b$closure$u7d$$u7d$::closure.31089
  11:        0x10ec70425 - driver::phase_3_run_analysis_passes::_$u7b$$u7b$closure$u7d$$u7d$::closure.30095
  12:        0x10ec6a3e9 - middle::ty::context::TyCtxt<'tcx>::create_and_enter::h11812805895441873020
  13:        0x10ec66dad - driver::phase_3_run_analysis_passes::h5261001332275006052
  14:        0x10ec39df7 - driver::compile_input::he5def18759d9424cPca
  15:        0x10ec276f6 - run_compiler::h61ff55f691b932efVMc
  16:        0x10ec24ca2 - sys_common::unwind::try::try_fn::h2984156204492186173
  17:        0x11312879b - __rust_try
  18:        0x113128723 - sys_common::unwind::inner_try::hbef67b929e699248JDt
  19:        0x10ec25539 - boxed::F.FnBox<A>::call_box::h8793623295134616532
  20:        0x1131332c8 - sys::thread::Thread::new::thread_start::h68477ad6a3ba8036fvy
  21:     0x7fff8fb6b9b0 - _pthread_body
  22:     0x7fff8fb6b92d - _pthread_start

error: Could not compile `eventual`.

To learn more, run the command again with --verbose.

EDIT: I had copied the rust nightly versions incorrectly

@plietar
Copy link
Contributor Author

plietar commented Mar 20, 2016

This is the smallest test case I could find :

use std::mem;
use std::marker::PhantomData;

trait Foo {
    type Error;
}

struct Bar<U: Foo> {
    stream: PhantomData<U::Error>,
}

fn foo<U: Foo>() -> Bar<U> {
    unsafe { mem::transmute([0u32; 4]) }
}
Unexpected tail in unsized_info_ty: core::marker::PhantomData<<isize as Foo>::Error> for ty=Bar<isize>

I am puzzled by the fact that isize does not appear once in the snippet.
Also, adding an implementation of Foo for isize with type Error = () makes the crash go away.

Potentially related, if I change the transmute to eg mem::transmute([0u32; 3]) and build on stable/the previous nightly, I get transmute called with differently sized types: [u32; 3] (96 bits) to Bar<U> (128 bits). The intriguing part is that I believe Bar<U> should be 0 bytes whatever U is, not 16 bytes. (It only contains a PhantomData)

@alexcrichton alexcrichton added I-nominated regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 21, 2016
@plietar
Copy link
Contributor Author

plietar commented Mar 21, 2016

Going down the rabbit hole,
So intrinsicck inserts some transmute restrictions by substituting the type parameters by a dummy type. This is where isize comes from.

Later, check_intrinsic calls sizing_type_of on that substituted type. The call to type_is_sized returns false for Bar<isize>.

From my limited understanding of librustc::middle::traits::select, it generates a list of obligations for Bar<isize> as Sized, yielding two obligations, TraitPredicate(<isize as Foo>) and TraitPredicate(<core::marker::PhantomData<_> as core::marker::Sized>), the first one failing.

Where the latest nightly differs, is at type_of.rs#L59. Instead of using a using a pointer for the second field, it now calls unsized_info_ty, which fails because the tail of Bar<U> isn't unsized.

This is going a bit too deep into the compiler internals for me, but I think 1d7c9bd only made this existing problem visible. I'm not sure how substituting an isize can really work, especially when dealing with associated types.

Here's the relevant debug output from librustc::middle::traits::select

INFO:rustc::middle::traits: type_known_to_meet_builtin_bound(ty=Bar<isize>, bound=Sized)
INFO:rustc::middle::traits::select: evaluate_obligation_conservatively(Obligation(predicate=Binder(TraitPredicate(<Bar<isize> as core::marker::Sized>)),depth=0))
INFO:rustc::middle::traits::select: evaluate_predicate_recursively(Obligation(predicate=Binder(TraitPredicate(<Bar<isize> as core::marker::Sized>)),depth=0))
INFO:rustc::middle::traits::select: evaluate_obligation_recursively(Obligation(predicate=Binder(TraitPredicate(<Bar<isize> as core::marker::Sized>)),depth=0))
INFO:rustc::middle::traits::select: candidate_from_obligation(cache_fresh_trait_pred=Binder(TraitPredicate(<Bar<isize> as core::marker::Sized>)), obligation=TraitObligationStack(Obligation(predicate=Binder(TraitPredicate(<Bar<isize> as core::marker::Sized>)),depth=0)))
INFO:rustc::middle::traits::select: is_knowable(intercrate=false)
INFO:rustc::middle::traits::select: builtin_bound: bound=Sized
INFO:rustc::middle::traits::select: assemble_candidates_for_projected_tys(Obligation(predicate=Binder(TraitPredicate(<Bar<isize> as core::marker::Sized>)),depth=0))
INFO:rustc::middle::traits::select: assemble_candidates_from_caller_bounds(Obligation(predicate=Binder(TraitPredicate(<Bar<isize> as core::marker::Sized>)),depth=0))
INFO:rustc::middle::traits::select: candidate list size: 1
INFO:rustc::middle::traits::select: assembled 1 candidates for TraitObligationStack(Obligation(predicate=Binder(TraitPredicate(<Bar<isize> as core::marker::Sized>)),depth=0)): [BuiltinCandidate(Sized)]
INFO:rustc::middle::traits::select: CACHE MISS: SELECT(Binder(TraitPredicate(<Bar<isize> as core::marker::Sized>)))=Ok(Some(BuiltinCandidate(Sized)))
INFO:rustc::middle::traits::select: evaluate_candidate: depth=0 candidate=BuiltinCandidate(Sized)
INFO:rustc::middle::traits::select: confirm_candidate(Obligation(predicate=Binder(TraitPredicate(<Bar<isize> as core::marker::Sized>)),depth=0), BuiltinCandidate(Sized))
INFO:rustc::middle::traits::select: confirm_builtin_candidate(Obligation(predicate=Binder(TraitPredicate(<Bar<isize> as core::marker::Sized>)),depth=0))
INFO:rustc::middle::traits::select: vtable_builtin_data(obligation=Obligation(predicate=Binder(TraitPredicate(<Bar<isize> as core::marker::Sized>)),depth=0), bound=Sized, nested=Binder([core::marker::PhantomData<<isize as Foo>::Error>]))
INFO:rustc::middle::traits::select: select(Obligation(predicate=Binder(TraitPredicate(<isize as Foo>)),depth=1))
INFO:rustc::middle::traits::select: candidate_from_obligation(cache_fresh_trait_pred=Binder(TraitPredicate(<isize as Foo>)), obligation=TraitObligationStack(Obligation(predicate=Binder(TraitPredicate(<isize as Foo>)),depth=1)))
INFO:rustc::middle::traits::select: is_knowable(intercrate=false)
INFO:rustc::middle::traits::select: assemble_candidates_from_impls(obligation=Obligation(predicate=Binder(TraitPredicate(<isize as Foo>)),depth=1))
INFO:rustc::middle::traits::select: assemble_candidates_from_object_ty(self_ty=isize)
INFO:rustc::middle::traits::select: assemble_candidates_for_projected_tys(Obligation(predicate=Binder(TraitPredicate(<isize as Foo>)),depth=1))
INFO:rustc::middle::traits::select: assemble_candidates_from_caller_bounds(Obligation(predicate=Binder(TraitPredicate(<isize as Foo>)),depth=1))
INFO:rustc::middle::traits::select: assemble_candidates_from_default_impls(self_ty=isize)
INFO:rustc::middle::traits::select: candidate list size: 0
INFO:rustc::middle::traits::select: assembled 0 candidates for TraitObligationStack(Obligation(predicate=Binder(TraitPredicate(<isize as Foo>)),depth=1)): []
INFO:rustc::middle::traits::select: CACHE MISS: SELECT(Binder(TraitPredicate(<isize as Foo>)))=Err(Unimplemented)
INFO:rustc::middle::traits::select: vtable_builtin_data: obligations=[Obligation(predicate=Binder(TraitPredicate(<isize as Foo>)),depth=1), Obligation(predicate=Binder(TraitPredicate(<core::marker::PhantomData<_> as core::marker::Sized>)),depth=1)]
INFO:rustc::middle::traits::select: evaluate_predicate_recursively(Obligation(predicate=Binder(TraitPredicate(<isize as Foo>)),depth=1))
INFO:rustc::middle::traits::select: evaluate_obligation_recursively(Obligation(predicate=Binder(TraitPredicate(<isize as Foo>)),depth=1))
INFO:rustc::middle::traits::select: candidate_from_obligation(cache_fresh_trait_pred=Binder(TraitPredicate(<isize as Foo>)), obligation=TraitObligationStack(Obligation(predicate=Binder(TraitPredicate(<isize as Foo>)),depth=1)))
INFO:rustc::middle::traits::select: CACHE HIT: SELECT(Binder(TraitPredicate(<isize as Foo>)))=Err(Unimplemented)
INFO:rustc::middle::traits::select: CACHE MISS: EVAL(Binder(<isize as Foo>))=EvaluatedToErr
INFO:rustc::middle::traits::select: evaluate_predicate_recursively(Obligation(predicate=Binder(TraitPredicate(<isize as Foo>)),depth=1)) = EvaluatedToErr
INFO:rustc::middle::traits::select: evaluate_candidate: depth=0 result=EvaluatedToErr
INFO:rustc::middle::traits::select: CACHE MISS: EVAL(Binder(<Bar<isize> as core::marker::Sized>))=EvaluatedToErr
INFO:rustc::middle::traits: type_known_to_meet_builtin_bound: ty=Bar<isize> bound=Sized => false

@plietar
Copy link
Contributor Author

plietar commented Mar 21, 2016

Here is a related ICE on stable http://is.gd/OozIh7
This makes me believe that substituting type parameters by isize/[isize] isn't correct, especially when associated types are involved.

Instead, it would have to substitute it first by a type whose associated type is Sized, then by one which isn't, recursively and exponentially for each type parameter / associated type.

However, this is probably hinting at the fact that substitution by dummy types isn't the right way to do it, no that I have any idea what that would be.

@eddyb
Copy link
Member

eddyb commented Mar 21, 2016

Guess this is a good time to move representation higher up to not depend on LLVM.

@eddyb eddyb self-assigned this Mar 21, 2016
@eddyb eddyb closed this as completed Mar 22, 2016
@eddyb eddyb reopened this Mar 22, 2016
@nikomatsakis
Copy link
Contributor

Discussed in @rust-lang/compiler meeting. This is tricky to prioritize. @eddyb is right that the best solution is to stop relying on LLVM to compute sizes for us. Moreover, the examples given -- or at least the first one -- are in error (even though they ought not to ICE). It's very odd that the stable compiler accepts it.

@plietar, do you have any inkling of the code in the original crate that is now ICEing?

@plietar
Copy link
Contributor Author

plietar commented Mar 24, 2016

I've run into this ICE when building the eventual crate.
Transmuting the process::Core struct is causing it, in the Deref for Inner implementation .

This is causing my librespot build to fail on nightly as opposed to nightly 2016-03-17 or earlier

@eddyb
Copy link
Member

eddyb commented Mar 24, 2016

@plietar Looking at that code, it is a transmute from a raw pointer to a reference, both pointing to the same Sized type (Core<T, U, F>).
We would definitely support that with higher-level layout computation (both input and output would be thin pointers and that's enough), but it's completely unnecessary.

You can just use &* to turn a raw pointer into a reference.

@comex
Copy link
Contributor

comex commented Mar 29, 2016

Just ran into this, FWIW, in some old hacky code that turned out to be unnecessary. (I wanted to write [u8]: Index<T>, <[u8] as Index<T>>::Output = [u8], got the error that equality constraints aren't supported, and forgot that I could write [u8]: Index<T, Output=[u8]>. So instead I used a different trait to limit T to the Range types, and used transmute to convert an unknown type, that I knew was actually &[u8], to &[u8]. This used to work but now ICEs with a similar error. I worked around it by just adding the proper Output constraint.)

euclio added a commit to euclio/eventual that referenced this issue Mar 30, 2016
These were exposing a compiler crash on the latest nightly. See
rust-lang/rust#32377.
@alexcrichton
Copy link
Member

This regression is also present in cbox and operational

@eddyb
Copy link
Member

eddyb commented Mar 31, 2016

@alexcrichton Hitting this means the code was wrong to begin with (and will receive a transmute error when this is fixed).

@alexcrichton
Copy link
Member

Ah yeah sorry to clarify I just ran a crate report against stable and those two crates just showed up with this same assertion (sorry I don't know much about the underlying issue here)

@comex
Copy link
Contributor

comex commented Mar 31, 2016

@eddyb For what definition of wrong? Are you saying that transmute in generic functions is going to start checking whether the sizes are guaranteed to match for any possible instantiations? (At least in my case, the only monomorphizations actually instantiated were valid transmutes, and I imagine the others are the same if they didn't get LLVM errors.) I suppose this matches the rest of the type system better, but it seems like an unnecessary stable-breaking change. Also, in theory this could mess up code that wants to be generic over "any type of size X", which of course is not expressible in the type system; it can just change from transmute(x) to *transmute(&x), but it seems odd to trade a compiler error for silent UB in the event the code is buggy and the sizes don't actually match up...

@eddyb
Copy link
Member

eddyb commented Mar 31, 2016

@comex Actually, I've taken a closer look and neither cbox or operational use the bug exposed by the original testcase for this issue (which was that PhantomData<UnresolvableAssociatedType> ended up being treated as unsized, AFAICT, allowing a transmute from it to (usize, usize)), so they shouldn't break.

I guess I'll try to track it down before working on the generalized solution.

@eddyb
Copy link
Member

eddyb commented Mar 31, 2016

Turns out that Bar<isize>: Sized requires not only PhantomData<<isize as Foo>::Error>: Sized but also isize: Foo because it tries to normalize PhantomData<<isize as Foo>::Error> (and thus <isize as Foo>::Error) eagerly, even if the Sized bound doesn't depend on it.

@pnkfelix
Copy link
Member

triage: P-high

@rust-highfive rust-highfive added P-high High priority and removed I-nominated labels Mar 31, 2016
carllerche pushed a commit to carllerche/eventual that referenced this issue Apr 2, 2016
These were exposing a compiler crash on the latest nightly. See
rust-lang/rust#32377.
@euclio euclio mentioned this issue Apr 2, 2016
@nikomatsakis
Copy link
Contributor

@eddyb are you saying that the value is considered unsized because isize: Foo doesn't hold?

bors added a commit that referenced this issue Apr 20, 2016
Compute LLVM-agnostic type layouts in rustc.

Layout for monomorphic types, and some polymorphic ones (e.g. `&T` where `T: Sized`),
can now be computed by rustc without involving LLVM in the actual process.

This gives rustc the ability to evaluate `size_of` or `align_of`, as well as obtain field offsets.
MIR-based CTFE will eventually make use of these layouts, as will MIR trans, shortly.

Layout computation also comes with a `[breaking-change]`, or two:
* `"data-layout"` is now mandatory in custom target specifications, reverting the decision from #27076.
This string is needed because it describes endianness, pointer size and alignments for various types.
We have the first two and we could allow tweaking alignments in target specifications.
Or we could also extract the data layout from LLVM and feed it back into rustc.
However, that can vary with the LLVM version, which is fragile and undermines stability.
For built-in targets, I've added a check that the hardcoded data-layout matches LLVM defaults.
* `transmute` calls are checked in a stricter fashion, which fixes #32377

To expand on `transmute`, there are only 2 allowed patterns: between types with statically known sizes and between pointers with the same potentially-unsized "tail" (which determines the type of unsized metadata they use, if any).
If you're affected, my suggestions are:
* try to use casts (and raw pointer deref) instead of transmutes
* *really* try to avoid `transmute` where possible
* if you have a structure, try working on individual fields and unpack/repack the structure instead of transmuting it whole, e.g. `transmute::<RefCell<Box<T>>, RefCell<*mut T>>(x)` doesn't work, but `RefCell::new(Box::into_raw(x.into_inner()))` does (and `Box::into_raw` is just a `transmute`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants