Add Windows error code and tidy formatting for system errors

[probinson]

Summary

In MakeErrMsg, the Windows error text was being pasted directly at the
end of the provided prefix, and only some callers provided appropriate
punctuation. Add the punctuation in MakeErrMsg and remove it from callers.
Also append the actual Windows error code, which can be really useful.

[probinson]

Using Greg Bedwell's test suggestion:

> set SYSTEMROOT=
> clang -c t.cpp

I was able to test the update to error reporting re. a crypto context. I got this message:

fatal error: error in backend: Could not acquire a cryptographic contextInvalid Signature.

(1) There ought to be nicer punctuation, e.g. "context: Invalid Signature."
(2) Providing the actual Windows error number could be more helpful than the text.

With this patch I get:

fatal error: error in backend: Could not acquire a cryptographic context: Invalid Signature. (0x80090006)

[rnk]

lgtm

lib/Support/Windows/WindowsSupport.h

51 ↗

(On Diff #40838)

According to MSDN, this call can update the last error code. You might want to cache it locally at the beginning of this function.

[probinson]

lib/Support/Windows/WindowsSupport.h

51 ↗

(On Diff #40838)

Ah, thanks for catching that.

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

if (!filename) {
     // Dummy library that represents "search all handles".
     // This is mostly to ensure that the return value still shows up as "valid".
     return DynamicLibrary(&OpenedHandles);
   }
 
   SmallVector<wchar_t, MAX_PATH> filenameUnicode;
   if (std::error_code ec = windows::UTF8ToUTF16(filename, filenameUnicode)) {
     SetLastError(ec.value());
-    MakeErrMsg(errMsg, std::string(filename) + ": Can't convert to UTF-16: ");
+    MakeErrMsg(errMsg, std::string(filename) + ": Can't convert to UTF-16");
     return DynamicLibrary();
   }
   
   HMODULE a_handle = LoadLibraryW(filenameUnicode.data());
 
   if (a_handle == 0) {
-    MakeErrMsg(errMsg, std::string(filename) + ": Can't open : ");
+    MakeErrMsg(errMsg, std::string(filename) + ": Can't open");
     return DynamicLibrary();
   }
 
   if (OpenedHandles == 0)
     OpenedHandles = new DenseSet<HMODULE>();
 
   // If we've already loaded this library, FreeLibrary() the handle in order to
   // keep the internal refcount at +1.

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

static DWORD getProtection(const void *addr) {
   if (sizeof(info) == ::VirtualQuery(addr, &info, sizeof(info))) {
     return info.Protect;
   }
   return 0;
 }
 
 bool Memory::setWritable(MemoryBlock &M, std::string *ErrMsg) {
   if (!setRangeWritable(M.Address, M.Size)) {
-    return MakeErrMsg(ErrMsg, "Cannot set memory to writeable: ");
+    return MakeErrMsg(ErrMsg, "Cannot set memory to writeable");
   }
   return true;
 }
 
 bool Memory::setExecutable(MemoryBlock &M, std::string *ErrMsg) {
   if (!setRangeExecutable(M.Address, M.Size)) {
-    return MakeErrMsg(ErrMsg, "Cannot set memory to executable: ");
+    return MakeErrMsg(ErrMsg, "Cannot set memory to executable");
   }
   return true;
 }
 
 bool Memory::setRangeWritable(const void *Addr, size_t Size) {
   DWORD prot = getProtection(Addr);
   if (!prot)
     return false;

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

if (path->empty()) {
     if (path::widenPath(fname, fnameUnicode))
       return INVALID_HANDLE_VALUE;
   }
   h = CreateFileW(fnameUnicode.data(), fd ? GENERIC_WRITE : GENERIC_READ,
                   FILE_SHARE_READ, &sa, fd == 0 ? OPEN_EXISTING : CREATE_ALWAYS,
                   FILE_ATTRIBUTE_NORMAL, NULL);
   if (h == INVALID_HANDLE_VALUE) {
     MakeErrMsg(ErrMsg, fname + ": Can't open file for " +
-        (fd ? "input: " : "output: "));
+        (fd ? "input" : "output"));
   }
 
   return h;
 }
 
 /// ArgNeedsQuotes - Check whether argument needs to be quoted when calling
 /// CreateProcess.
 static bool ArgNeedsQuotes(const char *Str) {
else if (SecondsToWait > 0)
     milliSecondsToWait = SecondsToWait * 1000;
 
   ProcessInfo WaitResult = PI;
   DWORD WaitStatus = WaitForSingleObject(PI.ProcessHandle, milliSecondsToWait);
   if (WaitStatus == WAIT_TIMEOUT) {
     if (SecondsToWait) {
       if (!TerminateProcess(PI.ProcessHandle, 1)) {
         if (ErrMsg)
-          MakeErrMsg(ErrMsg, "Failed to terminate timed-out program.");
+          MakeErrMsg(ErrMsg, "Failed to terminate timed-out program");
 
         // -2 indicates a crash or timeout as opposed to failure to execute.
         WaitResult.ReturnCode = -2;
         CloseHandle(PI.ProcessHandle);
         return WaitResult;
       }
       WaitForSingleObject(PI.ProcessHandle, INFINITE);
       CloseHandle(PI.ProcessHandle);
     } else {
       // Non-blocking wait.
       return ProcessInfo();
     }
   }
 
   // Get its exit status.
   DWORD status;
   BOOL rc = GetExitCodeProcess(PI.ProcessHandle, &status);
   DWORD err = GetLastError();
   if (err != ERROR_INVALID_HANDLE)
     CloseHandle(PI.ProcessHandle);
 
   if (!rc) {
     SetLastError(err);
     if (ErrMsg)
-      MakeErrMsg(ErrMsg, "Failed getting status for program.");
+      MakeErrMsg(ErrMsg, "Failed getting status for program");
 
     // -2 indicates a crash or timeout as opposed to failure to execute.
     WaitResult.ReturnCode = -2;
     return WaitResult;
   }
 
   if (!status)
     return WaitResult;

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

 #undef _WIN32_IE
 
 // Require at least Windows 7 API.
 #define _WIN32_WINNT 0x0601
 #define _WIN32_IE    0x0800 // MinGW at it again. FIXME: verify if still needed.
 #define WIN32_LEAN_AND_MEAN
 
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Config/config.h" // Get build system configuration settings
 #include "llvm/Support/Compiler.h"
 #include <system_error>
 #include <windows.h>
 #include <wincrypt.h>
 #include <cassert>
 #include <string>
 #include <vector>
 
 inline bool MakeErrMsg(std::string* ErrMsg, const std::string& prefix) {
   if (!ErrMsg)
     return true;
   char *buffer = NULL;
+  DWORD LastError = GetLastError();
   DWORD R = FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER |
-                          FORMAT_MESSAGE_FROM_SYSTEM,
-                          NULL, GetLastError(), 0, (LPSTR)&buffer, 1, NULL);
+                          FORMAT_MESSAGE_FROM_SYSTEM |
+                          FORMAT_MESSAGE_MAX_WIDTH_MASK,
+                          NULL, LastError, 0, (LPSTR)&buffer, 1, NULL);
   if (R)
-    *ErrMsg = prefix + buffer;
+    *ErrMsg = prefix + ": " + buffer;
   else
-    *ErrMsg = prefix + "Unknown error";
+    *ErrMsg = prefix + ": Unknown error";
+  *ErrMsg += " (0x" + llvm::utohexstr(LastError) + ")";
 
   LocalFree(buffer);
   return R != 0;
 }
 
 template <typename HandleTraits>
 class ScopedHandle {
   typedef typename HandleTraits::handle_type handle_type;