Fixes to possible GetLastError bugs

GetLastError needs to be called immediately after the function whose
error code it is retrieving. This has been made particularly important
by VS 2015 which sometimes clears the error code when allocating memory.
See bug 528394 for details of the underlying issue.

These changes came from code inspection that looked for patterns that
appeared wrong. None of the issues look critical, although that depends
on what callers do with the error codes.

BUG=529981

Review URL: https://codereview.chromium.org/1337223002

Cr-Commit-Position: refs/heads/master@{#349481}

6 files changed, 15 insertions, 6 deletions

diff --git a/base/debug/stack_trace_win.cc b/base/debug/stack_trace_win.cc
index 94c2e96..d5be5ef 100644
--- a/base/debug/stack_trace_win.cc
+++ b/base/debug/stack_trace_win.cc
@@ -77,8 +77,8 @@ bool InitializeSymbols() {
   if (!SymGetSearchPathW(GetCurrentProcess(),
                          symbols_path.get(),
                          kSymbolsArraySize)) {
-    DLOG(WARNING) << "SymGetSearchPath failed: " << g_init_error;
     g_init_error = GetLastError();
+    DLOG(WARNING) << "SymGetSearchPath failed: " << g_init_error;
     return false;
   }
 
diff --git a/chrome/browser/password_manager/password_manager_util_win.cc b/chrome/browser/password_manager/password_manager_util_win.cc
index 994a174..d5f7f3e 100644
--- a/chrome/browser/password_manager/password_manager_util_win.cc
+++ b/chrome/browser/password_manager/password_manager_util_win.cc
@@ -129,6 +129,7 @@ bool CheckBlankPasswordWithPrefs(const WCHAR* username,
                                    LOGON32_PROVIDER_DEFAULT,
                                    &handle);
 
+    auto last_error = GetLastError();
     // Win XP and later return ERROR_ACCOUNT_RESTRICTION for blank password.
     if (logon_result)
       CloseHandle(handle);
@@ -138,7 +139,7 @@ bool CheckBlankPasswordWithPrefs(const WCHAR* username,
     // ERROR_ACCOUNT_RESTRICTION.
     // http://msdn.microsoft.com/en-us/library/windows/desktop/ms681385
     blank_password = (logon_result ||
-                      GetLastError() == ERROR_ACCOUNT_RESTRICTION);
+                      last_error == ERROR_ACCOUNT_RESTRICTION);
   }
 
   // Account for clock skew between pulling the password age and
diff --git a/sandbox/win/src/process_thread_interception.cc b/sandbox/win/src/process_thread_interception.cc
index e6c8c2e..2d459b6 100644
--- a/sandbox/win/src/process_thread_interception.cc
+++ b/sandbox/win/src/process_thread_interception.cc
@@ -279,6 +279,8 @@ BOOL WINAPI TargetCreateProcessW(CreateProcessWFunction orig_CreateProcessW,
   if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled())
     return FALSE;
 
+  // Don't call GetLastError before InitCalled() succeeds because kernel32 may
+  // not be mapped yet.
   DWORD original_error = ::GetLastError();
 
   do {
@@ -338,6 +340,8 @@ BOOL WINAPI TargetCreateProcessA(CreateProcessAFunction orig_CreateProcessA,
   if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled())
     return FALSE;
 
+  // Don't call GetLastError before InitCalled() succeeds because kernel32 may
+  // not be mapped yet.
   DWORD original_error = ::GetLastError();
 
   do {
diff --git a/sandbox/win/src/restricted_token.cc b/sandbox/win/src/restricted_token.cc
index df5a87c..f0fc4cb 100644
--- a/sandbox/win/src/restricted_token.cc
+++ b/sandbox/win/src/restricted_token.cc
@@ -140,6 +140,7 @@ DWORD RestrictedToken::GetRestrictedToken(
                                 SecurityIdentification, TokenPrimary,
                                 &new_token_handle);
   }
+  auto last_error = ::GetLastError();
 
   if (deny_only_array)
     delete[] deny_only_array;
@@ -151,7 +152,7 @@ DWORD RestrictedToken::GetRestrictedToken(
     delete[] privileges_to_disable_array;
 
   if (!result)
-    return ::GetLastError();
+    return last_error;
 
   base::win::ScopedHandle new_token(new_token_handle);
 
diff --git a/sandbox/win/src/restricted_token_utils.cc b/sandbox/win/src/restricted_token_utils.cc
index 7f70e88..4a3d05c 100644
--- a/sandbox/win/src/restricted_token_utils.cc
+++ b/sandbox/win/src/restricted_token_utils.cc
@@ -218,9 +218,10 @@ DWORD SetTokenIntegrityLevel(HANDLE token, IntegrityLevel integrity_level) {
   DWORD size = sizeof(TOKEN_MANDATORY_LABEL) + ::GetLengthSid(integrity_sid);
   BOOL result = ::SetTokenInformation(token, TokenIntegrityLevel, &label,
                                       size);
+  auto last_error = ::GetLastError();
   ::LocalFree(integrity_sid);
 
-  return result ? ERROR_SUCCESS : ::GetLastError();
+  return result ? ERROR_SUCCESS : last_error;
 }
 
 DWORD SetProcessIntegrityLevel(IntegrityLevel integrity_level) {
diff --git a/sandbox/win/tools/launcher/launcher.cc b/sandbox/win/tools/launcher/launcher.cc
index 4f39822..a037702 100644
--- a/sandbox/win/tools/launcher/launcher.cc
+++ b/sandbox/win/tools/launcher/launcher.cc
@@ -102,17 +102,19 @@ DWORD StartRestrictedProcessInJob(wchar_t* command_line,
   {
     HANDLE temp_thread = process_info.thread_handle();
     if (!::SetThreadToken(&temp_thread, impersonation_token.Get())) {
+      auto last_error = ::GetLastError();
       ::TerminateProcess(process_info.process_handle(),
                          0);  // exit code
-      return ::GetLastError();
+      return last_error;
     }
   }
 
   err_code = job.AssignProcessToJob(process_info.process_handle());
   if (ERROR_SUCCESS != err_code) {
+    auto last_error = ::GetLastError();
     ::TerminateProcess(process_info.process_handle(),
                        0);  // exit code
-    return ::GetLastError();
+    return last_error;
   }
 
   // Start the application