-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Add Games MSVC targets #67745
Add Games MSVC targets #67745
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @zackmdavis (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
@@ -174,8 +174,10 @@ pub fn get_linker(sess: &Session, linker: &Path, flavor: LinkerFlavor) -> (PathB | |||
// UWP apps have API restrictions enforced during Store submissions. | |||
// To comply with the Windows App Certification Kit, | |||
// MSVC needs to link with the Store versions of the runtime libraries (vcruntime, msvcrt, etc). | |||
// Similarly, apps targeting the Games API partition need to link to the OneCore versions of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See second-to-last paragraph under Where to find the Universal CRT files here: https://docs.microsoft.com/en-us/cpp/porting/upgrade-your-code-to-the-universal-crt?view=vs-2019#where-to-find-the-universal-crt-files
For more background on OneCore: https://docs.microsoft.com/en-us/windows/win32/apiindex/windows-umbrella-libraries
@@ -0,0 +1,25 @@ | |||
use crate::spec::{LinkerFlavor, PanicStrategy, Target, TargetResult}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied from aarch64_pc_windows_msvc
with s/pc/games/
base.has_elf_tls = true; | ||
base.features = "+neon,+fp-armv8".to_string(); | ||
|
||
// FIXME: this shouldn't be panic=abort, it should be panic=unwind |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See prior discussion here: https://github.com/rust-lang/rust/pull/63155/files#r312060124
@@ -0,0 +1,31 @@ | |||
use crate::spec::{LinkerFlavor, Target, TargetResult}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like above, copied from i686_pc_windows_msvc
with pc
replaced with games
.
@@ -0,0 +1,36 @@ | |||
use crate::spec::{LinkArgs, LinkerFlavor, TargetOptions}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to windows_uwp_msvc_base.rs
except that we link against onecore.lib
instead of mincore.lib
.
mincore.lib
is a Windows 8 umbrella library, whereas onecore.lib
is a Windows 10 library. Since the games partition is new to Windows 10 it made sense to use the newer umbrella library.
See here for more info on umbrella libraries: https://docs.microsoft.com/en-us/windows/win32/apiindex/windows-umbrella-libraries
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add the above as a comment in code if it would be useful.
@@ -0,0 +1,22 @@ | |||
use crate::spec::{LinkerFlavor, Target, TargetResult}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied from x86_64_pc_windows_msvc
with s/pc/games
@@ -638,6 +635,34 @@ if #[cfg(not(target_vendor = "uwp"))] { | |||
pub type PVECTORED_EXCEPTION_HANDLER = extern "system" | |||
fn(ExceptionInfo: *mut EXCEPTION_POINTERS) -> LONG; | |||
|
|||
pub const HANDLE_FLAG_INHERIT: DWORD = 0x00000001; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These 4 items simply moved up a little bit in the file because other items moved out. These are still forbidden when targeting UWP, but are available for Games.
// Functions forbidden when targeting the Games API partition | ||
cfg_if::cfg_if! { | ||
if #[cfg(not(target_vendor = "games"))] { | ||
pub const SYMBOLIC_LINK_FLAG_DIRECTORY: DWORD = 0x1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the Windows SDK headers, symbolic links are only supported on desktop apps and drivers (not UWP or games):
#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP | WINAPI_PARTITION_SYSTEM)
#define SYMBOLIC_LINK_FLAG_DIRECTORY (0x1)
#define SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE (0x2)
The MSDN docs agree (they say [desktop apps only]
at the bottom instead of [desktop apps | UWP apps]
)
So I think these should be also compiled out for target_vendor = "uwp"
, but they weren't before for some reason. I can merge this with the block below if you think I should do that.
r? @nagisa cc @alexcrichton @retep998 |
|
||
// Functions forbidden when targeting UWP or the Games API partition | ||
cfg_if::cfg_if! { | ||
if #[cfg(not(any(target_vendor = "uwp", target_vendor = "games")))] { | ||
#[repr(C)] | ||
#[derive(Copy, Clone)] | ||
pub struct CONSOLE_READCONSOLE_CONTROL { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Console stuff is also unavailable in Games (a little ironic...)
@@ -732,7 +746,7 @@ if #[cfg(target_vendor = "uwp")] { | |||
lpFileInformation: LPVOID, | |||
dwBufferSize: DWORD) -> BOOL; | |||
pub fn BCryptGenRandom(hAlgorithm: LPVOID, pBuffer: *mut u8, | |||
cbBuffer: ULONG, dwFlags: ULONG) -> LONG; | |||
cbBuffer: ULONG, dwFlags: ULONG) -> LONG; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what happened here with whitespace but tidy didn't complain. I can fix on a later commit.
pub mod stdio_uwp; | ||
pub mod stack_overflow_uwp; | ||
pub use self::stdio_uwp as stdio; | ||
pub use self::stack_overflow_uwp as stack_overflow; | ||
} else if #[cfg(target_vendor = "games")] { | ||
pub mod stdio_uwp; | ||
pub mod stack_overflow; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vectored exception handling is available in Games (but not UWP).
After reading this comment by petrochenkov I started to think whether these new targets are needed at all, and I realized that I didn't fully understand how everything fits together, so I decided to spend some time digging into it. I'm writing my findings here in case others find it useful. See the final part of the comment regarding what I think this means for this PR. API families and partitions The API family indicates the platform an app is built for. An app must target a given API family. When building C code, the
API partitions define sets of APIs that are available on a given platform. Each API family corresponds to one or more API partitions. For example, the After looking into it, I've found that API partitions are not a good way to determine whether a given API is available on a given platform. Take
However, you can definitely use What this means for Rust: Rust targets shouldn't use API partitions to decide whether it's OK to reference a particular Windows API or not, since partitions are not a reliable indicator of whether the function is available. API sets API sets are a concept introduced on Windows 7. They represent groups of APIs that may be implemented in different system DLLs depending on the OS version. Here is the documentation for it on MSDN. Similar to API partitions, each platform may implement some API sets and not others. For example, the desktop platform implements the However, like with API partitions, I've found that API sets are not a reliable indicator of whether a given platform supports a particular API. It seems that API sets can be implemented only partially by some platforms. For example, the desktop platform and the Windows Store platform both implement the What this means for Rust: Similar to API partitions, Rust targets shouldn't use API sets to decide whether it's OK to reference a particular Windows API or not. Umbrella libraries Umbrella libraries are import libraries that export all the Win32 APIs available in a given platform. The MSDN docs have a good description on these libraries. The idea is that you can link against your chosen umbrella library - and nothing else - and you will get linker errors if you attempt to use a function that is not implemented by that platform. You still need to consider what the minimum OS version is for a given API, so as usual you need to use the The Windows SDK comes with a number of umbrella libraries:
Umbrella libraries appear to be the only reliable way to get linker errors when unconditionally linking against an API that's not available on a given platform. What this means for Rust:
MSVC Runtimes MSVC provides three separate runtimes based on the platforms to support:
All three runtimes appear to export the exact same set of APIs. What this means for Rust:
What this all means for this PR I don't think this PR should be merged, given what I discussed above. The proposed There are some improvements that I think could be made to the UWP targets, as I discussed above. If there's interest in that, I can open another PR for those. |
@jblazquez The overview makes sense to me. Feel free to close the PR yourself. You might want to open an issue with the proposed changes for the uwp targets to facilitate further discussion on that specifically. |
Thanks nagisa, will do that. |
@jblazquez good overview but I would caution against using APIs that aren't officially supported for that API set/family even if they work currently. By doing so you potentially lose all compatibility guarantees. I think the fact they currently work should be considered an implementation detail and not something to be relied upon. At least not by Rust itself. Otherwise it could cause problems for Rust applications if they suddenly stop working or start being rejected by certification. |
Thanks Chris, I think that's fair, but I'm wondering how we could define "officially supported" better. The
So one reading of this would be: if an API is in the umbrella library, then it's officially part of UWP. However, the headers disagree with this, so should the headers take precedence? I'd lean towards yes, but that's not a very satisfying answer to me. Wish Microsoft had explicit guidance on this. |
My thinking is that if it's not listed in the documentation and it's not in the headers than it definitely isn't part of the official UWP API. UWP apps need access to unsupported functions because some supported APIs are currently built on top of them (at least for desktop Windows). At the end of the day, a UWP app is mostly a normal I guess they could've separated the non-UWP functions into another library but that would undermine the convenience of having a single At least that's my reading of the situation. |
If we already have targets for the desktop family and the uwp family, then I think it makes sense to have targets for the games family. As for unsupported functions, |
This PR adds the following new targets to Rust:
Starting around Windows SDK 1903 (10.0.18362.0) a new Windows API family called
WINAPI_FAMILY_GAMES
and a new partition calledWINAPI_PARTITION_GAMES
were added. Here's the documentation from thewinapifamily.h
header in the SDK:See this three part article series for some background on API families. Currently, the
*-pc-windows-*
targets correspond to theWINAPI_PARTITION_DESKTOP
partition, and the*-uwp-windows-*
targets correspond to theWINAPI_PARTITION_[PC|PHONE]_APP
partitions. The only other partitions are games (which this PR adds support for) and drivers (I assume these are user-mode drivers).This new API family appears to include more Win32 API surface area than UWP but less than the full desktop environment. Here are some examples of APIs that are available in the games partition but not on UWP:
While UWP can be used to write games too, it's generally a limited API with a lot of restrictions. The new games partition provides access to a larger set of Win32 APIs than what UWP provides.
With these changes it would now be possible to build apps that target this new API family while avoiding the use of APIs that are not available there.
See #60260 and #63155 for similar past PRs where the UWP targets were added.
Tested these changes by building a hello_world executable for the following targets: