run-leaks should run leaks with --list (on Mojave)
authorap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Sep 2018 21:06:31 +0000 (21:06 +0000)
committerap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Sep 2018 21:06:31 +0000 (21:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187716
<rdar://problem/42261676>

Reviewed by Lucas Forschler.

Also enabled dumping memgraphs. We'll be pruning these aggressively, as they take
significant space.

* Scripts/run-leaks:
(main): Added an option to store memgraphs.
(runLeaks): As there is no way to test whether the new format is supported in advance,
we have to try with --list first, and retry if that fails. Also, made leaks operate
on a memgraph file if we are saving it anyway.

* Scripts/webkitpy/port/leakdetector.py:
(LeakDetector._leaks_args): Pass --memgraph-file to run-leaks.
(LeakDetector.leaks_file_name): Removed an incorrect comment.
(LeakDetector.memgraph_file_name): Added.
(LeakDetector.check_for_leaks): Changed how arguments are passed to _leaks_args.
It is a bit ugly that leaks path ends up being computed twice, but this is the least
ugly approach that I could find.

* Scripts/webkitpy/port/leakdetector_unittest.py: Updated for _leaks_args changes.

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

Tools/ChangeLog
Tools/Scripts/run-leaks
Tools/Scripts/webkitpy/port/leakdetector.py
Tools/Scripts/webkitpy/port/leakdetector_unittest.py

index 78cb212..09d96d6 100644 (file)
@@ -1,3 +1,30 @@
+2018-09-04  Alexey Proskuryakov  <ap@apple.com>
+
+        run-leaks should run leaks with --list (on Mojave)
+        https://bugs.webkit.org/show_bug.cgi?id=187716
+        <rdar://problem/42261676>
+
+        Reviewed by Lucas Forschler.
+
+        Also enabled dumping memgraphs. We'll be pruning these aggressively, as they take
+        significant space.
+
+        * Scripts/run-leaks:
+        (main): Added an option to store memgraphs.
+        (runLeaks): As there is no way to test whether the new format is supported in advance,
+        we have to try with --list first, and retry if that fails. Also, made leaks operate
+        on a memgraph file if we are saving it anyway.
+
+        * Scripts/webkitpy/port/leakdetector.py:
+        (LeakDetector._leaks_args): Pass --memgraph-file to run-leaks.
+        (LeakDetector.leaks_file_name): Removed an incorrect comment.
+        (LeakDetector.memgraph_file_name): Added.
+        (LeakDetector.check_for_leaks): Changed how arguments are passed to _leaks_args.
+        It is a bit ugly that leaks path ends up being computed twice, but this is the least
+        ugly approach that I could find.
+
+        * Scripts/webkitpy/port/leakdetector_unittest.py: Updated for _leaks_args changes.
+
 2018-09-04  Chris Dumez  <cdumez@apple.com>
 
         Add process pool configuration flag to turn on automatic process pre-warming
index af4b023..f09a469 100755 (executable)
@@ -34,7 +34,7 @@ use warnings;
 use File::Basename;
 use Getopt::Long;
 
-sub runLeaks($);
+sub runLeaks($$);
 sub parseLeaksOutput(\@);
 sub removeMatchingRecords(\@$\@);
 sub reportError($);
@@ -47,17 +47,20 @@ sub main()
         "  --exclude-callstack regexp   Exclude leaks whose call stacks match the regular expression 'regexp'.\n" .
         "  --exclude-type regexp        Exclude leaks whose data types match the regular expression 'regexp'.\n" .
         "  --output-file path           Store result in a file. If not specified, standard output is used.\n" .
+        "  --memgraph-file path         Store memgraph in a file.\n" .
         "  --help                       Show this help message.\n";
 
     my @callStacksToExclude = ();
     my @typesToExclude = ();
     my $outputPath;
+    my $memgraphPath;
     my $help = 0;
 
     my $getOptionsResult = GetOptions(
         'exclude-callstack:s' => \@callStacksToExclude,
         'exclude-type:s' => \@typesToExclude,
         'output-file:s' => \$outputPath,
+        'memgraph-file:s' => \$memgraphPath,
         'help' => \$help
     );
     my $pidOrExecutableName = $ARGV[0];
@@ -74,7 +77,7 @@ sub main()
     }
 
     # Run leaks tool.
-    my $leaksOutput = runLeaks($pidOrExecutableName);
+    my $leaksOutput = runLeaks($pidOrExecutableName, $memgraphPath);
     if (!$leaksOutput) {
         return 1;
     }
@@ -112,16 +115,28 @@ sub main()
 exit(main());
 
 # Returns the output of the leaks tool in list form.
