[iOS] run-webkit-tests records most DumpRenderTree.app crashes as time-outs
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Dec 2014 18:04:27 +0000 (18:04 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Dec 2014 18:04:27 +0000 (18:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=139143

Reviewed by David Kilzer.

Similar to the Windows-specific fix in <https://bugs.webkit.org/show_bug.cgi?id=37859>,
teach DumpRenderTree for iOS to write "#CRASHED" to the standard error stream when it
crashes. Run-webkit-tests will record as crashing the test associated with the
DumpRenderTree instance that wrote that string.

Currently almost all of the DumpRenderTree.app crashes are recorded by run-
webkit-tests as a time-out because ReportCrash(8) delays delivery of the
process exit notification for DumpRenderTree.app to LayoutTestRelay, which
launched DumpRenderTree.app, past the time-out time limit. Notice LayoutTestRelay
was launched by run-webkit-tests. So, run-webkit-tests kills LayoutTestRelay
(since it exceeded the time-out time limit) before it can inform rub-webkit-tests
about a crash.

Additionally, update the crash message format written to standard error when LayoutTestRelay
detects that {WebKitTestRunner, DumpRenderTree}.app crashed so as to be similar to the
crash message format used by WebKitTestRunner when it detects that the WebProcess crashed.
Then run-webkit-tests will collect the crash logs for {WebKitTestRunner, DumpRenderTree}.app
when they crash in their test machinery logic/UI process code.

* DumpRenderTree/mac/DumpRenderTree.mm:
(writeCrashedMessageOnFatalError): Added.
(dumpRenderTree): Register signal handler, writeCrashedMessageOnFatalError(), for signals:
SIGILL, SIGFPE, SIGBUS and SIGSEGV.
* LayoutTestRelay/LayoutTestRelay/LTRelayController.m:
(-[LTRelayController didCrashWithMessage:]): Emit a crash message with a format
similar to the format used by WebKitTestRunner so that run-webkit-tests will collect
the crash logs for WebKitTestRunner/DumpRenderTree.app.
* Scripts/webkitpy/port/driver.py:
(Driver.__init__): Update comment.
(Driver._check_for_driver_crash): Ditto.
* Scripts/webkitpy/port/ios.py:
(IOSSimulatorPort): Add class constant SUBPROCESS_CRASH_REGEX, which represents a compiled
regular expression. This constant is used as an optimization to avoid compiling the same
regular expression across invocations of _get_crash_log().
(IOSSimulatorPort._get_crash_log): Modified to parse the WebKitTestRunner-like crash message
for the subprocess name and pid. Also, moved variables crash_log, crash_logs, and now to be
closer to where they are used.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@176669 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/DumpRenderTree/mac/DumpRenderTree.mm
Tools/LayoutTestRelay/LayoutTestRelay/LTRelayController.m
Tools/Scripts/webkitpy/port/driver.py
Tools/Scripts/webkitpy/port/ios.py

index 27e687a..ae322ce 100644 (file)
@@ -1,3 +1,48 @@
+2014-12-02  Daniel Bates  <dabates@apple.com>
+
+        [iOS] run-webkit-tests records most DumpRenderTree.app crashes as time-outs
+        https://bugs.webkit.org/show_bug.cgi?id=139143
+
+        Reviewed by David Kilzer.
+
+        Similar to the Windows-specific fix in <https://bugs.webkit.org/show_bug.cgi?id=37859>,
+        teach DumpRenderTree for iOS to write "#CRASHED" to the standard error stream when it
+        crashes. Run-webkit-tests will record as crashing the test associated with the
+        DumpRenderTree instance that wrote that string.
+
+        Currently almost all of the DumpRenderTree.app crashes are recorded by run-
+        webkit-tests as a time-out because ReportCrash(8) delays delivery of the
+        process exit notification for DumpRenderTree.app to LayoutTestRelay, which
+        launched DumpRenderTree.app, past the time-out time limit. Notice LayoutTestRelay
+        was launched by run-webkit-tests. So, run-webkit-tests kills LayoutTestRelay
+        (since it exceeded the time-out time limit) before it can inform rub-webkit-tests
+        about a crash.
+
+        Additionally, update the crash message format written to standard error when LayoutTestRelay
+        detects that {WebKitTestRunner, DumpRenderTree}.app crashed so as to be similar to the
+        crash message format used by WebKitTestRunner when it detects that the WebProcess crashed.
+        Then run-webkit-tests will collect the crash logs for {WebKitTestRunner, DumpRenderTree}.app
+        when they crash in their test machinery logic/UI process code.
+
+        * DumpRenderTree/mac/DumpRenderTree.mm:
+        (writeCrashedMessageOnFatalError): Added.
+        (dumpRenderTree): Register signal handler, writeCrashedMessageOnFatalError(), for signals:
+        SIGILL, SIGFPE, SIGBUS and SIGSEGV.
+        * LayoutTestRelay/LayoutTestRelay/LTRelayController.m:
+        (-[LTRelayController didCrashWithMessage:]): Emit a crash message with a format
+        similar to the format used by WebKitTestRunner so that run-webkit-tests will collect
+        the crash logs for WebKitTestRunner/DumpRenderTree.app.
+        * Scripts/webkitpy/port/driver.py:
+        (Driver.__init__): Update comment.
+        (Driver._check_for_driver_crash): Ditto.
+        * Scripts/webkitpy/port/ios.py:
+        (IOSSimulatorPort): Add class constant SUBPROCESS_CRASH_REGEX, which represents a compiled
+        regular expression. This constant is used as an optimization to avoid compiling the same
+        regular expression across invocations of _get_crash_log().
+        (IOSSimulatorPort._get_crash_log): Modified to parse the WebKitTestRunner-like crash message
+        for the subprocess name and pid. Also, moved variables crash_log, crash_logs, and now to be
+        closer to where they are used.
+
 2014-12-01  Anders Carlsson  <andersca@apple.com>
 
         Remove WKBundleRemoveAllVisitedLinks
index 3fcde71..1e194a4 100644 (file)
@@ -1118,6 +1118,21 @@ static void prepareConsistentTestingEnvironment()
 #endif
 }
 
+#if PLATFORM(IOS)
+const char crashedMessage[] = "#CRASHED\n";
+
+void writeCrashedMessageOnFatalError(int signalCode)
+{
+    // Reset the default action for the signal so that we run ReportCrash(8) on pending and
+    // subsequent instances of the signal.
+    signal(signalCode, SIG_DFL);
+
+    // WRITE(2) and FSYNC(2) are considered safe to call from a signal handler by SIGACTION(2).
+    write(STDERR_FILENO, &crashedMessage[0], sizeof(crashedMessage) - 1);
+    fsync(STDERR_FILENO);
+}
+#endif
+
 void dumpRenderTree(int argc, const char *argv[])
 {
 #if PLATFORM(IOS)
@@ -1132,6 +1147,11 @@ void dumpRenderTree(int argc, const char *argv[])
     dup2(outfd, STDOUT_FILENO);
     int errfd = open(stderrPath, O_RDWR | O_NONBLOCK);
     dup2(errfd, STDERR_FILENO);
+
+    signal(SIGILL, &writeCrashedMessageOnFatalError);
+    signal(SIGFPE, &writeCrashedMessageOnFatalError);
+    signal(SIGBUS, &writeCrashedMessageOnFatalError);
+    signal(SIGSEGV, &writeCrashedMessageOnFatalError);
 #endif
 
     initializeGlobalsFromCommandLineOptions(argc, argv);
index 2a28c64..3529d40 100644 (file)
 - (void)didCrashWithMessage:(NSString *)message
 {
     [[self relay] disconnect];
-    NSString *crashMessage = [NSString stringWithFormat:@"\nCRASH: %@ %d\n", [self processName], [self pid]];
+    NSString *crashMessage = [NSString stringWithFormat:@"\n#CRASHED - %@ (pid %d)\n", [self processName], [self pid]];
 
     if (message)
         crashMessage = [crashMessage stringByAppendingFormat:@"%@\n", message];
index f038f2a..8422433 100644 (file)
@@ -130,10 +130,10 @@ class Driver(object):
         self._no_timeout = no_timeout
 
         self._driver_tempdir = None
-        # WebKitTestRunner can report back subprocess crashes by printing
-        # "#CRASHED - PROCESSNAME".  Since those can happen at any time
-        # and ServerProcess won't be aware of them (since the actual tool
-        # didn't crash, just a subprocess) we record the crashed subprocess name here.
+        # WebKitTestRunner/LayoutTestRelay can report back subprocess crashes by printing
+        # "#CRASHED - PROCESSNAME".  Since those can happen at any time and ServerProcess
+        # won't be aware of them (since the actual tool didn't crash, just a subprocess)
+        # we record the crashed subprocess name here.
         self._crashed_process_name = None
         self._crashed_pid = None
 
@@ -361,13 +361,13 @@ class Driver(object):
 
     def _check_for_driver_crash(self, error_line):
         if error_line == "#CRASHED\n":
-            # This is used on Windows to report that the process has crashed
+            # This is used on Windows and iOS to report that the process has crashed
             # See http://trac.webkit.org/changeset/65537.
             self._crashed_process_name = self._server_process.name()
             self._crashed_pid = self._server_process.pid()
         elif (error_line.startswith("#CRASHED - ")
             or error_line.startswith("#PROCESS UNRESPONSIVE - ")):
-            # WebKitTestRunner uses this to report that the WebProcess subprocess crashed.
+            # WebKitTestRunner/LayoutTestRelay uses this to report that the subprocess (e.g. WebProcess) crashed.
             match = re.match('#(?:CRASHED|PROCESS UNRESPONSIVE) - (\S+)', error_line)
             self._crashed_process_name = match.group(1) if match else 'WebProcess'
             match = re.search('pid (\d+)', error_line)
index d88d812..51f391e 100644 (file)
@@ -219,24 +219,33 @@ class IOSSimulatorPort(Port):
     def sample_file_path(self, name, pid):
         return self._filesystem.join(self.results_directory(), "{0}-{1}-sample.txt".format(name, pid))
 
+    SUBPROCESS_CRASH_REGEX = re.compile('#CRASHED - (?P<subprocess_name>\S+) \(pid (?P<subprocess_pid>\d+)\)')
+
     def _get_crash_log(self, name, pid, stdout, stderr, newer_than, time_fn=time.time, sleep_fn=time.sleep, wait_for_log=True):
         time_fn = time_fn or time.time
         sleep_fn = sleep_fn or time.sleep
-        crash_log = ''
-        crash_logs = CrashLogs(self.host)
-        now = time_fn()
 
-        crash_prefix = 'CRASH: '
+        # FIXME: We should collect the actual crash log for DumpRenderTree.app because it includes more
+        # information (e.g. exception codes) than is available in the stack trace written to standard error.
         stderr_lines = []
-        crash_lines = []
+        crashed_subprocess_name_and_pid = None  # e.g. ('DumpRenderTree.app', 1234)
         for line in (stderr or '').splitlines():
-            crash_lines.append(line) if line.startswith(crash_prefix) else stderr_lines.append(line)
-
-        for crash_line in crash_lines:
-            identifier, pid = crash_line[len(crash_prefix):].split(' ')
-            return self._get_crash_log(identifier, int(pid), stdout, '\n'.join(stderr_lines), newer_than, time_fn, sleep_fn, wait_for_log)
-
+            if not crashed_subprocess_name_and_pid:
+                match = self.SUBPROCESS_CRASH_REGEX.match(line)
+                if match:
+                    crashed_subprocess_name_and_pid = (match.group('subprocess_name'), int(match.group('subprocess_pid')))
+                    continue
+            stderr_lines.append(line)
+
+        if crashed_subprocess_name_and_pid:
+            return self._get_crash_log(crashed_subprocess_name_and_pid[0], crashed_subprocess_name_and_pid[1], stdout,
+                '\n'.join(stderr_lines), newer_than, time_fn, sleep_fn, wait_for_log)
+
+        # LayoutTestRelay crashed
         _log.debug('looking for crash log for %s:%s' % (name, str(pid)))
+        crash_log = ''
+        crash_logs = CrashLogs(self.host)
+        now = time_fn()
         deadline = now + 5 * int(self.get_option('child_processes', 1))
         while not crash_log and now <= deadline:
             crash_log = crash_logs.find_newest_log(name, pid, include_errors=True, newer_than=newer_than)