Skip to content

validate #[link_name = "..."] & #[link(name = "...")] parameters#155817

Open
folkertdev wants to merge 1 commit intorust-lang:mainfrom
folkertdev:link-name-null-empty
Open

validate #[link_name = "..."] & #[link(name = "...")] parameters#155817
folkertdev wants to merge 1 commit intorust-lang:mainfrom
folkertdev:link-name-null-empty

Conversation

@folkertdev
Copy link
Copy Markdown
Contributor

fixes #155776
fixes #155484

specifically, do not allow NULL bytes and the empty string in #[link_name = "..."] and #[link(name = "...")].

Like some of the others I think this formally needs to be looked at by T-lang because these errors would not show up if not linking.

The LLVM erorr on #[linke_name = "\0"] is emitted here, it is not e.g. target-specific.

https://github.com/llvm/llvm-project/blob/d593279c0b2891f0b0c8af3f70a1a0383b4ad1b5/llvm/lib/IR/Value.cpp#L336-L342

On #[link(name = "")] we already error today. A NULL byte in #[link(name = "\0")] is caught by the linker (https://godbolt.org/z/vnz9sYbPs), using #[link_name = ""] makes LLVM generate a name (https://godbolt.org/z/1hWEo4cxf) which is not useful and likely to cause linker errors.

r? jdonszelmann

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 26, 2026

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) 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, 2026
@folkertdev folkertdev changed the title validate #[link_name = "..."] parameter validate #[link_name = "..."] & #[link(name = "...")] parameters Apr 26, 2026
Copy link
Copy Markdown
Contributor

@mejrs mejrs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r? me

r=me on the implementation, pending lang approval

View changes since this review

Comment thread compiler/rustc_attr_parsing/src/attributes/link_attrs.rs Outdated
@rustbot rustbot assigned mejrs and unassigned jdonszelmann Apr 26, 2026
@rust-log-analyzer

This comment has been minimized.

@folkertdev folkertdev added I-lang-nominated Nominated for discussion during a lang team meeting. S-waiting-on-t-lang Status: Awaiting decision from T-lang labels Apr 26, 2026
@folkertdev folkertdev force-pushed the link-name-null-empty branch from b94ed3f to 2ffa233 Compare April 26, 2026 11:45
Comment on lines 12 to 13
#[link_name = "@GLIBC_2.2.5"]
pub safe fn exit_1(status: i32) -> !; //~ ERROR link name must be well-formed if link kind is `raw-dylib`
Copy link
Copy Markdown
Contributor Author

@folkertdev folkertdev Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should move these errors to rustc_attr_parsing too, for consistency and so that the ranges can actually point to the attribute instead of the item.

That's not something T-lang needs to worry about though, so probably best done separately.

cc @usamoi (this test was added by #144221)

View changes since the review

@folkertdev folkertdev force-pushed the link-name-null-empty branch from 2ffa233 to 4338dfc Compare April 26, 2026 17:17
@rust-log-analyzer

This comment has been minimized.

@folkertdev folkertdev force-pushed the link-name-null-empty branch from 4338dfc to 5a5ae4c Compare April 26, 2026 17:26
@rust-log-analyzer

This comment has been minimized.

specifically, do not allow NULL bytes and the empty string
@folkertdev folkertdev force-pushed the link-name-null-empty branch from 5a5ae4c to c08b9ab Compare April 26, 2026 17:36
@traviscross traviscross added P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang I-lang-radar Items that are on lang's radar and will need eventual work or consideration. T-lang Relevant to the language team labels Apr 29, 2026
@traviscross
Copy link
Copy Markdown
Contributor

Thanks @folkertdev for cleaning these up.

@rfcbot fcp merge lang

@rust-rfcbot
Copy link
Copy Markdown
Collaborator

rust-rfcbot commented Apr 29, 2026

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@rust-rfcbot rust-rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Apr 29, 2026
@rust-rfcbot
Copy link
Copy Markdown
Collaborator

🔔 This is now entering its final comment period, as per the review above. 🔔

@traviscross traviscross removed I-lang-nominated Nominated for discussion during a lang team meeting. P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang labels Apr 29, 2026
@nikomatsakis
Copy link
Copy Markdown
Contributor

@rfcbot reviewed

@rust-rfcbot rust-rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels May 9, 2026
@rust-rfcbot
Copy link
Copy Markdown
Collaborator

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

@folkertdev
Copy link
Copy Markdown
Contributor Author

@bors r=mejrs rollup

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 9, 2026

📋 This PR cannot be approved because it currently has the following label: S-waiting-on-t-lang.

@folkertdev folkertdev removed the S-waiting-on-t-lang Status: Awaiting decision from T-lang label May 9, 2026
@folkertdev
Copy link
Copy Markdown
Contributor Author

@bors r=mejrs rollup

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 9, 2026

📌 Commit c08b9ab has been approved by mejrs

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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 May 9, 2026
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 9, 2026
…=mejrs

validate `#[link_name = "..."]` & `#[link(name = "...")]` parameters

fixes rust-lang#155776
fixes rust-lang#155484

specifically, do not allow NULL bytes and the empty string in `#[link_name = "..."]` and `#[link(name = "...")]`.

Like some of the others I think this formally needs to be looked at by T-lang because these errors would not show up if not linking.

The LLVM erorr on `#[linke_name = "\0"]` is emitted here, it is not e.g. target-specific.

https://github.com/llvm/llvm-project/blob/d593279c0b2891f0b0c8af3f70a1a0383b4ad1b5/llvm/lib/IR/Value.cpp#L336-L342

On `#[link(name = "")]` we already error today. A NULL byte in `#[link(name = "\0")]` is caught by the linker (https://godbolt.org/z/vnz9sYbPs), using `#[link_name = ""]` makes LLVM generate a name (https://godbolt.org/z/1hWEo4cxf) which is not useful and likely to cause linker errors.

r? jdonszelmann
rust-bors Bot pushed a commit that referenced this pull request May 9, 2026
Rollup of 5 pull requests

Successful merges:

 - #148214 (Consider `Result<T, Uninhabited>` and `ControlFlow<Uninhabited, T>` to be equivalent to `T` for must use lint)
 - #149362 (Add Command::get_resolved_envs)
 - #155188 (Add regression test for issue 144329)
 - #155515 (error on empty `export_name`)
 - #155817 (validate `#[link_name = "..."]` & `#[link(name = "...")]` parameters)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request May 9, 2026
…=mejrs

validate `#[link_name = "..."]` & `#[link(name = "...")]` parameters

fixes rust-lang#155776
fixes rust-lang#155484

specifically, do not allow NULL bytes and the empty string in `#[link_name = "..."]` and `#[link(name = "...")]`.

Like some of the others I think this formally needs to be looked at by T-lang because these errors would not show up if not linking.

The LLVM erorr on `#[linke_name = "\0"]` is emitted here, it is not e.g. target-specific.

https://github.com/llvm/llvm-project/blob/d593279c0b2891f0b0c8af3f70a1a0383b4ad1b5/llvm/lib/IR/Value.cpp#L336-L342

On `#[link(name = "")]` we already error today. A NULL byte in `#[link(name = "\0")]` is caught by the linker (https://godbolt.org/z/vnz9sYbPs), using `#[link_name = ""]` makes LLVM generate a name (https://godbolt.org/z/1hWEo4cxf) which is not useful and likely to cause linker errors.

r? jdonszelmann
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. 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. T-lang Relevant to the language team to-announce Announce this issue on triage meeting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

special link_name confuses LLVM [ICE]: Result::unwrap() on an Err value: NulError(0, [0])

8 participants