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

Add Games MSVC targets #67745

Closed
wants to merge 1 commit into from
Closed

Conversation

jblazquez
Copy link
Contributor

@jblazquez jblazquez commented Dec 31, 2019

This PR adds the following new targets to Rust:

x86_64-games-windows-msvc
i686-games-windows-msvc
aarch64-games-windows-msvc

Starting around Windows SDK 1903 (10.0.18362.0) a new Windows API family called WINAPI_FAMILY_GAMES and a new partition called WINAPI_PARTITION_GAMES were added. Here's the documentation from the winapifamily.h header in the SDK:

/*
 * When compiling C and C++ code using SDK header files, the development
 * environment can specify a target platform by #define-ing the
 * pre-processor symbol WINAPI_FAMILY to one of the following values.
 * Each FAMILY value denotes an application family for which a different
 * subset of the total set of header-file-defined APIs are available.
 * Setting the WINAPI_FAMILY value will effectively hide from the
 * editing and compilation environments the existence of APIs that
 * are not applicable to the family of applications targeting a
 * specific platform.
 */

/* In Windows 10, WINAPI_PARTITIONs will be used to add additional  
 * device specific APIs to a particular WINAPI_FAMILY.  
 * For example, when writing Windows Universal apps, specifying 
 * WINAPI_FAMILY_APP will hide phone APIs from compilation.  
 * However, specifying WINAPI_PARTITION_PHONE_APP=1 additionally, will         
 * unhide any API hidden behind the partition, to the compiler.

 * The following partitions are currently defined:
 * WINAPI_PARTITION_DESKTOP            // usable for Desktop Win32 apps (but not store apps)
 * WINAPI_PARTITION_APP                // usable for Windows Universal store apps
 * WINAPI_PARTITION_PC_APP             // specific to Desktop-only store apps
 * WINAPI_PARTITION_PHONE_APP          // specific to Phone-only store apps
 * WINAPI_PARTITION_SYSTEM             // specific to System applications
 * WINAPI_PARTITION_GAMES              // specific to games and apps
*/

/*
 * The WINAPI_FAMILY values of 0 and 1 are reserved to ensure that
 * an error will occur if WINAPI_FAMILY is set to any
 * WINAPI_PARTITION value (which must be 0 or 1, see below).
 */
#define WINAPI_FAMILY_PC_APP               2   /* Windows Store Applications */
#define WINAPI_FAMILY_PHONE_APP            3   /* Windows Phone Applications */
#define WINAPI_FAMILY_SYSTEM               4   /* Windows Drivers and Tools */
#define WINAPI_FAMILY_SERVER               5   /* Windows Server Applications */
#define WINAPI_FAMILY_GAMES                6   /* Windows Games and Applications */
#define WINAPI_FAMILY_DESKTOP_APP          100   /* Windows Desktop Applications */

See this three part article series for some background on API families. Currently, the *-pc-windows-* targets correspond to the WINAPI_PARTITION_DESKTOP partition, and the *-uwp-windows-* targets correspond to the WINAPI_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:

#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP | WINAPI_PARTITION_SYSTEM | WINAPI_PARTITION_GAMES)

WINBASEAPI
_Ret_maybenull_
PVOID
WINAPI
AddVectoredExceptionHandler(
    _In_ ULONG First,
    _In_ PVECTORED_EXCEPTION_HANDLER Handler
    );


WINBASEAPI
ULONG
WINAPI
RemoveVectoredExceptionHandler(
    _In_ PVOID Handle
    );


WINBASEAPI
_Ret_maybenull_
PVOID
WINAPI
AddVectoredContinueHandler(
    _In_ ULONG First,
    _In_ PVECTORED_EXCEPTION_HANDLER Handler
    );


WINBASEAPI
ULONG
WINAPI
RemoveVectoredContinueHandler(
    _In_ PVOID Handle
    );

...
#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP | WINAPI_PARTITION_GAMES)

BOOL
WINAPI
MiniDumpWriteDump(
    _In_ HANDLE hProcess,
    _In_ DWORD ProcessId,
    _In_ HANDLE hFile,
    _In_ MINIDUMP_TYPE DumpType,
    _In_opt_ PMINIDUMP_EXCEPTION_INFORMATION ExceptionParam,
    _In_opt_ PMINIDUMP_USER_STREAM_INFORMATION UserStreamParam,
    _In_opt_ PMINIDUMP_CALLBACK_INFORMATION CallbackParam
    );

#endif /* WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP | WINAPI_PARTITION_GAMES) */

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:

x86_64-pc-windows-msvc
x86_64-uwp-windows-msvc
x86_64-games-windows-msvc
i686-pc-windows-msvc
i686-uwp-windows-msvc
i686-games-windows-msvc
aarch64-pc-windows-msvc
aarch64-uwp-windows-msvc
aarch64-games-windows-msvc

@rust-highfive
Copy link
Collaborator

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.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 31, 2019
@@ -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
Copy link
Contributor Author

