[Lldb-commits] [PATCH] D31367: Expression: add missing linkage to RuntimeDyld component

[Chris Bieneman via Phabricator via lldb-commits]

beanz added a subscriber: lhames.
beanz added a comment.

@mgorny, because of differences in linker semantics between Darwin and ELF, I can't reproduce the failure you have locally. I think that the patch below works around it in a more-portable way.

I've engaged with @lhames about an architectural solution to the problem, because I think we do need to change how the ExecutionEngine sub-libraries are intertwined, but that is a longer-term problem.

Can you please test this patch and see if it resolves your problem?

  diff --git a/cmake/modules/AddLLVM.cmake b/cmake/modules/AddLLVM.cmake
  index 7f7608cff33..1f27517c2df 100644
  --- a/cmake/modules/AddLLVM.cmake
  +++ b/cmake/modules/AddLLVM.cmake
  @@ -342,7 +342,7 @@ endfunction(set_windows_version_resource_properties)
   function(llvm_add_library name)
     cmake_parse_arguments(ARG
       "MODULE;SHARED;STATIC;OBJECT;DISABLE_LLVM_LINK_LLVM_DYLIB;SONAME"
  -    "OUTPUT_NAME;PLUGIN_TOOL"
  +    "OUTPUT_NAME;PLUGIN_TOOL;DEPENDENCY_LINK_TYPE"
       "ADDITIONAL_HEADERS;DEPENDS;LINK_COMPONENTS;LINK_LIBS;OBJLIBS"
       ${ARGN})
     list(APPEND LLVM_COMMON_DEPENDS ${ARG_DEPENDS})
  @@ -520,14 +520,16 @@ function(llvm_add_library name)
       get_property(lib_deps GLOBAL PROPERTY LLVMBUILD_LIB_DEPS_${name})
     endif()
   
  -  if(ARG_STATIC)
  -    set(libtype INTERFACE)
  -  else()
  -    # We can use PRIVATE since SO knows its dependent libs.
  -    set(libtype PRIVATE)
  +  if(NOT ARG_DEPENDENCY_LINK_TYPE)
  +    if(ARG_STATIC)
  +      set(ARG_DEPENDENCY_LINK_TYPE INTERFACE)
  +    else()
  +      # We can use PRIVATE since SO knows its dependent libs.
  +      set(ARG_DEPENDENCY_LINK_TYPE PRIVATE)
  +    endif()
     endif()
   
  -  target_link_libraries(${name} ${libtype}
  +  target_link_libraries(${name} ${ARG_DEPENDENCY_LINK_TYPE}
         ${ARG_LINK_LIBS}
         ${lib_deps}
         ${llvm_libs}
  diff --git a/lib/ExecutionEngine/CMakeLists.txt b/lib/ExecutionEngine/CMakeLists.txt
  index 2d9337bbefd..37a57eeaa79 100644
  --- a/lib/ExecutionEngine/CMakeLists.txt
  +++ b/lib/ExecutionEngine/CMakeLists.txt
  @@ -1,4 +1,9 @@
  -
  +# Execution engine is not neat and contained like other LLVM libraries. To work
  +# around this if BUILD_SHARED_LIBS is set we need to force the linkage type of
  +# LLVMExecutionEngine's dependencies to PUBLIC.
  +if(BUILD_SHARED_LIBS)
  +  set(dependency_hack DEPENDENCY_LINK_TYPE PUBLIC)
  +endif()
   
   add_llvm_library(LLVMExecutionEngine
     ExecutionEngine.cpp
  @@ -12,6 +17,7 @@ add_llvm_library(LLVMExecutionEngine
   
     DEPENDS
     intrinsics_gen
  +  ${dependency_hack}
     )
   
   add_subdirectory(Interpreter)


Repository:
  rL LLVM

https://reviews.llvm.org/D31367