-sub runLeaks($)
+sub runLeaks($$)
 {
-    my ($pidOrExecutableName) = @_;
-    
-    my @leaksOutput = `leaks $pidOrExecutableName`;
-    if (!@leaksOutput) {
-        reportError("Error running leaks tool.");
-        return;
+    my ($target, $memgraphPath) = @_;
+
+    if (defined $memgraphPath) {
+        `/usr/bin/leaks \"$target\" --outputGraph=\"$memgraphPath\"`;
+        $target = $memgraphPath;
     }
-    
+
+    # To get a result we can parse, we need to pass --list to all versions of the leaks tool
+    # that recognize it, but there is no certain way to tell in advance (the version of the
+    # tool is not always tied to OS version, and --help doesn't document the option in some
+    # of the tool versions where it's supported and needed).
+    print STDERR "Starting leaks\n";
+    my @leaksOutput = `/usr/bin/leaks \"$target\" --list`;
+
+    # FIXME: Remove the fallback once macOS Mojave is the oldest supported version.
+    my $leaksExitCode = $? >> 8;
+    if ($leaksExitCode > 1) {
+        @leaksOutput = `/usr/bin/leaks \"$target\"`;
+    }
+
     return \@leaksOutput;
 }
 
index beb7a9b..d07e095 100644 (file)
@@ -63,14 +63,15 @@ class LeakDetector(object):
         ]
         return callstacks
 
-    def _leaks_args(self, pid, leaks_output_path):
+    def _leaks_args(self, process_name, process_pid):
         leaks_args = []
         for callstack in self._callstacks_to_exclude_from_leaks():
             leaks_args += ['--exclude-callstack=%s' % callstack]
         for excluded_type in self._types_to_exclude_from_leaks():
             leaks_args += ['--exclude-type=%s' % excluded_type]
-        leaks_args += ['--output-file=%s' % leaks_output_path]
-        leaks_args.append(pid)
+        leaks_args += ['--output-file=%s' % self._filesystem.join(self._port.results_directory(), self.leaks_file_name(process_name, process_pid))]
+        leaks_args += ['--memgraph-file=%s' % self._filesystem.join(self._port.results_directory(), self.memgraph_file_name(process_name, process_pid))]
+        leaks_args.append(process_pid)
         return leaks_args
 
     def _parse_leaks_output(self, leaks_output):
@@ -83,9 +84,11 @@ class LeakDetector(object):
         return self._filesystem.glob(self._filesystem.join(directory, "*-leaks.txt"))
 
     def leaks_file_name(self, process_name, process_pid):
-        # We include the number of files this worker has already written in the name to prevent overwritting previous leak results..
         return "%s-%s-leaks.txt" % (process_name, process_pid)
 
+    def memgraph_file_name(self, process_name, process_pid):
+        return "%s-%s.memgraph" % (process_name, process_pid)
+
     def count_total_bytes_and_unique_leaks(self, leak_files):
         merge_depth = 5  # ORWT had a --merge-leak-depth argument, but that seems out of scope for the run-webkit-tests tool.
         args = [
@@ -115,13 +118,13 @@ class LeakDetector(object):
 
     def check_for_leaks(self, process_name, process_pid):
         _log.debug("Checking for leaks in %s" % process_name)
-        leaks_filename = self.leaks_file_name(process_name, process_pid)
-        leaks_output_path = self._filesystem.join(self._port.results_directory(), leaks_filename)
         try:
+            leaks_filename = self.leaks_file_name(process_name, process_pid)
+            leaks_output_path = self._filesystem.join(self._port.results_directory(), leaks_filename)
             # Oddly enough, run-leaks (or the underlying leaks tool) does not seem to always output utf-8,
             # thus we pass decode_output=False.  Without this code we've seen errors like:
             # "UnicodeDecodeError: 'utf8' codec can't decode byte 0x88 in position 779874: unexpected code byte"
-            self._port._run_script("run-leaks", self._leaks_args(process_pid, leaks_output_path), include_configuration_arguments=False, decode_output=False)
+            self._port._run_script("run-leaks", self._leaks_args(process_name, process_pid), include_configuration_arguments=False, decode_output=False)
             leaks_output = self._filesystem.read_binary_file(leaks_output_path)
         except ScriptError as e:
             _log.warn("Failed to run leaks tool: %s" % e.message_with_output())
index 25d4d42..1e6eb0b 100644 (file)
@@ -41,6 +41,9 @@ class LeakDetectorTest(unittest.TestCase):
                 self._filesystem = MockFileSystem()
                 self._executive = MockExecutive()
 
+            def results_directory(self):
+                return "/tmp/"
+
         return MockPort()
 
     def _make_detector(self):
@@ -50,8 +53,8 @@ class LeakDetectorTest(unittest.TestCase):
         detector = self._make_detector()
         detector._callstacks_to_exclude_from_leaks = lambda: ['foo bar', 'BAZ']
         detector._types_to_exclude_from_leaks = lambda: ['abcdefg', 'hi jklmno']
-        expected_args = ['--exclude-callstack=foo bar', '--exclude-callstack=BAZ', '--exclude-type=abcdefg', '--exclude-type=hi jklmno', '--output-file=leaks.txt', 1234]
-        self.assertEqual(detector._leaks_args(1234, 'leaks.txt'), expected_args)
+        expected_args = ['--exclude-callstack=foo bar', '--exclude-callstack=BAZ', '--exclude-type=abcdefg', '--exclude-type=hi jklmno', '--output-file=/tmp/DumpRenderTree-1234-leaks.txt', '--memgraph-file=/tmp/DumpRenderTree-1234.memgraph', 1234]
+        self.assertEqual(detector._leaks_args("DumpRenderTree", 1234), expected_args)
 
     example_leaks_output = """Process 5122: 663744 nodes malloced for 78683 KB
 Process 5122: 337301 leaks for 6525216 total leaked bytes.