@jblazquez jblazquez Dec 31, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -0,0 +1,25 @@
use crate::spec::{LinkerFlavor, PanicStrategy, Target, TargetResult};
Copy link
Contributor Author

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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -0,0 +1,31 @@
use crate::spec::{LinkerFlavor, Target, TargetResult};
Copy link
Contributor Author

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};
Copy link
Contributor Author

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

Copy link
Contributor Author

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};
Copy link
Contributor Author

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;
Copy link
Contributor Author

@jblazquez jblazquez Dec 31, 2019

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.

@Centril Centril added relnotes Marks issues that should be documented in the release notes of the next release. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 31, 2019
@Centril Centril added this to the 1.42 milestone Dec 31, 2019
// 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;
Copy link
Contributor Author

@jblazquez jblazquez Dec 31, 2019

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.

@Centril
Copy link
Contributor

Centril commented Dec 31, 2019

r? @nagisa cc @alexcrichton @retep998

@rust-highfive rust-highfive assigned nagisa and unassigned zackmdavis Dec 31, 2019

// 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 {
Copy link
Contributor Author

@jblazquez jblazquez Dec 31, 2019

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;
Copy link
Contributor Author

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;
Copy link
Contributor Author

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).

@jblazquez
Copy link
Contributor Author

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 WINAPI_FAMILY preprocessor macro must be set to one of the following values to determine the target platform:

#define WINAPI_FAMILY_PC_APP               2   /* Windows Store Applications */
#define WINAPI_FAMILY_PHONE_APP            3   /* Windows Phone Applications */
#define WINAPI_FAMILY_SYSTEM               4   /* Windows Drivers and Tools */
#define WINAPI_FAMILY_SERVER               5   /* Windows Server Applications */
#define WINAPI_FAMILY_GAMES                6   /* Windows Games and Applications */
#define WINAPI_FAMILY_DESKTOP_APP          100   /* Windows Desktop Applications */

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 WINAPI_FAMILY_DESKTOP_APP family brings three partitions: WINAPI_PARTITION_DESKTOP, WINAPI_PARTITION_APP and WINAPI_PARTITION_PC_APP. When building C code, all the APIs in those partitions will be available in the headers and can be referenced by code, but they may or may not exist in every OS version that supports that platform, so the _WIN32_WINNT macro must also be used to filter out some of those APIs based on the target OS version.

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 CreateFileW for example. According to the documentation, CreateFileW is only available in desktop apps, not Windows Store apps. The header file also agrees:

#pragma region Desktop Family or OneCore Family or Games Family
#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP | WINAPI_PARTITION_SYSTEM | WINAPI_PARTITION_GAMES)

WINBASEAPI
HANDLE
WINAPI
CreateFileW(
    _In_ LPCWSTR lpFileName,
    _In_ DWORD dwDesiredAccess,
    _In_ DWORD dwShareMode,
    _In_opt_ LPSECURITY_ATTRIBUTES lpSecurityAttributes,
    _In_ DWORD dwCreationDisposition,
    _In_ DWORD dwFlagsAndAttributes,
    _In_opt_ HANDLE hTemplateFile
    );

#endif // WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP | WINAPI_PARTITION_SYSTEM | WINAPI_PARTITION_GAMES)
#pragma endregion

However, you can definitely use CreateFileW in a Windows Store app (I just tested this). You can link against it, you will pass the Windows App Certification Kit's supported API test, and the call will succeed at runtime (at least for files). My guess is that in this case at least, Microsoft wants to encourage the use of the newer CreateFile2 API, so they make the older one harder to 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 api-ms-win-core-com-l1-1-2 API set, which includes the CLSIDFromProgIDEx and CoFileTimeNow functions, whereas the Windows Store platform doesn't. You can use the IsApiSetImplemented function to determine at runtime whether the platform you're running on supports a particular API set.

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 api-ms-win-core-com-l1-1-0 API set, but some of the functions in that API set (e.g. CoCancelCall) are only available on the desktop platform.

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 _WIN32_WINNT macro for additional filtering at build time or test for the API's existence at runtime with GetProcAddress.

