Statically link llvm-cfi-verify's libraries.

[hctim]

Summary

llvm-cfi-verify (D38379) introduced a potential build failure when compiling with -DLLVM_BUILD_LLVM_DYLIB=ON -DLLVM_LINK_LLVM_DYLIB=ON. Specific versions of cmake seem to treat the add_subdirectory() rule differently. It seems as if old versions of cmake BFS these rules, adding them to the fringe for expansion later. Newer versions of cmake seem to immediately execute CMakeFiles that are present in this subdirectory.

If the subdirectory is expanded through the fringe, the globbing resultant from llvm_add_implicit_projects() from cmake/modules/AddLLVM.cmake:1012 means that tools/llvm-shlib/CMakeFile.txt gets executed before tools/llvm-cfi-verify/lib/CMakeFile.txt. As the latter CMakeFile adds a new library, this expansion order means that the library files required the unit tests in unittests/tools/llvm-cfi-verify/ are not present in the dynamic library. This causes unit tests to fail as the required functions can't be found.

This change now ensures that the libraries created by llvm-cfi-verify are statically linked into the unit tests. As tools/llvm-cfi-verify/lib no longer adds anything to llvm-shlib, there should be no concern about the order-of-compilation.

[pcc]

Taking a step back, I don't think we even want the llvm-cfi-verify code in the shared library. That is because it is only intended to be used by one executable and the test suite. If you can avoid linking it in to the shlib, I guess you wouldn't need to do anything about the enumeration order of llvm-shlib.

Does it not work to declare your library with add_library instead of add_llvm_library? Alternatively it looks like add_llvm_library has a BUILDTREE_ONLY flag that appears to do what is needed here.

[hctim]

In D39020#900124, @pcc wrote:

Does it not work to declare your library with add_library instead of add_llvm_library? Alternatively it looks like add_llvm_library has a BUILDTREE_ONLY flag that appears to do what is needed here.

Adding BUILDTREE_ONLY both with and without STATIC are resulting in the same errors. Will take a look at explicitly creating the library without the llvm helpers through add_library.

[hctim]

Explicitly made llvm-cfi-verify build its own library that is statically linked into CFIVerifyTests. The target is not linked into the dynamic library when LLVM_BUILD_LLVM_DYLIB is enabled.

[pcc]

tools/llvm-cfi-verify/lib/CMakeLists.txt

13 ↗

(On Diff #119404)

Can you move the list of dependencies into this statement?

unittests/tools/llvm-cfi-verify/CMakeLists.txt

18 ↗

(On Diff #119404)

Why is this line necessary?

[hctim]

• pcc@'s comments

unittests/tools/llvm-cfi-verify/CMakeLists.txt

18 ↗

(On Diff #119404)

It isn't. Removed.

[pcc]

LGTM

tools/llvm-cfi-verify/lib/CMakeLists.txt

1 ↗

(On Diff #119411)

I think you want to depend on this library from llvm-cfi-verify instead of building it twice.

[skatkov]

Confirm, the patch fixes my issue.

[skatkov]

Please land it asap.

cmake/modules/AddLLVM.cmake

 function(create_llvm_tool_options)
   create_subdirectory_options(LLVM TOOL)
 endfunction(create_llvm_tool_options)
 
 function(llvm_add_implicit_projects project)
   set(list_of_implicit_subdirs "")
   file(GLOB sub-dirs "${CMAKE_CURRENT_SOURCE_DIR}/*")
   foreach(dir ${sub-dirs})
+    # Exclude shlib from being implicitly added.
+    string(FIND ${dir} llvm-shlib is_shlib)
+    if(NOT(${is_shlib} EQUAL -1))
+      continue()
+    endif()
+
     if(IS_DIRECTORY "${dir}" AND EXISTS "${dir}/CMakeLists.txt")
       canonicalize_tool_name(${dir} name)
       if (${project}_TOOL_${name}_BUILD)
         get_filename_component(fn "${dir}" NAME)
         list(APPEND list_of_implicit_subdirs "${fn}")
       endif()
     endif()
   endforeach()

tools/CMakeLists.txt

 # file as external projects.
 add_llvm_implicit_projects()
 
 # Add subprojects specified using LLVM_EXTERNAL_PROJECTS
 foreach(p ${LLVM_EXTERNAL_PROJECTS})
   add_llvm_external_project(${p})
 endforeach(p)
 
+# Add shlib as a subdirectory. This allows directories in this folder to add
+# libraries without worrying about which target will be executed first.
+add_llvm_subdirectory(LLVM TOOL llvm-shlib)
+
 set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS} PARENT_SCOPE)