[libcxx-commits] [libcxx] ec46cfe - [libcxx] Simplify back-deployment testing

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Thu Sep 10 05:17:43 PDT 2020


Author: Louis Dionne
Date: 2020-09-10T08:17:26-04:00
New Revision: ec46cfefe80d58cdc7068ad4e4f8efde6d94d835

URL: https://github.com/llvm/llvm-project/commit/ec46cfefe80d58cdc7068ad4e4f8efde6d94d835
DIFF: https://github.com/llvm/llvm-project/commit/ec46cfefe80d58cdc7068ad4e4f8efde6d94d835.diff

LOG: [libcxx] Simplify back-deployment testing

The needs of back-deployment testing currently require two different
ways of running the test suite: one based on the deployment target,
and one based on the target triple. Since the triple includes all the
information we need, it's better to have just one way of doing things.

Furthermore, `--param platform=XXX` is also supersedded by using the
target triple. Previously, this parameter would serve the purpose of
controling XFAILs for availability markup errors, however it is possible
to achieve the same thing by using with_system_cxx_lib only and using
.verify.cpp tests instead, as explained in the documentation changes.

The motivation for this change is twofold:
1. This part of the Lit config has always been really confusing and
   complicated, and it has been a source of bugs in the past. I have
   simplified it iteratively in the past, but the complexity is still
   there.
2. The deployment-target detection started failing in weird ways in
   recent Clangs, breaking our CI. Instead of band-aid patching the
   issue, I decided to remove the complexity altogether by using target
   triples even on Apple platforms.

A follow-up to this commit will bring the test suite in line with
the recommended way of handling availability markup tests.

Added: 
    

Modified: 
    libcxx/docs/DesignDocs/AvailabilityMarkup.rst
    libcxx/test/configs/legacy.cfg.in
    libcxx/utils/ci/macos-backdeployment.sh
    libcxx/utils/libcxx/test/config.py
    libcxx/utils/libcxx/test/target_info.py
    libcxxabi/test/lit.site.cfg.in
    libunwind/test/lit.site.cfg.in

Removed: 
    


################################################################################
diff  --git a/libcxx/docs/DesignDocs/AvailabilityMarkup.rst b/libcxx/docs/DesignDocs/AvailabilityMarkup.rst
index 87ad0abb62d7..238038539287 100644
--- a/libcxx/docs/DesignDocs/AvailabilityMarkup.rst
+++ b/libcxx/docs/DesignDocs/AvailabilityMarkup.rst
@@ -64,31 +64,35 @@ Testing
 Some parameters can be passed to lit to run the test-suite and exercise the
 availability.
 
-* The `platform` parameter controls the deployment target. For example lit can
-  be invoked with `--param=platform=macosx10.12`. Default is the current host.
-* The `use_system_cxx_lib` parameter indicates that the test suite is being run
-  against a system library.
+* The `target_triple` parameter controls the deployment target. For example lit
+  can be invoked with `--param=target_triple=x86_64-apple-macosx10.12`.
+  Default is the current host.
+* The `use_system_cxx_lib` parameter indicates that the test suite is being
+  compiled with the intent of being run against the system library for the
+  given triple, AND that it is being run against it.
 
-Tests can be marked as XFAIL based on multiple features made available by lit:
-
-* if `--param=platform=macosx10.12` is passed, the following features will be available:
-
-  - availability=macosx
-  - availability=macosx10.12
-
-  This feature is used to XFAIL a test that *is* using a class or a method marked
-  as unavailable *and* that is expected to *fail* if deployed on an older system.
-
-* if `use_system_cxx_lib` and `--param=platform=macosx10.12` are passed to lit,
-  the following features will also be available:
+Tests can be marked as XFAIL based on multiple features made available by lit.
+If `use_system_cxx_lib` is true, then assuming `target_triple=x86_64-apple-macosx10.12`,
+the following features will be made available:
 
   - with_system_cxx_lib=macosx
   - with_system_cxx_lib=macosx10.12
   - with_system_cxx_lib=x86_64-apple-macosx10.12
+  - availability=macosx
+  - availability=macosx10.12
 
-  This feature is used to XFAIL a test that is *not* using a class or a method
-  marked as unavailable *but* that is expected to fail if deployed on an older
-  system. For example, if the test exhibits a bug in the libc on a particular
-  system version, or if the test uses a symbol that is not available on an
-  older version of the dylib (but for which there is no availability markup,
-  otherwise the XFAIL should use `availability` above).
+These features are used to XFAIL a test that fails when deployed on (or is
+compiled for) an older system. For example, if the test exhibits a bug in the
+libc on a particular system version, or if the test uses a symbol that is not
+available on an older version of the dylib, it can be marked as XFAIL with
+one of the above features.
+
+It is sometimes useful to check that a test fails specifically when compiled
+for a given deployment target. For example, this is the case when testing
+availability markup, where we want to make sure that using the annotated
+facility on a deployment target that doesn't support it will fail at compile
+time, not at runtime. This can be achieved by creating a `.compile.pass.cpp`
+and XFAILing it for the right deployment target. If the test doesn't fail at
+compile-time like it's supposed to, the test will XPASS. Another option is to
+create a `.verify.cpp` test that checks for the right errors, and mark that
+test as requiring `with_system_cxx_lib=<something>`.

