Support MSan/TSan in test isolation and test_env.py

testing/test_env.py

 #!/usr/bin/env python
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Sets environment variables needed to run a chromium unit test."""
 
 import os
 import stat
 import subprocess
@@ skipping 17 matching lines @@
   if not chrome_sandbox_path:
     chrome_sandbox_path = CHROME_SANDBOX_PATH
   extra_env[CHROME_SANDBOX_ENV] = chrome_sandbox_path
 
   return extra_env
 
 
 def trim_cmd(cmd):
   """Removes internal flags from cmd since they're just used to communicate from
   the host machine to this script running on the swarm slaves."""
-  internal_flags = frozenset(['--asan=0', '--asan=1', '--lsan=0', '--lsan=1'])
+  internal_flags = frozenset(['--asan=0', '--asan=1', '--lsan=0', '--lsan=1',

[Alexander Potapenko, 2015/02/04 10:51:46]

Shall we replace this with a list comprehension?

[earthdok, 2015/02/04 14:34:47]

Done.

+                              '--msan=0', '--msan=1', '--tsan=0', '--tsan=1'])
   return [i for i in cmd if i not in internal_flags]
 
 
 def fix_python_path(cmd):
   """Returns the fixed command line to call the right python executable."""
   out = cmd[:]
   if out[0] == 'python':
     out[0] = sys.executable
   elif out[0].endswith('.py'):
     out.insert(0, sys.executable)
   return out
 
 
-def get_asan_env(cmd, lsan):
-  """Returns the envirnoment flags needed for ASan and LSan."""
+def get_sanitizer_env(cmd, asan, lsan, msan, tsan):

[Alexander Potapenko, 2015/02/04 10:51:46]

I think having a separate var for each sanitizer sucks.

[earthdok, 2015/02/04 14:34:47]

Your suggestion?

We could use a dict, but that would only simplify the function signature at the
expense of complicating everything else.

[Alexander Potapenko, 2015/02/04 16:31:04]

Acknowledged.

+  """Returns the envirnoment flags needed for sanitizer tools."""
 
   extra_env = {}
 
-  # Instruct GTK to use malloc while running ASan or LSan tests.
+  # Instruct GTK to use malloc while running sanitizer-instrumented tests.
   extra_env['G_SLICE'] = 'always-malloc'
 
   extra_env['NSS_DISABLE_ARENA_FREE_LIST'] = '1'
   extra_env['NSS_DISABLE_UNLOAD'] = '1'
 
   # TODO(glider): remove the symbolizer path once
   # https://code.google.com/p/address-sanitizer/issues/detail?id=134 is fixed.
   symbolizer_path = os.path.abspath(os.path.join(ROOT_DIR, 'third_party',
       'llvm-build', 'Release+Asserts', 'bin', 'llvm-symbolizer'))
 
-  asan_options = []
-  if lsan:
-    asan_options.append('detect_leaks=1')
-    if sys.platform == 'linux2':
-      # Use the debug version of libstdc++ under LSan. If we don't, there will
-      # be a lot of incomplete stack traces in the reports.
-      extra_env['LD_LIBRARY_PATH'] = '/usr/lib/x86_64-linux-gnu/debug:'
-
+  if lsan or tsan:
     # LSan is not sandbox-compatible, so we can use online symbolization. In
     # fact, it needs symbolization to be able to apply suppressions.
     symbolization_options = ['symbolize=1',
                              'external_symbolizer_path=%s' % symbolizer_path]
-
-    suppressions_file = os.path.join(ROOT_DIR, 'tools', 'lsan',
-        'suppressions.txt')
-    lsan_options = ['suppressions=%s' % suppressions_file,
-                    'print_suppressions=1']
-    extra_env['LSAN_OPTIONS'] = ' '.join(lsan_options)
-  else:
+  elif asan or msan:
     # ASan uses a script for offline symbolization.
     # Important note: when running ASan with leak detection enabled, we must use
     # the LSan symbolization options above.
     symbolization_options = ['symbolize=0']
     # Set the path to llvm-symbolizer to be used by asan_symbolize.py
     extra_env['LLVM_SYMBOLIZER_PATH'] = symbolizer_path
 
-  asan_options.extend(symbolization_options)
+  if asan:
+    asan_options = symbolization_options[:]
+    if lsan:
+      asan_options.append('detect_leaks=1')
 
-  extra_env['ASAN_OPTIONS'] = ' '.join(asan_options)
+    extra_env['ASAN_OPTIONS'] = ' '.join(asan_options)

[earthdok, 2015/02/03 17:56:52]

Alex: do we need replace_intrin/strict_memcmp here?

[Alexander Potapenko, 2015/02/04 10:51:46]

I think it's fine to omit them. Linux and Mac already have them by default. No
need to complicate all the scripts with a special case for CrOS, which doesn't
affect correctness.

 
-  if sys.platform == 'darwin':
-    isolate_output_dir = os.path.abspath(os.path.dirname(cmd[0]))
-    # This is needed because the test binary has @executable_path embedded in it
-    # it that the OS tries to resolve to the cache directory and not the mapped
-    #  directory.
-    extra_env['DYLD_LIBRARY_PATH'] = str(isolate_output_dir)
+    if sys.platform == 'darwin':
+      isolate_output_dir = os.path.abspath(os.path.dirname(cmd[0]))
+      # This is needed because the test binary has @executable_path embedded in
+      # it that the OS tries to resolve to the cache directory and not the
+      # mapped directory.
+      extra_env['DYLD_LIBRARY_PATH'] = str(isolate_output_dir)
+
+  if lsan:

[earthdok, 2015/02/03 17:56:52]

Currently we run LSan on top of ASan. The logic here, which I copied from
runtest.py, also supports:
- running LSan stand-alone (which is possible but not done in Chromium) and
- running LSan on top of MSan (which is in our plans but not yet implemented).

I think it's cool to have the option of using those modes without having to
patch the scripts again. However, one could argue that simplicity is more
important. I don't have a strong opinion.

The difference is about 5 LOC.

[Alexander Potapenko, 2015/02/04 10:51:46]

I don't think we ever want to run LSan without ASan.
I'm fine with having this option, but please reflect the difference between
modes in the comments.

+    if asan or msan:
+      lsan_options = []
+    else:
+      lsan_options += symbolization_options[:]
+    if sys.platform == 'linux2':
+      # Use the debug version of libstdc++ under LSan. If we don't, there will
+      # be a lot of incomplete stack traces in the reports.
+      extra_env['LD_LIBRARY_PATH'] = '/usr/lib/x86_64-linux-gnu/debug:'
+
+    suppressions_file = os.path.join(ROOT_DIR, 'tools', 'lsan',
+        'suppressions.txt')
+    lsan_options += ['suppressions=%s' % suppressions_file,
+                     'print_suppressions=1']
+    extra_env['LSAN_OPTIONS'] = ' '.join(lsan_options)
+
+  if msan:
+    msan_options = symbolization_options[:]
+    if lsan:
+      msan_options.append('detect_leaks=1')
+    extra_env['MSAN_OPTIONS'] = ' '.join(msan_options)
+
+  if tsan:
+    tsan_options = symbolization_options[:]
+    extra_env['TSAN_OPTIONS'] = ' '.join(tsan_options)
 
   return extra_env
 
 
 def get_sanitizer_symbolize_command(json_path=None):
   """Construct the command to invoke offline symbolization script."""
   script_path = '../tools/valgrind/asan/asan_symbolize.py'
   cmd = [sys.executable, script_path]
   if json_path is not None:
     cmd.append('--test-summary-json-file=%s' % json_path)
@@ skipping 38 matching lines @@
   # Many tests assume a English interface...
   extra_env['LANG'] = 'en_US.UTF-8'
   # Used by base/base_paths_linux.cc as an override. Just make sure the default
   # logic is used.
   env.pop('CR_SOURCE_ROOT', None)
   extra_env.update(get_sandbox_env(env))
 
   # Copy logic from  tools/build/scripts/slave/runtest.py.
   asan = '--asan=1' in cmd
   lsan = '--lsan=1' in cmd
-  use_symbolization_script = asan and not lsan
+  msan = '--msan=1' in cmd
+  tsan = '--tsan=1' in cmd
+  use_symbolization_script = (asan or msan) and not lsan
 
-  if asan:
-    extra_env.update(get_asan_env(cmd, lsan))
-    # ASan is not yet sandbox-friendly on Windows (http://crbug.com/382867).
-    if sys.platform == 'win32':
+  if asan or lsan or msan or tsan:
+    extra_env.update(get_sanitizer_env(cmd, asan, lsan, msan, tsan))
+
+  if lsan or tsan or (asan and sys.platform == 'win32'):
+      # ASan is not yet sandbox-friendly on Windows (http://crbug.com/382867).
+      # LSan and TSan are not sandbox-friendly.
       cmd.append('--no-sandbox')
-  if lsan:
-    cmd.append('--no-sandbox')
 
   cmd = trim_cmd(cmd)
 
   # Ensure paths are correctly separated on windows.
   cmd[0] = cmd[0].replace('/', os.path.sep)
   cmd = fix_python_path(cmd)
 
   print('Additional test environment:\n%s\n'
         'Command: %s\n' % (
         '\n'.join('    %s=%s' %
@@ skipping 20 matching lines @@
     print >> sys.stderr, 'Failed to start %s' % cmd
     raise
 
 
 def main():
   return run_executable(sys.argv[1:], os.environ.copy())
 
 
 if __name__ == '__main__':
   sys.exit(main())