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

windows-specific extensions for accessing Windows profileapi #70618

Closed
wants to merge 3 commits into from

Conversation

JOE1994
Copy link
Contributor

@JOE1994 JOE1994 commented Mar 31, 2020

Added new module time to std::os::windows, for accessing Windows profileapi as below.

  • QueryPerformanceCounter
  • QueryPerformanceFrequency

Need for these extensions arised while working on rust-lang/miri#997, which requires implementing
3 Windows shims

  • GetSystemTimeAsFileTime
  • QueryPerformanceCounter
  • QueryPerformanceFrequency

(The latter two shims are not yet mentioned in the issue, but they are currently needed to pass the latest version of test case : MIRI/tests/run-pass/time.rs))

Implementing the first shim didn't need new extensions to std::os::windows,
but implementing the other two shims needs direct access to Windows API.
(I couldn't find any public API from libstd to be used in MIRI for implementing the shims)

I also tried using winapi library in MIRI, but I got an error saying that "MIRI doesn't support foreign functions".

p.s. -
It's my first time pushing code to libstd, so some annotations might have to be fixed..

@rust-highfive
Copy link
Collaborator

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 31, 2020
@JOE1994
Copy link
Contributor Author

JOE1994 commented Mar 31, 2020

r? @RalfJung

@rust-highfive rust-highfive assigned RalfJung and unassigned sfackler Mar 31, 2020
@RalfJung
Copy link
Member

RalfJung commented Mar 31, 2020

Sorry, but there is no way I can review this. I didn't program on Windows for more than 10 years.

But I also think this is the wrong approach to get new Miri shims to work. The equivalent shims on macOS and Linux are all implemented entirely in terms of the public API of libstd, and thus work cross-platform (so on any host, you can run programs for the Linux/macOS target).
Even with these methods, how do you plan to implement the shims when the target is windows but the host is Linux?

@JOE1994
Copy link
Contributor Author

JOE1994 commented Mar 31, 2020

Even with these methods, how do you plan to implement the shims when the target is windows but the host is Linux?

I am currently not so sure..
While the docs for GetSystemTimeAsFileTime & FILETIME gives enough clue on how to imitate the behavior in MIRI, the docs for QueryPerformanceCounter & QueryPerformanceFrequency merely say that the functions fetch some value. I feel like I don't have enough info to be able to imitate such behavior in MIRI.

@RalfJung
Copy link
Member

You should be able to infer what you need from the code in libstd.

Remember that Miri doesn't have to behave exactly like real Windows, it just has to be a valid implementation of the Windows API. For example, on macOS, Miri implements mach_absolute_time which returns time in some unspecified unit. Miri just decides that unit is nanoseconds. Then there is mach_timebase_info which the program can use to query what the unit is, and Miri should implement that to indicate that it is nanoseconds. (Currently Miri just does not implement this, which means on macOS it is not currently possible to take the difference between two instants in Miri.)

With a similar approach, you should be able to implement the Windows API on any host.

@RalfJung
Copy link
Member

See rust-lang/miri#1243 for some inspiration.

@RalfJung
Copy link
Member

RalfJung commented Apr 1, 2020

And finally, even if we use direct host calls for the implementation in Miri (which I am inclined to reject as an implementation strategy), there is no reason to add new methods to the stable, forever-maintained backwards-compatible libstd API surface. As far as I know, there is also no precedent for directly exposing host functions like this.

Instead, in that (currently entirely hypothetical) case, Miri should use winapi. I don't know what you did that you got "MIRI doesn't support foreign functions", but that sounds like you made a mistake with integrating winapi. There is no reason that I know of that this should not work.

So, I recommend to close this PR.

@JOE1994 JOE1994 closed this Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants