commit 6bf4d2ab1a7e3e2254307de15f36a17509488125
author Vyacheslav Egorov <vegorov@google.com> Wed Dec 12 18:34:13 2018 +0000
committer commit-bot@chromium.org <commit-bot@chromium.org> Wed Dec 12 18:34:13 2018 +0000
tree 9be73ab362d360bb116ac0fc130b090d7145b4a0
parent 4ed049d7826026cb96d4709b62044f581e6458ef

[vm] Do not suppress WER when running VM outside of our test suite.

We prevent our crashing tests from hitting timeouts on the bots
by disabling Windows Error Reporting UI via SetErrorMode.

However this also disables builtin crash dump generation functionality
that WER has.

This change moves WER suppression for GP faults under a flag to
make sure that we can collect crash dumps when VM crashes on
user machines.

We also make sure that our exception handler call abort()
instead of calling exit() - because exit would not cause
WER to generate a dump.

Bug: https://github.com/flutter/flutter/issues/22558
Change-Id: I42f3e31cfaaa578f6a040b8f10621e5663cddc09
Reviewed-on: https://dart-review.googlesource.com/c/87061
Auto-Submit: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>

runtime/bin/platform_win.cc

diff --git a/runtime/bin/platform_win.cc b/runtime/bin/platform_win.cc
index b0fd774..5cdf6fc 100644
--- a/runtime/bin/platform_win.cc
+++ b/runtime/bin/platform_win.cc
@@ -43,11 +43,21 @@
     // Disable the message box for assertions in the CRT in Debug builds.
     // See: https://msdn.microsoft.com/en-us/library/1y71x448.aspx
     _CrtSetReportMode(_CRT_ASSERT, 0);
+
     // Disable dialog boxes for "critical" errors or when OpenFile cannot find
-    // the requested file. See:
+    // the requested file. However only disable error boxes for general
+    // protection faults if an environment variable is set. Passing
+    // SEM_NOGPFAULTERRORBOX completely disables WindowsErrorReporting (WER)
+    // for the process, which means users loose ability to enable local dump
+    // archiving to collect minidumps for Dart VM crashes.
+    // Our test runner would set DART_SUPPRESS_WER to suppress WER UI during
+    // test suite execution.
     // See: https://msdn.microsoft.com/en-us/library/windows/desktop/ms680621(v=vs.85).aspx
-    SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOOPENFILEERRORBOX |
-                 SEM_NOGPFAULTERRORBOX);
+    UINT uMode = SEM_FAILCRITICALERRORS | SEM_NOOPENFILEERRORBOX;
+    if (getenv("DART_SUPPRESS_WER") != nullptr) {
+      uMode |= SEM_NOGPFAULTERRORBOX;
+    }
+    SetErrorMode(uMode);
 #ifndef PRODUCT
     // Set up global exception handler to be able to dump stack trace on crash.
     SetExceptionHandler();
@@ -71,8 +81,12 @@
           ExceptionInfo->ExceptionRecord->ExceptionFlags,
           ExceptionInfo->ExceptionRecord->ExceptionAddress);
       Dart_DumpNativeStackTrace(ExceptionInfo->ContextRecord);
-      const int kAbortExitCode = 3;
-      Platform::Exit(kAbortExitCode);
+      Console::RestoreConfig();
+      // TODO(zra): Remove once VM shuts down cleanly.
+      ::dart::private_flag_windows_run_tls_destructors = false;
+      // Note: we want to abort(...) here instead of exiting because exiting
+      // would not cause WER to generate a minidump.
+      abort();
     }
     return EXCEPTION_CONTINUE_SEARCH;
   }

tools/testing/dart/test_suite.dart

diff --git a/tools/testing/dart/test_suite.dart b/tools/testing/dart/test_suite.dart
index cb2ca1e..e9892c8 100644
--- a/tools/testing/dart/test_suite.dart
+++ b/tools/testing/dart/test_suite.dart
@@ -128,9 +128,14 @@
     _environmentOverrides = {
       'DART_CONFIGURATION': configuration.configurationDirectory,
     };
-    if (configuration.copyCoreDumps && Platform.isWindows) {
-      _environmentOverrides['DART_CRASHPAD_HANDLER'] =
-          new Path(buildDir + '/crashpad_handler.exe').absolute.toNativePath();
+    if (Platform.isWindows) {
+      _environmentOverrides['DART_SUPPRESS_WER'] = '1';
+      if (configuration.copyCoreDumps) {
+        _environmentOverrides['DART_CRASHPAD_HANDLER'] =
+            new Path(buildDir + '/crashpad_handler.exe')
+                .absolute
+                .toNativePath();
+      }
     }
   }