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

Use clang for the UEFI targets #104622

Merged
merged 4 commits into from
Nov 22, 2022
Merged

Conversation

nicholasbishop
Copy link
Contributor

This fixes an issue where the C and asm sources built by compiler_builtins were being compiled as ELF objects instead of PE objects. This wasn't noticed before because it doesn't cause compiler_builtins or rustc to fail to build. You only see a failure when a program is built that references one of the symbols in an ELF object.

Compiling with clang fixes this because the cc crate converts the UEFI targets into Windows targets that clang understands, causing it to produce PE objects.

Also update compiler_builtins to 0.1.84 to pull in some necessary fixes for compiling the UEFI targets with clang.

Fixes #104326

This fixes an issue where the C and asm sources built by
compiler_builtins were being compiled as ELF objects instead of PE
objects. This wasn't noticed before because it doesn't cause
compiler_builtins or rustc to fail to build. You only see a failure when
a program is built that references one of the symbols in an ELF object.

Compiling with clang fixes this because the `cc` crate converts the UEFI
targets into Windows targets that clang understands, causing it to
produce PE objects.

Note that this requires compiler_builtins >= 0.1.84.

Fixes rust-lang#104326
@rustbot
Copy link
Collaborator

rustbot commented Nov 20, 2022

r? @Mark-Simulacrum

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

@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Nov 20, 2022
@Mark-Simulacrum
Copy link
Member

Hm, is there a chance we can add a test verifying we produce PE objects somewhere? It might be hard given tier 2 status, but I think we have some existing infrastructure for testing e.g. asm for arbitrary targets that we might be able to reuse.

@nicholasbishop
Copy link
Contributor Author

Are you thinking of a test that does something like objdump -a obj/build/x86_64-unknown-linux-gnu/stage2-std/x86_64-unknown-uefi/release/deps/*.rlib and checks that all the objects are PE?

@Mark-Simulacrum
Copy link
Member

Sure, if that works that seems quite workable. I think putting that in rustbuild itself as part of the dist step for example is obviously a hack but would be fine in practice.

This syncs it with how the UEFI targets are built in dist-various-2.
@nicholasbishop nicholasbishop changed the title dist-various-2: Use clang for the UEFI targets Use clang for the UEFI targets Nov 20, 2022
If clang isn't the C compiler used for the UEFI targets, or if the wrong
`--target` is passed to clang, we will get ELF objects in some
rlibs. This will cause problems at link time when trying to compile a
UEFI program that uses any of those objects. Add a check to the dist
step for UEFI targets that reads each rlib with the `object` crate and
fails with an error if any non-COFF objects are found.
@nicholasbishop
Copy link
Contributor Author

Sure, if that works that seems quite workable. I think putting that in rustbuild itself as part of the dist step for example is obviously a hack but would be fine in practice.

OK, I've now added a check to dist.rs that ensures all the objects are COFF.

@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=iffy

@bors
Copy link
Contributor

bors commented Nov 20, 2022

📌 Commit 6054608 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 20, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 22, 2022
…earth

Rollup of 7 pull requests

Successful merges:

 - rust-lang#83608 (Add slice methods for indexing via an array of indices.)
 - rust-lang#95583 (Deprecate the unstable `ptr_to_from_bits` feature)
 - rust-lang#101655 (Make the Box one-liner more descriptive)
 - rust-lang#102207 (Constify remaining `Layout` methods)
 - rust-lang#103193 (mark sys_common::once::generic::Once::new const-stable)
 - rust-lang#104622 (Use clang for the UEFI targets)
 - rust-lang#104638 (Move macro_rules diagnostics to diagnostics module)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 952d385 into rust-lang:master Nov 22, 2022
@rustbot rustbot added this to the 1.67.0 milestone Nov 22, 2022
@nicholasbishop nicholasbishop deleted the bishop-uefi-clang branch November 22, 2022 17:49
@workingjubilee workingjubilee added the O-UEFI UEFI label Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc O-UEFI UEFI S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Undefined symbols when linking i686-unknown-uefi without using -Zbuild-std
5 participants