diff  --git a/libcxx/test/configs/legacy.cfg.in b/libcxx/test/configs/legacy.cfg.in
index 1f3370ccc9bc..efb41a93e41b 100644
--- a/libcxx/test/configs/legacy.cfg.in
+++ b/libcxx/test/configs/legacy.cfg.in
@@ -21,7 +21,6 @@ config.abi_library_path         = "@LIBCXX_CXX_ABI_LIBRARY_PATH@"
 config.configuration_variant    = "@LIBCXX_LIT_VARIANT@"
 config.host_triple              = "@LLVM_HOST_TRIPLE@"
 config.target_triple            = "@TARGET_TRIPLE@"
-config.use_target               = bool("@LIBCXX_TARGET_TRIPLE@")
 config.sysroot                  = "@LIBCXX_SYSROOT@"
 config.gcc_toolchain            = "@LIBCXX_GCC_TOOLCHAIN@"
 config.generate_coverage        = @LIBCXX_GENERATE_COVERAGE@

diff  --git a/libcxx/utils/ci/macos-backdeployment.sh b/libcxx/utils/ci/macos-backdeployment.sh
index 24b866cdc1ae..04549aa34645 100755
--- a/libcxx/utils/ci/macos-backdeployment.sh
+++ b/libcxx/utils/ci/macos-backdeployment.sh
@@ -134,7 +134,7 @@ echo "@@@ Running tests for libc++ @@@"
                                  ${ENABLE_FILESYSTEM} \
                                  --param=cxx_headers="${LLVM_INSTALL_DIR}/include/c++/v1" \
                                  --param=std="${STD}" \
-                                 --param=platform="macosx${DEPLOYMENT_TARGET}" \
+                                 --param=target_triple="x86_64-apple-macosx${DEPLOYMENT_TARGET}" \
                                  --param=cxx_library_root="${LLVM_INSTALL_DIR}/lib" \
                                  --param=cxx_runtime_root="${LIBCXX_ROOT_ON_DEPLOYMENT_TARGET}" \
                                  --param=abi_library_path="${LIBCXXABI_ROOT_ON_DEPLOYMENT_TARGET}" \

diff  --git a/libcxx/utils/libcxx/test/config.py b/libcxx/utils/libcxx/test/config.py
index 82b696f76eec..c8bfdda91463 100644
--- a/libcxx/utils/libcxx/test/config.py
+++ b/libcxx/utils/libcxx/test/config.py
@@ -8,7 +8,6 @@
 
 import copy
 import os
-import platform
 import pkgutil
 import pipes
 import re
@@ -72,7 +71,6 @@ def __init__(self, lit_config, config):
         self.link_shared = self.get_lit_bool('enable_shared', default=True)
         self.debug_build = self.get_lit_bool('debug_build',   default=False)
         self.exec_env = dict()
-        self.use_target = False
         self.use_system_cxx_lib = self.get_lit_bool('use_system_cxx_lib', False)
         self.use_clang_verify = False
 
@@ -123,7 +121,6 @@ def configure(self):
         self.executor = self.get_lit_conf('executor')
         self.configure_cxx()
         self.configure_triple()
-        self.configure_deployment()
         self.configure_src_root()
         self.configure_obj_root()
         self.cxx_stdlib_under_test = self.get_lit_conf('cxx_stdlib_under_test', 'libc++')
@@ -248,22 +245,15 @@ def configure_features(self):
         # XFAIL markers for tests that are known to fail with versions of
         # libc++ as were shipped with a particular triple.
         if self.use_system_cxx_lib:
-            self.config.available_features.add('with_system_cxx_lib=%s' % self.config.target_triple)
-
-            # Add available features for more generic versions of the target
-            # triple attached to  with_system_cxx_lib.
-            if self.use_deployment:
-                (_, name, version) = self.config.deployment
-                self.config.available_features.add('with_system_cxx_lib=%s' % name)
-                self.config.available_features.add('with_system_cxx_lib=%s%s' % (name, version))
-
-        # Configure the availability feature. Availability is only enabled
-        # with libc++, because other standard libraries do not provide
-        # availability markup.
-        if self.use_deployment and self.cxx_stdlib_under_test == 'libc++':
-            (_, name, version) = self.config.deployment
-            self.config.available_features.add('availability=%s' % name)
-            self.config.available_features.add('availability=%s%s' % (name, version))
+            (arch, vendor, platform) = self.config.target_triple.split('-')
+            (sysname, version) = re.match(r'([^0-9]+)([0-9\.]*)', platform).groups()
+
+            self.config.available_features.add('with_system_cxx_lib={}-{}-{}{}'.format(arch, vendor, sysname, version))
+            self.config.available_features.add('with_system_cxx_lib={}{}'.format(sysname, version))
+            self.config.available_features.add('with_system_cxx_lib={}'.format(sysname))
+
+            self.config.available_features.add('availability={}'.format(sysname))
+            self.config.available_features.add('availability={}{}'.format(sysname, version))
 
         if self.target_info.is_windows():
             if self.cxx_stdlib_under_test == 'libc++':
@@ -317,20 +307,19 @@ def configure_default_compile_flags(self):
         # being elided.
         if self.target_info.is_windows() and self.debug_build:
             self.cxx.compile_flags += ['-D_DEBUG']
-        if self.use_target:
-            if not self.cxx.addFlagIfSupported(
-                    ['--target=' + self.config.target_triple]):
-                self.lit_config.warning('use_target is true but --target is '\
-                        'not supported by the compiler')
-        if self.use_deployment:
-            arch, name, version = self.config.deployment
-            self.cxx.flags += ['-arch', arch]
-            self.cxx.flags += ['-m' + name + '-version-min=' + version]
+        if not self.cxx.addFlagIfSupported(['--target=' + self.config.target_triple]):
+            self.lit_config.warning('Not adding any target triple -- the compiler does '
+                                    'not support --target=<triple>')
 
         # Add includes for support headers used in the tests.
         support_path = os.path.join(self.libcxx_src_root, 'test/support')
         self.cxx.compile_flags += ['-I' + support_path]
 
+        # If we're testing the upstream LLVM libc++, disable availability markup,
+        # which is not relevant for non-shipped flavors of libc++.
+        if not self.use_system_cxx_lib:
+            self.cxx.compile_flags += ['-D_LIBCPP_DISABLE_AVAILABILITY']
+
         # Add includes for the PSTL headers
         pstl_src_root = self.get_lit_conf('pstl_src_root')
         pstl_obj_root = self.get_lit_conf('pstl_obj_root')
@@ -641,37 +630,15 @@ def configure_substitutions(self):
         if self.get_lit_conf('libcxx_gdb'):
             sub.append(('%{libcxx_gdb}', self.get_lit_conf('libcxx_gdb')))
 
-    def can_use_deployment(self):
-        # Check if the host is on an Apple platform using clang.
-        if not self.target_info.is_darwin():
-            return False
-        if not self.target_info.is_host_macosx():
-            return False
-        if not self.cxx.type.endswith('clang'):
-            return False
-        return True
-
     def configure_triple(self):
         # Get or infer the target triple.
         target_triple = self.get_lit_conf('target_triple')
-        self.use_target = self.get_lit_bool('use_target', False)
-        if self.use_target and target_triple:
-            self.lit_config.warning('use_target is true but no triple is specified')
-
-        # Use deployment if possible.
-        self.use_deployment = not self.use_target and self.can_use_deployment()
-        if self.use_deployment:
-            return
-
-        # Save the triple (and warn on Apple platforms).
-        self.config.target_triple = target_triple
-        if self.use_target and 'apple' in target_triple:
-            self.lit_config.warning('consider using arch and platform instead'
-                                    ' of target_triple on Apple platforms')
 
         # If no target triple was given, try to infer it from the compiler
         # under test.
-        if not self.config.target_triple:
+        if not target_triple:
+            self.lit_config.note('Trying to infer the target_triple because none was specified')
+
             target_triple = self.cxx.getTriple()
             # Drop sub-major version components from the triple, because the
             # current XFAIL handling expects exact matches for feature checks.
@@ -686,44 +653,10 @@ def configure_triple(self):
             if (target_triple.endswith('redhat-linux') or
                 target_triple.endswith('suse-linux')):
                 target_triple += '-gnu'
-            self.config.target_triple = target_triple
-            self.lit_config.note(
-                "inferred target_triple as: %r" % self.config.target_triple)
-
-    def configure_deployment(self):
-        assert not self.use_deployment is None
-        assert not self.use_target is None
-        if not self.use_deployment:
-            # Warn about ignored parameters.
-            if self.get_lit_conf('arch'):
-                self.lit_config.warning('ignoring arch, using target_triple')
-            if self.get_lit_conf('platform'):
-                self.lit_config.warning('ignoring platform, using target_triple')
-            return
-
-        assert not self.use_target
-        assert self.target_info.is_host_macosx()
-
-        # Always specify deployment explicitly on Apple platforms, since
-        # otherwise a platform is picked up from the SDK.  If the SDK version
-        # doesn't match the system version, tests that use the system library
-        # may fail spuriously.
-        arch = self.get_lit_conf('arch')
-        if not arch:
-            arch = self.cxx.getTriple().split('-', 1)[0]
-
-        _, name, version = self.target_info.get_platform()
-        self.config.deployment = (arch, name, version)
-
-        # Set the target triple for use by lit.
-        self.config.target_triple = arch + '-apple-' + name + version
-        self.lit_config.note(
-            "computed target_triple as: %r" % self.config.target_triple)
 
