creader: Host crate loaded twice produces different CrateNums if host != target

[petrochenkov]

Reproduction (as an UI test):

// force-host
// no-prefer-dynamic

#![crate_type = "proc-macro"]

extern crate proc_macro;
use proc_macro::TokenStream;

#[proc_macro]
pub fn mac(input: TokenStream) -> TokenStream {
    input
}

// edition:2018
// compile-flags:--extern reproduction
// aux-build:reproduction.rs

reproduction::mac!();

fn main() {}

# Assuming x86_64-unknown-linux-gnu host
./x.py test --bless --stage 1 --target i686-unknown-linux-gnu src/test/ui --test-args reproduction

Name resolution will try to load the crate twice, first in initial pass, then in validation pass.
The results will be different due to different loaded CrateNums, so we get the "inconsistent resolution for a macro" assertion.

This happens due to the locate_ctxt.triple == &self.sess.opts.target_triple condition in CrateLoader::load preventing reuse of CrateNums for host crates during cross-compilation.

cc @alexcrichton probably

[petrochenkov]

If I'm not mistaken, the issue will affect any idiomatic Rust 2018 code trying to use proc macros together with cross-compilation.

[petrochenkov]

The workaround is to avoid using host crates through extern prelude and use them through extern crate items instead, like it's done in a5d4544.

[alexcrichton]

Sorry for the late delay in checking in on this. This is somewhat expected in the sense that we load proc-macro crates from the compiler's target and the actual target differently as they may be two different crates. That was done a really long time ago and I never really followed all of the macro developments and how they affected cross compilation.

We can probably add a function that compares two CrateNum instances though to see if they only differ by target and maybe that'd help here?

[jonas-schievink]

Posting a typical message and backtrace here to make this issue easier to find:

thread 'rustc' panicked at 'src/librustc_resolve/imports.rs:906: inconsistent resolution for an import', /rustc/1836e3b42a5b2f37fd79104eedbe8f48a5afdee6/src/libstd/macros.rs:13:23
stack backtrace:
   0: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
   1: core::fmt::write
   2: std::io::Write::write_fmt
   3: std::panicking::default_hook::{{closure}}
   4: std::panicking::default_hook
   5: rustc_driver::report_ice
   6: std::panicking::rust_panic_with_hook
   7: std::panicking::begin_panic
   8: rustc_middle::util::bug::opt_span_bug_fmt::{{closure}}
   9: rustc_middle::ty::context::tls::with_opt::{{closure}}
  10: rustc_middle::ty::context::tls::with_opt
  11: rustc_middle::util::bug::opt_span_bug_fmt
  12: rustc_middle::util::bug::span_bug_fmt
  13: rustc_resolve::imports::ImportResolver::finalize_import
  14: rustc_resolve::imports::ImportResolver::finalize_imports
  15: rustc_resolve::Resolver::resolve_crate
  16: rustc_interface::passes::configure_and_expand_inner
  17: rustc_interface::passes::configure_and_expand::{{closure}}
  18: rustc_data_structures::box_region::PinnedGenerator<I,A,R>::new
  19: rustc_interface::passes::configure_and_expand
  20: rustc_interface::queries::Queries::expansion
  21: rustc_interface::interface::run_compiler_in_existing_thread_pool
  22: scoped_tls::ScopedKey<T>::set
  23: rustc_ast::attr::with_globals

[petrochenkov]

It would be great if someone come up with a short-term fix for this, perhaps hacky.

Otherwise I hope to get to this in June and fix it together with a few other issues in crate loading.

[jyn514]

This is somewhat expected in the sense that we load proc-macro crates from the compiler's target and the actual target differently as they may be two different crates.

@alexcrichton do you remember what you meant by this? Why could they be different crates? Proc-macros are always compiled for the host, so they should be the same even when cross-compiling.

In general, I'm not sure when this could go wrong - the compiler already checks if the target doesn't match and gives an error:

error[E0461]: couldn't find crate `dependency` with expected target triple x86_64-unknown-linux-gnu
  --> /home/joshua/rustc4/src/test/ui/cfg-dependent.rs:8:2
   |
LL |     dependency::is_64();
   |     ^^^^^^^^^^
   |
   = note: the following crate versions were found:
           crate `dependency`, target triple i686-unknown-linux-gnu: /home/joshua/rustc4/build/x86_64-unknown-linux-gnu/test/ui/cfg-dependent/auxiliary/libdependency.so

So I'm not sure why we have to check again when loading the dependency. I don't think it's possible to load two crates compiled for different targets at the same time, even without this check, it will only load the crate that matches self.sess.opts.target_triple.

[TrueDoctor]

@jyn514 does it also make sense to add the test case described in #85386 (comment) ?
Would be nice if the ICE got resolved by your change as well

[jyn514]

@TrueDoctor how is that test case different from https://github.com/rust-lang/rust/blob/6882454bd3fabc36f54cfb2125e6edd26e50ec21/src/test/ui/crate-loading/cross-compiled-proc-macro.rs ?

[TrueDoctor]

@jyn514 I my testing I found that the compiler shows an error when you import one proc macro and panics with an ICE as soon as you import two or more proc macros from the same crate.
I would suspect your change fixes both but we might still to check just to be sure

[jyn514]

@TrueDoctor I can't replicate that ICE.

> cargo --version
cargo 1.55.0-nightly (495297903 2021-07-01)
> cat src/lib.rs 
pub use reproduction::{ToDiscriminant, TransitiveChild};
use proc_macro::TokenStream;
> cat reproduction/src/lib.rs
#[proc_macro_derive(ToDiscriminant)]
pub fn derive_discriminant(_: TokenStream) -> TokenStream {
    TokenStream::new()
}

#[proc_macro_derive(TransitiveChild)]
pub fn derive_transitive_child(_: TokenStream) -> TokenStream {
    TokenStream::new()
}
> cargo check --target=wasm32-unknown-unknown
   Compiling reproduction v0.1.0 (/home/joshua/test-rustdoc/creader/reproduction)
    Checking creader v0.1.0 (/home/joshua/test-rustdoc/creader)
    Finished dev [unoptimized + debuginfo] target(s) in 1.20s

[TrueDoctor]

@jyn514 I can still replicate the issue, both with stable and nightly:

graphite/editor on  ice-debugging [?] ➜ cat ../proc-macro/src/lib.rs
use proc_macro::TokenStream;

#[proc_macro_derive(ToDiscriminant)]
pub fn derive_discriminant(_: TokenStream) -> TokenStream {
    TokenStream::new()
}

#[proc_macro_derive(TransitiveChild)]
pub fn derive_transitive_child(_: TokenStream) -> TokenStream {
    TokenStream::new()
}

graphite/editor on  ice-debugging [?] ➜ cat src/lib.rs
pub use proc_macros::{ToDiscriminant, TransitiveChild};

graphite/editor on  ice-debugging [?] ➜ cargo check --target wasm32-unknown-unknown
    Checking graphite-editor-core v0.1.0 (/home/dennis/Projects/rust/Graphite/editor)
thread 'rustc' panicked at 'Failed to get crate data for crate15', compiler/rustc_metadata/src/creader.rs:139:32
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

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/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.55.0-nightly (b3d11f95c 2021-07-04) running on x86_64-unknown-linux-gnu

note: compiler flags: -C embed-bitcode=no -C debuginfo=2 -C incremental --crate-type lib

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

query stack during panic:
end of query stack
error: could not compile `graphite-editor-core`

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

I'm using this repo for testing: https://github.com/GraphiteEditor/Graphite/tree/ice-debugging

[TrueDoctor]

The only difference I can spot in which file use proc_macro::TokenStream;is, I have it in the proc macro crate and you seem to have in in the main file (unless the lines just got mixed up in your comment)

[alexcrichton]

@alexcrichton do you remember what you meant by this?

Sorry it's been so long I don't really remember most of this code here any more.