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

-Zno-link is broken #77857

Closed
bjorn3 opened this issue Oct 12, 2020 · 6 comments · Fixed by #80187
Closed

-Zno-link is broken #77857

bjorn3 opened this issue Oct 12, 2020 · 6 comments · Fixed by #80187
Labels
A-linkage Area: linking into static, shared libraries and binaries A-metadata Area: Crate metadata C-bug Category: This is a bug. requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@bjorn3
Copy link
Member

bjorn3 commented Oct 12, 2020

I tried this command:

$ echo "fn main() {}" | rustc -Zno-link -

I expected to see this happen: An .rlink file is generated.

Instead, this happened:

thread 'rustc' panicked at 'cannot encode `DefIndex` with `rustc_serialize::json::Encoder`', /rustc/381b445ff5751f9f39ec672b623372dff09c276e/compiler/rustc_span/src/def_id.rs:135:9
stack backtrace:
   0: rust_begin_unwind
             at /rustc/381b445ff5751f9f39ec672b623372dff09c276e/library/std/src/panicking.rs:483
   1: std::panicking::begin_panic_fmt
             at /rustc/381b445ff5751f9f39ec672b623372dff09c276e/library/std/src/panicking.rs:437
   2: <rustc_span::def_id::DefIndex as rustc_serialize::serialize::Encodable<E>>::encode
   3: <rustc_serialize::json::Encoder as rustc_serialize::serialize::Encoder>::emit_struct
   4: <rustc_serialize::json::Encoder as rustc_serialize::serialize::Encoder>::emit_struct
   5: <rustc_serialize::json::Encoder as rustc_serialize::serialize::Encoder>::emit_seq
   6: <rustc_serialize::json::Encoder as rustc_serialize::serialize::Encoder>::emit_map
   7: <rustc_serialize::json::Encoder as rustc_serialize::serialize::Encoder>::emit_struct
   8: <rustc_serialize::json::Encoder as rustc_serialize::serialize::Encoder>::emit_struct
   9: <rustc_codegen_llvm::LlvmCodegenBackend as rustc_codegen_ssa::traits::backend::CodegenBackend>::link
  10: rustc_interface::queries::Linker::link
  11: rustc_span::with_source_map
  12: rustc_interface::interface::create_compiler_and_run
  13: scoped_tls::ScopedKey<T>::set

Meta

rustc --version --verbose:

rustc 1.48.0-nightly (381b445ff 2020-09-29)
binary: rustc
commit-hash: 381b445ff5751f9f39ec672b623372dff09c276e
commit-date: 2020-09-29
host: x86_64-unknown-linux-gnu
release: 1.48.0-nightly
LLVM version: 11.0

Originally posted by @bjorn3 in #77795 (comment)

cc @0dvictor (original implementer of -Zno-link

@bjorn3 bjorn3 added the C-bug Category: This is a bug. label Oct 12, 2020
@jonas-schievink jonas-schievink added requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 12, 2020
@camelid camelid added A-linkage Area: linking into static, shared libraries and binaries A-metadata Area: Crate metadata labels Oct 12, 2020
@0dvictor
Copy link
Contributor

PR #73851 changed the encode/decode method of DefId to call the encode/decode method of its field pub index: DefIndex; however, DefIndex is only encodable in metadata. It panics if encoded anywhere else.

When no-link was implemented, the encode/decode of DefId was purposely crafted to avoid calling the encode/decode method of DefIndex. The most straightforward fix is to implement Encodable/Decodable for DefIndex to allow it be serialized outside of metadata.

After that, may be now it is the time to re-think the way we store data in the rlink file.

What do you think @bjorn3 ?

@bjorn3
Copy link
Member Author

bjorn3 commented Oct 26, 2020

I think DefId should completely be avoided in the input of the linker function. One of the uses if NativeLib in it's foreign_module field. This field doesn't seem to be used by the linking code.

@0dvictor
Copy link
Contributor

Good suggestion, let me look into it.

@bjorn3
Copy link
Member Author

bjorn3 commented Dec 12, 2020

@0dvictor are you still working on this?

@0dvictor
Copy link
Contributor

Thank you for reminding and apologize for the delay. I did create a prototype but haven't got a chance polish it yet. I'll create the PR this week.

@0dvictor
Copy link
Contributor

Sorry again about the delay. I've created PR #80187, which fixes this issue.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 23, 2020
Exclude unnecessary info from CodegenResults

`foreign_module` and `wasm_import_module` are not needed for linking, and hence can be removed from CodegenResults.

Fixes rust-lang#77857
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 24, 2020
Exclude unnecessary info from CodegenResults

`foreign_module` and `wasm_import_module` are not needed for linking, and hence can be removed from CodegenResults.

Fixes rust-lang#77857
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 24, 2020
Exclude unnecessary info from CodegenResults

`foreign_module` and `wasm_import_module` are not needed for linking, and hence can be removed from CodegenResults.

Fixes rust-lang#77857
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 24, 2020
Exclude unnecessary info from CodegenResults

`foreign_module` and `wasm_import_module` are not needed for linking, and hence can be removed from CodegenResults.

Fixes rust-lang#77857
@bors bors closed this as completed in 198ec34 Dec 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linkage Area: linking into static, shared libraries and binaries A-metadata Area: Crate metadata C-bug Category: This is a bug. requires-nightly This issue requires a nightly compiler in some way. 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.

4 participants