add support for the l4-bender linker on the x86_64-unknown-l4re-uclibc tier 3 target #85967

atopia · 2021-06-03T16:31:15Z

This PR contains the work by @humenda to update support for the x86_64-unknown-l4re-uclibc tier 3 target (published at humenda/rust), rebased and adapted to current rust in follow up commits by myself. The publishing of the rebased changes is authorized and preferred by the original author. As the goal was to distort the original work as little as possible, individual commits introduce changes that are incompatible to the newer code base that the changes were rebased on. These incompatibilities have been remedied in follow up commits, so that the PR as a whole should result in a clean update of the target.
If you prefer another strategy to mainline these changes while preserving attribution, please let me know.

rust-highfive · 2021-06-03T16:31:18Z

Some changes occured to rustc_codegen_cranelift

cc @bjorn3

rust-highfive · 2021-06-03T16:31:18Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @matthewjasper (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

library/std/src/sys/unix/l4re.rs

petrochenkov · 2021-06-10T15:49:12Z

compiler/rustc_codegen_ssa/src/back/linker.rs

+    fn control_flow_guard(&mut self) {}
+
+    fn no_crt_objects(&mut self) {}
+}


The interface looks like more or less like an ld linker, I'd suggest to avoid adding a new linker flavor for a start and reuse GccLinker (especially given that the set of linker flavors accepted by -C linker-flavor is a stable public interface).

humenda · 2021-06-10T16:17:40Z

The interface looks like more or less like an `ld` linker, I'd suggest to avoid adding a new linker flavor for a start and reuse `GccLinker` (especially given that the set of linker flavors accepted by `-C linker-flavor` is a stable public interface).

It is impossible to pass in arguments with spaces to the linker due to the way how this is implemented for linkers in Rustc. I talked to Alex Crichton @alexcrichton (hopefully the correct nick) a long time ago and the suggested way has been to add a new variant. L4-bender is indeed internally using ld but it `bends` the arguments and adds quite some things and custom options. If I recall correctly, even the order of the arguments matters, something that cannot be influenced from outside. One notable one is the `--` that separates l4-bender arguments from ld arguments that are passed through. How should this be done? If a new linker flavour is really not an option, I would ask the compiler team to resume the discussion on how to quote spaces in linker flags.

pnkfelix · 2021-09-09T14:39:02Z

It is impossible to pass in arguments with spaces to the linker due to the way how this is implemented for linkers in Rustc. I talked to Alex Crichton @alexcrichton (hopefully the correct nick) a long time ago and the suggested way has been to add a new variant. L4-bender is indeed internally using ld but it bends the arguments and adds quite some things and custom options. If I recall correctly, even the order of the arguments matters, something that cannot be influenced from outside. One notable one is the -- that separates l4-bender arguments from ld arguments that are passed through.

I think the points made here seem to give reasonable support for adding a new linker flavor for this case.

humenda · 2021-09-09T15:17:48Z

I think the points made here seem to give reasonable support for adding a new linker flavor for this case.

Could we get some more support, e.g. by a rustc CI, to detect breakage earlier? Or is this something that should be done on the L4Re side due to the tier 3 support level?

pnkfelix · 2021-09-09T15:49:56Z

I think the points made here seem to give reasonable support for adding a new linker flavor for this case.

Could we get some more support, e.g. by a rustc CI, to detect breakage earlier? Or is this something that should be done on the L4Re side due to the tier 3 support level?

I think the latter. My understanding of the tier 3 support level is that it is not meant to impose new burdens on the existing infrastructure.

pnkfelix · 2021-09-09T16:00:37Z

We discussed this in today's T-compiler weekly triage meeting.

Some further review of the target tier policy led us to conclude that an MCP is not strictly required here.

I'm going to double-check the points that @petrochenkov made about the linker, but I'm currently inclined to trust @humenda 's judgement here.

Question for @humenda and @atopia : should either or both of you be recorded as maintainers for the x86_64-unknown-l4re-uclibc target? Is there anyone else who should also be on such a list of maintainers for the target?

petrochenkov · 2021-09-10T11:56:08Z

I'll look in more details, this all still look pretty suspicious to me.
I'm not even sure the cmd quoting and env reading stuff belongs to rustc?
There's approximately zero documentation about l4-bender on the net (if you have some info about what it does, please share), so it seems niche enough to generally adapt to rustc and not otherwise.

petrochenkov · 2021-09-11T12:55:57Z

Here's the version of this PR, but without introducing a new linker flavor - petrochenkov@d3b02f3
It highlights the differences from a regular gcc-style linker, I left questions about them in code.
Most of them are related to the comma vs whitespace story which is still not clear to me.
If l4-bender simply passes the arguments after -- to the underlying linker, then why does e.g. -z relro needs to be changed to -z,relro?

Also, if there are some issues with passing arguments with whitespaces, then it's better to make a separate issue (with examples from practice, showing how they behave now, and how they should behave instead) and solve it once and for all, instead of introducing more hacks for specific targets.

r? @petrochenkov

compiler/rustc_target/src/spec/x86_64_unknown_l4re_uclibc.rs

petrochenkov · 2021-09-11T13:00:20Z

(I'd also suggest submitting the library changes separately, in that case they will get a more appropriate reviewer familiar with library conventions and will likely land faster.)

petrochenkov · 2022-01-20T13:50:38Z

r=me after squashing commits.

- Fix style errors. - L4-bender does not yet support dynamic linking. - Stack unwinding is not yet supported for x86_64-unknown-l4re-uclibc. For now, just abort on panics. - Use GNU-style linker options where possible. As suggested by review: - Use standard GNU-style ld syntax for relro flags. - Use standard GNU-style optimization flags and logic. - Use standard GNU-style ld syntax for --subsystem. - Don't read environment variables in L4Bender linker. Thanks to CARGO_ENCODED_RUSTFLAGS introduced in rust-lang#9601, l4-bender's arguments can now be passed from the L4Re build system without resorting to custom parsing of environment variables.

atopia · 2022-01-21T15:52:18Z

@rustbot ready

petrochenkov · 2022-01-22T04:07:28Z

@bors r+

bors · 2022-01-22T04:07:30Z

📌 Commit 29d6235 has been approved by petrochenkov

…henkov add support for the l4-bender linker on the x86_64-unknown-l4re-uclibc tier 3 target This PR contains the work by `@humenda` to update support for the `x86_64-unknown-l4re-uclibc` tier 3 target (published at [humenda/rust](https://github.com/humenda/rust)), rebased and adapted to current rust in follow up commits by myself. The publishing of the rebased changes is authorized and preferred by the original author. As the goal was to distort the original work as little as possible, individual commits introduce changes that are incompatible to the newer code base that the changes were rebased on. These incompatibilities have been remedied in follow up commits, so that the PR as a whole should result in a clean update of the target. If you prefer another strategy to mainline these changes while preserving attribution, please let me know.

…henkov add support for the l4-bender linker on the x86_64-unknown-l4re-uclibc tier 3 target This PR contains the work by ``@humenda`` to update support for the `x86_64-unknown-l4re-uclibc` tier 3 target (published at [humenda/rust](https://github.com/humenda/rust)), rebased and adapted to current rust in follow up commits by myself. The publishing of the rebased changes is authorized and preferred by the original author. As the goal was to distort the original work as little as possible, individual commits introduce changes that are incompatible to the newer code base that the changes were rebased on. These incompatibilities have been remedied in follow up commits, so that the PR as a whole should result in a clean update of the target. If you prefer another strategy to mainline these changes while preserving attribution, please let me know.

…askrgr Rollup of 9 pull requests Successful merges: - rust-lang#85967 (add support for the l4-bender linker on the x86_64-unknown-l4re-uclibc tier 3 target) - rust-lang#92828 (Print a helpful message if unwinding aborts when it reaches a nounwind function) - rust-lang#93012 (Update pulldown-cmark version to fix markdown list issue) - rust-lang#93116 (Simplify use of `map_or`) - rust-lang#93132 (Increase the format version of rustdoc-json-types) - rust-lang#93147 (Interner cleanups) - rust-lang#93153 (Reject unsupported naked functions) - rust-lang#93170 (Add missing GUI test explanations) - rust-lang#93172 (rustdoc: remove dashed underline under main heading) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Update stdlib for the l4re target This PR contains the work by `@humenda` and myself to update standard library support for the x86_64-unknown-l4re-uclibc tier 3 target, split out from humenda/rust as requested in rust-lang#85967. The changes have been rebased on current master and updated in follow up commits by myself. The publishing of the changes is authorized and preferred by the original author. To preserve attribution, when standard library changes were introduced as part of other changes to the compiler, I have kept the changes concerning the standard library and altered the commit messages as indicated. Any incompatibilities have been remedied in follow up commits, so that the PR as a whole should result in a clean update of the target.

Update stdlib for the l4re target This PR contains the work by ``@humenda`` and myself to update standard library support for the x86_64-unknown-l4re-uclibc tier 3 target, split out from humenda/rust as requested in rust-lang#85967. The changes have been rebased on current master and updated in follow up commits by myself. The publishing of the changes is authorized and preferred by the original author. To preserve attribution, when standard library changes were introduced as part of other changes to the compiler, I have kept the changes concerning the standard library and altered the commit messages as indicated. Any incompatibilities have been remedied in follow up commits, so that the PR as a whole should result in a clean update of the target.

rust-highfive assigned matthewjasper Jun 3, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 3, 2021

bjorn3 reviewed Jun 3, 2021

View reviewed changes

library/std/src/sys/unix/l4re.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

atopia force-pushed the update-l4re-target branch from 1bbb9f7 to f52fe68 Compare June 10, 2021 14:38

petrochenkov reviewed Jun 10, 2021

View reviewed changes

crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 26, 2021

This comment has been minimized.

Sign in to view

JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 25, 2021

JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 15, 2021

inquisitivecrystal added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 24, 2021

pnkfelix assigned pnkfelix and unassigned matthewjasper Sep 9, 2021

petrochenkov self-assigned this Sep 10, 2021

petrochenkov reviewed Sep 11, 2021

View reviewed changes

compiler/rustc_target/src/spec/x86_64_unknown_l4re_uclibc.rs Show resolved Hide resolved

petrochenkov 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 Sep 11, 2021

petrochenkov 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 Jan 20, 2022

humenda and others added 3 commits January 21, 2022 16:28

Add L4Bender as linker variant

d984287

Gate l4-bender linker flavor

29d6235

atopia force-pushed the update-l4re-target branch from f74ca9e to 29d6235 Compare January 21, 2022 15:51

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 21, 2022

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 Jan 22, 2022

matthiaskrgr mentioned this pull request Jan 22, 2022

Rollup of 11 pull requests #93189

Closed

matthiaskrgr mentioned this pull request Jan 22, 2022

Rollup of 10 pull requests #93191

Closed

matthiaskrgr mentioned this pull request Jan 22, 2022

Rollup of 9 pull requests #93202

Merged

bors merged commit ffd199d into rust-lang:master Jan 22, 2022

rustbot added this to the 1.60.0 milestone Jan 22, 2022

atopia deleted the update-l4re-target branch January 24, 2022 10:10

petrochenkov mentioned this pull request Aug 30, 2022

rustc_target: Add a compatibility layer to separate internal and user-facing linker flavors #100552

Merged

davidtwco mentioned this pull request Sep 20, 2023

target tier policy compliance #116004

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support for the l4-bender linker on the x86_64-unknown-l4re-uclibc tier 3 target #85967

add support for the l4-bender linker on the x86_64-unknown-l4re-uclibc tier 3 target #85967

atopia commented Jun 3, 2021

rust-highfive commented Jun 3, 2021

rust-highfive commented Jun 3, 2021

This comment has been minimized.

petrochenkov Jun 10, 2021

humenda commented Jun 10, 2021 via email

This comment has been minimized.

pnkfelix commented Sep 9, 2021

humenda commented Sep 9, 2021 via email

pnkfelix commented Sep 9, 2021 •

edited

pnkfelix commented Sep 9, 2021

petrochenkov commented Sep 10, 2021

petrochenkov commented Sep 11, 2021 •

edited

petrochenkov commented Sep 11, 2021

petrochenkov commented Jan 20, 2022

atopia commented Jan 21, 2022

petrochenkov commented Jan 22, 2022

bors commented Jan 22, 2022

add support for the l4-bender linker on the x86_64-unknown-l4re-uclibc tier 3 target #85967

add support for the l4-bender linker on the x86_64-unknown-l4re-uclibc tier 3 target #85967

Conversation

atopia commented Jun 3, 2021

rust-highfive commented Jun 3, 2021

rust-highfive commented Jun 3, 2021

This comment has been minimized.

petrochenkov Jun 10, 2021

Choose a reason for hiding this comment

humenda commented Jun 10, 2021 via email

This comment has been minimized.

pnkfelix commented Sep 9, 2021

humenda commented Sep 9, 2021 via email

pnkfelix commented Sep 9, 2021 • edited

pnkfelix commented Sep 9, 2021

petrochenkov commented Sep 10, 2021

petrochenkov commented Sep 11, 2021 • edited

petrochenkov commented Sep 11, 2021

petrochenkov commented Jan 20, 2022

atopia commented Jan 21, 2022

petrochenkov commented Jan 22, 2022

bors commented Jan 22, 2022

pnkfelix commented Sep 9, 2021 •

edited

petrochenkov commented Sep 11, 2021 •

edited