charles-zablit wrote: > This change broke building with LLVM configured as a dylib on Windows > ([mstorsjo/llvm-mingw/actions/runs/21889644772/job/63206027685](https://github.com/mstorsjo/llvm-mingw/actions/runs/21889644772/job/63206027685)). > Building with a dylib on Windows is only supported on mingw at the moment > (but there's work in progress for making it work in MSVC style build > configurations too). The build error is this: > > ``` > FAILED: [code=1] bin/lldb-dap.exe > : && /opt/llvm-mingw/bin/x86_64-w64-mingw32-g++ -fvisibility-inlines-hidden > -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra > -Wno-unused-parameter -Wwrite-strings -Wcast-qual > -Wmissing-field-initializers -pedantic -Wno-long-long > -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default > -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor > -Wsuggest-override -Wstring-conversion -Wno-pass-failed > -Wmisleading-indentation -Wctad-maybe-unsupported -ffunction-sections > -fdata-sections > -fprofile-instr-use="/home/runner/work/llvm-mingw/llvm-mingw/profile.profdata" > -flto=thin -Wno-unknown-pragmas -Wno-strict-aliasing -Wno-vla-extension -O3 > -DNDEBUG -Wl,--stack,16777216 > -fprofile-instr-use="/home/runner/work/llvm-mingw/llvm-mingw/profile.profdata" > -flto=thin -Wl,--gc-sections -Wl,--delayload=liblldb.dll > tools/lldb/tools/lldb-dap/tool/CMakeFiles/lldb-dap.dir/lldb-dap.cpp.obj -o > bin/lldb-dap.exe -Wl,--out-implib,lib/liblldb-dap.dll.a > -Wl,--major-image-version,0,--minor-image-version,0 lib/libLLVM-23git.dll.a > lib/liblldbDAP.a lib/liblldbHostPythonPathSetup.a lib/liblldb.dll.a > lib/libclang-cpp.dll.a lib/liblldbHost.a lib/liblldbUtility.a -lrpcrt4 > lib/libLLVMSupport.a -lpsapi -lshell32 -lole32 -luuid -ladvapi32 > -lws2_32 -lntdll lib/libLLVMDemangle.a -lkernel32 -luser32 -lgdi32 > -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 && : > ld.lld: error: duplicate symbol: llvm::raw_ostream::operator<<(unsigned long > long) > >>> defined at > >>> /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/llvm/lib/Support/raw_ostream.cpp > >>> libLLVMSupport.a(raw_ostream.cpp.obj) > >>> defined at libLLVM-23git.dll.a(libLLVM-23git.dll) > > ld.lld: error: duplicate symbol: > llvm::format_provider<std::__1::chrono::time_point<std::__1::chrono::system_clock, > std::__1::chrono::duration<long long, std::__1::ratio<1ll, 1000000000ll>>>, > void>::format(std::__1::chrono::time_point<std::__1::chrono::system_clock, > std::__1::chrono::duration<long long, std::__1::ratio<1ll, 1000000000ll>>> > const&, llvm::raw_ostream&, llvm::StringRef) > >>> defined at > >>> /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/llvm/lib/Support/Chrono.cpp > >>> libLLVMSupport.a(Chrono.cpp.obj) > >>> defined at libLLVM-23git.dll.a(libLLVM-23git.dll) > > ld.lld: error: duplicate symbol: llvm::raw_fd_ostream::~raw_fd_ostream() > >>> defined at > >>> /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/llvm/lib/Support/raw_ostream.cpp > >>> libLLVMSupport.a(raw_ostream.cpp.obj) > >>> defined at libLLVM-23git.dll.a(libLLVM-23git.dll) > > ld.lld: error: duplicate symbol: llvm::raw_ostream::operator<<(long) > >>> defined at > >>> /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/llvm/lib/Support/raw_ostream.cpp > >>> libLLVMSupport.a(raw_ostream.cpp.obj) > >>> defined at libLLVM-23git.dll.a(libLLVM-23git.dll) > > ld.lld: error: duplicate symbol: llvm::ErrorInfoBase::anchor() > >>> defined at > >>> /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/llvm/lib/Support/Error.cpp > >>> libLLVMSupport.a(Error.cpp.obj) > >>> defined at libLLVM-23git.dll.a(libLLVM-23git.dll) > clang: error: linker command failed with exit code 1 (use -v to see > invocation) > ``` > > When building with a dylib, all the various LLVM static libraries are linked > into one big libLLVM shared library, and that is linked instead of linking to > the individual small libraries; logic for that is in e.g. > [`llvmorg-21.1.8`/llvm/cmake/modules/AddLLVM.cmake#L773-L783](https://github.com/llvm/llvm-project/blob/llvmorg-21.1.8/llvm/cmake/modules/AddLLVM.cmake#L773-L783). > > To reproduce the issue, you can download a release of llvm-mingw from > [mstorsjo/llvm-mingw/releases](https://github.com/mstorsjo/llvm-mingw/releases) > (for a platform of your choice), unpack and add the `<toolchain>/bin` > directory to your `PATH`and configure a build like this: > > ``` > cmake .. \ > -G Ninja \ > \ > -DCMAKE_SYSTEM_NAME=Windows \ > -DCMAKE_SYSTEM_PROCESSOR=x86_64 \ > -DCMAKE_C_COMPILER=x86_64-w64-mingw32-clang \ > -DCMAKE_CXX_COMPILER=x86_64-w64-mingw32-clang++ \ > -DCMAKE_RC_COMPILER=x86_64-w64-mingw32-windres \ > -DCMAKE_FIND_ROOT_PATH=$(dirname $(which > x86_64-w64-mingw32-clang))/../x86_64-w64-mingw32 \ > \ > -DCMAKE_FIND_ROOT_PATH_MODE_PROGRAM=NEVER \ > -DCMAKE_FIND_ROOT_PATH_MODE_INCLUDE=ONLY \ > -DCMAKE_FIND_ROOT_PATH_MODE_LIBRARY=ONLY \ > -DCMAKE_FIND_ROOT_PATH_MODE_PACKAGE=ONLY \ > \ > -DCMAKE_BUILD_TYPE=Release \ > -DLLVM_TARGETS_TO_BUILD="ARM;AArch64;X86" \ > -DLLVM_ENABLE_PROJECTS="clang;lld;lldb" \ > -DLLVM_LINK_LLVM_DYLIB=ON > ``` > > (For a non-cross build on Windows, you can probably skip the first and second > section of options.) > > A change like this fixes the issue here: > > ```diff > diff --git a/lldb/source/Host/windows/PythonPathSetup/CMakeLists.txt > b/lldb/source/Host/windows/PythonPathSetup/CMakeLists.txt > index 6b84a7187160..96f2b482b376 100644 > --- a/lldb/source/Host/windows/PythonPathSetup/CMakeLists.txt > +++ b/lldb/source/Host/windows/PythonPathSetup/CMakeLists.txt > @@ -1,8 +1,5 @@ > add_lldb_library(lldbHostPythonPathSetup STATIC > PythonPathSetup.cpp > - > - LINK_LIBS > - LLVMSupport > ) > > if(DEFINED LLDB_PYTHON_DLL_RELATIVE_PATH) > ``` > > This change doesn't seem to break builds without `LLVM_LINK_LLVM_DYLIB` > either, so it seems ok in that sense, but I'm not entirely sure exactly > whether it is the semantically right change. > > Also FWIW, I tried simulating the same linking setup on Linux, by forcing > this library to be included there. I built with modifications like this: > > ```diff > diff --git a/lldb/source/Host/CMakeLists.txt b/lldb/source/Host/CMakeLists.txt > index 8c198c655e0a..e1dd013d7031 100644 > --- a/lldb/source/Host/CMakeLists.txt > +++ b/lldb/source/Host/CMakeLists.txt > @@ -64,8 +64,8 @@ add_host_subdirectory(posix > posix/ConnectionFileDescriptorPosix.cpp > ) > > -if (CMAKE_SYSTEM_NAME MATCHES "Windows") > add_subdirectory(windows/PythonPathSetup) > +if (CMAKE_SYSTEM_NAME MATCHES "Windows") > add_host_subdirectory(windows > windows/ConnectionGenericFileWindows.cpp > windows/FileSystem.cpp > diff --git a/lldb/source/Host/windows/PythonPathSetup/PythonPathSetup.cpp > b/lldb/source/Host/windows/PythonPathSetup/PythonPathSetup.cpp > index d378e6984b05..bf5f635a1bbc 100644 > --- a/lldb/source/Host/windows/PythonPathSetup/PythonPathSetup.cpp > +++ b/lldb/source/Host/windows/PythonPathSetup/PythonPathSetup.cpp > @@ -8,14 +8,18 @@ > > #include "lldb/Host/windows/PythonPathSetup/PythonPathSetup.h" > > +#ifdef _WIN32 > #include "lldb/Host/windows/windows.h" > #include "llvm/Support/Windows/WindowsSupport.h" > +#endif > > #include "llvm/ADT/SmallVector.h" > #include "llvm/Support/ConvertUTF.h" > #include "llvm/Support/FileSystem.h" > #include "llvm/Support/Path.h" > +#ifdef _WIN32 > #include <pathcch.h> > +#endif > > using namespace llvm; > > diff --git a/lldb/tools/driver/CMakeLists.txt > b/lldb/tools/driver/CMakeLists.txt > index 7043f648518d..2e6bad5b57dc 100644 > --- a/lldb/tools/driver/CMakeLists.txt > +++ b/lldb/tools/driver/CMakeLists.txt > @@ -22,9 +22,7 @@ set(LLDB_DRIVER_LINK_LIBS > lldbUtility > ) > > -if(WIN32) > list(APPEND LLDB_DRIVER_LINK_LIBS lldbHostPythonPathSetup) > -endif() > > add_lldb_tool(lldb > Driver.cpp > diff --git a/lldb/tools/lldb-dap/tool/CMakeLists.txt > b/lldb/tools/lldb-dap/tool/CMakeLists.txt > index 2216e54064bf..55f1ffa01c32 100644 > --- a/lldb/tools/lldb-dap/tool/CMakeLists.txt > +++ b/lldb/tools/lldb-dap/tool/CMakeLists.txt > @@ -4,10 +4,8 @@ add_public_tablegen_target(LLDBDAPOptionsTableGen) > > set(LLDB_DAP_LINK_LIBS lldbDAP) > > -if(WIN32) > list(APPEND LLDB_DAP_LINK_LIBS lldbHostPythonPathSetup) > list(APPEND LLDB_DAP_LINK_LIBS liblldb) > -endif() > > add_lldb_tool(lldb-dap > lldb-dap.cpp > diff --git a/lldb/tools/lldb-dap/tool/lldb-dap.cpp > b/lldb/tools/lldb-dap/tool/lldb-dap.cpp > index bfd7c5d39ec4..2c4f2dff6490 100644 > --- a/lldb/tools/lldb-dap/tool/lldb-dap.cpp > +++ b/lldb/tools/lldb-dap/tool/lldb-dap.cpp > @@ -59,6 +59,7 @@ > #include <utility> > #include <vector> > > +#include "lldb/Host/windows/PythonPathSetup/PythonPathSetup.h" > #if defined(_WIN32) > // We need to #define NOMINMAX in order to skip `min()` and `max()` macro > // definitions that conflict with other system headers. > @@ -70,7 +71,6 @@ > #undef GetObject > #include <io.h> > typedef int socklen_t; > -#include "lldb/Host/windows/PythonPathSetup/PythonPathSetup.h" > #else > #include <netinet/in.h> > #include <sys/socket.h> > @@ -523,10 +523,8 @@ int main(int argc, char *argv[]) { > "~/Library/Logs/DiagnosticReports/.\n"); > #endif > > -#ifdef _WIN32 > if (llvm::Error error = SetupPythonRuntimeLibrary()) > llvm::WithColor::error() << llvm::toString(std::move(error)) << '\n'; > -#endif > > llvm::SmallString<256> program_path(argv[0]); > llvm::sys::fs::make_absolute(program_path); > ``` > > If building this on Linux with `-DLLVM_ENABLE_PROJECTS="clang;lld;lldb" > -DLLVM_LINK_LLVM_DYLIB=ON`, I end up with this: > > ``` > $ ninja -v bin/llvm-dap > [..] > [4/4] : && /home/martin/clang+llvm-19.1.1-aarch64-linux-gnu/bin/clang++ -fPIC > -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time > -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter > -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic > -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough > -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor > -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion > -Wno-pass-failed -Wmisleading-indentation -Wctad-maybe-unsupported > -fdiagnostics-color -ffunction-sections -fdata-sections -Wno-unknown-pragmas > -Wno-strict-aliasing -Wno-vla-extension -O3 -DNDEBUG -fuse-ld=lld > -Wl,--color-diagnostics -Wl,--gc-sections > tools/lldb/tools/lldb-dap/tool/CMakeFiles/lldb-dap.dir/lldb-dap.cpp.o -o > bin/lldb-dap > -Wl,-rpath,"\$ORIGIN/../lib:/home/martin/code/llvm-project/llvm/build-clang/lib:" > -lpthread lib/liblldbDAP.a lib/liblldbHostPythonPathSetup.a > lib/liblldb.so.23.0.0git lib/liblldbHost.a lib/liblldbUtility.a > lib/libclang-cpp.so.23.0git lib/libLLVM.so.23.0git lib/libLLVMSupport.a > -lrt -lpthread -ldl -lm /usr/lib/aarch64-linux-gnu/libz.so > lib/libLLVMDemangle.a && : > ``` > > This doesn't fail the build in the same way as the mingw build, but the > problem is still present there, it links against both > `lib/libLLVM.so.23.0git` and `lib/libLLVMSupport.a` at the same time, which > can cause ending up with duplicate copies of the same constructs from > LLVMSupport in the same binary, which can give spurious issues at runtime. > > The suggested diff above gets rid of the duplicate `lib/libLLVMSupport.a` > from that linking command.
Thanks for the detailed explanation, I have opened a fix here: https://github.com/llvm/llvm-project/pull/180976. Building at desk works fine, I think that the fix is semantically correct, as `PythonPathSetup` does not require LLVMSupport. https://github.com/llvm/llvm-project/pull/179306 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
