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

rustc_target: support Unix-flavor linkers for UEFI #110869

Closed
wants to merge 1 commit into from

Conversation

alyssais
Copy link
Contributor

No description provided.

@rustbot
Copy link
Collaborator

rustbot commented Apr 26, 2023

r? @TaKO8Ki

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 26, 2023
@rustbot
Copy link
Collaborator

rustbot commented Apr 26, 2023

These commits modify compiler targets.
(See the Target Tier Policy.)

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-14 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
..............................F.......

failures:

---- spec::tests::aarch64_unknown_uefi stdout ----
thread 'spec::tests::aarch64_unknown_uefi' panicked at 'assertion failed: `(left matches right)`
  left: `Unix(Yes)`,
 right: `LinkerFlavor::Msvc(..)`', compiler/rustc_target/src/spec/tests/tests_impl.rs:57:25

---- spec::tests::i686_unknown_uefi stdout ----
---- spec::tests::i686_unknown_uefi stdout ----
thread 'spec::tests::i686_unknown_uefi' panicked at 'assertion failed: `(left matches right)`
error: test failed, to rerun pass `-p rustc_target --lib`
  left: `Unix(Yes)`,
 right: `LinkerFlavor::Msvc(..)`', compiler/rustc_target/src/spec/tests/tests_impl.rs:57:25
---- spec::tests::x86_64_unknown_uefi stdout ----
---- spec::tests::x86_64_unknown_uefi stdout ----
thread 'spec::tests::x86_64_unknown_uefi' panicked at 'assertion failed: `(left matches right)`
  left: `Unix(Yes)`,
 right: `LinkerFlavor::Msvc(..)`', compiler/rustc_target/src/spec/tests/tests_impl.rs:57:25

failures:
    spec::tests::aarch64_unknown_uefi
    spec::tests::i686_unknown_uefi

@petrochenkov petrochenkov self-assigned this Apr 27, 2023
@alyssais
Copy link
Contributor Author

In the test that's failing:

// Check that flavors mentioned in link args are compatible with the default flavor.

Can we not add linker arguments for non-default linkers?

@petrochenkov
Copy link
Contributor

This needs some context.

UEFI uses PE/COFF binary format, right?
I'm only aware of one Unix-style linker that can produce it, it's the linker from mingw toolchain - are you trying to cover this case or something else?

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 27, 2023
@alyssais
Copy link
Contributor Author

I'm only aware of one Unix-style linker that can produce it, it's the linker from mingw toolchain - are you trying to cover this case or something else?

No, I'd like to use ld.lld, which can also produce COFF.

The reason for specifically using the ld interface of lld is as follows: in Nixpkgs, cc is a wrapper script around the actual C compiler, which in this case is clang. The wrapper script is there so that we can set up the compiler with our library search paths and so on. Compiler wrappers like this are very common among package collections — I'm somewhat familiar with pkgsrc and Homebrew, both of which also use compiler wrappers. Our compiler wrapper was written for Unix compilers and linkers, so it understands Unix-flavor flags, but not the Windows-style ones rustc uses by default for UEFI targets. But since rustc supports choosing the linker flavor, the best way for us to add support to our toolchain for building UEFI binaries with Rust is to tell it to use the Unix-flavor arguments. This works fine with these small changes to teach rustc how to handle the UEFI specifics for Unix-flavor linkers.

@petrochenkov
Copy link
Contributor

Ok, I see.
It's hard to accommodate for differences between toolchains like msvc and clang+lld in one target.

I suggest adding new UEFI targets *-unknown-uefi-gnu for this.
I.e. a uefi_msvc_base/uefi_gnu_base pair and specific targets inheriting from it, in the same way like we currently have windows_msvc_base and windows_gnu_base.

As a bonus they will be able to use mingw toolchain for linking for UEFI too (if it works at all).

@dvdhrm
Copy link
Contributor

dvdhrm commented Jul 26, 2023

Using *-gnu as suffix might confuse people into thinking it follows the gnu-efi model.

Given that RISCV-on-PE is still not supported by LLVM, there might be people working to resurrect the gnu-efi model and compile ELF binaries which are then loaded by a PE-stub on UEFI. These would then likely use some *-gnu suffix. Hence, I would avoid the *-gnu suffix here.

I don't have a better idea, though. Sorry. Preferably, we would be able to detect the argument-style of lld at runtime and keep a single target.

@workingjubilee workingjubilee added the O-UEFI UEFI label Jul 30, 2023
@Dylan-DPC
Copy link
Member

@alyssais any updates on this? thanks

@Dylan-DPC
Copy link
Member

Closing this as inactive. Feel free to reöpen this pr or create a new pr if you get the time to work on this. Thanks

@Dylan-DPC Dylan-DPC closed this Oct 24, 2023
@Dylan-DPC Dylan-DPC added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-UEFI UEFI S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants