[lldb] Make sure we use the libc++ from the build dir

[JDevlieghere]

Summary

Make sure we use the libc++ from the build dir. Currently, by passing -stdlib=libc++, we might pick up the system libc++ on macOS. This change ensures that if LLVM_LIBS_DIR is set, we try to use the libc++ from there.

[labath]

I'm afraid this will not work on systems which do not default to libc++ (which includes at least linux and windows), because -stdlib=libc++ is not "equivalent" to -nostdlib++ -lc++ -- the former changes the include paths to use libc++, while the latter doesn't. And, for most of the things that we do here, headers are more important than the library itself.

If the goal here is to preclude any possibility of picking up a different library (it's not clear to me what kind of situation you're referring to in the description), then the most principled approach would be to also use -nostdinc++ and manually add the appropriate include paths. One nice property of that is that it would work with compilers (gcc) which do not understand the -stdlib flag.

The simple solution would be to use the -stdlib=libc++ -nostdlib++ as a combo, but I am not sure if that would solve what it is supposed to solve (as it still relies on clang to autodetect the header path).

Another thing to consider is that historically (back when we were running the tests against the system compiler by default), the USE_LIBCPP flag meant "use the system libc++" (or rather, the default library of the compiler in use). Now we use the in-tree compiler (by default), but our libc++ story is not so clear, as it mostly relies on clang. If the build contains libc++ then we will use that (and sometimes fail due to incorrect usage). If it doesn't, but there's a libc++ on the system somewhere, then we will use that one (and be non-hermetic). Otherwise, we will skip tests which depend on this (and the skipping code uses its own logic, which is not touched here).

Overall, I think it makes sense to use libc++ when we have it available. I am not so sure about the other modes. While I think it would be nice to support running the tests against a random c++ library (similar to how we do it for compilers), I don't think we need to support it unless there someone actively interested in running it. I do think we should support a mode where we don't have a libc++ in the build tree, but I'd be fine if that just causes all of the relevant tests to be skipped.

The thing where that gets interesting is with remote tests. Unlike system libc++, I think we have people (including myself, to some extent) whose job is to make sure those work. Right now, we don't make any special effort to copy the c++ library to the remote system. If it works, it works because the system already has a compatible c++ library lying around. The reason these two are related is that the system libc++ is more likely to be present on a remote system than the one we just built. Messing with the way use libc++ may break some of the remote use cases, so it might be good to take some position on that. Do we not support that? Do we expect the user to make sure the libc++ is on the system already? Do we expect the user to link libc++ statically (I think that's what happens in all of our use cases)?

I realize this isn't very helpful, but I'm trying to explain why I fear this may be more complicated than it may seem at first. Maybe it is not necessary to answer all of these questions for the problem at hand, but these are the questions that have stopped me from doing anything in this area in the past.

[JDevlieghere]

In D129166#3632597, @labath wrote:

I'm afraid this will not work on systems which do not default to libc++ (which includes at least linux and windows), because -stdlib=libc++ is not "equivalent" to -nostdlib++ -lc++ -- the former changes the include paths to use libc++, while the latter doesn't. And, for most of the things that we do here, headers are more important than the library itself.

If the goal here is to preclude any possibility of picking up a different library (it's not clear to me what kind of situation you're referring to in the description), then the most principled approach would be to also use -nostdinc++ and manually add the appropriate include paths. One nice property of that is that it would work with compilers (gcc) which do not understand the -stdlib flag.

The simple solution would be to use the -stdlib=libc++ -nostdlib++ as a combo, but I am not sure if that would solve what it is supposed to solve (as it still relies on clang to autodetect the header path).

Another thing to consider is that historically (back when we were running the tests against the system compiler by default), the USE_LIBCPP flag meant "use the system libc++" (or rather, the default library of the compiler in use). Now we use the in-tree compiler (by default), but our libc++ story is not so clear, as it mostly relies on clang. If the build contains libc++ then we will use that (and sometimes fail due to incorrect usage). If it doesn't, but there's a libc++ on the system somewhere, then we will use that one (and be non-hermetic). Otherwise, we will skip tests which depend on this (and the skipping code uses its own logic, which is not touched here).

Overall, I think it makes sense to use libc++ when we have it available. I am not so sure about the other modes. While I think it would be nice to support running the tests against a random c++ library (similar to how we do it for compilers), I don't think we need to support it unless there someone actively interested in running it. I do think we should support a mode where we don't have a libc++ in the build tree, but I'd be fine if that just causes all of the relevant tests to be skipped.

The thing where that gets interesting is with remote tests. Unlike system libc++, I think we have people (including myself, to some extent) whose job is to make sure those work. Right now, we don't make any special effort to copy the c++ library to the remote system. If it works, it works because the system already has a compatible c++ library lying around. The reason these two are related is that the system libc++ is more likely to be present on a remote system than the one we just built. Messing with the way use libc++ may break some of the remote use cases, so it might be good to take some position on that. Do we not support that? Do we expect the user to make sure the libc++ is on the system already? Do we expect the user to link libc++ statically (I think that's what happens in all of our use cases)?

I realize this isn't very helpful, but I'm trying to explain why I fear this may be more complicated than it may seem at first. Maybe it is not necessary to answer all of these questions for the problem at hand, but these are the questions that have stopped me from doing anything in this area in the past.

Thanks for the thoughtful reply Pavel. The remote tests are something we care about as well, so I'd like to have a solution for that. What do you think about adding a "stdlib" mode to dotest.py which allows you to pick between "system libc++", "system libstdc++" and "just built libc++". The latter would be hermetic, and the former would match what we do today.

[labath]

In D129166#3633116, @JDevlieghere wrote:

Thanks for the thoughtful reply Pavel. The remote tests are something we care about as well, so I'd like to have a solution for that. What do you think about adding a "stdlib" mode to dotest.py which allows you to pick between "system libc++", "system libstdc++" and "just built libc++". The latter would be hermetic, and the former would match what we do today.

Well.. I think that specifying some of this explicitly would be great, but I don't think a choice between a libc++ and libstdc++ makes sense.

The way I see it, we have three kinds of tests:

1. Tests which don't care which library we use. This is the vast majority of them. The only need it to be there, but the actual test result should not depend on the library used in any way. For these tests, we can use any library we like. (except maybe for the gmodules test variant, but I don't actually know how that one works). I don't think we need to offer a choice here. Ideally we would be able to just pick the option that works (it may not be the same option for each config).
2. Tests which explicitly require libc++. There shouldn't be too many of these, and ideally these would be limited to tests for the libc++ pretty printers and such. It doesn't make sense to run these against libstdc++. In fact, that would be harmful, because it might actually work, but test the wrong thing. Ideally, we'd give the user the option to choose between the system libc++, just-built libc++ or a way to specify the arguments needed to build&run these kinds of executables.
3. Tests which explicitly require libstdc++. These is the same thing, except for libstdc++ pretty printers. And that we obviously don't have an in-tree version of libstdc++. And I don't think we have many people interested in running tests against libstdc++, so we probably don't have to go overboard on this one, but it would be nice to be able to keep running the existing tests against system libstdc++ on systems which have one.

[JDevlieghere]

In D129166#3633243, @labath wrote:

In D129166#3633116, @JDevlieghere wrote:

Thanks for the thoughtful reply Pavel. The remote tests are something we care about as well, so I'd like to have a solution for that. What do you think about adding a "stdlib" mode to dotest.py which allows you to pick between "system libc++", "system libstdc++" and "just built libc++". The latter would be hermetic, and the former would match what we do today.

Well.. I think that specifying some of this explicitly would be great, but I don't think a choice between a libc++ and libstdc++ makes sense.

The way I see it, we have three kinds of tests:

1. Tests which don't care which library we use. This is the vast majority of them. The only need it to be there, but the actual test result should not depend on the library used in any way. For these tests, we can use any library we like. (except maybe for the gmodules test variant, but I don't actually know how that one works). I don't think we need to offer a choice here. Ideally we would be able to just pick the option that works (it may not be the same option for each config).
2. Tests which explicitly require libc++. There shouldn't be too many of these, and ideally these would be limited to tests for the libc++ pretty printers and such. It doesn't make sense to run these against libstdc++. In fact, that would be harmful, because it might actually work, but test the wrong thing. Ideally, we'd give the user the option to choose between the system libc++, just-built libc++ or a way to specify the arguments needed to build&run these kinds of executables.
3. Tests which explicitly require libstdc++. These is the same thing, except for libstdc++ pretty printers. And that we obviously don't have an in-tree version of libstdc++. And I don't think we have many people interested in running tests against libstdc++, so we probably don't have to go overboard on this one, but it would be nice to be able to keep running the existing tests against system libstdc++ on systems which have one.

Okay, so a binary option to specify the system libc++ or the just-built libc++. The libstd+++ stuff remains unchanged.

[JDevlieghere]

I still have to test different configurations, but putting it up here for feedback about the general direction. This doesn't account yet for remote runs. Should be easy to detect based on whether --platform-name is set in dotest.py.

[labath]

The overall logic and the include and library paths make sense to me. The rpath thingy will not work on windows as it (afaik) has no equivalent feature (it has the equivalent of (DY)LD_LIBRARY_PATH though). Do you just want to make that unsupported. I don't think we're running libc++ tests on windows right now, so I guess nothing will be lost there, though we will need to implement this differently at some point.

And I suppose that the plan is to not use this path at all for remote runs (?) That makes sense to me, since the build tree is unlikely to contain a usable libc++ (unless the remote machine has the same architecture).

lldb/packages/Python/lldbsuite/test/dotest.py

284

libcxx

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

394 ↗

(On Diff #442635)

I think this should be set in the "hermetic" case as well (or, given that it is unused, we might delete it completely).

[dblaikie]

The overall logic and the include and library paths make sense to me. The rpath thingy will not work on windows as it (afaik) has no equivalent feature (it has the equivalent of (DY)LD_LIBRARY_PATH though).

Any idea what the libc++ tests do on Windows then? (on linux they seem to use rpath to ensure the tests load the just-built libc++)

[labath]

In D129166#3636658, @dblaikie wrote:

The overall logic and the include and library paths make sense to me. The rpath thingy will not work on windows as it (afaik) has no equivalent feature (it has the equivalent of (DY)LD_LIBRARY_PATH though).

Any idea what the libc++ tests do on Windows then? (on linux they seem to use rpath to ensure the tests load the just-built libc++)

I believe the libc++ tests don't run at all on windows.

lldb/packages/Python/lldbsuite/test/builders/builder.py

def getModuleCacheSpec(self):
         Helper function to return the key-value string to specify the clang
         module cache used for the make system.
         """
         if configuration.clang_module_cache_dir:
             return ["CLANG_MODULE_CACHE_DIR={}".format(
                 configuration.clang_module_cache_dir)]
         return []
 
+    def getLibCxxArgs(self):
+        if configuration.hermetic_libcxx:
+            return ["USE_HERMETIC_LIBCPP=1"]
+        return []
+
     def _getDebugInfoArgs(self, debug_info):
         if debug_info is None:
             return []
         if debug_info == "dwarf":
             return ["MAKE_DSYM=NO"]
         if debug_info == "dwo":
             return ["MAKE_DSYM=NO", "MAKE_DWO=YES"]
         if debug_info == "gmodules":
             return ["MAKE_DSYM=NO", "MAKE_GMODULES=YES"]
         return None
 
     def getBuildCommand(self, debug_info, architecture=None, compiler=None,
             dictionary=None, testdir=None, testname=None):
         debug_info_args = self._getDebugInfoArgs(debug_info)
         if debug_info_args is None:
             return None
 
         command_parts = [
             self.getMake(testdir, testname), debug_info_args, ["all"],
             self.getArchCFlags(architecture), self.getArchSpec(architecture),
             self.getCCSpec(compiler), self.getExtraMakeArgs(),
             self.getSDKRootSpec(), self.getModuleCacheSpec(),
-            self.getCmdLine(dictionary)]
+            self.getLibCxxArgs(), self.getCmdLine(dictionary)]
         command = list(itertools.chain(*command_parts))
 
         return command
 
     def cleanup(self, dictionary=None):
         """Perform a platform-specific cleanup after the test."""
         return True

lldb/packages/Python/lldbsuite/test/configuration.py

 
 # The names of all tests. Used to assert we don't have two tests with the
 # same base name.
 all_tests = set()
 
 # LLDB library directory.
 lldb_libs_dir = None
 
+# Force us to use the just-built libcxx
+hermetic_libcxx = False
+
 # A plugin whose tests will be enabled, like intel-pt.
 enabled_plugins = []
 
 
 def shouldSkipBecauseOfCategories(test_categories):
     if use_categories:
         if len(test_categories) == 0 or len(
                 categories_list & set(test_categories)) == 0:

lldb/packages/Python/lldbsuite/test/dotest.py

def parseOptionsAndInitTestdirs():
     if args.llvm_tools_dir:
         configuration.filecheck = shutil.which("FileCheck", path=args.llvm_tools_dir)
         configuration.yaml2obj = shutil.which("yaml2obj", path=args.llvm_tools_dir)
 
     if not configuration.get_filecheck_path():
         logging.warning('No valid FileCheck executable; some tests may fail...')
         logging.warning('(Double-check the --llvm-tools-dir argument to dotest.py)')
 
+    configuration.hermetic_libcxx = args.hermetic_libcxx
+    if configuration.hermetic_libcxx and args.lldb_platform_name:
+        configuration.hermetic_libcxx = False
+        logging.warning('Hermetic libc++ is not supported for remote runs: ignoring --hermetic-libcxx')
+
     if args.channels:
         lldbtest_config.channels = args.channels
 
     if args.log_success:
         lldbtest_config.log_success = args.log_success
 
     if args.out_of_tree_debugserver:
         lldbtest_config.out_of_tree_debugserver = args.out_of_tree_debugserver

lldb/packages/Python/lldbsuite/test/dotest_args.py

group.add_argument(
         metavar='arch',
         dest='arch',
         help=textwrap.dedent('''Specify the architecture(s) to test. This option can be specified more than once'''))
     group.add_argument('-C', '--compiler', metavar='compiler', dest='compiler', help=textwrap.dedent(
         '''Specify the compiler(s) used to build the inferior executables. The compiler path can be an executable basename or a full path to a compiler executable. This option can be specified multiple times.'''))
     if sys.platform == 'darwin':
         group.add_argument('--apple-sdk', metavar='apple_sdk', dest='apple_sdk', default="", help=textwrap.dedent(
             '''Specify the name of the Apple SDK (macosx, macosx.internal, iphoneos, iphoneos.internal, or path to SDK) and use the appropriate tools from that SDK's toolchain.'''))
+    group.add_argument('--hermetic-libcxx', action='store_true', help=textwrap.dedent(
+        '''Force the just-built libcxx to be used for the libc++ formatter tests.'''))
     # FIXME? This won't work for different extra flags according to each arch.
     group.add_argument(
         '-E',
         metavar='extra-flags',
         help=textwrap.dedent('''Specify the extra flags to be passed to the toolchain when building the inferior programs to be debugged
                                                            suggestions: do not lump the "-A arch1 -A arch2" together such that the -E option applies to only one of the architectures'''))
 
     group.add_argument('--dsymutil', metavar='dsymutil', dest='dsymutil', help=textwrap.dedent('Specify which dsymutil to use.'))

lldb/test/API/lit.cfg.py

else:
   lit_config.warning("Could not set a default per-test timeout. " + errormsg)
 
 # Build dotest command.
 dotest_cmd = [os.path.join(config.lldb_src_root, 'test', 'API', 'dotest.py')]
 
 if is_configured('dotest_args_str'):
   dotest_cmd.extend(config.dotest_args_str.split(';'))
 
-# Library path may be needed to locate just-built clang.
+# Library path may be needed to locate just-built clang and libcxx.
 if is_configured('llvm_libs_dir'):
   dotest_cmd += ['--env', 'LLVM_LIBS_DIR=' + config.llvm_libs_dir]
 
+# Include path may be needed to locate just-built libcxx.
+if is_configured('llvm_include_dir'):
+  dotest_cmd += ['--env', 'LLVM_INCLUDE_DIR=' + config.llvm_include_dir]
+
 # This path may be needed to locate required llvm tools
 if is_configured('llvm_tools_dir'):
   dotest_cmd += ['--env', 'LLVM_TOOLS_DIR=' + config.llvm_tools_dir]
 
+# If we have a just-built libcxx, prefer it over the system one.
+if is_configured('has_libcxx') and platform.system() != 'Windows':
+  dotest_cmd += ['--hermetic-libcxx']
+
 # Forward ASan-specific environment variables to tests, as a test may load an
 # ASan-ified dylib.
 for env_var in ('ASAN_OPTIONS', 'DYLD_INSERT_LIBRARIES'):
   if env_var in config.environment:
     dotest_cmd += ['--inferior-env', env_var + '=' + config.environment[env_var]]
 
 if is_configured('test_arch'):
   dotest_cmd += ['--arch', config.test_arch]

lldb/test/API/lit.site.cfg.py.in

 @LIT_SITE_CFG_IN_HEADER@
 
 config.llvm_src_root = "@LLVM_SOURCE_DIR@"
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
 config.llvm_tools_dir = lit_config.substitute("@LLVM_TOOLS_DIR@")
 config.llvm_libs_dir = lit_config.substitute("@LLVM_LIBS_DIR@")
+config.llvm_include_dir = lit_config.substitute("@LLVM_INCLUDE_DIR@")
 config.llvm_shlib_dir = lit_config.substitute("@SHLIBDIR@")
 config.llvm_build_mode = lit_config.substitute("@LLVM_BUILD_MODE@")
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.lldb_obj_root = "@LLDB_BINARY_DIR@"
 config.lldb_src_root = "@LLDB_SOURCE_DIR@"
 config.lldb_libs_dir = lit_config.substitute("@LLDB_LIBS_DIR@")
 config.lldb_framework_dir = lit_config.substitute("@LLDB_FRAMEWORK_DIR@")
 config.cmake_cxx_compiler = "@CMAKE_CXX_COMPILER@"

 config.dotest_args_str = lit_config.substitute("@LLDB_DOTEST_ARGS@")
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
 config.dotest_lit_args_str = None
 config.enabled_plugins = []
 config.lldb_executable = lit_config.substitute('@LLDB_TEST_EXECUTABLE@')
 config.test_arch = '@LLDB_TEST_ARCH@'
 config.test_compiler = lit_config.substitute('@LLDB_TEST_COMPILER@')
 config.dsymutil = lit_config.substitute('@LLDB_TEST_DSYMUTIL@')
+config.has_libcxx = '@LLDB_HAS_LIBCXX@'
 # The API tests use their own module caches.
 config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-api")
 config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", "lldb-api")
 
 # Plugins
 lldb_build_intel_pt = '@LLDB_BUILD_INTEL_PT@'
 if lldb_build_intel_pt == '1':
     config.enabled_plugins.append('intel-pt')

lldb/test/CMakeLists.txt

else()
       endif()
     endif()
   endif()
 
   # Add libc++ dependency for libc++-specific tests. This is an optional
   # dependency as it's also possible to run the libc++ tests against the libc++
   # installed on the system.
   if (TARGET cxx)
+    set(LLDB_HAS_LIBCXX ON)
     add_lldb_test_dependency(cxx)
   endif()
 
 endif()
 
 if (LLDB_BUILT_STANDALONE)
   set(LLVM_HOST_TRIPLE ${LLVM_TARGET_TRIPLE})
 endif()

 # These values are not canonicalized within LLVM.
 llvm_canonicalize_cmake_booleans(
   LLDB_BUILD_INTEL_PT
   LLDB_ENABLE_PYTHON
   LLDB_ENABLE_LUA
   LLDB_ENABLE_LZMA
   LLVM_ENABLE_ZLIB
   LLVM_ENABLE_SHARED_LIBS
+  LLDB_HAS_LIBCXX
   LLDB_USE_SYSTEM_DEBUGSERVER
   LLDB_IS_64_BITS)
 
 # Configure the individual test suites.
 add_subdirectory(API)
 add_subdirectory(Shell)
 add_subdirectory(Unit)