The Windows SDK comes with a number of umbrella libraries:

  • mincore.lib (docs): Represents all the APIs defined in API sets available for Windows 8.x apps (4428 functions).
  • mincore_downlevel.lib (docs): Represents additional APIs defined in "well-layered system DLLs" available for Windows 8.x apps (451 functions).
  • onecore.lib (docs): Represents all the APIs available for Windows 10 apps, not including UWP APIs (7618 functions).
  • onecore_apiset.lib (undocumented): Similar to onecore.lib, but seems to only include APIs defined in API sets (5218 functions).
  • onecore_downlevel.lib (undocumented): Similar to onecore.lib, but seems to only include APIs not defined in API sets (4851 functions).
  • onecoreuap.lib (docs): Represents all the APIs available for Windows 10 apps, including UWP APIs (11565 functions).
  • onecoreuap_apiset.lib (undocumented): Similar to onecoreuap.lib, but seems to only include APIs defined in API sets (9261 functions).
  • onecoreuap_downlevel.lib (undocumented): Similar to onecoreuap.lib, but seems to only include APIs not defined in API sets (4851 functions - identical to onecore_downlevel.lib).
  • windowsapp.lib (docs): Represents all the APIs available for Windows Store apps (4627 functions).

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:

  • The *-pc-windows-msvc targets, which target the desktop platform, don't need to change anything. They can continue using all APIs available on Windows, just being careful to avoid unconditional references to functions that aren't available in older OS versions like they're doing now. Maybe in the future this explicit list of import libraries depended on by libstd could be removed in favor of passing an umbrella library (onecoreuap.lib for maximum coverage) in the linker command line.

  • The *-uwp-windows-msvc targets, which target the Windows Store platform, can probably be improved in a few ways:

    a) Instead of using the Windows 8.x mincore.lib umbrella library they should do what the *-uwp-windows-gnu targets are doing and link against the windowsapp.lib umbrella library instead.

    b) In order to avoid importing functions using the wrong DLL name, the UWP targets should invoke the linker with /NODEFAULTLIB to prevent references to non-umbrella import libraries from being used, as per the note here.

    c) This list of explicit import libraries could then be removed since the umbrella library subsumes them.

    d) This list of 10 APIs that were marked as forbidden for UWP apps can be reduced to only one that is actually forbidden: GetUserProfileDirectoryW. According to my testing, that's the only API that WACK complains about for a Windows Store app, and the only one that's not exported in the windowsapp.lib umbrella library (even SystemFunction036 is). This would simplify libstd since it would have fewer places where UWP and non-UWP diverge (see 1, 2, 3, 4). @chouquette, since you added this list of forbidden functions, could you double-check my assumptions here? Wondering if you had a better source of information for determining whether an API was forbidden in UWP.

  • There is no umbrella library for the games API partition, so there's no clear way to define a new games target based on that. Because the games partition appears to be mostly a superset of the Windows Store partition, one could probably use the existing *-uwp-microsoft-msvc targets to make sure there are no unconditional references to non-games APIs (e.g. GetUserProfileDirectoryW).

MSVC Runtimes

MSVC provides three separate runtimes based on the platforms to support:

  • %VCToolsInstallDir%\lib\<arch>: Runtime for desktop apps that supports all Windows versions since Windows XP.

  • %VCToolsInstallDir%\lib\onecore\<arch>: Runtime for desktop apps that supports all Windows versions since Windows 7. This runtime is subtly different than the previous one, with a number of codepaths that behave differently (example) and unconditional use of some Vista+ functions like InitializeSRWLock.

  • %VCToolsInstallDir%\lib\<arch>\store: Runtime for Windows Store apps. This runtime has a greater number of codepaths that differ compared to the desktop runtimes above (example).

All three runtimes appear to export the exact same set of APIs.

What this means for Rust:

  • The *-pc-windows-msvc targets don't need to change anything. They can continue linking against the non-onecore MSVC runtime, which is the default, for XP and Vista support. In the future, if Rust stops supporting XP and Vista and adds explicit targets for those then the main *-pc-windows-msvc targets could switch to linking against the onecore MSVC runtime.

  • The *-uwp-windows-msvc targets don't need to change anything either. They need to continue linking against the store MSVC runtime to ensure that they use the appropriate implementations internally.

What this all means for this PR

I don't think this PR should be merged, given what I discussed above. The proposed *-games-microsoft-msvc targets don't appear to provide any benefit as things stand right now.

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.

@nagisa
Copy link
Member

nagisa commented Jan 6, 2020

@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.

@jblazquez
Copy link
Contributor Author

Thanks nagisa, will do that.

@jblazquez jblazquez closed this Jan 6, 2020
@ChrisDenton
Copy link
Member

@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.

@jblazquez
Copy link
Contributor Author

Thanks Chris, I think that's fair, but I'm wondering how we could define "officially supported" better.

The windowsapp.lib umbrella library is described as follows:

This topic lists the Win32 APIs that are part of the UWP and that are implemented by all Windows 10 devices. For convenience, an umbrella library named WindowsApp.lib is provided in the Microsoft Windows Software Development Kit (SDK), which provides the exports for this set of Win32 APIs.

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.

@ChrisDenton
Copy link
Member

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 exe process, even though it's loaded a bit differently. Therefore the process itself still needs to import all the Win32 APIs it takes to run the exe and make the supported APIs work. In that light, I think the section you quoted should be read as meaning only that all UWP APIs are included in WindowsApp.lib but not that it contains only UWP functions. If that makes sense.

I guess they could've separated the non-UWP functions into another library but that would undermine the convenience of having a single WindowsApp.lib.

At least that's my reading of the situation.

@retep998
Copy link
Member

retep998 commented Jan 8, 2020

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, winapi will just stick strictly to the headers with regards to families and partitions.

@Centril Centril removed the relnotes Marks issues that should be documented in the release notes of the next release. label Jan 31, 2020
@Centril Centril removed this from the 1.42 milestone Jan 31, 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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants