rust-lang / miri

An interpreter for Rust's mid-level intermediate representation
Apache License 2.0
4.38k stars 338 forks source link

Remove GetCurrentProcessId's frame_in_std check #3716

Closed cgettys-microsoft closed 3 months ago

cgettys-microsoft commented 3 months ago

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 https://github.com/rust-lang/miri/issues/1727#issuecomment-2184679952.

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 commented 3 months ago

@rustbot ready

RalfJung commented 3 months ago

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

@bors r+

bors commented 3 months ago

:pushpin: Commit 3fc1560c76de36129cf68abfe7ed4cc5efd2091b has been approved by RalfJung

It is now in the queue for this repository.

bors commented 3 months ago

:hourglass: Testing commit 3fc1560c76de36129cf68abfe7ed4cc5efd2091b with merge f589ef52de4b26914722255b133c4e00905c6cb0...

bors commented 3 months ago

:sunny: Test successful - checks-actions Approved by: RalfJung Pushing f589ef52de4b26914722255b133c4e00905c6cb0 to master...