[Windows] Sink function bodies to .cpp file

[rnk]

Summary

These functions don't need to be inlined. I randomly picked Process.inc
for the Windows version helpers, since that's the most related file, and
Program.inc for MakeErrMsg since it's the main client.

While I'm here, move them into the llvm namespace, and delete the scoped
handle copy and assignment operators.

[aganea]

llvm/lib/Support/Windows/Process.inc

469 ↗

(On Diff #172844)

Can't you just replace the contents of this function by:

return GetWindowsOSVersion() >= llvm::VersionTuple(6, 2, 0, 0);

503 ↗

(On Diff #172844)

I'm wondering after all if we shouldn't just drop the part below? It's deprecated anyway, and the call to RtlGetVersion would probably always succeed (if you can't aquire a handle to ntdll.dll most likely you won't be able to run anything on your machine). What do you think?

[rnk]

llvm/lib/Support/Windows/Process.inc

469 ↗

(On Diff #172844)

Sounds good to me.

503 ↗

(On Diff #172844)

I'd be OK with that. I suppose we'd return VersionTuple(0,0,0,0) in that case, and we'd assume that flush file buffers was necessary, which is probably fine.

[rnk]

• simplify

[aganea]

llvm/lib/Support/Windows/Process.inc

485 ↗

(On Diff #172862)

It should say >= here.

[rnk]

• Fix thinko

[aganea]

LGTM!

[aganea]

I was also wondering - there are a lot of interesting things in /lldb/trunk/source/Host/windows/. The ´GetWindowsOSVersion’ comes from there. What was the rationale for not having the ‘Host’ folder inside llvm? Any plans to move it there?

[rnk]

Traditionally, LLDB has resisted efforts to more tightly couple it with LLVM. Many of the contributors at Apple seem to prefer to build LLVM separately and then build against that with the LLDB xcode project. You can see how that would disincentivize you from contributing to LLVM first. And in fairness, sometimes LLVM people object to adding extra complexity to Support when it's only used in LLDB. And, as with lib/DebugInfo/DWARF, sometimes code gets hoisted to LLVM without refactoring LLDB to use it. So, there's plenty of work to go around. :)

llvm/trunk/lib/Support/Windows/Process.inc

unsigned Process::GetRandomNumber() {
 
   ScopedCryptContext CryptoProvider(HCPC);
   unsigned Ret;
   if (!::CryptGenRandom(CryptoProvider, sizeof(Ret),
                         reinterpret_cast<BYTE *>(&Ret)))
     ReportLastErrorFatal("Could not generate a random number");
   return Ret;
 }
+
+typedef NTSTATUS(WINAPI* RtlGetVersionPtr)(PRTL_OSVERSIONINFOW);
+#define STATUS_SUCCESS ((NTSTATUS)0x00000000L)
+
+llvm::VersionTuple llvm::GetWindowsOSVersion() {
+  HMODULE hMod = ::GetModuleHandleW(L"ntdll.dll");
+  if (hMod) {
+    auto getVer = (RtlGetVersionPtr)::GetProcAddress(hMod, "RtlGetVersion");
+    if (getVer) {
+      RTL_OSVERSIONINFOEXW info{};
+      info.dwOSVersionInfoSize = sizeof(info);
+      if (getVer((PRTL_OSVERSIONINFOW)&info) == STATUS_SUCCESS) {
+        return llvm::VersionTuple(info.dwMajorVersion, info.dwMinorVersion, 0,
+                                  info.dwBuildNumber);
+      }
+    }
+  }
+  return llvm::VersionTuple(0, 0, 0, 0);
+}
+
+bool llvm::RunningWindows8OrGreater() {
+  // Windows 8 is version 6.2, service pack 0.
+  return GetWindowsOSVersion() >= llvm::VersionTuple(6, 2, 0, 0);
+}

llvm/trunk/lib/Support/Windows/Program.inc

ErrorOr<std::string> sys::findProgramByName(StringRef Name,
   SmallVector<char, MAX_PATH> U8Result;
   if (std::error_code EC =
           windows::UTF16ToUTF8(U16Result.data(), U16Result.size(), U8Result))
     return EC;
 
   return std::string(U8Result.begin(), U8Result.end());
 }
 
+bool MakeErrMsg(std::string *ErrMsg, const std::string &prefix) {
+  if (!ErrMsg)
+    return true;
+  char *buffer = NULL;
+  DWORD LastError = GetLastError();
+  DWORD R = FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER |
+                               FORMAT_MESSAGE_FROM_SYSTEM |
+                               FORMAT_MESSAGE_MAX_WIDTH_MASK,
+                           NULL, LastError, 0, (LPSTR)&buffer, 1, NULL);
+  if (R)
+    *ErrMsg = prefix + ": " + buffer;
+  else
+    *ErrMsg = prefix + ": Unknown error";
+  *ErrMsg += " (0x" + llvm::utohexstr(LastError) + ")";
+
+  LocalFree(buffer);
+  return R != 0;
+}
+
 static HANDLE RedirectIO(Optional<StringRef> Path, int fd,
                          std::string *ErrMsg) {
   HANDLE h;
   if (!Path) {
     if (!DuplicateHandle(GetCurrentProcess(), (HANDLE)_get_osfhandle(fd),
                          GetCurrentProcess(), &h,
                          0, TRUE, DUPLICATE_SAME_ACCESS))
       return INVALID_HANDLE_VALUE;

llvm/trunk/lib/Support/Windows/WindowsSupport.h

 #include <cassert>
 #include <string>
 #include <system_error>
 #include <windows.h>
 
 // Must be included after windows.h
 #include <wincrypt.h>
 
+namespace llvm {
+
 /// Determines if the program is running on Windows 8 or newer. This
 /// reimplements one of the helpers in the Windows 8.1 SDK, which are intended
 /// to supercede raw calls to GetVersionEx. Old SDKs, Cygwin, and MinGW don't
 /// yet have VersionHelpers.h, so we have our own helper.
-inline bool RunningWindows8OrGreater() {
-  // Windows 8 is version 6.2, service pack 0.
-  OSVERSIONINFOEXW osvi = {};
-  osvi.dwOSVersionInfoSize = sizeof(OSVERSIONINFO);
-  osvi.dwMajorVersion = 6;
-  osvi.dwMinorVersion = 2;
-  osvi.wServicePackMajor = 0;
-
-  DWORDLONG Mask = 0;
-  Mask = VerSetConditionMask(Mask, VER_MAJORVERSION, VER_GREATER_EQUAL);
-  Mask = VerSetConditionMask(Mask, VER_MINORVERSION, VER_GREATER_EQUAL);
-  Mask = VerSetConditionMask(Mask, VER_SERVICEPACKMAJOR, VER_GREATER_EQUAL);
-
-  return VerifyVersionInfoW(&osvi, VER_MAJORVERSION | VER_MINORVERSION |
-                                       VER_SERVICEPACKMAJOR,
-                            Mask) != FALSE;
-}
-
-typedef NTSTATUS(WINAPI* RtlGetVersionPtr)(PRTL_OSVERSIONINFOW);
-#define STATUS_SUCCESS ((NTSTATUS)0x00000000L)
-
-inline llvm::VersionTuple GetWindowsOSVersion() {
-  HMODULE hMod = ::GetModuleHandleW(L"ntdll.dll");
-  if (hMod) {
-    auto getVer = (RtlGetVersionPtr)::GetProcAddress(hMod, "RtlGetVersion");
-    if (getVer) {
-      RTL_OSVERSIONINFOEXW info{};
-      info.dwOSVersionInfoSize = sizeof(info);
-      if (getVer((PRTL_OSVERSIONINFOW)&info) == STATUS_SUCCESS) {
-        return llvm::VersionTuple(info.dwMajorVersion, info.dwMinorVersion, 0,
-                                  info.dwBuildNumber);
-      }
-    }
-  }
-
-  OSVERSIONINFOEX info;
-  ZeroMemory(&info, sizeof(OSVERSIONINFOEX));
-  info.dwOSVersionInfoSize = sizeof(OSVERSIONINFOEX);
-#if defined(_MSC_VER)
-#pragma warning(push)
-#pragma warning(disable : 4996)
-#endif // _MSC_VER
-#if defined(__clang__)
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated"
-#endif // __clang__
-  // Starting with Microsoft SDK for Windows 8.1, this function is deprecated
-  // in favor of the new Windows Version Helper APIs.  Since we don't specify a
-  // minimum SDK version, it's easier to simply disable the warning rather than
-  // try to support both APIs.
-  if (GetVersionEx((LPOSVERSIONINFO)&info) == 0)
-    return llvm::VersionTuple();
-#if defined(__clang__)
-#pragma clang diagnostic pop
-#endif // __clang__
-#if defined(_MSC_VER)
-#pragma warning(pop)
-#endif // _MSC_VER
-
-  return llvm::VersionTuple(info.dwMajorVersion, info.dwMinorVersion, 0,
-                            info.dwBuildNumber);
-}
-
-inline bool MakeErrMsg(std::string *ErrMsg, const std::string &prefix) {
-  if (!ErrMsg)
-    return true;
-  char *buffer = NULL;
-  DWORD LastError = GetLastError();
-  DWORD R = FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER |
-                               FORMAT_MESSAGE_FROM_SYSTEM |
-                               FORMAT_MESSAGE_MAX_WIDTH_MASK,
-                           NULL, LastError, 0, (LPSTR)&buffer, 1, NULL);
-  if (R)
-    *ErrMsg = prefix + ": " + buffer;
-  else
-    *ErrMsg = prefix + ": Unknown error";
-  *ErrMsg += " (0x" + llvm::utohexstr(LastError) + ")";
+bool RunningWindows8OrGreater();
 
-  LocalFree(buffer);
-  return R != 0;
-}
+/// Returns the Windows version as Major.Minor.0.BuildNumber. Uses
+/// RtlGetVersion or GetVersionEx under the hood depending on what is available.
+/// GetVersionEx is deprecated, but this API exposes the build number which can
+/// be useful for working around certain kernel bugs.
+llvm::VersionTuple GetWindowsOSVersion();
+
+bool MakeErrMsg(std::string *ErrMsg, const std::string &prefix);
 
 template <typename HandleTraits>
 class ScopedHandle {
   typedef typename HandleTraits::handle_type handle_type;
   handle_type Handle;
 
-  ScopedHandle(const ScopedHandle &other); // = delete;
-  void operator=(const ScopedHandle &other); // = delete;
+  ScopedHandle(const ScopedHandle &other) = delete;
+  void operator=(const ScopedHandle &other) = delete;
 public:
   ScopedHandle()
     : Handle(HandleTraits::GetInvalid()) {}
 
   explicit ScopedHandle(handle_type h)
     : Handle(h) {}
 
   ~ScopedHandle() {

 
 typedef ScopedHandle<CommonHandleTraits> ScopedCommonHandle;
 typedef ScopedHandle<FileHandleTraits>   ScopedFileHandle;
 typedef ScopedHandle<CryptContextTraits> ScopedCryptContext;
 typedef ScopedHandle<RegTraits>          ScopedRegHandle;
 typedef ScopedHandle<FindHandleTraits>   ScopedFindHandle;
 typedef ScopedHandle<JobHandleTraits>    ScopedJobHandle;
 
-namespace llvm {
 template <class T>
 class SmallVectorImpl;
 
 template <class T>
 typename SmallVectorImpl<T>::const_pointer
 c_str(SmallVectorImpl<T> &str) {
   str.push_back(0);
   str.pop_back();