[RFC] Solve linking inconsistency, proposal one

[labath]

Summary

This solves the ODR violations by exporting all symbols in the llvm namespace to
all users of liblldb, which can then avoid including their own copy of the llvm
libraries. The LLDB_EXPORT_ALL_SYMBOLS=OFF option then becomes purely a size
optimization for liblldb, and does not affect the way in which dependant
executables are linked, and we can even consider turning it off by default
(modulo backtrace(3) problems on Linux).

This effect of this on the SB API firewall is that it would turn it into a sort
of a gentlemans agreement: "If you only use the SB API of liblldb, we guarantee
ABI compatibility. If you don't care about that, you can use the llvm utilities
that are bundled in this library, but then don't blame us if your code gets
broken by an update."

This is my preferred solution. However, I am not sure if this will actually work
on Windows. AFAIK, there you have to specially annotate the API in order for it
to be exported, and I think llvm API don't have those annotations.

[tfiala]

I'll let Greg comment on the public ABI expansion (i.e. including llvm of a specific version, which may differ from LLDB.framework clients that contain different versions of LLVM). My guess is this is not going to work for us, since we have long-lived frameworks shipped with Xcode that get used by clients where we cannot assume what they are or are not using. However, we could address that by adding the reverse flag, which would be "don't export LLVM", and have that be exported by default.

The bit I care about is the "modulo backtrace(3)..." part. This change is taking away my ability to use log-based stacktraces on Linux if I read it right. How were you envisioning I do that with this change?

[labath]

In D26188#584614, @tfiala wrote:

I'll let Greg comment on the public ABI expansion (i.e. including llvm of a specific version, which may differ from LLDB.framework clients that contain different versions of LLVM). My guess is this is not going to work for us, since we have long-lived frameworks shipped with Xcode that get used by clients where we cannot assume what they are or are not using. However, we could address that by adding the reverse flag, which would be "don't export LLVM", and have that be exported by default.

I'd like to avoid that, as it means we will have to have two completely different ways of linking lldb-mi (and maybe lldb, if it starts using some llvm APIs -- it already does on windows, mostly accidentally). I'd rather stick to the proposal two then.

The bit I care about is the "modulo backtrace(3)..." part. This change is taking away my ability to use log-based stacktraces on Linux if I read it right. How were you envisioning I do that with this change?

This does not affect affect the state of backtracing. The option to export all symbols could still stay there, and maybe could even be the default. The "modulo" part was about the fact that this would not be a _completely_ side-effect free optimisation, as it would affect the backtracing.

[tfiala]

source/API/CMakeLists.txt

109

This does not affect affect the state of backtracing. The option to export all symbols could still stay there, and maybe could even be the default.

But you eliminate it here, don't you? (Maybe you just missed that I was also exporting the lldb_private symbols in the case of "LLDB_EXPORT_ALL_SYMBOLS:BOOL=YES"?)

[tfiala]

@zturner wrote:
Can I have some background? What is the linking problem?

Hi Zachary,

This review has comments that pretty much cover it exhaustively:
https://reviews.llvm.org/D26093

That'd be a good place to start.

[labath]

I thought removing it will cause it to export everything. If it doesn't, then we can put whatever magic is necessary there. :)

Although, if exporting everything does require some fancy linker flags, then I probably wouldn't make it the default.

[zturner]

On Windows if you have a DLL (.so) that links against a .lib (.a), and an EXE links against both the DLL and the .lib, then both the DLL and the EXE will each get their own private copy of the symbols in the lib, and the linker won't try to merge them.

Do I understand correctly that this is not the case on Linux? If you have this:

foo
  |__ lib.a
  |__ slib.so
      |__  lib.a

That lib.a is mapped at one location, and all the constructors are invoked twice?

If I understand correctly, then it sounds like you can do whatever you want to fix this, and just disable all of this machinery on Windows since it appears unnecessary.

[clayborg]

I would rather not see us export everything in LLVM. The gentleman's agreement doesn't enforce anything and this will cause crashes when people try to launch a tool against a different liblldb.dylib. It will also increase the size of liblldb dramatically. We seem to worry about lldb-server being 11MB, so I can't see us being OK with liblldb increasing in size by a few hundred K. All the mangled names from all of llvm will be quite large. As soon as we do this, people might then want everything in clang, and then everything in swift. I would like to see our liblldb only export the stuff from the "lldb" namespace like it does. We guarantee the API will work with future versions of things.

One alternate solution is to have two dylibs:

• liblldbprivate
• liblldb

liblldb would remain as it is. liblldbprivate would export lldb_private::*, llvm::* and clang::*. liblldb would link against liblldbprivate, and lldb-mi would link against both. We would then need to build something extra into the tools that link againt liblldbprivate where they would check a version number to ensure they are using the correct liblldbprivate. This check would happen in liblldb during the SBDebugger::Initialize() call and it would cause the program to exit if things don't match. lldb-mi would need to do the same thing during startup and exit if things don't match. I am not too thrilled with this either as we will still have one very very large shared library in liblldbprivate since it would contain all exported symbols from lldb_private::*, llvm::* and clang::*.

I still think your other solution of statically linking in your own copy is the right thing to do.

[labath]

In D26188#584710, @zturner wrote:

On Windows if you have a DLL (.so) that links against a .lib (.a), and an EXE links against both the DLL and the .lib, then both the DLL and the EXE will each get their own private copy of the symbols in the lib, and the linker won't try to merge them.

Do I understand correctly that this is not the case on Linux? If you have this:

foo
  |__ lib.a
  |__ slib.so
      |__  lib.a

That lib.a is mapped at one location, and all the constructors are invoked twice?

If I understand correctly, then it sounds like you can do whatever you want to fix this, and just disable all of this machinery on Windows since it appears unnecessary.

Basically true. It's not that the lib will be mapped only once, but all reference to the each of the symbols from that library will be resolved so that they all point to exactly one copy (as required by the c++ spec). If you try to prevent that, funny things can happen, and that may well depend on the linker then. FWIW, the machinery is already disabled on windows.

[zturner]

BTW, in general static linking on Windows is "bad", but as long as no SBI API header directly or indirectly includes an LLVM header, and there is no LLVM usage on the API boundary, it should be ok IIUC.

[labath]

Ok, I guess we're leaning towards option two then, which is more-or-less status quo as well. I am going to leave this open for a while in case anyone has more input.

Todd, for you this means that you cannot export *all* symbols from liblldb, as that would break the link. The current solution of just exporting lldb_private should be fine though, and llvm symbols are not likely to show up in the backtrace output anyway.

source/API/CMakeLists.txt

 if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
   if (NOT LLDB_EXPORT_ALL_SYMBOLS)
     # If we're not exporting all symbols, we'll want to explicitly set
     # the exported symbols here.  This prevents 'log enable --stack ...'
     # from working on some systems but limits the liblldb size.
     MESSAGE("-- Symbols (liblldb): exporting all symbols from the lldb namespace")
     add_llvm_symbol_exports(liblldb ${CMAKE_CURRENT_SOURCE_DIR}/liblldb.exports)
   else()
-    # Don't use an explicit export.  Instead, tell the linker to
-    # export all symbols.
-    MESSAGE("-- Symbols (liblldb): exporting all symbols from the lldb and lldb_private namespaces")
-    add_llvm_symbol_exports(liblldb ${CMAKE_CURRENT_SOURCE_DIR}/liblldb-private.exports)
   endif()
 endif()
 
 if ( CMAKE_SYSTEM_NAME MATCHES "Windows" )
   # Only MSVC has the ABI compatibility problem and avoids using FindPythonLibs,
   # so only it needs to explicitly link against ${PYTHON_LIBRARY}
   if (MSVC AND NOT LLDB_DISABLE_PYTHON)
     target_link_libraries(liblldb PRIVATE ${PYTHON_LIBRARY})

source/API/liblldb.exports

 _ZN4lldb*
 _ZNK4lldb*
 init_lld*
 PyInit__lldb*
+_ZN4llvm*
+_ZNK4llvm*

tools/lldb-mi/CMakeLists.txt

-include(${LLDB_PROJECT_ROOT}/cmake/LLDBDependencies.cmake)
-
 set(LLDB_MI_SOURCES
   MICmdArgContext.cpp
   MICmdArgSet.cpp
   MICmdArgValBase.cpp
   MICmdArgValConsume.cpp
   MICmdArgValFile.cpp
   MICmdArgValListBase.cpp
   MICmdArgValListOfN.cpp

 
 add_lldb_executable(lldb-mi ${LLDB_MI_SOURCES})
 
 target_link_libraries(lldb-mi liblldb)
 if ( NOT CMAKE_SYSTEM_NAME MATCHES "Windows" )
   target_link_libraries(lldb-mi pthread)
 endif ()
 
-llvm_config(lldb-mi ${LLVM_LINK_COMPONENTS})
-
 set_target_properties(lldb-mi PROPERTIES VERSION ${LLDB_VERSION})
 
 install(TARGETS lldb-mi
   RUNTIME DESTINATION bin)