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

Compiler crashed with cannot create local mono-item #50865

Closed
zonyitoo opened this Issue May 18, 2018 · 31 comments

Comments

Projects
None yet
@zonyitoo
Copy link

zonyitoo commented May 18, 2018

Relevant logs:

Log hidden by @eddyb (click to expand)
   Compiling shadowsocks-rust v1.7.0-alpha.7 (file:///Users/zonyitoo/Projects/shadowsocks-rust)
error: internal compiler error: librustc_mir/monomorphize/collector.rs:750: Cannot create local mono-item for DefId(19/0:1252 ~ shadowsocks[e91a]::relay[0]::udprelay[0]::dns[0]::{{impl}}[2]::new[0])

thread 'main' panicked at 'Box<Any>', librustc_errors/lib.rs:554:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.
error: aborting due to previous error


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

note: rustc 1.27.0-nightly (2f2a11dfc 2018-05-16) running on x86_64-apple-darwin

note: compiler flags: -C debuginfo=2 -C incremental --crate-type bin

note: some of the compiler flags provided by cargo are hidden

error: Could not compile `shadowsocks-rust`.
warning: build failed, waiting for other jobs to finish...
error: build failed

Reproduce steps:

  1. Clone the master branch of https://github.com/shadowsocks/shadowsocks-rust
  2. Build with cargo build
  3. Compiler crashes

Meta

$ rustc --version --verbose
rustc 1.27.0-nightly (2f2a11dfc 2018-05-16)
binary: rustc
commit-hash: 2f2a11dfc436fc0f401b595f22ed043c46dbebe7
commit-date: 2018-05-16
host: x86_64-apple-darwin
release: 1.27.0-nightly
LLVM version: 6.0

Backtrace

stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
   1: std::sys_common::backtrace::print
   2: std::panicking::default_hook::{{closure}}
   3: std::panicking::default_hook
   4: rustc::util::common::panic_hook
   5: std::panicking::rust_panic_with_hook
   6: std::panicking::begin_panic
   7: rustc_errors::Handler::bug
   8: rustc::session::opt_span_bug_fmt::{{closure}}
   9: rustc::ty::context::tls::with_opt::{{closure}}
  10: rustc::ty::context::tls::with_context_opt
  11: rustc::ty::context::tls::with_opt
  12: rustc::session::opt_span_bug_fmt
  13: rustc::session::bug_fmt
  14: rustc_mir::monomorphize::collector::should_monomorphize_locally
  15: rustc_mir::monomorphize::collector::visit_instance_use
  16: <rustc_mir::monomorphize::collector::MirNeighborCollector<'a, 'tcx> as rustc::mir::visit::Visitor<'tcx>>::visit_terminator_kind
  17: rustc_mir::monomorphize::collector::collect_items_rec
  18: rustc_mir::monomorphize::collector::collect_items_rec
  19: rustc_mir::monomorphize::collector::collect_items_rec
  20: rustc_mir::monomorphize::collector::collect_items_rec
  21: rustc_mir::monomorphize::collector::collect_items_rec
  22: rustc_mir::monomorphize::collector::collect_items_rec
  23: rustc_mir::monomorphize::collector::collect_items_rec
  24: rustc_mir::monomorphize::collector::collect_items_rec
  25: rustc_mir::monomorphize::collector::collect_items_rec
  26: rustc_mir::monomorphize::collector::collect_items_rec
  27: rustc_mir::monomorphize::collector::collect_items_rec
  28: rustc_mir::monomorphize::collector::collect_items_rec
  29: rustc_mir::monomorphize::collector::collect_items_rec
  30: rustc_mir::monomorphize::collector::collect_items_rec
  31: rustc_mir::monomorphize::collector::collect_items_rec
  32: rustc_mir::monomorphize::collector::collect_items_rec
  33: rustc_mir::monomorphize::collector::collect_items_rec
  34: rustc_mir::monomorphize::collector::collect_items_rec
  35: rustc_mir::monomorphize::collector::collect_items_rec
  36: rustc_mir::monomorphize::collector::collect_items_rec
  37: rustc_mir::monomorphize::collector::collect_items_rec
  38: rustc_mir::monomorphize::collector::collect_items_rec
  39: rustc_mir::monomorphize::collector::collect_items_rec
  40: rustc_mir::monomorphize::collector::collect_items_rec
  41: rustc_mir::monomorphize::collector::collect_items_rec
  42: rustc_mir::monomorphize::collector::collect_items_rec
  43: rustc_mir::monomorphize::collector::collect_crate_mono_items
  44: rustc::util::common::time
  45: rustc_trans::base::collect_and_partition_translation_items
  46: rustc::ty::context::tls::with_context
  47: rustc::dep_graph::graph::DepGraph::with_task_impl
  48: rustc::ty::context::tls::with_related_context
  49: rustc::ty::maps::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::force_query_with_job
  50: rustc::ty::maps::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::get_query
  51: rustc_trans::base::trans_crate
  52: <rustc_trans::LlvmTransCrate as rustc_trans_utils::trans_crate::TransCrate>::trans_crate
  53: rustc::util::common::time
  54: rustc_driver::driver::phase_4_translate_to_llvm
  55: rustc_driver::driver::compile_input::{{closure}}
  56: rustc::ty::context::tls::enter_context
  57: <std::thread::local::LocalKey<T>>::with
  58: rustc::ty::context::TyCtxt::create_and_enter
  59: rustc_driver::driver::compile_input
  60: rustc_driver::run_compiler_with_pool
  61: <scoped_tls::ScopedKey<T>>::set
  62: syntax::with_globals
  63: <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
  64: __rust_maybe_catch_panic
  65: rustc_driver::run
  66: rustc_driver::main
  67: std::rt::lang_start::{{closure}}
  68: std::panicking::try::do_call
  69: __rust_maybe_catch_panic
  70: std::rt::lang_start_internal
  71: main
query stack during panic:
#0 [collect_and_partition_translation_items] collect_and_partition_translation_items
end of query stack

And I think it also happens in stable rustc 1.26.0.
@zonyitoo

This comment has been minimized.

Copy link
Author

zonyitoo commented May 18, 2018

BTW, by replacing

pub fn run(config: Arc<Config>) -> impl Future<Item = (), Error = io::Error> + Send

with

pub fn run(config: Arc<Config>) -> Box<Future<Item = (), Error = io::Error> + Send>

in src/relay/udprelay/dns.rs, it won't crash again.

@khuey

This comment has been minimized.

Copy link
Contributor

khuey commented May 19, 2018

I see this too in a separate (not open source) codebase. My stack is slightly different:

Log hidden by @eddyb (click to expand)
error: internal compiler error: librustc_mir/monomorphize/collector.rs:750: Cannot create local mono-item for DefId(164/0:347 ~ gdbserver[d9d4]::sysroot_populator[0]::get_file_name_for_open_syscall[0])

thread 'main' panicked at 'Box<Any>', librustc_errors/lib.rs:554:9
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::default_hook::{{closure}}
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:227
   4: rustc::util::common::panic_hook
   5: std::panicking::rust_panic_with_hook
             at libstd/panicking.rs:467
   6: std::panicking::begin_panic
   7: rustc_errors::Handler::bug
   8: rustc::session::opt_span_bug_fmt::{{closure}}
   9: rustc::ty::context::tls::with_opt::{{closure}}
  10: rustc::ty::context::tls::with_context_opt
  11: rustc::ty::context::tls::with_opt
  12: rustc::session::opt_span_bug_fmt
  13: rustc::session::bug_fmt
  14: rustc_mir::monomorphize::collector::should_monomorphize_locally
  15: rustc_mir::monomorphize::collector::visit_instance_use
  16: <rustc_mir::monomorphize::collector::MirNeighborCollector<'a, 'tcx> as rustc::mir::visit::Visitor<'tcx>>::visit_terminator_kind
  17: rustc_mir::monomorphize::collector::collect_items_rec
  18: rustc_mir::monomorphize::collector::collect_items_rec
  19: rustc_mir::monomorphize::collector::collect_items_rec
  20: rustc_mir::monomorphize::collector::collect_items_rec
  21: rustc_mir::monomorphize::collector::collect_items_rec
  22: rustc_mir::monomorphize::collector::collect_items_rec
  23: rustc_mir::monomorphize::collector::collect_items_rec
  24: rustc_mir::monomorphize::collector::collect_items_rec
  25: rustc_mir::monomorphize::collector::collect_items_rec
  26: rustc_mir::monomorphize::collector::collect_items_rec
  27: rustc_mir::monomorphize::collector::collect_items_rec
  28: rustc_mir::monomorphize::collector::collect_items_rec
  29: rustc_mir::monomorphize::collector::collect_items_rec
  30: rustc_mir::monomorphize::collector::collect_items_rec
  31: rustc_mir::monomorphize::collector::collect_items_rec
  32: rustc_mir::monomorphize::collector::collect_items_rec
  33: rustc_mir::monomorphize::collector::collect_items_rec
  34: rustc_mir::monomorphize::collector::collect_items_rec
  35: rustc_mir::monomorphize::collector::collect_items_rec
  36: rustc_mir::monomorphize::collector::collect_items_rec
  37: rustc_mir::monomorphize::collector::collect_items_rec
  38: rustc_mir::monomorphize::collector::collect_items_rec
  39: rustc_mir::monomorphize::collector::collect_items_rec
  40: rustc_mir::monomorphize::collector::collect_items_rec
  41: rustc_mir::monomorphize::collector::collect_items_rec
  42: rustc_mir::monomorphize::collector::collect_items_rec
  43: rustc_mir::monomorphize::collector::collect_items_rec
  44: rustc_mir::monomorphize::collector::collect_items_rec
  45: rustc_mir::monomorphize::collector::collect_items_rec
  46: rustc_mir::monomorphize::collector::collect_items_rec
  47: rustc_mir::monomorphize::collector::collect_items_rec
  48: rustc_mir::monomorphize::collector::collect_items_rec
  49: rustc_mir::monomorphize::collector::collect_items_rec
  50: rustc_mir::monomorphize::collector::collect_items_rec
  51: rustc_mir::monomorphize::collector::collect_items_rec
  52: rustc_mir::monomorphize::collector::collect_items_rec
  53: rustc_mir::monomorphize::collector::collect_items_rec
  54: rustc_mir::monomorphize::collector::collect_items_rec
  55: rustc_mir::monomorphize::collector::collect_items_rec
  56: rustc_mir::monomorphize::collector::collect_items_rec
  57: rustc_mir::monomorphize::collector::collect_items_rec
  58: rustc_mir::monomorphize::collector::collect_items_rec
  59: rustc_mir::monomorphize::collector::collect_items_rec
  60: rustc_mir::monomorphize::collector::collect_items_rec
  61: rustc_mir::monomorphize::collector::collect_items_rec
  62: rustc_mir::monomorphize::collector::collect_items_rec
  63: rustc_mir::monomorphize::collector::collect_items_rec
  64: rustc_mir::monomorphize::collector::collect_items_rec
  65: rustc_mir::monomorphize::collector::collect_items_rec
  66: rustc_mir::monomorphize::collector::collect_crate_mono_items
  67: rustc::util::common::time
  68: rustc_codegen_llvm::base::collect_and_partition_mono_items
  69: rustc::ty::context::tls::with_context
  70: rustc::dep_graph::graph::DepGraph::with_task_impl
  71: rustc::ty::context::tls::with_related_context
  72: rustc::ty::maps::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::force_query_with_job
  73: rustc::ty::maps::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::get_query
  74: rustc_codegen_llvm::back::symbol_export::exported_symbols_provider_local
  75: rustc::ty::context::tls::with_context
  76: rustc::dep_graph::graph::DepGraph::with_task_impl
  77: rustc::ty::context::tls::with_related_context
  78: rustc::ty::maps::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::force_query_with_job
  79: rustc::ty::maps::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::get_query
  80: rustc_metadata::encoder::encode_metadata
  81: rustc_metadata::cstore_impl::<impl rustc::middle::cstore::CrateStore for rustc_metadata::cstore::CStore>::encode_metadata
  82: rustc::ty::context::TyCtxt::encode_metadata
  83: rustc_codegen_llvm::base::write_metadata
  84: rustc::util::common::time
  85: rustc_codegen_llvm::base::codegen_crate
  86: <rustc_codegen_llvm::LlvmCodegenBackend as rustc_codegen_utils::codegen_backend::CodegenBackend>::codegen_crate
  87: rustc::util::common::time
  88: rustc_driver::driver::phase_4_codegen
  89: rustc_driver::driver::compile_input::{{closure}}
  90: rustc::ty::context::tls::enter_context
  91: <std::thread::local::LocalKey<T>>::with
  92: rustc::ty::context::TyCtxt::create_and_enter
  93: rustc_driver::driver::compile_input
  94: rustc_driver::run_compiler_with_pool
  95: syntax::with_globals
  96: <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
  97: __rust_maybe_catch_panic
             at libpanic_unwind/lib.rs:105
  98: rustc_driver::run
  99: rustc_driver::main
query stack during panic:
#0 [collect_and_partition_mono_items] collect_and_partition_mono_items
#1 [exported_symbols] exported_symbols
end of query stack

The relevant diff in my code is

-                           -> Box<Future<Item = Option<SysrootEntry>, Error = Never> + Send> {
+                           -> impl Future<Item = Option<SysrootEntry>, Error = Never> + Send {

Version:

rustc -V
rustc 1.28.0-nightly (952f344cd 2018-05-18)

Happens on 1.26.0 stable too (which is where I first noticed it).

@qthree

This comment has been minimized.

Copy link

qthree commented May 24, 2018

Also met this error. My simplified example:

// lib.rs
pub trait Baz{}
pub struct Bar;
impl Baz for Bar{}

pub fn bar<P: Baz>( // Error won't happen if "bar" is not generic
    _baz: P,
) {
    let draw_data: Vec<u32> = iterate().collect();
    
}

fn iterate() -> impl Iterator<Item=u32> { // Error won't happen if "iterate" hasn't impl Trait
    std::iter::once(0).map(foo)
}


fn foo(input: u32) -> u32 { // Error won't happen if "foo" isn't used in "iterate"
    input
}
//main.rs
extern crate lib;

fn main() {
    lib::bar(lib::Bar); // Error won't happen if bar is called from same crate
}
@estokes

This comment has been minimized.

Copy link

estokes commented Jun 5, 2018

In qthree's example, if you make foo public the error doesn't happen.

I've looked at the code for rustc_mir::monomorphize::collector::should_monomorphize_locally, and it's checking if the code it needs is something that it should link to, or if it should find it in the mir. In this case it's decided it should find it in the Mir, but it blows up when it doesn't. If you remove the check, then you'll get past compilation but it will fail with a link error to lib::foo.

I don't know the compiler internals well, but my gut feeling is that this is a bug somewhere else than where it's actually detected. It seems things are missing from the Mir when using impl Trait.

I'd also like to add that this bug has come up a lot for me, basically any time you want to write a library of moderate complexity using futures-await, and then you try to use that library in a program, you will hit this bug. Now at least we know that you can work around it by making everything it blows up on public. Not a great long term solution obviously ...

@michaelwoerister

This comment has been minimized.

Copy link
Contributor

michaelwoerister commented Jun 27, 2018

@jocutajar

This comment has been minimized.

Copy link

jocutajar commented Jun 30, 2018

Confirmed with cargo --version 1.28.0-nightly (e2348c2db 2018-06-07) and rustc --version 1.28.0-nightly (01cc982 2018-06-24). It happened in a lib+main project. I tried to reproduce it in a single main.rs file, but the same code (impl Trait stuff) compiles fine there on the same tool chain. I can pass the project archive if someone is interested.

After rustup update, still the case with rustc 1.28.0-nightly (e3bf634 2018-06-28) running on x86_64-unknown-linux-gnu

As I said, it compiles fine in a single main.rs file. As soon as I moved the functions into a lib, it came back. This may be significant.

Curious: only one fn serve is public in the lib when it fails. When I make the fn accept public, it compiles. When I make the fn bind public and fn accept private, it compiles. However, if I make fn resolve public and leave the others private, it also fails. It seems thus related to the chain of calls and that public fns are handled differently than private. I can also call the pub fn accept directly from the main.rs and that compiles. Also, if I do not call into the lib from the main.rs, it compiles.

Log hidden by @eddyb (click to expand)

error: internal compiler error: librustc_mir/monomorphize/collector.rs:765: Cannot create local mono-item for DefId(55/0:18 ~ samotop[7786]::accept[0])

thread 'main' panicked at 'Box', librustc_errors/lib.rs:554:9
stack backtrace:
0: std::sys::unix::backtrace::tracing:👿:unwind_backtrace
at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
1: std::sys_common::backtrace::print
at libstd/sys_common/backtrace.rs:71
at libstd/sys_common/backtrace.rs:59
2: std::panicking::default_hook::{{closure}}
at libstd/panicking.rs:211
3: std::panicking::default_hook
at libstd/panicking.rs:227
4: rustc::util::common::panic_hook
5: std::panicking::rust_panic_with_hook
at libstd/panicking.rs:515
6: std::panicking::begin_panic
7: rustc_errors::Handler::bug
8: rustc::session::opt_span_bug_fmt::{{closure}}
9: rustc::ty::context::tls::with_opt::{{closure}}
10: rustc::ty::context::tls::with_context_opt
11: rustc::ty::context::tls::with_opt
12: rustc::session::opt_span_bug_fmt
13: rustc::session::bug_fmt
14: rustc_mir::monomorphize::collector::should_monomorphize_locally
15: rustc_mir::monomorphize::collector::visit_instance_use
16: <rustc_mir::monomorphize::collector::MirNeighborCollector<'a, 'tcx> as rustc::mir::visit::Visitor<'tcx>>::visit_terminator_kind
17: rustc_mir::monomorphize::collector::collect_items_rec
18: rustc_mir::monomorphize::collector::collect_items_rec
19: rustc_mir::monomorphize::collector::collect_items_rec
20: rustc_mir::monomorphize::collector::collect_items_rec
21: rustc_mir::monomorphize::collector::collect_items_rec
22: rustc_mir::monomorphize::collector::collect_items_rec
23: rustc_mir::monomorphize::collector::collect_items_rec
24: rustc_mir::monomorphize::collector::collect_items_rec
25: rustc_mir::monomorphize::collector::collect_items_rec
26: rustc_mir::monomorphize::collector::collect_items_rec
27: rustc_mir::monomorphize::collector::collect_crate_mono_items::{{closure}}
28: rustc_mir::monomorphize::collector::collect_crate_mono_items
29: rustc::util::common::time
30: rustc_codegen_llvm::base::collect_and_partition_mono_items
31: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors<'tcx> for rustc::ty::query::queries::collect_and_partition_mono_items<'tcx>>::compute
32: rustc::ty::context::tls::with_context
33: rustc::dep_graph::graph::DepGraph::with_task_impl
34: rustc::ty::context::tls::with_related_context
35: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::force_query_with_job
36: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::get_query
37: rustc_codegen_llvm::base::codegen_crate
38: <rustc_codegen_llvm::LlvmCodegenBackend as rustc_codegen_utils::codegen_backend::CodegenBackend>::codegen_crate
39: rustc::util::common::time
40: rustc_driver::driver::phase_4_codegen
41: rustc_driver::driver::compile_input::{{closure}}
42: rustc::ty::context::tls::enter_context
43: <std::thread::local::LocalKey>::with
44: rustc::ty::context::TyCtxt::create_and_enter
45: rustc_driver::driver::compile_input
46: rustc_driver::run_compiler_with_pool
47: <scoped_tls::ScopedKey>::set
48: syntax::with_globals
49: <std::panic::AssertUnwindSafe as core::ops::function::FnOnce<()>>::call_once
50: __rust_maybe_catch_panic
at libpanic_unwind/lib.rs:105
51: rustc_driver::run
52: rustc_driver::main
53: std::rt::lang_start::{{closure}}
54: std::panicking::try::do_call
at libstd/rt.rs:59
at libstd/panicking.rs:310
55: __rust_maybe_catch_panic
at libpanic_unwind/lib.rs:105
56: std::rt::lang_start_internal
at libstd/panicking.rs:289
at libstd/panic.rs:397
at libstd/rt.rs:58
57: main
58: __libc_start_main
59:
query stack during panic:
#0 [collect_and_partition_mono_items] collect_and_partition_mono_items
end of query stack
error: aborting due to previous error

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

note: rustc 1.28.0-nightly (01cc982 2018-06-24) running on x86_64-unknown-linux-gnu

note: compiler flags: -C debuginfo=2 -C incremental --crate-type bin

note: some of the compiler flags provided by cargo are hidden

@michaelwoerister

This comment has been minimized.

Copy link
Contributor

michaelwoerister commented Jul 2, 2018

Nominating for priority assignment.

@michaelwoerister

This comment has been minimized.

Copy link
Contributor

michaelwoerister commented Jul 2, 2018

Thanks for the bug reports everyone!

@oli-obk oli-obk self-assigned this Jul 2, 2018

@oli-obk

This comment has been minimized.

Copy link
Contributor

oli-obk commented Jul 2, 2018

I found the cause for the symptom, although not the root cause yet. The def collector (during compilation of the lib) doesn't realize it doesn't need to export the iterate function.

I have a feeling this is due to the reachability analysis operating on the HIR, while the collector is operating on the MIR and we end up with some incompatibility from that.

@oli-obk

This comment has been minimized.

Copy link
Contributor

oli-obk commented Jul 2, 2018

And I found the root cause: zst constants of fn type do not get their functions collected.

@Bert-Proesmans

This comment has been minimized.

Copy link

Bert-Proesmans commented Jul 2, 2018

I was about to respond with the details of my error. I'm glad you found out the issue so quick!
Any ideas coming to mind how we can work around this, until the next nightly/stable release?

Thanks @oli-obk !

@oli-obk

This comment has been minimized.

Copy link
Contributor

oli-obk commented Jul 2, 2018

PR incoming shortly ;)

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Jul 2, 2018

This is the same old reachability bug, right? I think I asked @petrochenkov and/or @cramertj to look at it but I'm not sure it ever got fixed. I guess people are noticing it now because of the stabilization.
EDIT: went ahead and used <details> on a few of the logs in this thread.

@oli-obk

This comment has been minimized.

Copy link
Contributor

oli-obk commented Jul 2, 2018

Minimal repro:

// lib.rs
pub fn bar<P>( // Error won't happen if "bar" is not generic
    _baz: P,
) {
    hide_foo()();
}

fn hide_foo() -> impl Fn() { // Error won't happen if "iterate" hasn't impl Trait or has generics
    foo
}

fn foo() { // Error won't happen if "foo" isn't used in "iterate" or has generics
}
// main
// aux-build:impl_trait_bug.rs

extern crate impl_trait_bug;

fn main() {
    impl_trait_bug::bar(()); // Error won't happen if bar is called from same crate
}
@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Jul 2, 2018

There is a reachability special-case which allows private impls to work:

// Some methods from non-exported (completely private) trait impls still have to be
// reachable if they are called from inlinable code. Generally, it's not known until
// monomorphization if a specific trait impl item can be reachable or not. So, we
// conservatively mark all of them as reachable.

However, this does not apply to functions that may be called statically but not obviously so.

EDIT: it seems that the privacy's EmbargoVisitor can handle a public function returning impl Trait hiding some private items including other functions (i.e. if hide_foo were public, foo would also be considered to be exported).
However, only reachability knows hide_foo is used in an exported item, and only EmbargoVisitor knows how to traverse types to propagate access levels.

@petrochenkov Should we merge reachability and EmbargoVisitor somehow? Or make EmbargoVisitor compute reachability edges from types?

@oli-obk oli-obk removed their assignment Jul 2, 2018

@jocutajar

This comment has been minimized.

Copy link

jocutajar commented Jul 4, 2018

@Bert-Proesmans , try make the fn's public. That should work around it.

@Bert-Proesmans

This comment has been minimized.

Copy link

Bert-Proesmans commented Jul 4, 2018

@jocutajar Thanks for the explicit answer. My issue is similar but not related to functions (all relevant functions were already public). I had to make fields on the offending structure public instead.

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Jul 12, 2018

triage: P-high

@pnkfelix pnkfelix added the P-high label Jul 12, 2018

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Jul 19, 2018

visited for triage, leaving assigned to @oli-obk for now.

FelixMcFelix added a commit to FelixMcFelix/rust that referenced this issue Jul 25, 2018

@FelixMcFelix

This comment has been minimized.

Copy link
Contributor

FelixMcFelix commented Jul 26, 2018

@eddyb Got bored and tried something like your proposed solution: merging the output of reachability into the privacy checking query seems to have no effect on the bug here, as in the minimum test case specified above tcx.reachable_set(...) finds no new reachable items. This does turn up many more technically reachable items in libcore and other libraries (which in turn need to be hidden again from lints, stability and so on).

Something could maybe turn up if we try to calculate the reachable set while the EmbargoVisitor does its work; I'll keep looking into this. If you have any further ideas then they'd be appreciated.

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Jul 26, 2018

visiting for triage. Taking off @oli-obk assignment as they will be very busy with non-Rust stuff very soon.

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Jul 26, 2018

@eddyb @petrochenkov would one of you be the right person to try to take on the work here?

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Aug 2, 2018

visiting for triage. @oli-obk points out that this is an impl Trait adoption blocker, and therefore it is appropriate to put it on some edition milestone.

Putting on "Rust 2018 RC" milestone.

@pnkfelix pnkfelix added this to the Rust 2018 RC milestone Aug 2, 2018

@petrochenkov

This comment has been minimized.

Copy link
Contributor

petrochenkov commented Aug 2, 2018

@nikomatsakis asked me about this issue on internals, copy-pasting the answer here:

IIRC, there are two passes calculating reachability.

The first one is "language reachability" calculated in rustc_privacy (what items can named from other crates, objects of which types can be used from other crates, but not link-time reachability).

The second one is in rustc/middle/reachable that takes results of the first pass and extends it with inline functions, generics and similar things to get link-time reachability.

The first pass was (re-)written by me long time ago when impl Trait wasn't implemented yet, then extended by someone else to support impl Trait. I don't know if those extensions follow the logic of separation of "language reachability" and "link-time reachability" or not.

I'm not sure what kind of issue #50865 is - more "link time", or "more language", looks like something in the middle.
I remember suggesting adding one more variant to enum AccessLevel - something like "Reachable, but with actual impl Trait types revealed" (that would be a superset of Reachable) and continue calculating it in rustc_privacy, while still leaving all the #[inline]-related stuff in the second pass.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Aug 9, 2018

Visiting for compiler team meeting.

@eddyb or @petrochenkov — if either of you have time to pick this off, would be good. (Or to leave some detailed instructions.)

Otherwise, we should try to find someone.

@FelixMcFelix

This comment has been minimized.

Copy link
Contributor

FelixMcFelix commented Aug 10, 2018

Increasingly, I'm becoming convinced that the problem isn't in privacy or reachability. Using this "fixed" lib.rs:

// lib.rs
pub fn bar<P>( // Error won't happen if "bar" is not generic
    _baz: P,
) {
    hide_foo()();
}

fn hide_foo() -> fn() -> () {
    foo
}

fn foo() { // Error won't happen if "foo" isn't used in "iterate" or has generics
}

This compiles as expected. We end up getting identical results from both reachabililty and privacy for the inner crate between this and the impl Trait version:

DEBUG 2018-08-10T09:50:14Z: rustc_privacy: access levels after embargo: {NodeId(0): Public, NodeId(4): Public}
DEBUG 2018-08-10T09:50:14Z: rustc_privacy: final access levels: {NodeId(0): Public, NodeId(4): Public}
DEBUG 2018-08-10T09:50:14Z: rustc::middle::reachable: Inline reachability shows: {NodeId(14), NodeId(4)}

where:

  • 0 -> crate root
  • 4 -> bar
  • 14 -> hide_foo

A quick test shows the same behaviour for a lib.rs based on generics.

This suggests that foo being marked for inclusion must happen elsewhere. Any other suggestions on where the root of the problem might lie?

@FelixMcFelix

This comment has been minimized.

Copy link
Contributor

FelixMcFelix commented Aug 17, 2018

Dug into this a little more. It seems this arises from the MIR generation step: using generics, foo is in fact only ever used internally (so it never needs to be exported), while the impl Trait version tries (and fails) to access foo externally.

Either we can explicitly mark all items who do not already have an assigned level as having a new privacy level like @petrochenkov suggested (probably visible to only tcx::reachable_non_generics), or the MIR can be changed to match how it appears to work for generics (which I'm not able to reason about the soundness of). I'm going to give the former a try.

EDIT: fixed truncated comment. This is specifically observed from the pattern/context of lookups made via rustc_mir::monomorphize::collector::should_monomorphize_locally.

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Aug 17, 2018

@FelixMcFelix Not sure what you mean about MIR - your comment appears cut off, too.

@FelixMcFelix

This comment has been minimized.

Copy link
Contributor

FelixMcFelix commented Aug 20, 2018

@eddyb Using one of the impl Trait examples based on Iterators and comparing it against the equivalent for generics, the main difference seems to be where foo is codegenned. In the explicit type and generic cases this happens while codegenning the inner crate, but for impl Trait this happens when processing main.rs. This is why the non-impl Trait cases work despite having identical output from privacy/reachability. I don't see any reason why foo shouldn't be used in the same way between impl Trait and generics. Setting RUST_LOG="rustc_mir::monomorphize::collector", we can see this to some extent.

impl Trait:

#///
# inner crate
#///
DEBUG 2018-08-20T10:26:39Z: rustc_mir::monomorphize::collector: Collecting roots
DEBUG 2018-08-20T10:26:39Z: rustc_mir::monomorphize::collector: collect_roots: entry_fn = None
# ...

#///
# main.rs
#///
DEBUG 2018-08-20T10:26:39Z: rustc_mir::monomorphize::collector: Collecting roots
DEBUG 2018-08-20T10:26:39Z: rustc_mir::monomorphize::collector: collect_roots: entry_fn = Some(DefId(0/0:4 ~ testtest[106a]::main[0]))
# ...
DEBUG 2018-08-20T10:26:40Z: rustc_mir::monomorphize::collector: visit_item_use(Instance { def: Item(DefId(9/0:8 ~ 
testtest[dd34]::foo[0])), substs: [] }, is_direct_call=true)

error: internal compiler error
: librustc_mir\monomorphize\collector.rs:767: Cannot create local mono-item for DefId(9/0:8 ~ testtest[dd34]::foo[0])

Generics:

#///
# inner crate
#///
DEBUG 2018-08-20T11:14:02Z: rustc_mir::monomorphize::collector: Collecting roots
DEBUG 2018-08-20T11:14:02Z: rustc_mir::monomorphize::collector: collect_roots: entry_fn = None
#...
DEBUG 2018-08-20T11:14:02Z: rustc_mir::monomorphize::collector: visit_item_use(Instance { def: Item(DefId(0/0:8 ~ testtest[dd34]::foo[0])), substs: [] }, is_direct_call=true)
# ...

#///
# main.rs
#///
DEBUG 2018-08-20T11:14:02Z: rustc_mir::monomorphize::collector: Collecting roots
DEBUG 2018-08-20T11:14:02Z: rustc_mir::monomorphize::collector: collect_roots: entry_fn = Some(DefId(0/0:4 ~ testtest[106a]::main[0]))
#...

Not sure if this 100% explains why I think the error stems from here, I'm available on discord if need be (FelixMcFelix#2443).

FelixMcFelix added a commit to FelixMcFelix/rust that referenced this issue Aug 20, 2018

kennytm added a commit to kennytm/rust that referenced this issue Aug 24, 2018

Rollup merge of rust-lang#53545 - FelixMcFelix:fix-50865-beta, r=petr…
…ochenkov

Fix rust-lang#50865: ICE on impl-trait returning functions reaching private items

Adds a test case as suggested in rust-lang#50865, and implements @petrochenkov's suggestion. Fixes rust-lang#50865.

Impl-trait-returning functions are marked under a new (low) access level, which they propagate rather than `AccessLevels::Reachable`. `AccessLevels::is_reachable` returns false for such items (leaving stability analysis unaffected), these items may still be visible to the lints phase however.

bors added a commit that referenced this issue Aug 24, 2018

Auto merge of #53662 - kennytm:rollup, r=kennytm
Rollup of 16 pull requests

Successful merges:

 - #53311 (Window Mutex: Document that we properly initialize the SRWLock)
 - #53503 (Discourage overuse of mem::forget)
 - #53545 (Fix #50865: ICE on impl-trait returning functions reaching private items)
 - #53559 (add macro check for lint)
 - #53562 (Lament the invincibility of the Turbofish)
 - #53563 (use String::new() instead of String::from(""), "".to_string(), "".to_owned() or "".into())
 - #53592 (docs: minor stylistic changes to str/string docs)
 - #53594 (Update RELEASES.md to include clippy-preview)
 - #53600 (Fix a grammatical mistake in "expected generic arguments" errors)
 - #53614 (update nomicon and book)
 - #53617 (tidy: Stop requiring a license header)
 - #53618 (Add missing fmt examples)
 - #53636 (Prefer `.nth(n)` over `.skip(n).next()`.)
 - #53644 (Use SmallVec for SmallCStr)
 - #53664 (Remove unnecessary closure in rustc_mir/build/mod.rs)
 - #53666 (Added rustc_codegen_llvm to compiler documentation.)

@bors bors closed this in #53545 Aug 24, 2018

@retep007

This comment has been minimized.

Copy link
Member

retep007 commented Aug 27, 2018

This fix is available in 1.30.0-nightly
However, I have very similar ICE #53468 (comment).

@bobbobbio

This comment has been minimized.

Copy link

bobbobbio commented Oct 26, 2018

I also have a similar looking ICE dating back quite a while ago: #51388 and still fails in 1.30.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment