Skip to content

cfi: Remove #[no_sanitize(cfi)] for extern weak functions#139667

Merged
bors merged 2 commits intorust-lang:masterfrom
1c3t3a:remove-no-sanitize
Apr 17, 2025
Merged

cfi: Remove #[no_sanitize(cfi)] for extern weak functions#139667
bors merged 2 commits intorust-lang:masterfrom
1c3t3a:remove-no-sanitize

Conversation

@1c3t3a
Copy link
Member

@1c3t3a 1c3t3a commented Apr 11, 2025

Previously (#115200, #138002), we added #[no_sanitize(cfi)] to all code paths that call to a weakly linked function.

In #138349 we fixed the root cause for this issue, which means we can now remove the corresponding attributes.

r? @rcvalle

@1c3t3a
cfi: Remove #[no_sanitize(cfi)] for extern weak functions
068a332 
Previously (rust-lang#115200,
rust-lang#138002), we
added `#[no_sanitize(cfi)]` to all code paths that call to a weakly
linked function.

In rust-lang#138349 we fixed the root cause
for this issue, which means we can now remove the corresponding
attributes.
@rustbot rustbot added O-unix Operating system: Unix-like S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 11, 2025
@tamird
Copy link
Contributor

tamird commented Apr 12, 2025

I think

rust/library/std/src/lib.rs

Line 315 in 7cd6e2f

#![feature(no_sanitize)]
can also be removed.

@1c3t3a
Copy link
Member Author

1c3t3a commented Apr 13, 2025

I think

rust/library/std/src/lib.rs

Line 315 in 7cd6e2f

#![feature(no_sanitize)]
can also be removed.

True, but for that #139632 has to land first. I can wait for it to be through and then also remove this line!

@tamird
Copy link
Contributor

tamird commented Apr 13, 2025

I think

rust/library/std/src/lib.rs

Line 315 in 7cd6e2f

#![feature(no_sanitize)]

can also be removed.

True, but for that #139632 has to land first. I can wait for it to be through and then also remove this line!

Are you sure? libcore has its own:

rust/library/core/src/lib.rs

Line 172 in 53d4476

#![feature(no_sanitize)]

@rcvalle
Copy link
Member

rcvalle commented Apr 14, 2025

r? @m-ou-se

@rustbot rustbot assigned m-ou-se and unassigned rcvalle Apr 14, 2025
@rcvalle
Copy link
Member

rcvalle commented Apr 14, 2025

@m-ou-se It LGTM. Mind taking a look and r+ whenever you have time? I don't want to r+ core and std changes directly.

@m-ou-se
Copy link
Member

m-ou-se commented Apr 15, 2025

Can you also remove #![feature(no_sanitize)] from library/std/src/lib.rs?

As @tamird mentions, that doesn't have to wait on #139632. That PR is all in library/core, while this PR is all in library/std.

@m-ou-se m-ou-se 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 15, 2025
@1c3t3a
Copy link
Member Author

1c3t3a commented Apr 15, 2025

I think

rust/library/std/src/lib.rs

Line 315 in 7cd6e2f

#![feature(no_sanitize)]

can also be removed.

True, but for that #139632 has to land first. I can wait for it to be through and then also remove this line!

Are you sure? libcore has its own:

rust/library/core/src/lib.rs

Line 172 in 53d4476

#![feature(no_sanitize)]

Yes you are right! I wasn't aware of that, it is now fixed :)

@rcvalle
Copy link
Member

rcvalle commented Apr 16, 2025

@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 Apr 16, 2025
@m-ou-se
Copy link
Member

m-ou-se commented Apr 16, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 16, 2025

📌 Commit c4d3563 has been approved by m-ou-se

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 Apr 16, 2025
@matthiaskrgr matthiaskrgr mentioned this pull request Apr 16, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 17, 2025
@bors
Auto merge of rust-lang#139940 - matthiaskrgr:rollup-rd4d3fn, r=matth…
15c4cce 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#135340 (Add `explicit_extern_abis` Feature and Enforce Explicit ABIs)
 - rust-lang#139440 (rustc_target: RISC-V: feature addition batch 2)
 - rust-lang#139667 (cfi: Remove #[no_sanitize(cfi)] for extern weak functions)
 - rust-lang#139828 (Don't require rigid alias's trait to hold)
 - rust-lang#139854 (Improve parse errors for stray lifetimes in type position)
 - rust-lang#139889 (Clean UI tests 3 of n)
 - rust-lang#139894 (Fix `opt-dist` CLI flag and make it work without LLD)
 - rust-lang#139900 (stepping into impls for normalization is unproductive)
 - rust-lang#139915 (replace some #[rustc_intrinsic] usage with use of the libcore declarations)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f14e632 into rust-lang:master Apr 17, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 17, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 17, 2025
@rust-timer
Unrolled build for rust-lang#139667
365e919 
Rollup merge of rust-lang#139667 - 1c3t3a:remove-no-sanitize, r=m-ou-se

cfi: Remove #[no_sanitize(cfi)] for extern weak functions

Previously (rust-lang#115200, rust-lang#138002), we added `#[no_sanitize(cfi)]` to all code paths that call to a weakly linked function.

In rust-lang#138349 we fixed the root cause for this issue, which means we can now remove the corresponding attributes.

r? `@rcvalle`
@rcvalle rcvalle mentioned this pull request Apr 18, 2025
Closed
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Apr 19, 2025
@matthiaskrgr
Rollup merge of rust-lang#139667 - 1c3t3a:remove-no-sanitize, r=m-ou-se
9a520f8 
cfi: Remove #[no_sanitize(cfi)] for extern weak functions

Previously (rust-lang#115200, rust-lang#138002), we added `#[no_sanitize(cfi)]` to all code paths that call to a weakly linked function.

In rust-lang#138349 we fixed the root cause for this issue, which means we can now remove the corresponding attributes.

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

Reviewers

@rcvalle rcvalle rcvalle left review comments

Assignees

@m-ou-se m-ou-se

Labels

O-unix Operating system: Unix-like S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.88.0

Development

Successfully merging this pull request may close these issues.

6 participants

@1c3t3a @tamird @rcvalle @m-ou-se @bors @rustbot