From c817d0320f86c6a8c40fc92894fa02fb1b90e9b0 Mon Sep 17 00:00:00 2001 From: Fabrice de Gans Date: Fri, 7 Mar 2025 17:07:06 -0800 Subject: [PATCH 1/2] [WindowsDriver] Always consider `WinSdkVersion` Currently, the `-Xmicrosoft-windows-sdk-version` is only used if `-Xmicrosoft-windows-sdk-root` is also provided. This is a surprising behavior since the argument should still be taking effect if LLVM uses the Windows SDK root from the registry. Tested locally in a simple Hello World program including `Windows.h` and compiled with `-Xmicrosoft-windows-sdk-version 10.0.18362.0` on a system where the SDK 10.0.22621.0 is also installed and verified that the correct header was included. --- llvm/lib/WindowsDriver/MSVCPaths.cpp | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/llvm/lib/WindowsDriver/MSVCPaths.cpp b/llvm/lib/WindowsDriver/MSVCPaths.cpp index a7bffbb20eba1..60fc096059c62 100644 --- a/llvm/lib/WindowsDriver/MSVCPaths.cpp +++ b/llvm/lib/WindowsDriver/MSVCPaths.cpp @@ -85,11 +85,22 @@ getHighestNumericTupleInDirectory(llvm::vfs::FileSystem &VFS, return Highest; } -static bool getWindows10SDKVersionFromPath(llvm::vfs::FileSystem &VFS, - const std::string &SDKPath, - std::string &SDKVersion) { +static bool getWindows10SDKVersionFromPath( + llvm::vfs::FileSystem &VFS, const std::string &SDKPath, + std::optional WinSdkVersion, std::string &SDKVersion) { llvm::SmallString<128> IncludePath(SDKPath); llvm::sys::path::append(IncludePath, "Include"); + + if (WinSdkVersion) { + // Use the provided version, if it exists. + llvm::SmallString<128> VersionIncludePath(IncludePath); + llvm::sys::path::append(VersionIncludePath, *WinSdkVersion); + if (VFS.exists(VersionIncludePath)) { + SDKVersion = *WinSdkVersion; + return true; + } + } + SDKVersion = getHighestNumericTupleInDirectory(VFS, IncludePath); return !SDKVersion.empty(); } @@ -122,7 +133,8 @@ static bool getWindowsSDKDirViaCommandLine( if (!SDKVersion.empty()) { Major = SDKVersion.getMajor(); Version = SDKVersion.getAsString(); - } else if (getWindows10SDKVersionFromPath(VFS, Path, Version)) { + } else if (getWindows10SDKVersionFromPath(VFS, Path, WinSdkVersion, + Version)) { Major = 10; } return true; @@ -444,7 +456,8 @@ bool getWindowsSDKDir(vfs::FileSystem &VFS, std::optional WinSdkDir, return !WindowsSDKLibVersion.empty(); } if (Major == 10) { - if (!getWindows10SDKVersionFromPath(VFS, Path, WindowsSDKIncludeVersion)) + if (!getWindows10SDKVersionFromPath(VFS, Path, WinSdkVersion, + WindowsSDKIncludeVersion)) return false; WindowsSDKLibVersion = WindowsSDKIncludeVersion; return true; @@ -475,7 +488,7 @@ bool getUniversalCRTSdkDir(vfs::FileSystem &VFS, Path, nullptr)) return false; - return getWindows10SDKVersionFromPath(VFS, Path, UCRTVersion); + return getWindows10SDKVersionFromPath(VFS, Path, WinSdkVersion, UCRTVersion); } bool findVCToolChainViaCommandLine(vfs::FileSystem &VFS, From a7fd7bc9d4f0f0d36ebcd2071ac96c27d5a92c25 Mon Sep 17 00:00:00 2001 From: Fabrice de Gans Date: Mon, 10 Mar 2025 15:51:59 -0700 Subject: [PATCH 2/2] Use the user-provided version as-is Skip checking that the directory actually exists, trust the user input. --- llvm/lib/WindowsDriver/MSVCPaths.cpp | 38 ++++++++++++++-------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/llvm/lib/WindowsDriver/MSVCPaths.cpp b/llvm/lib/WindowsDriver/MSVCPaths.cpp index 60fc096059c62..1fc89749955df 100644 --- a/llvm/lib/WindowsDriver/MSVCPaths.cpp +++ b/llvm/lib/WindowsDriver/MSVCPaths.cpp @@ -85,22 +85,11 @@ getHighestNumericTupleInDirectory(llvm::vfs::FileSystem &VFS, return Highest; } -static bool getWindows10SDKVersionFromPath( - llvm::vfs::FileSystem &VFS, const std::string &SDKPath, - std::optional WinSdkVersion, std::string &SDKVersion) { +static bool getWindows10SDKVersionFromPath(llvm::vfs::FileSystem &VFS, + const std::string &SDKPath, + std::string &SDKVersion) { llvm::SmallString<128> IncludePath(SDKPath); llvm::sys::path::append(IncludePath, "Include"); - - if (WinSdkVersion) { - // Use the provided version, if it exists. - llvm::SmallString<128> VersionIncludePath(IncludePath); - llvm::sys::path::append(VersionIncludePath, *WinSdkVersion); - if (VFS.exists(VersionIncludePath)) { - SDKVersion = *WinSdkVersion; - return true; - } - } - SDKVersion = getHighestNumericTupleInDirectory(VFS, IncludePath); return !SDKVersion.empty(); } @@ -133,8 +122,7 @@ static bool getWindowsSDKDirViaCommandLine( if (!SDKVersion.empty()) { Major = SDKVersion.getMajor(); Version = SDKVersion.getAsString(); - } else if (getWindows10SDKVersionFromPath(VFS, Path, WinSdkVersion, - Version)) { + } else if (getWindows10SDKVersionFromPath(VFS, Path, Version)) { Major = 10; } return true; @@ -456,8 +444,14 @@ bool getWindowsSDKDir(vfs::FileSystem &VFS, std::optional WinSdkDir, return !WindowsSDKLibVersion.empty(); } if (Major == 10) { - if (!getWindows10SDKVersionFromPath(VFS, Path, WinSdkVersion, - WindowsSDKIncludeVersion)) + if (WinSdkVersion) { + // Use the user-provided version as-is. + WindowsSDKIncludeVersion = WinSdkVersion->str(); + WindowsSDKLibVersion = WindowsSDKIncludeVersion; + return true; + } + + if (!getWindows10SDKVersionFromPath(VFS, Path, WindowsSDKIncludeVersion)) return false; WindowsSDKLibVersion = WindowsSDKIncludeVersion; return true; @@ -488,7 +482,13 @@ bool getUniversalCRTSdkDir(vfs::FileSystem &VFS, Path, nullptr)) return false; - return getWindows10SDKVersionFromPath(VFS, Path, WinSdkVersion, UCRTVersion); + if (WinSdkVersion) { + // Use the user-provided version as-is. + UCRTVersion = WinSdkVersion->str(); + return true; + } + + return getWindows10SDKVersionFromPath(VFS, Path, UCRTVersion); } bool findVCToolChainViaCommandLine(vfs::FileSystem &VFS,