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

Remove GetCurrentProcessId's frame_in_std check #3716

Merged

Conversation

cgettys-microsoft
Copy link
Contributor

@cgettys-microsoft cgettys-microsoft commented Jun 27, 2024

Most of the support required to close #1727 was actually added a while back, in #2215.

However, for some reason, even though the Unix/Linux syscall equivalent has no frame_in_std() check, the Windows GetCurrentProcessId check did. While the vast majority of use cases use std::process::id, there's no particular reason to penalize any Windows code that is no_std or for whatever other reason choses to call the function directly (e.g. via the generated windows-sys method). The emulation should still work fine. Given there's no reason not to, we might as well simplify the code a tiny bit and save that branch / frame check during runtime too.

This PR removes the frame_in_std restriction for GetCurrentProcessId, and also moves it into the environment related shim section per discussion in #1727 (comment).

Still passes existing tests/pass/getpid.rs test.

Closes #1727 unless we wish to give a dummy value when isolated, which we don't seem to want to do at this time.

@cgettys-microsoft
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added the S-waiting-on-review Status: Waiting for a review to complete label Jun 28, 2024
@RalfJung
Copy link
Member

Yeah not sure why we made that function std-only... thanks for the PR!

@bors r+

@bors
Copy link
Contributor

bors commented Jun 28, 2024

📌 Commit 3fc1560 has been approved by RalfJung

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jun 28, 2024

⌛ Testing commit 3fc1560 with merge f589ef5...

@bors
Copy link
Contributor

bors commented Jun 28, 2024

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing f589ef5 to master...

@bors bors merged commit f589ef5 into rust-lang:master Jun 28, 2024
8 checks passed
@cgettys-microsoft cgettys-microsoft deleted the dev/cgettys/process_id_fixup-03 branch June 28, 2024 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Waiting for a review to complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't call foreign function: GetCurrentProcessId on windows
4 participants