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

Can't call foreign function: GetCurrentProcessId on windows #1727

Closed
Legorooj opened this issue Mar 2, 2021 · 4 comments · Fixed by #3716
Closed

Can't call foreign function: GetCurrentProcessId on windows #1727

Legorooj opened this issue Mar 2, 2021 · 4 comments · Fixed by #3716
Labels
A-shims Area: This affects the external function shims C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement

Comments

@Legorooj
Copy link

Legorooj commented Mar 2, 2021

I'm getting this when I call cargo miri run on my project:

error: unsupported operation: can't call foreign function: GetCurrentProcessId
   --> C:\Users\legor\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\sys\windows\os.rs:332:14
    |
332 |     unsafe { c::GetCurrentProcessId() as u32 }
    |              ^^^^^^^^^^^^^^^^^^^^^^^^ can't call foreign function: GetCurrentProcessId
    |
    = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support

    = note: inside `std::sys::windows::os::getpid` at C:\Users\legor\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\sys\windows\os.rs:332:14
    = note: inside `std::process::id` at C:\Users\legor\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\process.rs:1802:5
note: inside `main` at src\macros.rs:17:33
   --> src\macros.rs:17:33
    |
17  |                 print!("[{}] ", std::process::id());
    |                                 ^^^^^^^^^^^^^^^^^^
    |
   ::: src/main.rs:22:5
    |
22  |     debug!("PyInstaller Bootloader 4.x");
    |     ------------------------------------- in this macro invocation
    = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at C:\Users\legor\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\ops\function.rs:227:5
    = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at C:\Users\legor\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\sys_common\backtrace.rs:125:18
    = note: inside closure at C:\Users\legor\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\rt.rs:66:18
    = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at C:\Users\legor\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\ops\function.rs:259:13
    = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at C:\Users\legor\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panicking.rs:379:40
    = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at C:\Users\legor\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panicking.rs:343:19
    = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at C:\Users\legor\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panic.rs:431:14
    = note: inside `std::rt::lang_start_internal` at C:\Users\legor\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\rt.rs:51:25
    = note: inside `std::rt::lang_start::<()>` at C:\Users\legor\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\rt.rs:65:5
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

error: process didn't exit successfully: `C:\Users\legor\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\bin\cargo-miri.exe target\x86_64-pc-windows-msvc\debug\run.exe` (exit code: 1)

Let me know what other data I can provide.

Let me know what other data I can provide.

@RalfJung
Copy link
Member

RalfJung commented Mar 2, 2021

Yeah, std::process::id is not currently supported on any platform by Miri.

In non-isolated mode, we can probably just call process::id() on the host and use that; in isolated mode we could return a fake ID but I am not sure if that could cause any problems? An isolated program cannot talk to other processed anyway so there's nothing that ID can really be used for...

@RalfJung RalfJung added A-shims Area: This affects the external function shims C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement labels Mar 2, 2021
@Legorooj
Copy link
Author

Legorooj commented Mar 8, 2021

I don't see how it would cause problems, but then I'm not exactly an expert when it comes to this.

@Cgettys
Copy link

Cgettys commented Jun 22, 2024

Isn't this already pretty much done since 2022 for the non-isolated case?
#2215 /
3e03054
seems to have added support for it. The only thing that would be required that I see to make this work on Windows for outside-std usage is change

"GetCurrentProcessId" if this.frame_in_std()

to

"GetCurrentProcessId"

I don't have a pressing need for it, but it's slightly strange today that this has a if this.frame_in_std() but the unix one does not - see https://github.com/rust-lang/miri/blob/3e03054ef0056e728a081b03cde8546f91c822aa/src/shims/unix/foreign_items.rs .

RE: fake PID, can't speak to that either. Probably best to steer away from allowing it in isolated mode. Maybe one could safely use zero: https://blog.dave.tf/post/linux-pid0/
Windows tools typically report process id 0 for system idle process too, I'm guessing for much the same reasons, but I don't know for sure.
But probably better to just make it required non-isolated mode like it is today.

Happy to put in a 1-line patch to close this out if it'd be useful.

@RalfJung
Copy link
Member

Hm, good point, I don't see why GetCurrentProcessId has to be std-only.

So yes please submit a PR. :) Please also move that match arm up into the "Environment related shims" section of the file then, to keep up the organization.

@bors bors closed this as completed in f589ef5 Jun 28, 2024
RalfJung pushed a commit to RalfJung/rust that referenced this issue Jun 29, 2024
…id_fixup-03, r=RalfJung

Remove GetCurrentProcessId's frame_in_std check

Most of the support required to close rust-lang#1727 was actually added a while back, in rust-lang#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](https://docs.rs/windows-sys/latest/windows_sys/Win32/System/Threading/fn.GetCurrentProcessId.html) 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 rust-lang/miri#1727 (comment).

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

Closes rust-lang#1727 unless we wish to give a dummy value when isolated, which we don't seem to want to do at this time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-shims Area: This affects the external function shims C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants