diff --git a/build/util/count_ctors.py b/build/util/count_ctors.py deleted file mode 100644 index 5b3745f1c047..000000000000 --- a/build/util/count_ctors.py +++ /dev/null @@ -1,66 +0,0 @@ -#!/usr/bin/python -import json - -import re -import subprocess -import sys - - -def count_ctors(filename): - proc = subprocess.Popen( - ['readelf', '-W', '-S', filename], stdout=subprocess.PIPE) - - # Some versions of ld produce both .init_array and .ctors. So we have - # to check for both. - n_init_array_ctors = 0 - have_init_array = False - n_ctors_ctors = 0 - have_ctors = False - - for line in proc.stdout: - f = line.split() - if len(f) != 11: - continue - # Don't try to int()-parse the header line for the section summaries. - if not re.match("\\[\\d+\\]", f[0]): - continue - section_name, contents, size, align = f[1], f[2], int(f[5], 16), int(f[10]) - if section_name == ".ctors" and contents == "PROGBITS": - have_ctors = True - # Subtract 2 for the uintptr_t(-1) header and the null terminator. - n_ctors_ctors = size / align - 2 - if section_name == ".init_array" and contents == "INIT_ARRAY": - have_init_array = True - n_init_array_ctors = size / align - - if have_init_array: - # Even if we have .ctors, we shouldn't have any constructors in .ctors. - # Complain if .ctors does not look how we expect it to. - if have_ctors and n_ctors_ctors != 0: - print >>sys.stderr, "Unexpected .ctors contents for", filename - sys.exit(1) - return n_init_array_ctors - if have_ctors: - return n_ctors_ctors - - # We didn't find anything; somebody switched initialization mechanisms on - # us, or the binary is completely busted. Complain either way. - print >>sys.stderr, "Couldn't find .init_array or .ctors in", filename - sys.exit(1) - - -if __name__ == '__main__': - for f in sys.argv[1:]: - perfherder_data = { - "framework": {"name": "build_metrics"}, - "suites": [{ - "name": "compiler_metrics", - "subtests": [{ - "name": "num_static_constructors", - "value": count_ctors(f), - "alertChangeType": "absolute", - "alertThreshold": 3 - }]} - ] - } - print("PERFHERDER_DATA: %s" % json.dumps(perfherder_data)) diff --git a/config/rules.mk b/config/rules.mk index d020f0d286af..e68334bafe0f 100644 --- a/config/rules.mk +++ b/config/rules.mk @@ -780,7 +780,7 @@ define syms_template syms:: $(2) $(2): $(1) ifdef MOZ_CRASHREPORTER - $$(call py_action,dumpsymbols,$$(abspath $$<) $$(abspath $$@)) + $$(call py_action,dumpsymbols,$$(abspath $$<) $$(abspath $$@) $$(DUMP_SYMBOLS_FLAGS)) endif endef diff --git a/python/mozbuild/mozbuild/action/dumpsymbols.py b/python/mozbuild/mozbuild/action/dumpsymbols.py index a7d43721f3b9..b4511275a78a 100644 --- a/python/mozbuild/mozbuild/action/dumpsymbols.py +++ b/python/mozbuild/mozbuild/action/dumpsymbols.py @@ -11,7 +11,7 @@ import shutil import sys import os -def dump_symbols(target, tracking_file): +def dump_symbols(target, tracking_file, count_ctors=False): # Our tracking file, if present, will contain path(s) to the previously generated # symbols. Remove them in this case so we don't simply accumulate old symbols # during incremental builds. @@ -60,6 +60,8 @@ def dump_symbols(target, tracking_file): 'dist', 'crashreporter-symbols'), os.path.abspath(target)]) + if count_ctors: + args.append('--count-ctors') print('Running: %s' % ' '.join(args)) out_files = subprocess.check_output(args) with open(tracking_file, 'w') as fh: @@ -67,12 +69,19 @@ def dump_symbols(target, tracking_file): fh.flush() def main(argv): - if len(argv) != 2: - print("Usage: dumpsymbols.py ", - file=sys.stderr) - return 1 + parser = argparse.ArgumentParser( + usage="Usage: dumpsymbols.py ") + parser.add_argument("--count-ctors", + action="store_true", default=False, + help="Count static initializers") + parser.add_argument("library_or_program", + help="Path to library or program") + parser.add_argument("tracking_file", + help="Tracking file") + args = parser.parse_args() - return dump_symbols(*argv) + return dump_symbols(args.library_or_program, args.tracking_file, + args.count_ctors) if __name__ == '__main__': diff --git a/taskcluster/ci/build/linux.yml b/taskcluster/ci/build/linux.yml index c9a8a85bcb35..82964ef4cdf0 100644 --- a/taskcluster/ci/build/linux.yml +++ b/taskcluster/ci/build/linux.yml @@ -15,7 +15,6 @@ linux64/opt: config: - builds/releng_base_firefox.py - builds/releng_base_linux_64_builds.py - - builds/releng_sub_linux_configs/enable_count_ctors.py script: "mozharness/scripts/fx_desktop_build.py" secrets: true tooltool-downloads: public @@ -103,7 +102,6 @@ linux64/pgo: config: - builds/releng_base_firefox.py - builds/releng_base_linux_64_builds.py - - builds/releng_sub_linux_configs/enable_count_ctors.py script: "mozharness/scripts/fx_desktop_build.py" secrets: true tooltool-downloads: public @@ -312,7 +310,6 @@ linux/opt: config: - builds/releng_base_firefox.py - builds/releng_base_linux_32_builds.py - - builds/releng_sub_linux_configs/enable_count_ctors.py script: "mozharness/scripts/fx_desktop_build.py" secrets: true tooltool-downloads: public @@ -371,7 +368,6 @@ linux/pgo: config: - builds/releng_base_firefox.py - builds/releng_base_linux_32_builds.py - - builds/releng_sub_linux_configs/enable_count_ctors.py script: "mozharness/scripts/fx_desktop_build.py" secrets: true tooltool-downloads: public diff --git a/testing/mozharness/configs/builds/releng_sub_linux_configs/64_tup.py b/testing/mozharness/configs/builds/releng_sub_linux_configs/64_tup.py index 07811d10a024..bccbacc277a8 100644 --- a/testing/mozharness/configs/builds/releng_sub_linux_configs/64_tup.py +++ b/testing/mozharness/configs/builds/releng_sub_linux_configs/64_tup.py @@ -27,7 +27,6 @@ config = { 'SCCACHE_DISABLE': '1', }, 'mozconfig_variant': 'tup', - 'enable_count_ctors': False, # TODO: libxul.so needs to be linked for this to work 'disable_package_metrics': True, # TODO: the package needs to be created for this to work 'artifact_flag_build_variant_in_try': None, } diff --git a/testing/mozharness/configs/builds/releng_sub_linux_configs/enable_count_ctors.py b/testing/mozharness/configs/builds/releng_sub_linux_configs/enable_count_ctors.py deleted file mode 100644 index ba281be26727..000000000000 --- a/testing/mozharness/configs/builds/releng_sub_linux_configs/enable_count_ctors.py +++ /dev/null @@ -1,5 +0,0 @@ -config = { - # this enables counting constructors, which we only want to do on a - # very small number of variants (see bug 1261081) - 'enable_count_ctors': True -} diff --git a/testing/mozharness/mozharness/mozilla/building/buildbase.py b/testing/mozharness/mozharness/mozilla/building/buildbase.py index 839c6c35dfef..7f392f57e986 100755 --- a/testing/mozharness/mozharness/mozilla/building/buildbase.py +++ b/testing/mozharness/mozharness/mozilla/building/buildbase.py @@ -1083,24 +1083,6 @@ or run without that action (ie: --no-{action})" ) return revision.encode('ascii', 'replace') if revision else None - def _count_ctors(self): - """count num of ctors and set testresults.""" - dirs = self.query_abs_dirs() - python_path = os.path.join(dirs['abs_work_dir'], 'venv', 'bin', - 'python') - abs_count_ctors_path = os.path.join(dirs['abs_src_dir'], - 'build', - 'util', - 'count_ctors.py') - abs_libxul_path = os.path.join(dirs['abs_obj_dir'], - 'dist', - 'bin', - 'libxul.so') - - cmd = [python_path, abs_count_ctors_path, abs_libxul_path] - self.get_output_from_command(cmd, cwd=dirs['abs_src_dir'], - throw_exception=True) - def _query_props_set_by_mach(self, console_output=True, error_level=FATAL): mach_properties_path = os.path.join( self.query_abs_dirs()['abs_obj_dir'], 'dist', 'mach_build_properties.json' @@ -1717,12 +1699,6 @@ or run without that action (ie: --no-{action})" c = self.config - if c.get('enable_count_ctors'): - self.info("counting ctors...") - self._count_ctors() - else: - self.info("ctors counts are disabled for this build.") - # Report some important file sizes for display in treeherder perfherder_data = { diff --git a/toolkit/crashreporter/tools/symbolstore.py b/toolkit/crashreporter/tools/symbolstore.py index 7edd4f206240..11a726d36b31 100755 --- a/toolkit/crashreporter/tools/symbolstore.py +++ b/toolkit/crashreporter/tools/symbolstore.py @@ -492,12 +492,12 @@ class Dumper: def CopyDebug(self, file, debug_file, guid, code_file, code_id): pass - def Process(self, file_to_process): + def Process(self, file_to_process, count_ctors=False): """Process the given file.""" if self.ShouldProcess(os.path.abspath(file_to_process)): - self.ProcessFile(file_to_process) + self.ProcessFile(file_to_process, count_ctors=count_ctors) - def ProcessFile(self, file, dsymbundle=None): + def ProcessFile(self, file, dsymbundle=None, count_ctors=False): """Dump symbols from these files into a symbol file, stored in the proper directory structure in |symbol_path|; processing is performed asynchronously, and Finish must be called to wait for it complete and cleanup. @@ -509,7 +509,8 @@ class Dumper: # the tinderbox vcs path will be assigned further down vcs_root = os.environ.get('MOZ_SOURCE_REPO') for arch_num, arch in enumerate(self.archs): - self.ProcessFileWork(file, arch_num, arch, vcs_root, dsymbundle) + self.ProcessFileWork(file, arch_num, arch, vcs_root, dsymbundle, + count_ctors=count_ctors) def dump_syms_cmdline(self, file, arch, dsymbundle=None): ''' @@ -518,7 +519,9 @@ class Dumper: # The Mac dumper overrides this. return [self.dump_syms, file] - def ProcessFileWork(self, file, arch_num, arch, vcs_root, dsymbundle=None): + def ProcessFileWork(self, file, arch_num, arch, vcs_root, dsymbundle=None, + count_ctors=False): + ctors = 0 t_start = time.time() print("Processing file: %s" % file, file=sys.stderr) @@ -582,6 +585,16 @@ class Dumper: code_id, code_file = bits[2:] f.write(line) else: + if count_ctors and line.startswith("FUNC "): + # Static initializers, as created by clang and gcc + # have symbols that start with "_GLOBAL_sub" + if '_GLOBAL__sub_' in line: + ctors += 1 + # MSVC creates `dynamic initializer for '...'` + # symbols. + elif "`dynamic initializer for '" in line: + ctors += 1 + # pass through all other lines unchanged f.write(line) f.close() @@ -605,6 +618,24 @@ class Dumper: if dsymbundle: shutil.rmtree(dsymbundle) + if count_ctors: + import json + + perfherder_data = { + "framework": {"name": "build_metrics"}, + "suites": [{ + "name": "compiler_metrics", + "subtests": [{ + "name": "num_static_constructors", + "value": ctors, + "alertChangeType": "absolute", + "alertThreshold": 3 + }]} + ] + } + print('PERFHERDER_DATA: %s' % json.dumps(perfherder_data), + file=sys.stderr) + elapsed = time.time() - t_start print('Finished processing %s in %.2fs' % (file, elapsed), file=sys.stderr) @@ -768,13 +799,14 @@ class Dumper_Mac(Dumper): return self.RunFileCommand(file).startswith("Mach-O") return False - def ProcessFile(self, file): + def ProcessFile(self, file, count_ctors=False): print("Starting Mac pre-processing on file: %s" % file, file=sys.stderr) dsymbundle = self.GenerateDSYM(file) if dsymbundle: # kick off new jobs per-arch with our new list of files - Dumper.ProcessFile(self, file, dsymbundle=dsymbundle) + Dumper.ProcessFile(self, file, dsymbundle=dsymbundle, + count_ctors=count_ctors) def dump_syms_cmdline(self, file, arch, dsymbundle=None): ''' @@ -863,6 +895,9 @@ def main(): to canonical locations in the source repository. Specify , as a comma-separated pair. """) + parser.add_option("--count-ctors", + action="store_true", dest="count_ctors", default=False, + help="Count static initializers") (options, args) = parser.parse_args() #check to see if the pdbstr.exe exists @@ -897,7 +932,7 @@ to canonical locations in the source repository. Specify s3_bucket=bucket, file_mapping=file_mapping) - dumper.Process(args[2]) + dumper.Process(args[2], options.count_ctors) # run main if run directly if __name__ == "__main__": diff --git a/toolkit/library/Makefile.in b/toolkit/library/Makefile.in index bbc7a8602fbc..09cb47679d1c 100644 --- a/toolkit/library/Makefile.in +++ b/toolkit/library/Makefile.in @@ -11,3 +11,5 @@ include $(topsrcdir)/config/rules.mk .PHONY: gtestxul gtestxul: $(MAKE) -C $(DEPTH) toolkit/library/gtest/target LINK_GTEST_DURING_COMPILE=1 + +DUMP_SYMBOLS_FLAGS = --count-ctors