[libcxx] Add support for Android targets to lit.cfg.

[danalbert]

Summary

For accurate testing on Android, test compilation needs to more closely match the Android build system. This means flipping all the switches for -nostdlibinc, -nostdlib, etc. and instead using the libraries that were either prebuilt or built by the system.

Android's build system isn't able to efficiently handle many small builds run concurrently. The downside of this is that directly using the Android build system to compile each test cannot be done in parallel, and would take roughly 7 hours to complete a test run.

To deal with this, when config.android is true, lit.cfg will pull all of the cppflags and ldflags right out of config instead of trying to determine them on its own. It's dirtied a little bit by crtbegin and crtend.

The other thing this patch does is deal with pushing the built test to the android device, running it on the device, and removing it when done. There are a few intricacies wrapped around adb shell, as adb considers all commands that were able to be run on the device, regardless of their exit codes, to be successful. As such, the commands are run as adb shell COMMAND; echo $?, and the last line of stdout is read to determine the real exit status.

[danalbert]

test/lit.cfg

287

Nothing past here is actually a change (or at least, shouldn't be). It's just diff doing an awful job of detecting a whole block being indented.

[danalbert]

Previous patch had accidentally removed the -c flag from expected compile failure tests.

[abdulras]

test/lit.cfg

94

The compile_only is misleading. !compile_only implies compile and link. I think that using an enum would be much nicer here:

(COMPILE_ONLY, LINK_ONLY, COMPILE_AND_LINK) = xrange(3)

209

Does this have to be /data/nativetest? Personally, I would vote for /data/local/tmp/nativetest/.

210

Might be nice to make a parameter to specify where adb is.

212

Ick. Can you chain this together using exit code checks please? This feels fragile (quoting and reliance on the exec passing it through to the shell appropriately).

219

Hmm... perhaps make the test location a member variable?

[danalbert]

test/lit.cfg

94

self._build(..., compile_only=False) might be misleading, but callers wouldn't write that. They'd write self._build(...), which does exactly what it says it does.

209

I'll make it configurable.

210

This set up is only intended to be used as part of a platform build, so it uses the freshly built adb. Expanding this config to work with the NDK is a task for another day.

212

This cmd is never run. It's only returned so LIT can dump the command that was run in the even of an error. All this line is doing is printing clang ... && adb push ....

[danalbert]

Make the on device directory that tests copy to configurable.

[danalbert]

Last diff upload was the wrong patch.

[danalbert]

Major change in this revision is that tests get placed into their own working directory on the device rather than being lumped in together. This is done because we need to also push any .dat files that live in the same directory as the test source file to the device as well, and there are duplicate .dat files in the test tree that prevent us from placing them all in the same directory.

There's also a small refactoring of some of the ADB calls.

[EricWF]

There are going to be some merge conflicts between this and D4329. If my patch gets accepted first I would be more than willing to re-generate the diff for this patch. Hopefully I'll be able to take a look at this tonight depending on how long it takes me to get my android environment set up.

[danalbert]

I probably need to regenerate it anyway. I believe I've made some changes in our tree since this was updated (this patch will already be applied when you sync aosp).

test/lit.cfg

95

Whoops. Somehow the compiler got passed twice.

[danalbert]

Rebased onto current lit.cfg so that this change merges cleanly.

[danalbert]

Rebased on to current master.

[danalbert]

Did phabricator get smart enough to realize that most of the diff is just an indent? O_o

Someone teach git to do this.

[danalbert]

This has gotten far too gross and fallen way out of sync with the rest of the changes that have been going in. I'm waiting for http://reviews.llvm.org/D4952 to land, and then I'll massage these changes in to that.

[jroelofs]

In D4594#23, @danalbert wrote:

Someone teach git to do this.

Amen!

[danalbert]

We're going to find a more generic approach to this problem (one that will also support things like QEMU targets).

test/lit.cfg

         # Evaluate the test.
         return self._evaluate_test(test, lit_config)
 
+    def _build(self, exec_path, source_path, compile_only=False):
+        cmd = [self.cxx_under_test, self.cxx_under_test, '-o', exec_path,
+               source_path] + self.cpp_flags
+        if compile_only:
+            cmd += ['-c']
+        else:
+            cmd += self.ld_flags
+        out, err, exitCode = self.execute_command(cmd)
+        return cmd, out, err, exitCode
+
+    def _clean(self, exec_path):
+        os.remove(exec_path)
+
+    def _run(self, exec_path, lit_config, in_dir=None):
+        cmd = []
+        if self.exec_env:
+            cmd.append('env')
+            cmd.extend('%s=%s' % (name, value)
+                       for name, value in self.exec_env.items())
+        cmd.append(exec_path)
+        if lit_config.useValgrind:
+            cmd = lit_config.valgrindArgs + cmd
+        out, err, exitCode = self.execute_command(cmd, in_dir)
+        return cmd, out, err, exitCode
+
     def _evaluate_test(self, test, lit_config):
         name = test.path_in_suite[-1]
         source_path = test.getSourcePath()

 
         # If this is a compile (failure) test, build it and check for failure.
         if expected_compile_fail:
-            cmd = [self.cxx_under_test, '-c',
-                   '-o', '/dev/null', source_path] + self.cpp_flags
-            out, err, exitCode = self.execute_command(cmd)
+            cmd, out, err, exitCode = self._build('/dev/null', source_path,
+                                                  compile_only=True)
             if exitCode == 1:
                 return lit.Test.PASS, ""
             else:

             exec_file.close()
 
             try:
-                compile_cmd = [self.cxx_under_test, '-o', exec_path,
-                       source_path] + self.cpp_flags + self.ld_flags
-                cmd = compile_cmd
-                out, err, exitCode = self.execute_command(cmd)
+                cmd, out, err, exitCode = self._build(exec_path, source_path)
+                compile_cmd = cmd
                 if exitCode != 0:
                     report = """Command: %s\n""" % ' '.join(["'%s'" % a
                                                              for a in cmd])

                     report += "\n\nCompilation failed unexpectedly!"
                     return lit.Test.FAIL, report
 
-                cmd = []
-                if self.exec_env:
-                    cmd.append('env')
-                    cmd.extend('%s=%s' % (name, value)
-                               for name,value in self.exec_env.items())
-                cmd.append(exec_path)
-                if lit_config.useValgrind:
-                    cmd = lit_config.valgrindArgs + cmd
-                out, err, exitCode = self.execute_command(cmd, source_dir)
+                cmd, out, err, exitCode = self._run(exec_path, lit_config,
+                                                    source_dir)
                 if exitCode != 0:
                     report = """Compiled With: %s\n""" % \
                         ' '.join(["'%s'" % a for a in compile_cmd])

                     return lit.Test.FAIL, report
             finally:
                 try:
-                    os.remove(exec_path)
+                    self._clean(exec_path)
                 except:
                     pass
         return lit.Test.PASS, ""
 
+
+class AndroidLibcxxTestFormat(LibcxxTestFormat):
+    def __init__(self, cxx_under_test, libcxx_src_root, libcxx_obj_root,
+                 cpp_flags, ld_flags, crtbegin, crtend, timeout):
+        self.cxx_under_test = cxx_under_test
+        self.libcxx_src_root = libcxx_src_root
+        self.libcxx_obj_root = libcxx_obj_root
+        self.cpp_flags = cpp_flags
+        self.ld_flags = ld_flags
+        self.crtbegin = crtbegin
+        self.crtend = crtend
+        self.timeout = timeout
+
+    def _build(self, exec_path, source_path, compile_only=False):
+        cmd = [self.cxx_under_test, '-o', exec_path] + self.cpp_flags
+        if compile_only:
+            cmd += ['-c', source_path]
+        else:
+            cmd += [self.crtbegin, source_path] + self.ld_flags + [self.crtend]
+        try:
+            build_cmd = cmd
+            out, err, exit_code = self.execute_command(build_cmd)
+            if exit_code != 0:
+                return build_cmd, out, err, exit_code
+            exec_file = os.path.basename(exec_path)
+            device_path = os.path.join('/data/nativetest/', exec_file)
+            cmd = ['adb', 'push', exec_path, device_path]
+            out, err, exit_code = self.execute_command(cmd)
+            cmd = build_cmd + ['&&'] + cmd
+            return cmd, out, err, exit_code
+        except:
+            return cmd, out, err, exit_code
+
+    def _clean(self, exec_path):
+        exec_file = os.path.basename(exec_path)
+        device_path = os.path.join('/data/nativetest/', exec_file)
+        cmd = ['adb', 'shell', 'rm', device_path]
+        self.execute_command(cmd)
+        os.remove(exec_path)
+
+    def _run(self, exec_path, lit_config, in_dir=None):
+        exec_path = os.path.basename(exec_path)
+        device_path = os.path.join('/data/nativetest', exec_path)
+        shell_cmd = '{}; echo $?'.format(device_path)
+        cmd = ['timeout', self.timeout, 'adb', 'shell', shell_cmd]
+
+        # Tests will commonly fail with ETXTBSY. Possibly related to this bug:
+        # https://code.google.com/p/android/issues/detail?id=65857. Work around
+        # it by just waiting a second and then retrying.
+        for _ in range(10):
+            out, err, exit_code = self.execute_command(cmd)
+            if exit_code == 0:
+                if 'Text file busy' in out:
+                    time.sleep(1)
+                else:
+                    out = out.strip().split('\r\n')
+                    status_line = out[-1:][0]
+                    out = '\n'.join(out[:-1])
+                    exit_code = int(status_line)
+                    break
+            else:
+                err += '\nTimed out after {} seconds'.format(self.timeout)
+                break
+        return cmd, out, err, exit_code
+
+
 # name: The name of this test suite.
 config.name = 'libc++'
 

 # test_source_root: The root path where tests are located.
 config.test_source_root = os.path.dirname(__file__)
 
-# Gather various compiler parameters.
-cxx_under_test = lit_config.params.get('cxx_under_test', None)
-if cxx_under_test is None:
-    cxx_under_test = getattr(config, 'cxx_under_test', None)
-
-    # If no specific cxx_under_test was given, attempt to infer it as clang++.
+if getattr(config, 'android', False):
+    android_root = getattr(config, 'android_root', None)
+    if not android_root:
+        lit_config.fatal('config.android_root must be set')
+    cxx_under_test = lit_config.params.get('cxx_under_test', None)
     if cxx_under_test is None:
-        clangxx = lit.util.which('clang++', config.environment['PATH'])
-        if clangxx is not None:
-            cxx_under_test = clangxx
-    lit_config.note("inferred cxx_under_test as: %r" % (cxx_under_test,))
-if cxx_under_test is None:
-    lit_config.fatal('must specify user parameter cxx_under_test '
-                     '(e.g., --param=cxx_under_test=clang++)')
-
-libcxx_src_root = lit_config.params.get('libcxx_src_root', None)
-if libcxx_src_root is None:
-    libcxx_src_root = getattr(config, 'libcxx_src_root', None)
+        cxx_under_test = getattr(config, 'cxx_under_test', None)
+        if cxx_under_test is None:
+            lit_config.fatal('config.cxx_under_test must be set')
+    libcxx_src_root = lit_config.params.get('libcxx_src_root', None)
     if libcxx_src_root is None:
-        libcxx_src_root = os.path.dirname(config.test_source_root)
+        libcxx_src_root = getattr(config, 'libcxx_src_root', None)
+        if libcxx_src_root is None:
+            libcxx_src_root = os.path.dirname(config.test_source_root)
+    libcxx_obj_root = lit_config.params.get('libcxx_obj_root', None)
+    if libcxx_obj_root is None:
+        libcxx_obj_root = getattr(config, 'libcxx_obj_root', None)
+        if libcxx_obj_root is None:
+            libcxx_obj_root = libcxx_src_root
+    config.test_format = AndroidLibcxxTestFormat(
+        cxx_under_test,
+        libcxx_src_root,
+        libcxx_obj_root,
+        config.cppflags,
+        config.ldflags,
+        config.crtbegin,
+        config.crtend,
+        getattr(config, 'timeout', '30'))
+else:
+    # Gather various compiler parameters.
+    cxx_under_test = lit_config.params.get('cxx_under_test', None)
+    if cxx_under_test is None:
+        cxx_under_test = getattr(config, 'cxx_under_test', None)
+
+        # If no specific cxx_under_test was given, attempt to infer it as clang++.
+        if cxx_under_test is None:
+            clangxx = lit.util.which('clang++', config.environment['PATH'])
+            if clangxx is not None:
+                cxx_under_test = clangxx
+        lit_config.note("inferred cxx_under_test as: %r" % (cxx_under_test,))
+    if cxx_under_test is None:
+        lit_config.fatal('must specify user parameter cxx_under_test '
+                         '(e.g., --param=cxx_under_test=clang++)')
+
+    libcxx_src_root = lit_config.params.get('libcxx_src_root', None)
+    if libcxx_src_root is None:
+        libcxx_src_root = getattr(config, 'libcxx_src_root', None)
+        if libcxx_src_root is None:
+            libcxx_src_root = os.path.dirname(config.test_source_root)
 
     libcxx_obj_root = lit_config.params.get('libcxx_obj_root', None)
-if libcxx_obj_root is None:
-    libcxx_obj_root = getattr(config, 'libcxx_obj_root', None)
     if libcxx_obj_root is None:
-        libcxx_obj_root = libcxx_src_root
-
-cxx_has_stdcxx0x_flag_str = lit_config.params.get('cxx_has_stdcxx0x_flag', None)
-if cxx_has_stdcxx0x_flag_str is not None:
-    if cxx_has_stdcxx0x_flag_str.lower() in ('1', 'true'):
-        cxx_has_stdcxx0x_flag = True
-    elif cxx_has_stdcxx0x_flag_str.lower() in ('', '0', 'false'):
-        cxx_has_stdcxx0x_flag = False
+        libcxx_obj_root = getattr(config, 'libcxx_obj_root', None)
+        if libcxx_obj_root is None:
+            libcxx_obj_root = libcxx_src_root
+
+    cxx_has_stdcxx0x_flag_str = lit_config.params.get('cxx_has_stdcxx0x_flag', None)
+    if cxx_has_stdcxx0x_flag_str is not None:
+        if cxx_has_stdcxx0x_flag_str.lower() in ('1', 'true'):
+            cxx_has_stdcxx0x_flag = True
+        elif cxx_has_stdcxx0x_flag_str.lower() in ('', '0', 'false'):
+            cxx_has_stdcxx0x_flag = False
+        else:
+            lit_config.fatal(
+                'user parameter cxx_has_stdcxx0x_flag_str should be 0 or 1')
     else:
-        lit_config.fatal(
-            'user parameter cxx_has_stdcxx0x_flag_str should be 0 or 1')
-else:
-    cxx_has_stdcxx0x_flag = getattr(config, 'cxx_has_stdcxx0x_flag', True)
-
-# This test suite supports testing against either the system library or the
-# locally built one; the former mode is useful for testing ABI compatibility
-# between the current headers and a shipping dynamic library.
-use_system_lib_str = lit_config.params.get('use_system_lib', None)
-if use_system_lib_str is not None:
-    if use_system_lib_str.lower() in ('1', 'true'):
-        use_system_lib = True
-    elif use_system_lib_str.lower() in ('', '0', 'false'):
-        use_system_lib = False
+        cxx_has_stdcxx0x_flag = getattr(config, 'cxx_has_stdcxx0x_flag', True)
+
+    # This test suite supports testing against either the system library or the
+    # locally built one; the former mode is useful for testing ABI compatibility
+    # between the current headers and a shipping dynamic library.
+    use_system_lib_str = lit_config.params.get('use_system_lib', None)
+    if use_system_lib_str is not None:
+        if use_system_lib_str.lower() in ('1', 'true'):
+            use_system_lib = True
+        elif use_system_lib_str.lower() in ('', '0', 'false'):
+            use_system_lib = False
+        else:
+            lit_config.fatal('user parameter use_system_lib should be 0 or 1')
     else:
-        lit_config.fatal('user parameter use_system_lib should be 0 or 1')
-else:
-    # Default to testing against the locally built libc++ library.
-    use_system_lib = False
-    lit_config.note("inferred use_system_lib as: %r" % (use_system_lib,))
-
-link_flags = []
-link_flags_str = lit_config.params.get('link_flags', None)
-if link_flags_str is None:
-    link_flags_str = getattr(config, 'link_flags', None)
+        # Default to testing against the locally built libc++ library.
+        use_system_lib = False
+        lit_config.note("inferred use_system_lib as: %r" % (use_system_lib,))
+
+    link_flags = []
+    link_flags_str = lit_config.params.get('link_flags', None)
     if link_flags_str is None:
-      cxx_abi = getattr(config, 'cxx_abi', 'libcxxabi')
-      if cxx_abi == 'libstdc++':
-        link_flags += ['-lstdc++']
-      elif cxx_abi == 'libsupc++':
-        link_flags += ['-lsupc++']
-      elif cxx_abi == 'libcxxabi':
-        link_flags += ['-lc++abi']
-      elif cxx_abi == 'none':
-        pass
-      else:
-        lit_config.fatal('C++ ABI setting %s unsupported for tests' % cxx_abi)
-
-      if sys.platform == 'darwin':
-        link_flags += ['-lSystem']
-      elif sys.platform == 'linux2':
-        link_flags += [ '-lgcc_eh', '-lc', '-lm', '-lpthread',
-              '-lrt', '-lgcc_s']
-      else:
+        link_flags_str = getattr(config, 'link_flags', None)
+        if link_flags_str is None:
+          cxx_abi = getattr(config, 'cxx_abi', 'libcxxabi')
+          if cxx_abi == 'libstdc++':
+            link_flags += ['-lstdc++']
+          elif cxx_abi == 'libsupc++':
+            link_flags += ['-lsupc++']
+          elif cxx_abi == 'libcxxabi':
+            link_flags += ['-lc++abi']
+          elif cxx_abi == 'none':
+            pass
+          else:
+            lit_config.fatal('C++ ABI setting %s unsupported for tests' % cxx_abi)
+
+          if sys.platform == 'darwin':
+            link_flags += ['-lSystem']
+          elif sys.platform == 'linux2':
+            link_flags += [ '-lgcc_eh', '-lc', '-lm', '-lpthread',
+                  '-lrt', '-lgcc_s']
+          else:
+            lit_config.fatal("unrecognized system")
+
+          lit_config.note("inferred link_flags as: %r" % (link_flags,))
+    if not link_flags_str is None:
+        link_flags += shlex.split(link_flags_str)
+
+    # Configure extra compiler flags.
+    include_paths = ['-I' + libcxx_src_root + '/include',
+        '-I' + libcxx_src_root + '/test/support']
+    library_paths = ['-L' + libcxx_obj_root + '/lib']
+    compile_flags = []
+    if cxx_has_stdcxx0x_flag:
+        compile_flags += ['-std=c++0x']
+
+    # Configure extra linker parameters.
+    exec_env = {}
+    if sys.platform == 'darwin':
+        if not use_system_lib:
+            exec_env['DYLD_LIBRARY_PATH'] = os.path.join(libcxx_obj_root, 'lib')
+    elif sys.platform == 'linux2':
+        if not use_system_lib:
+            link_flags += ['-Wl,-R', libcxx_obj_root + '/lib']
+        compile_flags += ['-D__STDC_FORMAT_MACROS', '-D__STDC_LIMIT_MACROS',
+            '-D__STDC_CONSTANT_MACROS']
+    else:
         lit_config.fatal("unrecognized system")
 
-      lit_config.note("inferred link_flags as: %r" % (link_flags,))
-if not link_flags_str is None:
-    link_flags += shlex.split(link_flags_str)
-
-# Configure extra compiler flags.
-include_paths = ['-I' + libcxx_src_root + '/include',
-    '-I' + libcxx_src_root + '/test/support']
-library_paths = ['-L' + libcxx_obj_root + '/lib']
-compile_flags = []
-if cxx_has_stdcxx0x_flag:
-    compile_flags += ['-std=c++0x']
-
-# Configure extra linker parameters.
-exec_env = {}
-if sys.platform == 'darwin':
-    if not use_system_lib:
-        exec_env['DYLD_LIBRARY_PATH'] = os.path.join(libcxx_obj_root, 'lib')
-elif sys.platform == 'linux2':
-    if not use_system_lib:
-        link_flags += ['-Wl,-R', libcxx_obj_root + '/lib']
-    compile_flags += ['-D__STDC_FORMAT_MACROS', '-D__STDC_LIMIT_MACROS',
-        '-D__STDC_CONSTANT_MACROS']
-else:
-    lit_config.fatal("unrecognized system")
-
-config.test_format = LibcxxTestFormat(
-    cxx_under_test,
-    cpp_flags = ['-nostdinc++'] + compile_flags + include_paths,
-    ld_flags = ['-nodefaultlibs'] + library_paths + ['-lc++'] + link_flags,
-    exec_env = exec_env)
-
-# Get or infer the target triple.
-config.target_triple = lit_config.params.get('target_triple', None)
-# If no target triple was given, try to infer it from the compiler under test.
-if config.target_triple is None:
-    config.target_triple = lit.util.capture(
-        [cxx_under_test, '-dumpmachine']).strip()
-    lit_config.note("inferred target_triple as: %r" % (config.target_triple,))
-
-# Write an "available feature" that combines the triple when use_system_lib is
-# enabled. This is so that we can easily write XFAIL markers for tests that are
-# known to fail with versions of libc++ as were shipped with a particular
-# triple.
-if use_system_lib:
-    # Drop sub-major version components from the triple, because the current
-    # XFAIL handling expects exact matches for feature checks.
-    sanitized_triple = re.sub(r"([^-]+)-([^-]+)-([^-.]+).*", r"\1-\2-\3",
-                              config.target_triple)
-    config.available_features.add('with_system_lib=%s' % (sanitized_triple,))
+    config.test_format = LibcxxTestFormat(
+        cxx_under_test,
+        cpp_flags = ['-nostdinc++'] + compile_flags + include_paths,
+        ld_flags = ['-nodefaultlibs'] + library_paths + ['-lc++'] + link_flags,
+        exec_env = exec_env)
+
+    # Get or infer the target triple.
+    config.target_triple = lit_config.params.get('target_triple', None)
+    # If no target triple was given, try to infer it from the compiler under test.
+    if config.target_triple is None:
+        config.target_triple = lit.util.capture(
+            [cxx_under_test, '-dumpmachine']).strip()
+        lit_config.note("inferred target_triple as: %r" % (config.target_triple,))
+
+    # Write an "available feature" that combines the triple when use_system_lib is
+    # enabled. This is so that we can easily write XFAIL markers for tests that are
+    # known to fail with versions of libc++ as were shipped with a particular
+    # triple.
+    if use_system_lib:
+        # Drop sub-major version components from the triple, because the current
+        # XFAIL handling expects exact matches for feature checks.
+        sanitized_triple = re.sub(r"([^-]+)-([^-]+)-([^-.]+).*", r"\1-\2-\3",
+                                  config.target_triple)
+        config.available_features.add('with_system_lib=%s' % (sanitized_triple,))