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

WASI: forbid unsafe_op_in_unsafe_fn for std::{os, sys} #128432

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

g0djan
Copy link
Contributor

@g0djan g0djan commented Jul 31, 2024

Part of #127747 for WASI

try-job: test-various

@rustbot
Copy link
Collaborator

rustbot commented Jul 31, 2024

r? @cuviper

rustbot has assigned @cuviper.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added O-wasi Operating system: Wasi, Webassembly System Interface 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 Jul 31, 2024
Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

Thanks, a few suggestions to match the others.

@rustbot rustbot added the O-unix Operating system: Unix-like label Aug 1, 2024
@rust-log-analyzer

This comment has been minimized.

@g0djan
Copy link
Contributor Author

g0djan commented Aug 1, 2024

hmm @tgross35 it fails with an odd error for unix

Can this change to forbid and move to the parent library/std/src/sys/pal/wasi/mod.rs

I can do this only with changes that I did to unix, will it be fine if I don't do that and just enforce forbid in each wasi file besides mod.rs ?

@workingjubilee
Copy link
Member

I can do this only with changes that I did to unix, will it be fine if I don't do that and just enforce forbid in each wasi file besides mod.rs ?

Yes, just enforcing this in the wasi files is fine.

@workingjubilee
Copy link
Member

The behavior of modules with #[path] and linting contexts is slightly nonintuitive.

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

A few last style nits then looks good to me. Please squash into one commit.

Comment on lines 2 to 6
//!
//! [`std::ffi`]: crate::ffi
#![stable(feature = "rust1", since = "1.0.0")]

Copy link
Contributor

Choose a reason for hiding this comment

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

Everything in library/std/src/os/wasi/ looks like it got a line removed but the file is otherwise untouched. Assume this just happened because of fixups, could you reset these? Just look at the diff.

I think a space between inner docs and inner attributes is common anyway (@workingjubilee could certainly say for sure).

Copy link
Member

Choose a reason for hiding this comment

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

Common, not required? Certainly better to not diff the file if nothing actually changed otherwise.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking more about library/std/src/os/wasip2/mod.rs that both had an update and removed the line. But yeah, I'm mildly surprised rustfmt doesn't do something here.

@g0djan g0djan force-pushed the godjan/wasi_prohibit_implicit_unsafe branch from 8a7a495 to dcd6fb6 Compare August 5, 2024 08:46
@g0djan g0djan force-pushed the godjan/wasi_prohibit_implicit_unsafe branch from dcd6fb6 to 8a61674 Compare August 5, 2024 08:48
@g0djan
Copy link
Contributor Author

g0djan commented Aug 20, 2024

@rustbot ready

@tgross35
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Aug 20, 2024

📌 Commit 8a61674 has been approved by tgross35

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 Aug 20, 2024
@tgross35 tgross35 changed the title WASI fixing unsafe_op_in_unsafe_fn for std::{os, sys} WASI: fix unsafe_op_in_unsafe_fn for std::{os, sys} Aug 21, 2024
@tgross35 tgross35 changed the title WASI: fix unsafe_op_in_unsafe_fn for std::{os, sys} WASI: foirbid unsafe_op_in_unsafe_fn for std::{os, sys} Aug 21, 2024
@tgross35
Copy link
Contributor

Actually, this didn't get tested yet. Might as well verify before it gets picked up.

@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 21, 2024
@tgross35
Copy link
Contributor

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 21, 2024
…_unsafe, r=<try>

WASI: foirbid `unsafe_op_in_unsafe_fn` for `std::{os, sys}`

Part of rust-lang#127747 for WASI

try-job: test-various
@bors
Copy link
Contributor

bors commented Aug 21, 2024

⌛ Trying commit 8a61674 with merge 7d7e7bd...

@tgross35 tgross35 changed the title WASI: foirbid unsafe_op_in_unsafe_fn for std::{os, sys} WASI: forbid unsafe_op_in_unsafe_fn for std::{os, sys} Aug 21, 2024
@bors
Copy link
Contributor

bors commented Aug 21, 2024

☀️ Try build successful - checks-actions
Build commit: 7d7e7bd (7d7e7bd2d74143633fb38c67be31b8d531f68f7a)

@tgross35
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Aug 21, 2024

📌 Commit 8a61674 has been approved by tgross35

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 21, 2024
tgross35 added a commit to tgross35/rust that referenced this pull request Aug 21, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…it_unsafe, r=tgross35

WASI: forbid `unsafe_op_in_unsafe_fn` for `std::{os, sys}`

Part of rust-lang#127747 for WASI

try-job: test-various
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 21, 2024
Rollup of 7 pull requests

Successful merges:

 - rust-lang#128432 (WASI: forbid `unsafe_op_in_unsafe_fn` for `std::{os, sys}`)
 - rust-lang#128627 (Special case DUMMY_SP to emit line 0/column 0 locations on DWARF platforms.)
 - rust-lang#129187 (bootstrap: fix clean's remove_dir_all implementation)
 - rust-lang#129257 (Allow rust staticlib to work with MSVC's /WHOLEARCHIVE)
 - rust-lang#129264 (Update `library/Cargo.toml` in weekly job)
 - rust-lang#129284 (rustdoc: animate the `:target` highlight)
 - rust-lang#129332 (Avoid extra `cast()`s after `CStr::as_ptr()`)

r? `@ghost`
`@rustbot` modify labels: rollup
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Aug 22, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…it_unsafe, r=tgross35

WASI: forbid `unsafe_op_in_unsafe_fn` for `std::{os, sys}`

Part of rust-lang#127747 for WASI

try-job: test-various
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 22, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#128432 (WASI: forbid `unsafe_op_in_unsafe_fn` for `std::{os, sys}`)
 - rust-lang#129373 (Add missing module flags for CFI and KCFI sanitizers)
 - rust-lang#129374 (Use `assert_unsafe_precondition!` in `AsciiChar::digit_unchecked`)
 - rust-lang#129376 (Change `assert_unsafe_precondition` docs to refer to `check_language_ub`)
 - rust-lang#129382 (Add `const_cell_into_inner` to `OnceCell`)
 - rust-lang#129387 (Advise against removing the remaining Python scripts from `tests/run-make`)
 - rust-lang#129388 (Do not rely on names to find lifetimes.)
 - rust-lang#129395 (Pretty-print own args of existential projections (dyn-Trait w/ GAT constraints))

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a8d5c6d into rust-lang:master Aug 22, 2024
7 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 22, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 22, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Rollup merge of rust-lang#128432 - g0djan:godjan/wasi_prohibit_implicit_unsafe, r=tgross35

WASI: forbid `unsafe_op_in_unsafe_fn` for `std::{os, sys}`

Part of rust-lang#127747 for WASI

try-job: test-various
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-unix Operating system: Unix-like O-wasi Operating system: Wasi, Webassembly System Interface 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
Development

Successfully merging this pull request may close these issues.

None yet

7 participants