Commit 5a797379 authored by Ted Choc's avatar Ted Choc Committed by Commit Bot

Revert "Make generate_breakpad_symbols.py not silently ignore subprocess errors."

This reverts commit 924c8465.

Reason for revert: This appears to break the Android WebView bots.

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Android%20WebView%20N%20%28dbg%29
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Android%20WebView%20O%20(dbg)

Traceback (most recent call last):
  File "/b/s/w/ir/cache/builder/src/components/crash/content/tools/generate_breakpad_symbols.py", line 327, in <module>
    sys.exit(main())
  File "/b/s/w/ir/cache/builder/src/components/crash/content/tools/generate_breakpad_symbols.py", line 316, in main
    deps = GetSharedLibraryDependencies(options, queue.pop(0), loader_path)
  File "/b/s/w/ir/cache/builder/src/components/crash/content/tools/generate_breakpad_symbols.py", line 151, in GetSharedLibraryDependencies
    deps = GetSharedLibraryDependenciesLinux(binary)
  File "/b/s/w/ir/cache/builder/src/components/crash/content/tools/generate_breakpad_symbols.py", line 67, in GetSharedLibraryDependenciesLinux
    ldd = subprocess.check_output(['ldd', binary])
  File "/b/s/w/ir/cipd_bin_packages/lib/python2.7/subprocess.py", line 219, in check_output
    raise CalledProcessError(retcode, cmd, output=output)

Original change's description:
> Make generate_breakpad_symbols.py not silently ignore subprocess errors.
> 
> GetCommandOuput() used to pipe stderr to /dev/null, and it ignored
> the command's return code. Use check_output() to check the return
> code, and keep stderr attached to parent's stderr.
> 
> Also make breakpad_integration_test.py a bit simpler (this part is
> supposed to be behavior-preserving.)
> 
> Bug: 813163
> Change-Id: I8c55d3da9fff3b944111c3e868121ac34bf65c17
> Reviewed-on: https://chromium-review.googlesource.com/1086981
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Commit-Queue: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#564531}

TBR=thakis@chromium.org,jochen@chromium.org

