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

Properly pass linker arguments that contain commas #132974

Merged
merged 3 commits into from
Dec 1, 2024

Conversation

madsmtm
Copy link
Contributor

@madsmtm madsmtm commented Nov 13, 2024

When linking with the system C compiler, we sometimes want to forward certain arguments unchanged to the linker. This can be done with -Wl,arg1,arg2 or -Xlinker arg1 -Xlinker arg2. -Wl is used when possible, since it is more compact, but it does not support commas in the argument itself - in those cases, we need to use -Xlinker, and that is what this PR implements.

This also fixes using sanitizers on macOS with -Clinker-flavor=ld, as those were previously manually using -Wl/-Xlinker (probably since the support wasn't present in the link_args function).

Note that there has been a previous PR for this, but it only implemented this in certain cases when passing -rpath.

r? compiler

@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 Nov 13, 2024
@petrochenkov
Copy link
Contributor

r? @petrochenkov

@rustbot rustbot assigned petrochenkov and unassigned pnkfelix Nov 13, 2024
compiler/rustc_codegen_ssa/src/back/rpath/tests.rs Outdated Show resolved Hide resolved
compiler/rustc_codegen_ssa/src/back/rpath.rs Outdated Show resolved Hide resolved
compiler/rustc_codegen_ssa/src/back/linker.rs Outdated Show resolved Hide resolved
compiler/rustc_codegen_ssa/src/back/linker.rs Outdated Show resolved Hide resolved
compiler/rustc_codegen_ssa/src/back/linker.rs Outdated Show resolved Hide resolved
@petrochenkov 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 Nov 23, 2024
Using `cc_args` panics when using `-Clinker-flavor=ld`, because the
arguments are in a form tailored for `-Clinker-flavor=gcc`.

So instead, we use `link_args` and let that wrap the arguments with the
appropriate `-Wl` or `-Xlinker` when needed.
@madsmtm madsmtm force-pushed the linker-arguments-with-commas branch from bb24a92 to 6bbf832 Compare November 24, 2024 00:23
@madsmtm
Copy link
Contributor Author

madsmtm commented Nov 24, 2024

Thanks for the review, resolved now

@rustbot ready

@rustbot 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 Nov 24, 2024
@petrochenkov
Copy link
Contributor

Thanks!
@bors r+

@bors
Copy link
Contributor

bors commented Nov 30, 2024

📌 Commit 6bbf832 has been approved by petrochenkov

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 30, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 1, 2024
…as, r=petrochenkov

Properly pass linker arguments that contain commas

When linking with the system C compiler, we sometimes want to forward certain arguments unchanged to the linker. This can be done with `-Wl,arg1,arg2` or `-Xlinker arg1 -Xlinker arg2`. `-Wl` is used when possible, since it is more compact, but it does not support commas in the argument itself - in those cases, we need to use `-Xlinker`, and that is what this PR implements.

This also fixes using sanitizers on macOS with `-Clinker-flavor=ld`, as those were previously manually using `-Wl`/`-Xlinker` (probably since the support wasn't present in the `link_args` function).

Note that there has been [a previous PR for this](rust-lang#38798), but it only implemented this in certain cases when passing `-rpath`.

r? compiler
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 1, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#128004 (codegen `#[naked]` functions using global asm)
 - rust-lang#132974 (Properly pass linker arguments that contain commas)
 - rust-lang#133403 (Make `adjust_fulfillment_errors` work with `HostEffectPredicate` and `const_conditions`)
 - rust-lang#133482 (Only error raw lifetime followed by `\'` in edition 2021+)
 - rust-lang#133595 (Do not emit `missing_doc_code_examples` rustdoc lint on module and a few other items)
 - rust-lang#133669 (Move some functions out of const_swap feature gate)
 - rust-lang#133674 (Fix chaining `carrying_add`s)
 - rust-lang#133691 (Check let source before suggesting annotation)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 1, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#132974 (Properly pass linker arguments that contain commas)
 - rust-lang#133403 (Make `adjust_fulfillment_errors` work with `HostEffectPredicate` and `const_conditions`)
 - rust-lang#133482 (Only error raw lifetime followed by `\'` in edition 2021+)
 - rust-lang#133595 (Do not emit `missing_doc_code_examples` rustdoc lint on module and a few other items)
 - rust-lang#133669 (Move some functions out of const_swap feature gate)
 - rust-lang#133674 (Fix chaining `carrying_add`s)
 - rust-lang#133691 (Check let source before suggesting annotation)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 1, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#132974 (Properly pass linker arguments that contain commas)
 - rust-lang#133403 (Make `adjust_fulfillment_errors` work with `HostEffectPredicate` and `const_conditions`)
 - rust-lang#133482 (Only error raw lifetime followed by `\'` in edition 2021+)
 - rust-lang#133595 (Do not emit `missing_doc_code_examples` rustdoc lint on module and a few other items)
 - rust-lang#133669 (Move some functions out of const_swap feature gate)
 - rust-lang#133674 (Fix chaining `carrying_add`s)
 - rust-lang#133691 (Check let source before suggesting annotation)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ae6a7db into rust-lang:master Dec 1, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 1, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 1, 2024
Rollup merge of rust-lang#132974 - madsmtm:linker-arguments-with-commas, r=petrochenkov

Properly pass linker arguments that contain commas

When linking with the system C compiler, we sometimes want to forward certain arguments unchanged to the linker. This can be done with `-Wl,arg1,arg2` or `-Xlinker arg1 -Xlinker arg2`. `-Wl` is used when possible, since it is more compact, but it does not support commas in the argument itself - in those cases, we need to use `-Xlinker`, and that is what this PR implements.

This also fixes using sanitizers on macOS with `-Clinker-flavor=ld`, as those were previously manually using `-Wl`/`-Xlinker` (probably since the support wasn't present in the `link_args` function).

Note that there has been [a previous PR for this](rust-lang#38798), but it only implemented this in certain cases when passing `-rpath`.

r? compiler
@madsmtm madsmtm deleted the linker-arguments-with-commas branch December 1, 2024 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

5 participants