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

ICE compiling the objrs crate #54059

Closed
mjbshaw opened this issue Sep 8, 2018 · 10 comments · Fixed by #54265
Closed

ICE compiling the objrs crate #54059

mjbshaw opened this issue Sep 8, 2018 · 10 comments · Fixed by #54265
Assignees
Labels
A-macros-1.2 Area: Declarative macros 1.2 C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@mjbshaw
Copy link
Contributor

mjbshaw commented Sep 8, 2018

cargo --version: cargo 1.29.0-nightly (0ec7281b9 2018-08-20)
rustc --version: rustc 1.30.0-nightly (fc81e3624 2018-09-07)

Steps to reproduce:

$ git clone https://gitlab.com/objrs/objrs.git
$ cd objrs
$ cargo +nightly build

This is failing when the objrs crate attempts to import the objrs_macros crate (which is a proc-macro crate). That is, if I modify objrs's lib.rs file to only contain:

extern crate objrs_macros;

rustc will ICE.

Backtrace:
$ RUST_BACKTRACE=1 cargo +nightly build
   Compiling objrs v0.0.1 (file:///private/tmp/objrs)
thread 'main' panicked at 'index out of bounds: the len is 1 but the index is 150', /Users/travis/build/rust-lang/rust/src/libcore/slice/mod.rs:2046:10
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::continue_panic_fmt
   7: rust_begin_unwind
   8: core::panicking::panic_fmt
   9: core::panicking::panic_bounds_check
  10: rustc_metadata::cstore_impl::<impl rustc::middle::cstore::CrateStore for rustc_metadata::cstore::CStore>::def_path_hash
  11: <[T] as rustc_data_structures::stable_hasher::HashStable<CTX>>::hash_stable
  12: rustc::dep_graph::graph::DepGraph::with_task_impl
  13: rustc::ty::context::tls::with_related_context
  14: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::force_query_with_job
  15: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::get_query
  16: rustc_codegen_llvm::base::<impl rustc_codegen_llvm::CrateInfo>::new
  17: rustc_codegen_llvm::back::write::start_async_codegen
  18: rustc_codegen_llvm::base::codegen_crate
  19: <rustc_codegen_llvm::LlvmCodegenBackend as rustc_codegen_utils::codegen_backend::CodegenBackend>::codegen_crate
  20: rustc::util::common::time
  21: rustc_driver::driver::phase_4_codegen
  22: rustc_driver::driver::compile_input::{{closure}}
  23: rustc::ty::context::tls::enter_context
  24: <std::thread::local::LocalKey<T>>::with
  25: rustc::ty::context::TyCtxt::create_and_enter
  26: rustc_driver::driver::compile_input
  27: rustc_driver::run_compiler_with_pool
  28: <scoped_tls::ScopedKey<T>>::set
  29: rustc_driver::run_compiler
  30: syntax::with_globals
  31: __rust_maybe_catch_panic
  32: rustc_driver::run
  33: rustc_driver::main
  34: std::rt::lang_start::{{closure}}
  35: std::panicking::try::do_call
  36: __rust_maybe_catch_panic
  37: std::rt::lang_start_internal
  38: main
query stack during panic:
#0 [native_libraries] looking up the native libraries of a linked crate
end of query stack

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

note: rustc 1.30.0-nightly (fc81e3624 2018-09-07) running on x86_64-apple-darwin

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

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

error: Could not compile `objrs`.

To learn more, run the command again with --verbose.
@kennytm kennytm added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. A-macros-1.2 Area: Declarative macros 1.2 labels Sep 8, 2018
@kennytm
Copy link
Member

kennytm commented Sep 8, 2018

I suspect this is a duplicate of #52370, but I only count 2 #[proc_macro] and 1 #[proc_macro_attribute] in the repository.

@mjbshaw
Copy link
Contributor Author

mjbshaw commented Sep 9, 2018

The objrs_macros crate only has 1 #[proc_macro] and 1 #[proc_macro_attribute]. If I comment both of them out, the compiler still ICEs. The objrs crate used to compile a couple weeks ago, so this was a fairly recent change.

@mjbshaw
Copy link
Contributor Author

mjbshaw commented Sep 9, 2018

To give some bounds on this: the objrs crate compiles with rustc 1.30.0-nightly (b2028828d 2018-08-16). It fails with rustc 1.30.0-nightly (fc81e3624 2018-09-07). I would bisect this with rustup to determine which nightly the issue started with, but rls's failing build means that rustup can't install nightlies for many of the days in that range.

@mjbshaw
Copy link
Contributor Author

mjbshaw commented Sep 9, 2018

Derp, I just remembered I could remove the rls-preview component. Okay, doing that, I was able to bisect the nightlies:

Last working version: rustc 1.30.0-nightly (7061b2775 2018-08-28)
First failing version: rustc 1.30.0-nightly (02cb8f2a4 2018-08-29)

@kennytm kennytm added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc labels Sep 9, 2018
@mjbshaw
Copy link
Contributor Author

mjbshaw commented Sep 9, 2018

I just did a git bisect, and it looks like commit 5415bb6 (from PR #53711) is the source of the breakage.

cc @arielb1

@pnkfelix pnkfelix self-assigned this Sep 13, 2018
@pnkfelix
Copy link
Member

visited for triage. Going to try to followup with @arielb1 . Assigning to self to keep myself honest about that claim.

@pnkfelix
Copy link
Member

also, P-high

@pnkfelix pnkfelix added the P-high High priority label Sep 13, 2018
@pnkfelix pnkfelix removed the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Sep 13, 2018
@pnkfelix pnkfelix removed their assignment Sep 13, 2018
@arielb1
Copy link
Contributor

arielb1 commented Sep 15, 2018

Am looking at this.

@arielb1
Copy link
Contributor

arielb1 commented Sep 15, 2018

Minified:

mac.rs:

extern crate proc_macro;

macro_rules! proc_macro_tokenstream {
    () => {
        ::proc_macro::TokenStream
    };
}

macro_rules! proc_macro_expr_impl {
    ($(
        $( #[$attr:meta] )*
        pub fn $func:ident($input:ident: &str) -> String $body:block
    )+) => {
        $(
            // Parses an input that looks like:
            //
            // ```
            // #[allow(unused)]
            // enum ProcMacroHack {
            //     Input = (stringify!(ARGS), 0).1,
            // }
            // ```
            $( #[$attr] )*
            #[proc_macro_derive($func)]
            pub fn $func(input: proc_macro_tokenstream!()) -> proc_macro_tokenstream!() {
                unsafe { puts("hello!\n\0" as *const str as *const u8); }
                panic!()
            }
        )+
    };
}

proc_macro_expr_impl! {
    pub fn base2_impl(input: &str) -> String {
        panic!()
    }
}

#[link(name="c")]
extern "C" {
    fn puts(inp: *const u8);
}

use.rs:

#[macro_use]
extern crate mac;

fn main() {}

The problem is caused by native_libraries containing "host" defids and being used in trans. I don't think that this field has any business containing host libraries when compiling a proc macro, but I'll ask @alexcrichton for confirmation.

arielb1 added a commit to arielb1/rust that referenced this issue Sep 15, 2018
proc macro crates are essentially implemented as dynamic libraries using
a dlopen-based ABI. They are also Rust crates, so they have 2 worlds -
the "host" world in which they are defined, and the "target" world in
which they are used.

For all the "target" world knows, the proc macro crate might not even
be implemented in Rust, so leaks of details from the host to the target
must be avoided for correctness.

Because the "host" DefId space is different from the "target" DefId
space, any leak involving a DefId will have a nonsensical or
out-of-bounds DefKey, and will cause all sorts of crashes.

This PR fixes all leaks I have found in `decoder`. In particular, rust-lang#54059
was caused by host native libraries leaking into the target, which feels
like it might even be a correctness issue if it doesn't cause an ICE.

Fixes rust-lang#54059
@pietroalbini pietroalbini added regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Sep 20, 2018
@pietroalbini pietroalbini added this to Triaged in 1.30 regressions via automation Sep 20, 2018
@pietroalbini pietroalbini moved this from Triaged to Fix in progress in 1.30 regressions Sep 20, 2018
@pnkfelix
Copy link
Member

visited for triage. Seems like PR #54265 handles this. Leaving as P-high

bors added a commit that referenced this issue Sep 21, 2018
avoid leaking host details in proc macro metadata decoding

proc macro crates are essentially implemented as dynamic libraries using
a dlopen-based ABI. They are also Rust crates, so they have 2 worlds -
the "host" world in which they are defined, and the "target" world in
which they are used.

For all the "target" world knows, the proc macro crate might not even
be implemented in Rust, so leaks of details from the host to the target
must be avoided for correctness.

Because the "host" DefId space is different from the "target" DefId
space, any leak involving a DefId will have a nonsensical or
out-of-bounds DefKey, and will cause all sorts of crashes.

This PR fixes all leaks I have found in `decoder`. In particular, #54059
was caused by host native libraries leaking into the target, which feels
like it might even be a correctness issue if it doesn't cause an ICE.

Fixes #54059
bors added a commit that referenced this issue Sep 22, 2018
avoid leaking host details in proc macro metadata decoding

proc macro crates are essentially implemented as dynamic libraries using
a dlopen-based ABI. They are also Rust crates, so they have 2 worlds -
the "host" world in which they are defined, and the "target" world in
which they are used.

For all the "target" world knows, the proc macro crate might not even
be implemented in Rust, so leaks of details from the host to the target
must be avoided for correctness.

Because the "host" DefId space is different from the "target" DefId
space, any leak involving a DefId will have a nonsensical or
out-of-bounds DefKey, and will cause all sorts of crashes.

This PR fixes all leaks I have found in `decoder`. In particular, #54059
was caused by host native libraries leaking into the target, which feels
like it might even be a correctness issue if it doesn't cause an ICE.

Fixes #54059
@pietroalbini pietroalbini moved this from Fix in progress to Backport in progress in 1.30 regressions Sep 22, 2018
bors pushed a commit that referenced this issue Sep 22, 2018
proc macro crates are essentially implemented as dynamic libraries using
a dlopen-based ABI. They are also Rust crates, so they have 2 worlds -
the "host" world in which they are defined, and the "target" world in
which they are used.

For all the "target" world knows, the proc macro crate might not even
be implemented in Rust, so leaks of details from the host to the target
must be avoided for correctness.

Because the "host" DefId space is different from the "target" DefId
space, any leak involving a DefId will have a nonsensical or
out-of-bounds DefKey, and will cause all sorts of crashes.

This PR fixes all leaks I have found in `decoder`. In particular, #54059
was caused by host native libraries leaking into the target, which feels
like it might even be a correctness issue if it doesn't cause an ICE.

Fixes #54059
@pietroalbini pietroalbini moved this from Backport in progress to Fixed in 1.30 regressions Sep 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros-1.2 Area: Declarative macros 1.2 C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

6 participants