Change-Id: Iee5a81e9b7e9db21f1dda9bdc272d3392438f481
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 813163
Reviewed-on: https://chromium-review.googlesource.com/1087871Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564707}
parent 8e1b1e2e
...@@ -29,6 +29,24 @@ BINARY_INFO = collections.namedtuple('BINARY_INFO', ...@@ -29,6 +29,24 @@ BINARY_INFO = collections.namedtuple('BINARY_INFO',
['platform', 'arch', 'hash', 'name']) ['platform', 'arch', 'hash', 'name'])
def GetCommandOutput(command, env=None):
"""Runs the command list, returning its output (stdout).
Args:
command: (list of strings) a command with arguments
env: (dict or None) optional environment for the command. If None,
inherits the existing environment, otherwise completely overrides it.
From chromium_utils.
"""
devnull = open(os.devnull, 'w')
proc = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=devnull,
bufsize=1, env=env)
output = proc.communicate()[0]
return output
def GetDumpSymsBinary(build_dir=None): def GetDumpSymsBinary(build_dir=None):
"""Returns the path to the dump_syms binary.""" """Returns the path to the dump_syms binary."""
DUMP_SYMS = 'dump_syms' DUMP_SYMS = 'dump_syms'
...@@ -64,7 +82,7 @@ def GetSharedLibraryDependenciesLinux(binary): ...@@ -64,7 +82,7 @@ def GetSharedLibraryDependenciesLinux(binary):
"""Return absolute paths to all shared library dependecies of the binary. """Return absolute paths to all shared library dependecies of the binary.
This implementation assumes that we're running on a Linux system.""" This implementation assumes that we're running on a Linux system."""
ldd = subprocess.check_output(['ldd', binary]) ldd = GetCommandOutput(['ldd', binary])
lib_re = re.compile('\t.* => (.+) \(.*\)$') lib_re = re.compile('\t.* => (.+) \(.*\)$')
result = [] result = []
for line in ldd.splitlines(): for line in ldd.splitlines():
...@@ -88,7 +106,7 @@ def GetDeveloperDirMac(): ...@@ -88,7 +106,7 @@ def GetDeveloperDirMac():
if 'DEVELOPER_DIR' in os.environ: if 'DEVELOPER_DIR' in os.environ:
candidate_paths.append(os.environ['DEVELOPER_DIR']) candidate_paths.append(os.environ['DEVELOPER_DIR'])
candidate_paths.extend([ candidate_paths.extend([
subprocess.check_output(['xcode-select', '-p']).strip(), GetCommandOutput(['xcode-select', '-p']).strip(),
# Most Mac 10.1[0-2] bots have at least one Xcode installed. # Most Mac 10.1[0-2] bots have at least one Xcode installed.
'/Applications/Xcode.app', '/Applications/Xcode.app',
'/Applications/Xcode9.0.app', '/Applications/Xcode9.0.app',
...@@ -112,7 +130,7 @@ def GetSharedLibraryDependenciesMac(binary, loader_path): ...@@ -112,7 +130,7 @@ def GetSharedLibraryDependenciesMac(binary, loader_path):
developer_dir = GetDeveloperDirMac() developer_dir = GetDeveloperDirMac()
if developer_dir: if developer_dir:
env['DEVELOPER_DIR'] = developer_dir env['DEVELOPER_DIR'] = developer_dir
otool = subprocess.check_output(['otool', '-l', binary], env=env).splitlines() otool = GetCommandOutput(['otool', '-l', binary], env=env).splitlines()
rpaths = [] rpaths = []
dylib_id = None dylib_id = None
for idx, line in enumerate(otool): for idx, line in enumerate(otool):
...@@ -123,7 +141,7 @@ def GetSharedLibraryDependenciesMac(binary, loader_path): ...@@ -123,7 +141,7 @@ def GetSharedLibraryDependenciesMac(binary, loader_path):
m = re.match(' *name (.*) \(offset .*\)$', otool[idx+2]) m = re.match(' *name (.*) \(offset .*\)$', otool[idx+2])
dylib_id = m.group(1) dylib_id = m.group(1)
otool = subprocess.check_output(['otool', '-L', binary], env=env).splitlines() otool = GetCommandOutput(['otool', '-L', binary], env=env).splitlines()
lib_re = re.compile('\t(.*) \(compatibility .*\)$') lib_re = re.compile('\t(.*) \(compatibility .*\)$')
deps = [] deps = []
for line in otool: for line in otool:
...@@ -206,7 +224,7 @@ def GenerateSymbols(options, binaries): ...@@ -206,7 +224,7 @@ def GenerateSymbols(options, binaries):
break break
binary_info = GetBinaryInfoFromHeaderInfo( binary_info = GetBinaryInfoFromHeaderInfo(
subprocess.check_output([dump_syms, '-i', binary]).splitlines()[0]) GetCommandOutput([dump_syms, '-i', binary]).splitlines()[0])
if not binary_info: if not binary_info:
should_dump_syms = False should_dump_syms = False
reason = "Could not obtain binary information." reason = "Could not obtain binary information."
......
...@@ -22,12 +22,10 @@ import time ...@@ -22,12 +22,10 @@ import time
CONCURRENT_TASKS=4 CONCURRENT_TASKS=4
BREAKPAD_TOOLS_DIR = os.path.join(
os.path.dirname(__file__), '..', '..', '..',
'components', 'crash', 'content', 'tools')
def run_test(options, crash_dir, symbols_dir, additional_arguments = []): def run_test(options, crash_dir, additional_arguments = []):
global failure global failure
global breakpad_tools_dir
print "# Run content_shell and make it crash." print "# Run content_shell and make it crash."
cmd = [options.binary, cmd = [options.binary,
...@@ -63,7 +61,7 @@ def run_test(options, crash_dir, symbols_dir, additional_arguments = []): ...@@ -63,7 +61,7 @@ def run_test(options, crash_dir, symbols_dir, additional_arguments = []):
if sys.platform not in ('darwin', 'win32'): if sys.platform not in ('darwin', 'win32'):
minidump = os.path.join(crash_dir, 'minidump') minidump = os.path.join(crash_dir, 'minidump')
dmp_to_minidump = os.path.join(BREAKPAD_TOOLS_DIR, 'dmp2minidump.py') dmp_to_minidump = os.path.join(breakpad_tools_dir, 'dmp2minidump.py')
cmd = [dmp_to_minidump, dmp_file, minidump] cmd = [dmp_to_minidump, dmp_file, minidump]
if options.verbose: if options.verbose:
print ' '.join(cmd) print ' '.join(cmd)
...@@ -89,6 +87,7 @@ def run_test(options, crash_dir, symbols_dir, additional_arguments = []): ...@@ -89,6 +87,7 @@ def run_test(options, crash_dir, symbols_dir, additional_arguments = []):
# stack = proc.communicate()[0] # stack = proc.communicate()[0]
else: else:
minidump_stackwalk = os.path.join(options.build_dir, 'minidump_stackwalk') minidump_stackwalk = os.path.join(options.build_dir, 'minidump_stackwalk')
global symbols_dir
cmd = [minidump_stackwalk, minidump, symbols_dir] cmd = [minidump_stackwalk, minidump, symbols_dir]
if options.verbose: if options.verbose:
print ' '.join(cmd) print ' '.join(cmd)
...@@ -117,6 +116,7 @@ def run_test(options, crash_dir, symbols_dir, additional_arguments = []): ...@@ -117,6 +116,7 @@ def run_test(options, crash_dir, symbols_dir, additional_arguments = []):
def main(): def main():
global failure global failure
global breakpad_tools_dir
parser = optparse.OptionParser() parser = optparse.OptionParser()
parser.add_option('', '--build-dir', default='', parser.add_option('', '--build-dir', default='',
...@@ -150,15 +150,19 @@ def main(): ...@@ -150,15 +150,19 @@ def main():
# Create a temporary directory to store the crash dumps and symbols in. # Create a temporary directory to store the crash dumps and symbols in.
crash_dir = tempfile.mkdtemp() crash_dir = tempfile.mkdtemp()
symbols_dir = os.path.join(crash_dir, 'symbols')
crash_service = None crash_service = None
try: try:
if sys.platform != 'win32': if sys.platform != 'win32':
print "# Generate symbols." print "# Generate symbols."
global symbols_dir
breakpad_tools_dir = os.path.join(
os.path.dirname(__file__), '..', '..', '..',
'components', 'crash', 'content', 'tools')
generate_symbols = os.path.join( generate_symbols = os.path.join(
BREAKPAD_TOOLS_DIR, 'generate_breakpad_symbols.py') breakpad_tools_dir, 'generate_breakpad_symbols.py')
symbols_dir = os.path.join(crash_dir, 'symbols')
cmd = [generate_symbols, cmd = [generate_symbols,
'--build-dir=%s' % options.build_dir, '--build-dir=%s' % options.build_dir,
'--binary=%s' % options.binary, '--binary=%s' % options.binary,
...@@ -171,11 +175,11 @@ def main(): ...@@ -171,11 +175,11 @@ def main():
subprocess.check_call(cmd) subprocess.check_call(cmd)
print "# Running test without trap handler." print "# Running test without trap handler."
run_test(options, crash_dir, symbols_dir) run_test(options, crash_dir);
print "# Running test with trap handler." print "# Running test with trap handler."
run_test(options, crash_dir, symbols_dir, run_test(options, crash_dir,
additional_arguments = additional_arguments =
['--enable-features=WebAssemblyTrapHandler']) ['--enable-features=WebAssemblyTrapHandler']);
except: except:
print "FAIL: %s" % failure print "FAIL: %s" % failure
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment