[sanitizers] Force pickup of new symbolizer (round #2).

[hctim]

Summary

It's possible currently that the sanitizer runtimes when testing grab
the path to the symbolizer through *SAN_SYMBOLIZER_PATH=...

This can be polluted by things like Android's setup script. This patch
forces external_symbolizer_path=$new_build_out_dir/llvm-symbolizer when
%env_tool_options is used.

Same principle with Android, except that we need to replace the path to
the new symbolizer with the path to the new symbolizer on the device.

Previously committed, fixed-forward, then reverted to bring us back to
stable before the weekend.

[oontvoo]

compiler-rt/test/sanitizer_common/android_commands/android_common.py

41

Maybe test -e ?

( or -r since "exists" isn't the same as readable? or executable?)

[vitalybuka]

compiler-rt/test/sanitizer_common/android_commands/android_common.py

37

do not add unneeded args

40

why not just call adb(['shell', 'ls', ....]);

compiler-rt/test/sanitizer_common/android_commands/android_run.py

12

why do we need device_file_exists at all?
can we just make sure that it's always there?

[hctim]

compiler-rt/test/sanitizer_common/android_commands/android_common.py

41

good idea, test -x done.

compiler-rt/test/sanitizer_common/android_commands/android_run.py

12

The symbolizer needs to be cross-compiled, which isn't easy (possible?) to accomplish with a standalone CRT build. The buildbot script always pushes it, but I'd rather not force people to use the buildbot script.

[hctim]

• Changed to 'test -x' instead of 'ls' in test, and removed timeout blob.

[vitalybuka]

compiler-rt/test/sanitizer_common/android_commands/android_common.py

40

you probably marked this one as done by mistake?

compiler-rt/test/sanitizer_common/android_commands/android_run.py

12

So what is going to happen if has_symbolizer is false? Most tests will fail?

[hctim]

compiler-rt/test/sanitizer_common/android_commands/android_common.py

40

thanks for the catch

compiler-rt/test/sanitizer_common/android_commands/android_run.py

12

We remove any metion of external_symbolizer_path=... from *SAN_OPTIONS. So sanitizer-common will do the normal thing of looking in the CWD and PATH and will likely fall back to addr2line/no symbolizer as normal. Tests should continue as usual (just they don't have the symbolizer). I figured this was a better option than forcing that tests can only be run on devices that have the symbolizer.

[hctim]

• Remove timeout command from 'test'.

[oontvoo]

compiler-rt/test/sanitizer_common/android_commands/android_common.py

40

you probably marked this one as done by mistake?

I think vitalybuka@ was asking why you needed to do a manual subprocess.call(), as opposed to just using the the adb() function already defined (line 16)

something like adb(['shell', 'test', '-x', path]) ?

[hctim]

compiler-rt/test/sanitizer_common/android_commands/android_common.py

40

ah, because adb() "fails" if the retcode is non-zero, and we want to observe the value of the retcode.

[vitalybuka]

compiler-rt/test/sanitizer_common/android_commands/android_common.py

41

I guess the following should be a part of adb() then
it returns exit code so caller should do them
maybe extract some adb_internal() and use it in adb and in device_file...?

if ret != 0:
  print "adb command failed", args
  print tmpname
  out.close()
  out = open(tmpname, 'r')
  print out.read()

compiler-rt/test/sanitizer_common/android_commands/android_run.py

12

do they actually work? I suspect this case is not supported at all and you unnecessary complicate the code.
if so you can remove device_file_exists from this patch.

Another concern is testing time. How longer to it takes to execute all tests with device_file_exists and without?

[hctim]

Scratch this patch - polluted environment probably doesn't necessitate a workaround that increases the complexity of the runner.

compiler-rt/test/sanitizer_common/android_commands/android_common.py

if ret != 0:
       print tmpname
       out.close()
       out = open(tmpname, 'r')
       print out.read()
     out.close()
     os.unlink(tmpname)
     return ret
 
+def device_file_exists(path):
+    ret = 255
+    devnull = open(os.devnull, 'w')
+    ret = subprocess.call([ADB, "shell", "test", "-x", path], stdout=devnull, stderr=devnull)
+
+    devnull.close()
+    if ret == 0:
+        return True
+    elif ret == 1:
+        return False
+    else:
+        print "adb connection failed"
+        return False
+
 def pull_from_device(path):
     tmp = tempfile.mktemp()
     adb(['pull', path, tmp], 5, 60)
     text = open(tmp, 'r').read()
     os.unlink(tmp)
     return text
 
 def push_to_device(path):
     dst_path = host_to_device_path(path)
     adb(['push', path, dst_path], 5, 60)

compiler-rt/test/sanitizer_common/android_commands/android_run.py

 #!/usr/bin/env python
 
-import os, signal, sys, subprocess, tempfile
+import os, signal, sys, subprocess, tempfile, re
 from android_common import *
 
 ANDROID_TMPDIR = '/data/local/tmp/Output'
 
 device_binary = host_to_device_path(sys.argv[0])
 
 def build_env():
+    symbolizer_path = "{}/llvm-symbolizer".format(ANDROID_TMPDIR)
+    has_symbolizer = device_file_exists(symbolizer_path)
+
     args = []
     # Android linker ignores RPATH. Set LD_LIBRARY_PATH to Output dir.
     args.append('LD_LIBRARY_PATH=%s' % (ANDROID_TMPDIR,))
     for (key, value) in os.environ.items():
         if key in ['ASAN_ACTIVATION_OPTIONS', 'SCUDO_OPTIONS'] or key.endswith('SAN_OPTIONS'):
-            args.append('%s="%s"' % (key, value.replace('"', '\\"')))
+            val = value.replace('"', '\\"')
+            val = re.sub(
+                r"external_symbolizer_path=(\'.*?\'|\".*?\"|.*?)(:|$)",
+                "external_symbolizer_path=\"{}\"\\2".format(symbolizer_path) if has_symbolizer else "",
+                val)
+            args.append('%s="%s"' % (key, val))
     return ' '.join(args)
 
 is_64bit = (subprocess.check_output(['file', sys.argv[0] + '.real']).find('64-bit') != -1)
 
 device_env = build_env()
 device_args = ' '.join(sys.argv[1:]) # FIXME: escape?
 device_stdout = device_binary + '.stdout'
 device_stderr = device_binary + '.stderr'

compiler-rt/test/sanitizer_common/lit.common.cfg.py

if config.host_os == 'Darwin':
   if config.tool_name == "tsan":
     default_tool_options += ['ignore_interceptors_accesses=0']
 elif config.android:
   # The same as on Darwin, we default to "abort_on_error=1" which slows down
   # testing. Also, all existing tests are using "not" instead of "not --crash"
   # which does not work for abort()-terminated programs.
   default_tool_options += ['abort_on_error=0']
 
+# If the user has a poisoned *SAN_SYMBOLIZER_PATH (like what's setup by
+# build/envsetup.sh on Android), then they can end up with an out-of-date
+# symbolizer for the tests. Ensure they get the one from the recent build tree.
+symbolizer_path = os.path.join(config.binary_path, "bin", "llvm-symbolizer")
+if os.path.exists(symbolizer_path):
+  default_tool_options += ['external_symbolizer_path=' + symbolizer_path]
+
 default_tool_options_str = ':'.join(default_tool_options)
 if default_tool_options_str:
   config.environment[tool_options] = default_tool_options_str
   default_tool_options_str += ':'
 
 extra_link_flags = []
 
 if config.host_os in ['Linux']:

compiler-rt/test/sanitizer_common/lit.site.cfg.py.in

 @LIT_SITE_CFG_IN_HEADER@
 
 # Tool-specific config options.
 config.name_suffix = "@CONFIG_NAME@"
 config.tool_name = "@SANITIZER_COMMON_LIT_TEST_MODE@"
 config.target_cflags = "@SANITIZER_COMMON_TEST_TARGET_CFLAGS@"
 config.target_arch = "@SANITIZER_COMMON_TEST_TARGET_ARCH@"
+config.binary_path = "@LLVM_BINARY_DIR@"
 
 # Load common config for all compiler-rt lit tests.
 lit_config.load_config(config, "@COMPILER_RT_BINARY_DIR@/test/lit.common.configured")
 
 # Load tool-specific config that would do the real work.
 lit_config.load_config(config, "@SANITIZER_COMMON_LIT_SOURCE_DIR@/lit.common.cfg.py")