Merge libc++abi's lit configuration with libc++'s

[jroelofs]

Summary

This is WIP, it doesn't quite work yet. On my dawin machine, I think it is picking up the system libc++abi, rather than the just-built one (maybe I've got the default backwards? not sure).

@danalbert, what do you think? Am I missing anything big?

[jroelofs]

Also, the diff lies... not sure why. It is showing that I deleted all these files, but in fact I've "svn mv"-ed them.

[danalbert]

Should improve test times significantly, since you seem to have deleted all of them ;)

I'm assuming you renamed them with .pass.cpp and forgot to add the new names (git mv is your friend). As I said in IRC, I don't really like building metadata in to the test name. I'd rather see // XCOMPILE-FAIL or similar added to the test runner instead.

Looks much better. I'm especially liking all the red. I'll try cherry-picking this tomorrow to see if it meets all my needs, but it looks like it should.

libcxxabi/test/config.py

24

os.path.join

28

I thought obj_root was supposed to be the build output directory.

31

?

36

os.path.join (and anywhere else you've done this)

56

?

lit.cfg

56

config_module = importlib.import_module(config_module_name)

I would have done that when I wrote it the first time but I had thought we bumped the minimum version to 2.6, not 2.7.

190

Did I see a patch to libc++ that added this to libcxx.test.config.Configuration earlier? I assume that's why this isn't needed any more.

lit.site.cfg.in

8

Is this actually used for anything? If it is it's in the libc++ side of this, but I can't think of what we'd actually be using it for...

[jroelofs]

I svn mv'd all the files to their *.pass.cpp equivalents. Not sure why they aren't showing up in the diff.

I could go either way on the metadata thing. Or do both, and have checks that the extension matches the metadata. Don't really have a preference.

libcxxabi/test/config.py

28

Copy-pasta from the libc++ config. That's just a default for when it can't find the right thing from cmake. I'm not sure what else to default it to.

31

That was something @EricWF added to turn off measuring the durations of things like the demangler tests.

56

The guard is named differently between libc++abi and libc++.

lit.cfg

190

Yeah.

lit.site.cfg.in

8

You're right, enable_exceptions, enable_rtti, and enable_shared aren't useful for libc++abi. Good catch.

[jroelofs]

Address most of @danalbert's comments. I'm still not sure what I'm doing wrong with the library paths.... this still breaks tests on darwin.

[jroelofs]

CMakeLists.txt

47 ↗

(On Diff #18232)

Diffs in this file bring libc++abi's version of this file most of the way in sync with libc++'s. I'm not too familiar with cmake to know whether 'add_custom_target' vs 'add_lit_testsuite' is better here. I suspect the change should be going the other way?

libcxxabi/test/config.py

23

this seems like a reasonable default now. I should delete this comment.

[danalbert]

Pinging in the hope that it will reach cfe-commits.

[jroelofs]

Fix up the library path stuff. This at least works on darwin now. Haven't checked on linux yet.

[EricWF]

I can't patch CMakeLists.txt completely, Can you make sure that you diffed against the trunk?

[jroelofs]

I suspect that's because of @danalbert's recent cmake changes for cross compiling... lame.

[jroelofs]

Rebase, and add detection/configuration bits that make the libcxx imports work. Symlinking test/libcxx is no longer necessary.

Tests work on darwin now. Still haven't checked on linux.

[jroelofs]

I think I ought to separate the "rename all the files" change from this.

[jroelofs]

Here's a separate patch that does the file renaming: http://reviews.llvm.org/D7101

[jroelofs]

Rebase after D7101

[danalbert]

CMakeLists.txt

47 ↗

(On Diff #18542)

What prevents us from continuing to use add_lit_testsuite? If I'm reading the definition correctly, the function will automatically use LLVM_LIT_ARGS (used indirectly in add_lit_target). That also keeps the FindPythonInterp` stuff delegated to the LLVM cmake code.

libcxxabi/test/config.py

44

os.path.join (The leading slash isn't necessary with os.path.join. I'd like to believe Windows will handle that gracefully, but I don't actually know.)

[jroelofs]

I could have merged that either way, actually. I wasn't sure which one was more preferable.

[danalbert]

Reusing the LLVM cmake code is preferred. Once that is fixed up, LGTM.

[jroelofs]

Merge the CMakeLists.txt the other way.

[jroelofs]

Committed in r226737, (and buildbots appeased in r226749)

libcxxabi/test/config.py

+import locale
+import os
+import platform
+import re
+import shlex
+import sys
+
+import lit.Test  # pylint: disable=import-error,no-name-in-module
+import lit.util  # pylint: disable=import-error,no-name-in-module
+
+from libcxx.test.format import LibcxxTestFormat
+from libcxx.test.config import Configuration as LibcxxConfiguration
+
+class Configuration(LibcxxConfiguration):
+    # pylint: disable=redefined-outer-name
+    def __init__(self, lit_config, config):
+        super(Configuration, self).__init__(lit_config, config)
+
+    def configure_src_root(self):
+        self.libcxxabi_src_root = self.get_lit_conf(
+            'libcxxabi_src_root', os.path.dirname(self.config.test_source_root))
+        # TODO(jroelofs): not sure if I've defaulted libcxx_src_root correctly
+        self.libcxx_src_root = self.get_lit_conf(
+            'libcxx_src_root', self.libcxxabi_src_root + '/../libcxx')
+
+    def configure_obj_root(self):
+        self.obj_root = self.get_lit_conf('libcxxabi_obj_root',
+                                          self.libcxxabi_src_root)
+
+    def configure_compile_flags(self):
+        self.compile_flags += ['-DLIBCXXABI_NO_TIMER']
+        super(Configuration, self).configure_compile_flags()
+
+    def configure_compile_flags_header_includes(self):
+        cxx_headers = self.get_lit_conf('cxx_headers',
+                                        self.libcxx_src_root + '/include')
+        print 'libcxx_src_root is ', self.libcxx_src_root
+        if not os.path.isdir(cxx_headers):
+            self.lit_config.fatal("cxx_headers='%s' is not a directory."
+                                  % cxx_headers)
+        self.compile_flags += ['-I' + cxx_headers]
+        libcxxabi_headers = self.get_lit_conf('libcxxabi_headers',
+                                              self.libcxxabi_src_root + '/include')
+        if not os.path.isdir(libcxxabi_headers):
+            self.lit_config.fatal("libcxxabi_headers='%s' is not a directory."
+                                  % libcxxabi_headers)
+        self.compile_flags += ['-I' + libcxxabi_headers]
+
+    def configure_compile_flags_exceptions(self):
+        pass
+
+    def configure_compile_flags_rtti(self):
+        pass
+
+    def configure_compile_flags_no_threads(self):
+        self.compile_flags += ['-DLIBCXXABI_HAS_NO_THREADS=1']
+
+    def configure_compile_flags_no_monotonic_clock(self):
+        pass

lit.cfg

-# -*- Python -*- vim: set syntax=python tabstop=4 expandtab cc=80:
-
+# -*- Python -*- vim: set ft=python ts=4 sw=4 expandtab tw=79:
 # Configuration file for the 'lit' test runner.
-
-import errno
 import os
-import platform
-import re
-import shlex
-import signal
-import subprocess
-import sys
-import tempfile
-import time
+import site
 
-import lit.Test
-import lit.formats
-import lit.util
+site.addsitedir(os.path.dirname(__file__))
 
-class LibcxxabiTestFormat(lit.formats.FileBasedTest):
-    """
-    Custom test format handler for use with the test format use by libc++abi.
-    """
 
-    def __init__(self, cxx_under_test, cpp_flags, ld_flags, exec_env):
-        self.cxx_under_test = cxx_under_test
-        self.cpp_flags = list(cpp_flags)
-        self.ld_flags = list(ld_flags)
-        self.exec_env = dict(exec_env)
+# Tell pylint that we know config and lit_config exist somewhere.
+if 'PYLINT_IMPORT' in os.environ:
+    config = object()
+    lit_config = object()
 
-    def execute(self, test, lit_config):
-        while True:
-            try:
-                return self._execute(test, lit_config)
-            except OSError as oe:
-                if oe.errno != errno.ETXTBSY:
-                    raise
-                time.sleep(0.1)
-
-    def _execute(self, test, lit_config):
-        # Extract test metadata from the test file.
-        requires = []
-        unsupported = []
-        with open(test.getSourcePath()) as f:
-            for ln in f:
-                if 'XFAIL:' in ln:
-                    items = ln[ln.index('XFAIL:') + 6:].split(',')
-                    test.xfails.extend([s.strip() for s in items])
-                elif 'REQUIRES:' in ln:
-                    items = ln[ln.index('REQUIRES:') + 9:].split(',')
-                    requires.extend([s.strip() for s in items])
-                elif 'UNSUPPORTED:' in ln:
-                    items = ln[ln.index('UNSUPPORTED:') + 12:].split(',')
-                    unsupported.extend([s.strip() for s in items])
-                elif not ln.strip().startswith("//") and ln.strip():
-                    # Stop at the first non-empty line that is not a C++
-                    # comment.
-                    break
-
-        # Check that we have the required features.
-        #
-        # FIXME: For now, this is cribbed from lit.TestRunner, to avoid
-        # introducing a dependency there. What we more ideally would like to do
-        # is lift the "requires" handling to be a core lit framework feature.
-        missing_required_features = [f for f in requires
-                                     if f not in test.config.available_features]
-        if missing_required_features:
-            return (lit.Test.UNSUPPORTED,
-                    "Test requires the following features: %s" % (
-                      ', '.join(missing_required_features),))
-
-        unsupported_features = [f for f in unsupported
-                             if f in test.config.available_features]
-        if unsupported_features:
-            return (lit.Test.UNSUPPORTED,
-                    "Test is unsupported with the following features: %s" % (
-                       ', '.join(unsupported_features),))
-
-        # Evaluate the test.
-        return self._evaluate_test(test, lit_config)
-
-    def _evaluate_test(self, test, lit_config):
-        name = test.path_in_suite[-1]
-        source_path = test.getSourcePath()
-        source_dir = os.path.dirname(source_path)
-
-        # If this is a compile (failure) test, build it and check for failure.
-        exec_file = tempfile.NamedTemporaryFile(suffix="exe", delete=False)
-        exec_path = exec_file.name
-        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 = lit.util.executeCommand(cmd)
-            if exitCode != 0:
-                report = """Command: %s\n""" % ' '.join(["'%s'" % a
-                                                         for a in cmd])
-                report += """Exit Code: %d\n""" % exitCode
-                if out:
-                    report += """Standard Output:\n--\n%s--""" % out
-                if err:
-                    report += """Standard Error:\n--\n%s--""" % err
-                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 = lit.util.executeCommand(cmd, cwd=source_dir)
-            if exitCode != 0:
-                report = """Compiled With: %s\n""" % \
-                    ' '.join(["'%s'" % a for a in compile_cmd])
-                report += """Command: %s\n""" % \
-                    ' '.join(["'%s'" % a for a in cmd])
-                report += """Exit Code: %d\n""" % exitCode
-                if out:
-                    report += """Standard Output:\n--\n%s--""" % out
-                if err:
-                    report += """Standard Error:\n--\n%s--""" % err
-                report += "\n\nCompiled test failed unexpectedly!"
-                return lit.Test.FAIL, report
-        finally:
-            try:
-                os.remove(exec_path)
-            except:
-                pass
-        return lit.Test.PASS, ""
-
 # name: The name of this test suite.
 config.name = 'libc++abi'
 

 # 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)
+# Infer the test_exec_root from the libcxxabi_object root.
+libcxxabi_obj_root = getattr(config, 'libcxxabi_obj_root', None)
+if libcxxabi_obj_root is not None:
+    config.test_exec_root = os.path.join(libcxxabi_obj_root, 'test')
 
-    # 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++)')
-
-libcxxabi_src_root = lit_config.params.get('libcxxabi_src_root', None)
-if libcxxabi_src_root is None:
-    libcxxabi_src_root = getattr(config, 'libcxxabi_src_root', None)
-    if libcxxabi_src_root is None:
-        libcxxabi_src_root = os.path.dirname(config.test_source_root)
-
-libcxxabi_library_root = lit_config.params.get('libcxxabi_library_root', None)
-if libcxxabi_library_root is None:
-    libcxxabi_library_root = getattr(config, 'libcxxabi_library_root', None)
-    if libcxxabi_library_root is None:
-        lit_config.fatal("libcxxabi_library_root must be defined")
-
-libcxx_includes = lit_config.params.get('libcxx_includes', None)
-if libcxx_includes is None:
-    libcxx_includes = getattr(config, 'libcxx_includes', None)
-    if libcxx_includes is None:
-        lit_config.fatal("libcxx_includes must be defined")
-
-enable_shared = lit_config.params.get('enable_shared', None)
-if enable_shared is None:
-    enable_shared = getattr(config, 'enable_shared', None)
-    if enable_shared is None:
-        lit_config.fatal("enable_shared must be defined")
-
-enable_threads = lit_config.params.get('enable_threads', None)
-if enable_threads is None:
-    enable_threads = getattr(config, 'enable_threads', None)
-    if enable_threads is None:
-        lit_config.fatal('enable_threads must be defined')
-
-llvm_unwinder = getattr(config, 'llvm_unwinder', None)
-if llvm_unwinder is None:
-    lit_config.fatal("llvm_unwinder must be defined")
-
-
-# Compiler parameters
-include_paths = ['-I' + libcxxabi_src_root + '/include',
-                 '-I' + libcxx_includes]
-library_paths = ['-L' + libcxxabi_library_root]
-compile_flags = ['-std=c++11']
-link_flags = []
-exec_env = {}
-
-link_flags_str = lit_config.params.get('link_flags', None)
-if link_flags_str is None:
-    link_flags_str = getattr(config, 'link_flags', None)
-    if link_flags_str is None:
-        if enable_shared:
-            link_flags += ['-lc++abi']
-        if sys.platform == 'darwin':
-            link_flags += ['-lSystem']
-        elif sys.platform.startswith('linux'):
-            if not llvm_unwinder:
-                link_flags += ['-lgcc_eh']
-            link_flags += ['-lc', '-lm']
-            if enable_threads:
-                link_flags += ['-lpthread']
-            if llvm_unwinder:
-                link_flags += ['-lunwind', '-ldl']
-            else:
-                link_flags += ['-lgcc_s']
-        else:
-            lit_config.fatal("unrecognized system")
-
-        lit_config.note("inferred link_flags as: %r" % (link_flags,))
-if link_flags_str is not None:
-    link_flags += shlex.split(link_flags_str)
-
-# Configure extra compiler flags.
-if not enable_threads:
-    compile_flags += ['-DLIBCXXABI_HAS_NO_THREADS=1']
-
-# Always disable timed test when using LIT since the output gets lost and since
-# using the timer requires <chrono> as a dependancy.
-compile_flags += ['-DLIBCXXABI_NO_TIMER']
-
-san = lit_config.params.get('llvm_use_sanitizer', None)
-if san is None:
-    san = getattr(config, 'llvm_use_sanitizer', None)
-if san:
-    # Search for llvm-symbolizer along the compiler path first
-    # and then along the PATH env variable.
-    symbolizer_search_paths = os.environ.get('PATH', '')
-    cxx_path = lit.util.which(cxx_under_test)
-    if cxx_path is not None:
-        symbolizer_search_paths = os.path.dirname(cxx_path) + \
-                                    os.pathsep + symbolizer_search_paths
-    llvm_symbolizer = lit.util.which('llvm-symbolizer',
-                                        symbolizer_search_paths)
-    compile_flags += ['-g', '-fno-omit-frame-pointer']
-    if sys.platform.startswith('linux'):
-        link_flags += ['-ldl']
-    if san == 'Address':
-        compile_flags += ['-fsanitize=address']
-        if llvm_symbolizer is not None:
-            exec_env['ASAN_SYMBOLIZER_PATH'] = llvm_symbolizer
-        config.available_features.add('asan')
-    elif san == 'Memory' or san == 'MemoryWithOrigins':
-        compile_flags += ['-fsanitize=memory']
-        if llvm_symbolizer is not None:
-            exec_env['MSAN_SYMBOLIZER_PATH'] = llvm_symbolizer
-        config.available_features.add('msan')
-        if san == 'MemoryWithOrigins':
-            compile_flags += ['-fsanitize-memory-track-origins']
-    elif san == 'Undefined':
-        compile_flags += ['-fsanitize=undefined',
-                          '-fno-sanitize=vptr,function',
-                          '-fno-sanitize-recover']
-        config.available_features.add('ubsan')
-    elif san == 'Thread':
-        compile_flags += ['-fsanitize=thread']
-        config.available_features.add('tsan')
+# Check that the test exec root is known.
+if config.test_exec_root is None:
+    # Otherwise, we haven't loaded the site specific configuration (the user is
+    # probably trying to run on a test file directly, and either the site
+    # configuration hasn't been created by the build system, or we are in an
+    # out-of-tree build situation).
+    site_cfg = lit_config.params.get('libcxxabi_site_config',
+                                     os.environ.get('LIBCXX_SITE_CONFIG'))
+    if not site_cfg:
+        lit_config.warning('No site specific configuration file found!'
+                           ' Running the tests in the default configuration.')
+        # TODO: Set test_exec_root to a temporary directory where output files
+        # can be placed. This is needed for ShTest.
+    elif not os.path.isfile(site_cfg):
+        lit_config.fatal(
+            "Specified site configuration file does not exist: '%s'" %
+            site_cfg)
     else:
-        lit_config.fatal('unsupported value for '
-                         'llvm_use_sanitizer: {0}'.format(san))
+        lit_config.note('using site specific configuration at %s' % site_cfg)
+        lit_config.load_config(config, site_cfg)
+        raise SystemExit()
 
+cfg_variant = getattr(config, 'configuration_variant', 'libcxxabi')
+if cfg_variant:
+    print 'Using configuration variant: %s' % cfg_variant
 
-# Configure extra linker parameters.
+# Load the Configuration class from the module name <cfg_variant>.test.config.
+config_module_name = '.'.join([cfg_variant, 'test', 'config'])
+config_module = __import__(config_module_name, fromlist=['Configuration'])
 
-if sys.platform == 'darwin':
-    exec_env['DYLD_LIBRARY_PATH'] = os.path.join(libcxxabi_library_root)
-elif sys.platform.startswith('linux'):
-    link_flags += ['-Wl,-R', libcxxabi_library_root]
-else:
-    lit_config.fatal("unrecognized system")
-
-config.available_features.add(sys.platform)
-
-config.test_format = LibcxxabiTestFormat(
-    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,))
+configuration = config_module.Configuration(lit_config, config)
+configuration.configure()
+config.test_format = configuration.get_test_format()

lit.site.cfg.in

 @AUTO_GEN_COMMENT@
-config.cxx_under_test         = "@LIBCXXABI_COMPILER@"
-config.libcxxabi_src_root     = "@LIBCXXABI_SOURCE_DIR@"
-config.libcxxabi_library_root = "@LIBCXXABI_LIBRARY_DIR@"
-config.python_executable      = "@PYTHON_EXECUTABLE@"
-config.enable_shared          = @LIBCXXABI_ENABLE_SHARED@
-config.libcxx_includes        = "@LIBCXXABI_LIBCXX_INCLUDES@"
-config.llvm_unwinder          = @LIBCXXABI_USE_LLVM_UNWINDER@
-config.enable_threads         = @LIBCXXABI_ENABLE_THREADS@
-config.llvm_use_sanitizer     = "@LLVM_USE_SANITIZER@"
+config.cxx_under_test           = "@LIBCXXABI_COMPILER@"
+config.libcxxabi_src_root       = "@LIBCXXABI_SOURCE_DIR@"
+config.libcxxabi_obj_root       = "@LIBCXXABI_BINARY_DIR@"
+config.libcxxabi_library_root   = "@LIBCXXABI_LIBRARY_DIR@"
+config.enable_exceptions        = "@LIBCXXABI_ENABLE_EXCEPTIONS@"
+config.enable_rtti              = "@LIBCXXABI_ENABLE_RTTI@"
+config.enable_shared            = "@LIBCXXABI_ENABLE_SHARED@"
+config.enable_32bit             = "@LIBCXXABI_BUILD_32_BITS@"
+config.enable_threads           = "@LIBCXXABI_ENABLE_THREADS@"
+config.use_sanitizer            = "@LLVM_USE_SANITIZER@"
+config.llvm_unwinder            = "@LIBCXXABI_USE_LLVM_UNWINDER@"
 
 # Let the main config do the real work.
 lit_config.load_config(config, "@LIBCXXABI_SOURCE_DIR@/test/lit.cfg")