Skip to content

Hermit: Unify std::env::args with Unix #139711

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Apr 21, 2025
Merged

Conversation

thaliaarchi
Copy link
Contributor

@thaliaarchi thaliaarchi commented Apr 12, 2025

The only differences between these implementations of std::env::args are that Unix uses relaxed ordering, but Hermit uses acquire/release, and Unix truncates argv at the first null pointer, but Hermit doesn't. Since Hermit aims for Unix compatibility, unify it with Unix.

The atomic orderings were established in #74006 (cc @euclio) for Unix and #100579 (cc @joboet) for Hermit and, before those, they used mutexes and non-atomic statics. I think the difference in orderings is simply from them being changed at different times. The commented explanation for using acquire/release for Hermit is “to broadcast writes by the OS”. I'm not experienced enough with atomics to accurately judge, but I think acquire/release is stronger than needed. Either way, they should match.

Truncating at the first null pointer seems desirable, though I don't know whether it is necessary in practice on Hermit.

cc @mkroening @stlankes for Hermit

@rustbot
Copy link
Collaborator

rustbot commented Apr 12, 2025

r? @jhpratt

rustbot has assigned @jhpratt.
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-hermit Operating System: Hermit O-SGX Target: SGX O-solid Operating System: SOLID O-unix Operating system: Unix-like O-wasi Operating system: Wasi, Webassembly System Interface O-wasm Target: WASM (WebAssembly), http://webassembly.org/ O-windows Operating system: Windows 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. S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Apr 12, 2025
@bors
Copy link
Collaborator

bors commented Apr 13, 2025

☔ The latest upstream changes (presumably #139746) made this pull request unmergeable. Please resolve the merge conflicts.

The only differences between these implementations are that Unix uses
relaxed ordering, but Hermit uses acquire/release, and Unix truncates
`argv` at the first null pointer, but Hermit doesn't. Since Hermit aims
for Unix compatibility, unify it with Unix.
@thaliaarchi
Copy link
Contributor Author

thaliaarchi commented Apr 13, 2025

#139710 merged. @rustbot label -S-blocked

@rustbot rustbot removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Apr 13, 2025
@thaliaarchi
Copy link
Contributor Author

@rustbot label -O-SGX -O-solid -O-unix -O-wasi -O-wasm -O-windows

@rustbot rustbot removed O-SGX Target: SGX O-solid Operating System: SOLID O-unix Operating system: Unix-like O-wasi Operating system: Wasi, Webassembly System Interface O-wasm Target: WASM (WebAssembly), http://webassembly.org/ O-windows Operating system: Windows labels Apr 14, 2025
@jhpratt
Copy link
Member

jhpratt commented Apr 19, 2025

r=me with either/both of the target maintainers approval.

Copy link
Contributor

@mkroening mkroening left a comment

Choose a reason for hiding this comment

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

This looks good to me! 👍

I have nothing to add to your assessments, I agree. :D

@joboet
Copy link
Member

joboet commented Apr 21, 2025

@bors r=@jhpratt

@bors
Copy link
Collaborator

bors commented Apr 21, 2025

📌 Commit c1f0498 has been approved by jhpratt

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 21, 2025
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 21, 2025
@joboet
Copy link
Member

joboet commented Apr 21, 2025

@bors rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 21, 2025
…enton

Rollup of 11 pull requests

Successful merges:

 - rust-lang#134213 (Stabilize `naked_functions`)
 - rust-lang#139711 (Hermit: Unify `std::env::args` with Unix)
 - rust-lang#139795 (Clarify why SGX code specifies linkage/symbol names for certain statics)
 - rust-lang#140036 (Advent of `tests/ui` (misc cleanups and improvements) [4/N])
 - rust-lang#140047 (remove a couple clones)
 - rust-lang#140052 (Fix error when an intra doc link is trying to resolve an empty associated item)
 - rust-lang#140074 (rustdoc-json: Improve test for auto-trait impls)
 - rust-lang#140076 (jsondocck: Require command is at start of line)
 - rust-lang#140107 (rustc-dev-guide subtree update)
 - rust-lang#140111 (cleanup redundant pattern instances)
 - rust-lang#140118 ({B,C}Str: minor cleanup)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5779843 into rust-lang:master Apr 21, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 21, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 21, 2025
Rollup merge of rust-lang#139711 - thaliaarchi:hermit-args, r=jhpratt

Hermit: Unify `std::env::args` with Unix

The only differences between these implementations of `std::env::args` are that Unix uses relaxed ordering, but Hermit uses acquire/release, and Unix truncates `argv` at the first null pointer, but Hermit doesn't. Since Hermit aims for Unix compatibility, unify it with Unix.

The atomic orderings were established in rust-lang#74006 (cc `@euclio)` for Unix and rust-lang#100579 (cc `@joboet)` for Hermit and, before those, they used mutexes and non-atomic statics. I think the difference in orderings is simply from them being changed at different times. The commented explanation for using acquire/release for Hermit is “to broadcast writes by the OS”. I'm not experienced enough with atomics to accurately judge, but I think acquire/release is stronger than needed. Either way, they should match.

Truncating at the first null pointer seems desirable, though I don't know whether it is necessary in practice on Hermit.

cc `@mkroening` `@stlankes` for Hermit
@thaliaarchi thaliaarchi deleted the hermit-args branch April 22, 2025 01:19
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Apr 23, 2025
Hermit: Unify `std::env::args` with Unix

The only differences between these implementations of `std::env::args` are that Unix uses relaxed ordering, but Hermit uses acquire/release, and Unix truncates `argv` at the first null pointer, but Hermit doesn't. Since Hermit aims for Unix compatibility, unify it with Unix.

The atomic orderings were established in rust-lang#74006 (cc `@euclio)` for Unix and rust-lang#100579 (cc `@joboet)` for Hermit and, before those, they used mutexes and non-atomic statics. I think the difference in orderings is simply from them being changed at different times. The commented explanation for using acquire/release for Hermit is “to broadcast writes by the OS”. I'm not experienced enough with atomics to accurately judge, but I think acquire/release is stronger than needed. Either way, they should match.

Truncating at the first null pointer seems desirable, though I don't know whether it is necessary in practice on Hermit.

cc `@mkroening` `@stlankes` for Hermit
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Apr 23, 2025
…enton

Rollup of 11 pull requests

Successful merges:

 - rust-lang#134213 (Stabilize `naked_functions`)
 - rust-lang#139711 (Hermit: Unify `std::env::args` with Unix)
 - rust-lang#139795 (Clarify why SGX code specifies linkage/symbol names for certain statics)
 - rust-lang#140036 (Advent of `tests/ui` (misc cleanups and improvements) [4/N])
 - rust-lang#140047 (remove a couple clones)
 - rust-lang#140052 (Fix error when an intra doc link is trying to resolve an empty associated item)
 - rust-lang#140074 (rustdoc-json: Improve test for auto-trait impls)
 - rust-lang#140076 (jsondocck: Require command is at start of line)
 - rust-lang#140107 (rustc-dev-guide subtree update)
 - rust-lang#140111 (cleanup redundant pattern instances)
 - rust-lang#140118 ({B,C}Str: minor cleanup)

r? `@ghost`
`@rustbot` modify labels: rollup
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
O-hermit Operating System: Hermit 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.

6 participants