-        # If we're testing the upstream LLVM libc++, disable availability markup,
-        # which is not relevant for non-shipped flavors of libc++.
-        if not self.use_system_cxx_lib:
-            self.cxx.compile_flags += ['-D_LIBCPP_DISABLE_AVAILABILITY']
+        # Save the triple
+        self.lit_config.note("Setting target_triple to {}".format(target_triple))
+        self.config.target_triple = target_triple
 
     def configure_env(self):
         self.config.environment = dict(os.environ)

diff  --git a/libcxx/utils/libcxx/test/target_info.py b/libcxx/utils/libcxx/test/target_info.py
index 3197276ffa5b..4f19d60a1a87 100644
--- a/libcxx/utils/libcxx/test/target_info.py
+++ b/libcxx/utils/libcxx/test/target_info.py
@@ -73,34 +73,8 @@ def get_sdk_version(self, name):
 
         return re.sub(r'.*/[^0-9]+([0-9.]+)\.sdk', r'\1', out)
 
-    def get_platform(self):
-        platform = self.full_config.get_lit_conf('platform')
-        if platform:
-            platform = re.sub(r'([^0-9]+)([0-9\.]*)', r'\1-\2', platform)
-            name, version = tuple(platform.split('-', 1))
-        else:
-            name = 'macosx'
-            version = None
-
-        if version:
-            return (False, name, version)
-
-        # Infer the version, either from the SDK or the system itself.  For
-        # macosx, ignore the SDK version; what matters is what's at
-        # /usr/lib/libc++.dylib.
-        if name == 'macosx':
-            version = self.get_macosx_version()
-        else:
-            version = self.get_sdk_version(name)
-        return (True, name, version)
-
     def add_cxx_compile_flags(self, flags):
-        if self.full_config.use_deployment:
-            _, name, _ = self.full_config.config.deployment
-            cmd = ['xcrun', '--sdk', name, '--show-sdk-path']
-        else:
-            cmd = ['xcrun', '--show-sdk-path']
-        out, err, exit_code = executeCommand(cmd)
+        out, err, exit_code = executeCommand(['xcrun', '--show-sdk-path'])
         if exit_code != 0:
             self.full_config.lit_config.warning("Could not determine macOS SDK path! stderr was " + err)
         if exit_code == 0 and out:

diff  --git a/libcxxabi/test/lit.site.cfg.in b/libcxxabi/test/lit.site.cfg.in
index 06d5706da7d2..87f955e32161 100644
--- a/libcxxabi/test/lit.site.cfg.in
+++ b/libcxxabi/test/lit.site.cfg.in
@@ -25,7 +25,6 @@ config.enable_shared            = @LIBCXXABI_LINK_TESTS_WITH_SHARED_LIBCXX@
 config.enable_exceptions        = @LIBCXXABI_ENABLE_EXCEPTIONS@
 config.host_triple              = "@LLVM_HOST_TRIPLE@"
 config.target_triple            = "@TARGET_TRIPLE@"
-config.use_target               = bool("@LIBCXXABI_TARGET_TRIPLE@")
 config.sysroot                  = "@LIBCXXABI_SYSROOT@"
 config.gcc_toolchain            = "@LIBCXXABI_GCC_TOOLCHAIN@"
 config.cxx_ext_threads          = @LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY@

diff  --git a/libunwind/test/lit.site.cfg.in b/libunwind/test/lit.site.cfg.in
index 30a996cf3783..8ff770fe29bc 100644
--- a/libunwind/test/lit.site.cfg.in
+++ b/libunwind/test/lit.site.cfg.in
@@ -25,7 +25,6 @@ config.enable_shared            = @LIBCXX_ENABLE_SHARED@
 config.arm_ehabi                = @LIBUNWIND_USES_ARM_EHABI@
 config.host_triple              = "@LLVM_HOST_TRIPLE@"
 config.target_triple            = "@TARGET_TRIPLE@"
-config.use_target               = bool("@LIBUNWIND_TARGET_TRIPLE@")
 config.sysroot                  = "@LIBUNWIND_SYSROOT@"
 config.gcc_toolchain            = "@LIBUNWIND_GCC_TOOLCHAIN@"
 config.cxx_ext_threads          = @LIBUNWIND_BUILD_EXTERNAL_THREAD_LIBRARY@


        


More information about the libcxx-commits mailing list