Diff
Modified: trunk/Tools/ChangeLog (225936 => 225937)
--- trunk/Tools/ChangeLog 2017-12-14 23:13:41 UTC (rev 225936)
+++ trunk/Tools/ChangeLog 2017-12-14 23:26:12 UTC (rev 225937)
@@ -1,3 +1,30 @@
+2017-12-14 Alexey Proskuryakov <a...@apple.com>
+
+ Improve leaks detector output
+ https://bugs.webkit.org/show_bug.cgi?id=180828
+
+ Reviewed by Joseph Pecoraro.
+
+ Fixing two issues:
+ 1. run-leaks omits many lines from leaks tool output, making it incompatible with
+ other tools. Notably, symbolication cannot be performed.
+ 2. run-leaks output goes to "run-webkit-tests --debug-rwt-logging" output. This
+ makes it much longer than needed, sometimes even overloading buildbot. We don't
+ need full output in test log, as separate files are created for each of these.
+
+ * Scripts/run-leaks: Represent each line in leaks output when parsing, and print
+ everything except for explicitly excluded leaks. From my testing and reading
+ the code, it looks like none of our tools should be broken by this change.
+
+ * Scripts/webkitpy/port/leakdetector.py: I couldn't find a way to run a helper tool
+ without dumping all of its output to debug log, so switched to using a file for leaks.
+
+ * Scripts/webkitperl/run-leaks_unittest/run-leaks-report-v1.0.pl:
+ * Scripts/webkitperl/run-leaks_unittest/run-leaks-report-v2.0-new.pl:
+ * Scripts/webkitperl/run-leaks_unittest/run-leaks-report-v2.0-old.pl:
+ * Scripts/webkitpy/port/leakdetector_unittest.py:
+ Updated tests for new behavior.
+
2017-12-14 Brady Eidson <beid...@apple.com>
REGRESSION (r225789): API tests WKProcessPool.InitialWarmedProcessUsed and WebKit.WebsiteDataStoreCustomPaths are failing.
Modified: trunk/Tools/Scripts/run-leaks (225936 => 225937)
--- trunk/Tools/Scripts/run-leaks 2017-12-14 23:13:41 UTC (rev 225936)
+++ trunk/Tools/Scripts/run-leaks 2017-12-14 23:26:12 UTC (rev 225937)
@@ -46,15 +46,18 @@
"Usage: " . basename($0) . " [options] pid | executable name\n" .
" --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" .
" --help Show this help message.\n";
my @callStacksToExclude = ();
my @typesToExclude = ();
+ my $outputPath;
my $help = 0;
my $getOptionsResult = GetOptions(
'exclude-callstack:s' => \@callStacksToExclude,
'exclude-type:s' => \@typesToExclude,
+ 'output-file:s' => \$outputPath,
'help' => \$help
);
my $pidOrExecutableName = $ARGV[0];
@@ -76,26 +79,31 @@
return 1;
}
- my $leakList = parseLeaksOutput(@$leaksOutput);
- if (!$leakList) {
+ my $parsedOutput = parseLeaksOutput(@$leaksOutput);
+ if (!$parsedOutput) {
return 1;
}
# Filter output.
- my $leakCount = @$leakList;
- removeMatchingRecords(@$leakList, "callStack", @callStacksToExclude);
- removeMatchingRecords(@$leakList, "type", @typesToExclude);
- my $excludeCount = $leakCount - @$leakList;
+ # FIXME: Adjust the overall leak count in the output accordingly. This will affect callers, notably leakdetector.py.
+ my $parsedLineCount = @$parsedOutput;
+ removeMatchingRecords(@$parsedOutput, "callStack", @callStacksToExclude);
+ removeMatchingRecords(@$parsedOutput, "type", @typesToExclude);
+ my $excludeCount = $parsedLineCount - @$parsedOutput;
- # Dump results.
- print $leaksOutput->[0];
- print $leaksOutput->[1];
- foreach my $leak (@$leakList) {
- print $leak->{"leaksOutput"};
+ my $outputFH;
+ if (defined $outputPath) {
+ open($outputFH , '>', $outputPath) or die "Could not open $outputPath for writing";
+ } else {
+ $outputFH = *STDOUT;
}
+ foreach my $leak (@$parsedOutput) {
+ print $outputFH $leak->{"leaksOutput"};
+ }
+
if ($excludeCount) {
- print "$excludeCount leaks excluded (not printed)\n";
+ print $outputFH "$excludeCount leaks excluded (not printed)\n";
}
return 0;
@@ -132,24 +140,22 @@
#
# We treat every line except for Process 00000: and Leak: as optional
- # Skip header section until the first two "Process " lines.
- # FIXME: In the future we may wish to propagate the header section through to our output.
- until ($leaksOutput->[0] =~ /^Process /) {
- shift @$leaksOutput;
- }
+ my $leakCount;
+ my @parsedOutput = ();
+ my $parsingLeak = 0;
+ my $parsedLeakCount = 0;
+ for my $line (@$leaksOutput) {
+ if ($line =~ /^Process \d+: (\d+) leaks?/) {
+ $leakCount = $1;
+ }
- my ($leakCount) = ($leaksOutput->[1] =~ /[[:blank:]]+([0-9]+)[[:blank:]]+leaks?/);
- if (!defined($leakCount)) {
- reportError("Could not parse leak count reported by leaks tool.");
- return;
- }
+ if ($line eq "\n") {
+ $parsingLeak = 0;
+ }
- my @leakList = ();
- for my $line (@$leaksOutput) {
- next if $line =~ /^Process/;
- next if $line =~ /^node buffer added/;
-
if ($line =~ /^Leak: /) {
+ $parsingLeak = 1;
+ $parsedLeakCount++;
my ($address) = ($line =~ /Leak: ([[:xdigit:]x]+)/);
if (!defined($address)) {
reportError("Could not parse Leak address.");
@@ -162,6 +168,9 @@
return;
}
+ # FIXME: This code seems wrong, the leaks tool doesn't actually use single quotes.
+ # We should reconcile with other format changes that happened since, such as the
+ # addition of zone information.
my ($type) = ($line =~ /'([^']+)'/); #'
if (!defined($type)) {
$type = ""; # The leaks tool sometimes omits the type.
@@ -174,22 +183,26 @@
"callStack" => "", # The leaks tool sometimes omits the call stack.
"leaksOutput" => $line
);
- push(@leakList, \%leak);
- } else {
- $leakList[$#leakList]->{"leaksOutput"} .= $line;
+ push(@parsedOutput, \%leak);
+ } elsif ($parsingLeak) {
+ $parsedOutput[$#parsedOutput]->{"leaksOutput"} .= $line;
if ($line =~ /Call stack:/) {
- $leakList[$#leakList]->{"callStack"} = $line;
+ $parsedOutput[$#parsedOutput]->{"callStack"} = $line;
}
+ } else {
+ my %nonLeakLine = (
+ "leaksOutput" => $line
+ );
+ push(@parsedOutput, \%nonLeakLine);
}
}
- if (@leakList != $leakCount) {
- my $parsedLeakCount = @leakList;
- reportError("Parsed leak count($parsedLeakCount) does not match leak count reported by leaks tool($leakCount).");
+ if ($parsedLeakCount != $leakCount) {
+ reportError("Parsed leak count ($parsedLeakCount) does not match leak count reported by leaks tool ($leakCount).");
return;
}
- return \@leakList;
+ return \@parsedOutput;
}
sub removeMatchingRecords(\@$\@)
@@ -200,7 +213,7 @@
my $record = $recordList->[$i];
foreach my $regexp (@$regexpList) {
- if ($record->{$key} =~ $regexp) {
+ if (defined $record->{$key} and $record->{$key} =~ $regexp) {
splice(@$recordList, $i, 1);
next RECORD;
}
Modified: trunk/Tools/Scripts/webkitperl/run-leaks_unittest/run-leaks-report-v1.0.pl (225936 => 225937)
--- trunk/Tools/Scripts/webkitperl/run-leaks_unittest/run-leaks-report-v1.0.pl 2017-12-14 23:13:41 UTC (rev 225936)
+++ trunk/Tools/Scripts/webkitperl/run-leaks_unittest/run-leaks-report-v1.0.pl 2017-12-14 23:26:12 UTC (rev 225937)
@@ -63,6 +63,12 @@
my $expectedOutput =
[
{
+ 'leaksOutput' => 'Process 1602: 86671 nodes malloced for 13261 KB'
+ },
+ {
+ 'leaksOutput' => 'Process 1602: 8 leaks for 160 total leaked bytes.'
+ },
+ {
'leaksOutput' => join('', split(/\n/, <<EOF)),
Leak: 0x114d54708 size=24 zone: _javascript_Core FastMalloc_0x7fff70a09d20
Modified: trunk/Tools/Scripts/webkitperl/run-leaks_unittest/run-leaks-report-v2.0-new.pl (225936 => 225937)
--- trunk/Tools/Scripts/webkitperl/run-leaks_unittest/run-leaks-report-v2.0-new.pl 2017-12-14 23:13:41 UTC (rev 225936)
+++ trunk/Tools/Scripts/webkitperl/run-leaks_unittest/run-leaks-report-v2.0-new.pl 2017-12-14 23:26:12 UTC (rev 225937)
@@ -78,6 +78,54 @@
my $expectedOutput =
[
{
+ 'leaksOutput' => 'Process: DumpRenderTree [29903]'
+ },
+ {
+ 'leaksOutput' => 'Path: /Volumes/Data/Build/Debug/DumpRenderTree'
+ },
+ {
+ 'leaksOutput' => 'Load Address: 0x102116000'
+ },
+ {
+ 'leaksOutput' => 'Identifier: DumpRenderTree'
+ },
+ {
+ 'leaksOutput' => 'Version: ??? (???)'
+ },
+ {
+ 'leaksOutput' => 'Code Type: X86-64 (Native)'
+ },
+ {
+ 'leaksOutput' => 'Parent Process: Python [29892]'
+ },
+ {
+ 'leaksOutput' => ''
+ },
+ {
+ 'leaksOutput' => 'Date/Time: 2011-11-14 11:12:45.706 -0800'
+ },
+ {
+ 'leaksOutput' => 'OS Version: Mac OS X 10.7.2 (11C74)'
+ },
+ {
+ 'leaksOutput' => 'Report Version: 7'
+ },
+ {
+ 'leaksOutput' => ''
+ },
+ {
+ 'leaksOutput' => 'leaks Report Version: 2.0'
+ },
+ {
+ 'leaksOutput' => 'leaks(12871,0xacdfa2c0) malloc: process 89617 no longer exists, stack logs deleted from /tmp/stack-logs.89617.DumpRenderTree.A2giy6.index'
+ },
+ {
+ 'leaksOutput' => 'Process 29903: 60015 nodes malloced for 7290 KB'
+ },
+ {
+ 'leaksOutput' => 'Process 29903: 2 leaks for 1008 total leaked bytes.'
+ },
+ {
'leaksOutput' => join('', split(/\n/, <<EOF)),
Leak: 0x7f9a3a612810 size=576 zone: DefaultMallocZone_0x10227b000 URLConnectionLoader::LoaderConnectionEventQueue C++ CFNetwork
0x7f3af460 0x00007fff 0x7edf2f40 0x00007fff `.:.....@/.~....
Modified: trunk/Tools/Scripts/webkitperl/run-leaks_unittest/run-leaks-report-v2.0-old.pl (225936 => 225937)
--- trunk/Tools/Scripts/webkitperl/run-leaks_unittest/run-leaks-report-v2.0-old.pl 2017-12-14 23:13:41 UTC (rev 225936)
+++ trunk/Tools/Scripts/webkitperl/run-leaks_unittest/run-leaks-report-v2.0-old.pl 2017-12-14 23:26:12 UTC (rev 225937)
@@ -58,6 +58,54 @@
my $expectedOutput =
[
{
+ 'leaksOutput' => 'leaks Report Version: 2.0'
+ },
+ {
+ 'leaksOutput' => 'Process: Safari [53606]'
+ },
+ {
+ 'leaksOutput' => 'Path: /Applications/Safari.app/Contents/MacOS/Safari'
+ },
+ {
+ 'leaksOutput' => 'Load Address: 0x100000000'
+ },
+ {
+ 'leaksOutput' => 'Identifier: com.apple.Safari'
+ },
+ {
+ 'leaksOutput' => 'Version: 5.0 (6533.9)'
+ },
+ {
+ 'leaksOutput' => 'Build Info: WebBrowser-75330900~1'
+ },
+ {
+ 'leaksOutput' => 'Code Type: X86-64 (Native)'
+ },
+ {
+ 'leaksOutput' => 'Parent Process: perl5.10.0 [53599]'
+ },
+ {
+ 'leaksOutput' => ''
+ },
+ {
+ 'leaksOutput' => 'Date/Time: 2010-05-27 11:42:27.356 -0700'
+ },
+ {
+ 'leaksOutput' => 'OS Version: Mac OS X 10.6.3 (10D571)'
+ },
+ {
+ 'leaksOutput' => 'Report Version: 6'
+ },
+ {
+ 'leaksOutput' => ''
+ },
+ {
+ 'leaksOutput' => 'Process 53606: 112295 nodes malloced for 22367 KB'
+ },
+ {
+ 'leaksOutput' => 'Process 53606: 1 leak for 32 total leaked bytes.'
+ },
+ {
'leaksOutput' => join('', split(/\n/, <<EOF)),
Leak: 0x1118c0e60 size=32 zone: DefaultMallocZone_0x105a92000 string 'com.apple.quarantine'
Call stack: [thread 0x7fff70126be0]: | 0x100001e84 | NSApplicationMain | +[NSBundle(NSNibLoading) loadNibNamed:owner:] | +[NSBundle(NSNibLoading) _loadNibFile:nameTable:withZone:ownerBundle:] | loadNib | -[NSIBObjectData nibInstantiateWithOwner:topLevelObjects:] | -[NSSet makeObjectsPerformSelector:] | 0x100003494 | 0x1001013ff | 0x10014dbb9 | 0x10014d923 | 0x10014d7d7 | 0x10014ccd9 | 0x100149c8e | 0x100149bd8 | xar_open | xar_file_unserialize | xar_prop_unserialize | xar_prop_unserialize | strdup | malloc | malloc_zone_malloc
Modified: trunk/Tools/Scripts/webkitpy/port/leakdetector.py (225936 => 225937)
--- trunk/Tools/Scripts/webkitpy/port/leakdetector.py 2017-12-14 23:13:41 UTC (rev 225936)
+++ trunk/Tools/Scripts/webkitpy/port/leakdetector.py 2017-12-14 23:26:12 UTC (rev 225937)
@@ -62,12 +62,13 @@
]
return callstacks
- def _leaks_args(self, pid):
+ def _leaks_args(self, pid, leaks_output_path):
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)
return leaks_args
@@ -113,11 +114,14 @@
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:
# 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"
- leaks_output = self._port._run_script("run-leaks", self._leaks_args(process_pid), include_configuration_arguments=False, decode_output=False)
+ self._port._run_script("run-leaks", self._leaks_args(process_pid, leaks_output_path), 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())
return
@@ -126,12 +130,9 @@
count, excluded, bytes = self._parse_leaks_output(leaks_output)
adjusted_count = count - excluded
if not adjusted_count:
+ self._filesystem.remove(leaks_output_path)
return
- leaks_filename = self.leaks_file_name(process_name, process_pid)
- leaks_output_path = self._filesystem.join(self._port.results_directory(), leaks_filename)
- self._filesystem.write_binary_file(leaks_output_path, leaks_output)
-
# FIXME: Ideally we would not be logging from the worker process, but rather pass the leak
# information back to the manager and have it log.
if excluded:
Modified: trunk/Tools/Scripts/webkitpy/port/leakdetector_unittest.py (225936 => 225937)
--- trunk/Tools/Scripts/webkitpy/port/leakdetector_unittest.py 2017-12-14 23:13:41 UTC (rev 225936)
+++ trunk/Tools/Scripts/webkitpy/port/leakdetector_unittest.py 2017-12-14 23:26:12 UTC (rev 225937)
@@ -50,8 +50,8 @@
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', 1234]
- self.assertEqual(detector._leaks_args(1234), expected_args)
+ 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)
example_leaks_output = """Process 5122: 663744 nodes malloced for 78683 KB
Process 5122: 337301 leaks for 6525216 total leaked bytes.
