[libcxx] Framework to allow testing of static libc++abi

[bcraig]

Summary

NOTE: related libc++abi changes are up for review here: http://reviews.llvm.org/D16545 .

Tested with static libcxx and libcxxabi against custom embedded target and Linux x64.
Tested with shared libcxx and libcxxabi against Linux x64.
Really hoping that I didn't accidentally break Mac.

These changes make linking against static libraries more explicit. Instead
of using -lc++ and -lc++abi in the tests, an absolute path to the library is
provided instead.

The choices of shared vs. static, and the choices of library paths for both
libcxx and libcxxabi needed to be exchanged for this to work. In other words,
libcxx tests need to know the library path of libcxxabi, and whether libcxxabi
is a static or shared library.

Some Mac specific logic for testing against libc++abi had to be moved from
libcxxabi's config.py, as it was overriding choices made in libcxx's config.py.
That logic is now in libcxx's target_info.py.

Testing a static libcxx on Linux will now automatically link in librt most of
the time. Previously, lots of pthread tests would fail because of an
unresolved clock_gettime.

[bcraig]

ping

[bcraig]

ping

[EricWF]

Did you consider simply wrapping the library in "-Wl,-Bstatic" and "-Wl,-Bdynamic" linker flags? It seems like that would work almost as well as explicitly naming the target.

test/libcxx/test/config.py

463

libcxx_shared = self.get_lit_bool('enable_shared', default=True)

469

Why not just wrap it in '-B,-static'?

482

Same as above.

test/libcxx/test/target_info.py

173

Hmm... this would have a different default compared to elsewhere after my suggested edits.

[bcraig]

test/libcxx/test/config.py

469

I know exactly which binary I want to link against. Running through the linker search path can't make this work better, but it can make it work worse.

test/libcxx/test/target_info.py

173

When testing libcxx, it's going to be difficult to get an unset enable_shared, so the default doesn't matter that much in that situation. It might matter more with libcxxabi. I will investigate.

Regardless, I'm pretty sure "False" is the default here due to copy / paste rather than long, careful consideration.

[bcraig]

Addressed EricWF's feedback (hopefully).

[bcraig]

ping

[bcraig]

ping

[bcraig]

ping

[EricWF]

Just spitballing but have you considered simply passing the full path, including the library name, to LIT? Instead of needing enable_shared variables we would simply use the named library, be it DSO or archive.

test/libcxx/test/config.py

488

os.path.join

test/libcxx/test/target_info.py

113

This part makes me uncomfortable. Half the reason this was disabled in the first place was to help find bugs like this.

Make the comment a FIXME and file a bug and I'm OK with this going in for now.

[EricWF]

If we choose not to use full library paths could you please remove the 'libcxx_library' option? It just does what your trying here but worse, no need to keep it around.

[bcraig]

In D16544#403273, @EricWF wrote:

Just spitballing but have you considered simply passing the full path, including the library name, to LIT? Instead of needing enable_shared variables we would simply use the named library, be it DSO or archive.

I agree that this would be nice. I don't really like the nest of if / else statements that I currently have in place. However, I have now attempted to do this, and failed. My cmake-fu just isn't strong enough.

My approach was to populate variables like so...

get_target_property(LIBCXXABI_FULL_LIB_PATH cxxabi_static LOCATION)
get_target_property(LIBCXXABI_FULL_LIB_PATH cxxabi_shared LOCATION)
get_target_property(LIBCXX_FULL_LIB_PATH cxx LOCATION)

... and then to plumb that through lit.site.cfg.in. This would work great if the core of cxxabi and cxx were processed first, followed by their respective tests. Unfortunately, cxxabi and it's tests get processed before cxx gets processed. This means that I can't rely on cxx's CMakeLists.txt to populate a path on behalf of cxxabi. I could attempt to rebuild the path from the pieces of information that I do have access to, but that seems really hacky.

Note that getting a full path in doesn't remove all the if / elif complexity in getting cxx[abi] to the link line. If no desired path exists in the config, then something like -lc++ or -lc++abi still needs to be passed in. This is to handle the case where someone is building one of libcxx[abi] without the counterpart being available. We don't know the absolute path to the respective library, but it is (presumably) in the library path.

[EricWF]

In D16544#404283, @bcraig wrote:

In D16544#403273, @EricWF wrote:

Just spitballing but have you considered simply passing the full path, including the library name, to LIT? Instead of needing enable_shared variables we would simply use the named library, be it DSO or archive.

I agree that this would be nice. I don't really like the nest of if / else statements that I currently have in place. However, I have now attempted to do this, and failed. My cmake-fu just isn't strong enough.

OK. Thanks for looking into this. I figured CMake would stand in our way. Last I looked into it CMake doesn't actually know the name of the library it's going to produce until way to late in the configuration process.

Lets go ahead with this.

[bcraig]

EricWF's feedback. Removed libcxx_library option. I plan on submitting this tomorrow morning.

[EricWF]

LGTM. It's up to you if you want to address the inline comments before committing.

Thanks for updating the Doc. I really need to kill one version of it.

test/libcxx/test/config.py

454

In this fallback case do we want to explicitly ask for static linkage using -Wl,-Bstatic -lc++ -Wl,-Bdynamic? I think that will cause the linker to diagnose that your not actually getting a static library.

473

In this fallback case do we want to explicitly ask for static linkage using -Wl,-Bstatic -lc++abi -Wl,-Bdynamic?

[bcraig]

r266730

test/libcxx/test/config.py

454

While I would like to get a the diagnosis, I don't plan on making this change. The hexagon linker (and maybe others?) default to -Bstatic, so the "restoration" half of that snippet does the wrong thing. If there were a way to save / push the linker state, then restore / pop it then I would do that.

docs/TestingLibcxx.rst

   Specify the libc++ headers that are tested. By default the headers in the
   source tree are used.
 
-.. option:: libcxx_library=<path/to/libc++.so>
-
-  Specify the libc++ library that is tested. By default the library in the
-  build directory is used. This option cannot be used when use_system_lib is
-  provided.
-
 .. option:: use_system_lib=<bool>
 
   **Default**: False

test/CMakeLists.txt

 pythonize_bool(LIBCXX_ENABLE_SHARED)
 pythonize_bool(LIBCXX_BUILD_32_BITS)
 pythonize_bool(LIBCXX_GENERATE_COVERAGE)
+pythonize_bool(LIBCXXABI_ENABLE_SHARED)
 pythonize_bool(LIBCXXABI_USE_LLVM_UNWINDER)
 pythonize_bool(LIBCXX_HAS_ATOMIC_LIB)
 

   set(LIBCXX_CXX_ABI_LIBNAME "none")
 endif()
 
+# By default, for non-standalone builds, libcxx and libcxxabi share a library
+# directory.
+if (NOT LIBCXX_CXX_ABI_LIBRARY_PATH)
+  set(LIBCXX_CXX_ABI_LIBRARY_PATH "${LIBCXX_LIBRARY_DIR}" CACHE PATH
+      "The path to libc++abi library.")
+endif()
+
 set(LIBCXX_TARGET_INFO "libcxx.test.target_info.LocalTI" CACHE STRING
     "TargetInfo to use when setting up test environment.")
 set(LIBCXX_EXECUTOR "None" CACHE STRING

test/libcxx/test/config.py

         self.cxx.link_flags += shlex.split(link_flags_str)
 
     def configure_link_flags_cxx_library_path(self):
-        libcxx_library = self.get_lit_conf('libcxx_library')
-        # Configure libc++ library paths.
-        if libcxx_library is not None:
-            # Check that the given value for libcxx_library is valid.
-            if not os.path.isfile(libcxx_library):
-                self.lit_config.fatal(
-                    "libcxx_library='%s' is not a valid file." %
-                    libcxx_library)
-            if self.use_system_cxx_lib:
-                self.lit_config.fatal(
-                    "Conflicting options: 'libcxx_library' cannot be used "
-                    "with 'use_system_cxx_lib=true'")
-            self.cxx.link_flags += ['-Wl,-rpath,' +
-                                    os.path.dirname(libcxx_library)]
-        elif not self.use_system_cxx_lib and self.cxx_library_root:
+        if not self.use_system_cxx_lib and self.cxx_library_root:
             self.cxx.link_flags += ['-L' + self.cxx_library_root,
                                     '-Wl,-rpath,' + self.cxx_library_root]
 

                                     '-Wl,-rpath,' + self.abi_library_root]
 
     def configure_link_flags_cxx_library(self):
-        libcxx_library = self.get_lit_conf('libcxx_library')
-        if libcxx_library:
-            self.cxx.link_flags += [libcxx_library]
-        else:
+        libcxx_shared = self.get_lit_bool('enable_shared', default=True)
+        if libcxx_shared:
             self.cxx.link_flags += ['-lc++']
+        else:
+            cxx_library_root = self.get_lit_conf('cxx_library_root')
+            if cxx_library_root:
+                abs_path = os.path.join(cxx_library_root, 'libc++.a')
+                self.cxx.link_flags += [abs_path]
+            else:
+                self.cxx.link_flags += ['-lc++']
 
     def configure_link_flags_abi_library(self):
         cxx_abi = self.get_lit_conf('cxx_abi', 'libcxxabi')

             self.cxx.link_flags += ['-lsupc++']
         elif cxx_abi == 'libcxxabi':
             if self.target_info.allow_cxxabi_link():
-                self.cxx.link_flags += ['-lc++abi']
+                libcxxabi_shared = self.get_lit_bool('libcxxabi_shared', default=True)
+                if libcxxabi_shared:
+                    self.cxx.link_flags += ['-lc++abi']
+                else:
+                    cxxabi_library_root = self.get_lit_conf('abi_library_path')
+                    if cxxabi_library_root:
+                        abs_path = os.path.join(cxxabi_library_root, 'libc++abi.a')
+                        self.cxx.link_flags += [abs_path]
+                    else:
+                        self.cxx.link_flags += ['-lc++abi']
         elif cxx_abi == 'libcxxrt':
             self.cxx.link_flags += ['-lcxxrt']
         elif cxx_abi == 'none':

test/libcxx/test/target_info.py

     def configure_env(self, env):
         library_paths = []
         # Configure the library path for libc++
-        libcxx_library = self.full_config.get_lit_conf('libcxx_library')
         if self.full_config.use_system_cxx_lib:
             pass
-        elif libcxx_library:
-            library_paths += [os.path.dirname(libcxx_library)]
         elif self.full_config.cxx_library_root:
             library_paths += [self.full_config.cxx_library_root]
         # Configure the abi library path

             env['DYLD_LIBRARY_PATH'] = ':'.join(library_paths)
 
     def allow_cxxabi_link(self):
+        # FIXME: PR27405
+        # libc++ *should* export all of the symbols found in libc++abi on OS X.
+        # For this reason LibcxxConfiguration will not link libc++abi in OS X.
+        # However __cxa_throw_bad_new_array_length doesn't get exported into
+        # libc++ yet so we still need to explicitly link libc++abi when testing
+        # libc++abi
+        # See PR22654.
+        if(self.full_config.get_lit_conf('name', '') == 'libc++abi'):
+            return True
         # Don't link libc++abi explicitly on OS X because the symbols
         # should be available in libc++ directly.
         return False

         enable_threads = ('libcpp-has-no-threads' not in
                           self.full_config.config.available_features)
         llvm_unwinder = self.full_config.get_lit_bool('llvm_unwinder', False)
+        shared_libcxx = self.full_config.get_lit_bool('enable_shared', True)
         flags += ['-lm']
         if not llvm_unwinder:
             flags += ['-lgcc_s', '-lgcc']
         if enable_threads:
             flags += ['-lpthread']
+            if not shared_libcxx:
+              flags += ['-lrt']
         flags += ['-lc']
         if llvm_unwinder:
             flags += ['-lunwind', '-ldl']

test/lit.site.cfg.in

 config.executor                 = "@LIBCXX_EXECUTOR@"
 config.llvm_unwinder            = "@LIBCXXABI_USE_LLVM_UNWINDER@"
 config.use_libatomic            = "@LIBCXX_HAS_ATOMIC_LIB@"
+config.libcxxabi_shared         = "@LIBCXXABI_ENABLE_SHARED@"
 
 # Let the main config do the real work.
 lit_config.load_config(config, "@LIBCXX_SOURCE_DIR@/test/lit.cfg")

www/lit_usage.html

 </p>
 
 <p>
-<h3 class="lit-option">libcxx_library=&lt;path/to/libc++.so&gt;</h3>
-<blockquote class="lit-option-desc">
-Specify the libc++ library that is tested. By default the library in the build
-directory is used. This option cannot be used when <code>use_system_lib</code>
-is provided.
-</blockquote>
-</p>
-
-<p>
 <h3 class="lit-option">use_system_lib=&lt;bool&gt;</h3>
 <blockquote class="lit-option-desc">
 <b>Default: </b><code>False</code></br>