[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)
This revision was automatically updated to reflect the committed changes.
Closed by commit rL327490: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds
(pr36687) (authored by labath, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D44379?vs=138034&id=138316#toc
Repository:
rL LLVM
https://reviews.llvm.org/D44379
Files:
lldb/trunk/cmake/modules/LLDBConfig.cmake
Index: lldb/trunk/cmake/modules/LLDBConfig.cmake
===
--- lldb/trunk/cmake/modules/LLDBConfig.cmake
+++ lldb/trunk/cmake/modules/LLDBConfig.cmake
@@ -346,14 +346,18 @@
endif()
-if (HAVE_LIBPTHREAD)
- list(APPEND system_libs pthread)
-endif(HAVE_LIBPTHREAD)
+if( WIN32 AND NOT CYGWIN )
+ set(PURE_WINDOWS 1)
+endif()
-if (HAVE_LIBDL)
- list(APPEND system_libs ${CMAKE_DL_LIBS})
+if(NOT PURE_WINDOWS)
+ set(CMAKE_THREAD_PREFER_PTHREAD TRUE)
+ find_package(Threads REQUIRED)
+ list(APPEND system_libs ${CMAKE_THREAD_LIBS_INIT})
endif()
+list(APPEND system_libs ${CMAKE_DL_LIBS})
+
# Figure out if lldb could use lldb-server. If so, then we'll
# ensure we build lldb-server when an lldb target is being built.
if (CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|FreeBSD|Linux|NetBSD")
Index: lldb/trunk/cmake/modules/LLDBConfig.cmake
===
--- lldb/trunk/cmake/modules/LLDBConfig.cmake
+++ lldb/trunk/cmake/modules/LLDBConfig.cmake
@@ -346,14 +346,18 @@
endif()
-if (HAVE_LIBPTHREAD)
- list(APPEND system_libs pthread)
-endif(HAVE_LIBPTHREAD)
+if( WIN32 AND NOT CYGWIN )
+ set(PURE_WINDOWS 1)
+endif()
-if (HAVE_LIBDL)
- list(APPEND system_libs ${CMAKE_DL_LIBS})
+if(NOT PURE_WINDOWS)
+ set(CMAKE_THREAD_PREFER_PTHREAD TRUE)
+ find_package(Threads REQUIRED)
+ list(APPEND system_libs ${CMAKE_THREAD_LIBS_INIT})
endif()
+list(APPEND system_libs ${CMAKE_DL_LIBS})
+
# Figure out if lldb could use lldb-server. If so, then we'll
# ensure we build lldb-server when an lldb target is being built.
if (CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|FreeBSD|Linux|NetBSD")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)
labath added inline comments.
Comment at: cmake/modules/LLDBConfig.cmake:349
-if (HAVE_LIBPTHREAD)
- list(APPEND system_libs pthread)
-endif(HAVE_LIBPTHREAD)
-
-if (HAVE_LIBDL)
- list(APPEND system_libs ${CMAKE_DL_LIBS})
+if(UNIX)
+ set(CMAKE_THREAD_PREFER_PTHREAD TRUE)
krytarowski wrote:
> labath wrote:
> > krytarowski wrote:
> > > Why UNIX here?
> > >
> > > Why CMAKE_THREAD_PREFER_PTHREAD? It looks like used only on IRIX and that
> > > one is not going anywhere nowadays. (And certainly similarly to other
> > > commercial OSes, due to legal work/removing not-owned code, it's not
> > > possible to push it to Open-Source).
> > >
> > > Assuming that system_libs can accept "-pthreads", this patch looks good
> > > to me.
> > For the second part, I copied it out of the llvm's build scripts without
> > looking at what it does (with the idea of trying to maintain a consistent
> > build). However, if it's only used at irix, then I guess I can remove that.
> >
> > The UNIX part is also inspired by llvm, but I simplified it a bit (they use
> > CYGWIN OR NOT WINDOWS). I was assuming the idea was to make sure we use
> > native thread support and not pthreads (which are present there sometimes,
> > I think).
> I see, I would use `CYGWIN OR NOT WINDOWS` without changing the logic.
>
> Keeping here `CMAKE_THREAD_PREFER_PTHREAD` does not make harm. non-pthreading
> on UNIX systems is rather in extinct and remnant of 90ties (Minix has
> something like that.. and lack of pthreads).
>
> The UNIX part looks correct.
Thanks, I'll update the logic to match and submit.
https://reviews.llvm.org/D44379
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)
krytarowski accepted this revision.
krytarowski added a comment.
This revision is now accepted and ready to land.
I cannot test it shortly on NetBSD, but if there are any issues left (probably
none) - I will fix it in future.
Comment at: cmake/modules/LLDBConfig.cmake:349
-if (HAVE_LIBPTHREAD)
- list(APPEND system_libs pthread)
-endif(HAVE_LIBPTHREAD)
-
-if (HAVE_LIBDL)
- list(APPEND system_libs ${CMAKE_DL_LIBS})
+if(UNIX)
+ set(CMAKE_THREAD_PREFER_PTHREAD TRUE)
labath wrote:
> krytarowski wrote:
> > Why UNIX here?
> >
> > Why CMAKE_THREAD_PREFER_PTHREAD? It looks like used only on IRIX and that
> > one is not going anywhere nowadays. (And certainly similarly to other
> > commercial OSes, due to legal work/removing not-owned code, it's not
> > possible to push it to Open-Source).
> >
> > Assuming that system_libs can accept "-pthreads", this patch looks good to
> > me.
> For the second part, I copied it out of the llvm's build scripts without
> looking at what it does (with the idea of trying to maintain a consistent
> build). However, if it's only used at irix, then I guess I can remove that.
>
> The UNIX part is also inspired by llvm, but I simplified it a bit (they use
> CYGWIN OR NOT WINDOWS). I was assuming the idea was to make sure we use
> native thread support and not pthreads (which are present there sometimes, I
> think).
I see, I would use `CYGWIN OR NOT WINDOWS` without changing the logic.
Keeping here `CMAKE_THREAD_PREFER_PTHREAD` does not make harm. non-pthreading
on UNIX systems is rather in extinct and remnant of 90ties (Minix has something
like that.. and lack of pthreads).
The UNIX part looks correct.
https://reviews.llvm.org/D44379
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)
labath added inline comments.
Comment at: cmake/modules/LLDBConfig.cmake:349
-if (HAVE_LIBPTHREAD)
- list(APPEND system_libs pthread)
-endif(HAVE_LIBPTHREAD)
-
-if (HAVE_LIBDL)
- list(APPEND system_libs ${CMAKE_DL_LIBS})
+if(UNIX)
+ set(CMAKE_THREAD_PREFER_PTHREAD TRUE)
krytarowski wrote:
> Why UNIX here?
>
> Why CMAKE_THREAD_PREFER_PTHREAD? It looks like used only on IRIX and that one
> is not going anywhere nowadays. (And certainly similarly to other commercial
> OSes, due to legal work/removing not-owned code, it's not possible to push it
> to Open-Source).
>
> Assuming that system_libs can accept "-pthreads", this patch looks good to me.
For the second part, I copied it out of the llvm's build scripts without
looking at what it does (with the idea of trying to maintain a consistent
build). However, if it's only used at irix, then I guess I can remove that.
The UNIX part is also inspired by llvm, but I simplified it a bit (they use
CYGWIN OR NOT WINDOWS). I was assuming the idea was to make sure we use native
thread support and not pthreads (which are present there sometimes, I think).
https://reviews.llvm.org/D44379
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)
krytarowski added inline comments.
Comment at: cmake/modules/LLDBConfig.cmake:349
-if (HAVE_LIBPTHREAD)
- list(APPEND system_libs pthread)
-endif(HAVE_LIBPTHREAD)
-
-if (HAVE_LIBDL)
- list(APPEND system_libs ${CMAKE_DL_LIBS})
+if(UNIX)
+ set(CMAKE_THREAD_PREFER_PTHREAD TRUE)
Why UNIX here?
Why CMAKE_THREAD_PREFER_PTHREAD? It looks like used only on IRIX and that one
is not going anywhere nowadays. (And certainly similarly to other commercial
OSes, due to legal work/removing not-owned code, it's not possible to push it
to Open-Source).
Assuming that system_libs can accept "-pthreads", this patch looks good to me.
https://reviews.llvm.org/D44379
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)
labath updated this revision to Diff 138034.
labath added a comment.
This drops the LLDBStandalone changes (they are replaced by
https://reviews.llvm.org/D44391 on the llvm
side), and simplifies the CMAKE_DL_LIBS logic.
I've tested this out both with BUILD_SHARED_LIBS and LLVM_LINK_LLVM_DYLIB builds
(for the latter, be sure to specify LLVM_LINK_LLVM_DYLIB also when configuring
lldb), but I'd appreciate more testing (I'm no hurry to land this).
https://reviews.llvm.org/D44379
Files:
cmake/modules/LLDBConfig.cmake
Index: cmake/modules/LLDBConfig.cmake
===
--- cmake/modules/LLDBConfig.cmake
+++ cmake/modules/LLDBConfig.cmake
@@ -346,14 +346,14 @@
endif()
-if (HAVE_LIBPTHREAD)
- list(APPEND system_libs pthread)
-endif(HAVE_LIBPTHREAD)
-
-if (HAVE_LIBDL)
- list(APPEND system_libs ${CMAKE_DL_LIBS})
+if(UNIX)
+ set(CMAKE_THREAD_PREFER_PTHREAD TRUE)
+ find_package(Threads REQUIRED)
+ list(APPEND system_libs ${CMAKE_THREAD_LIBS_INIT})
endif()
+list(APPEND system_libs ${CMAKE_DL_LIBS})
+
# Figure out if lldb could use lldb-server. If so, then we'll
# ensure we build lldb-server when an lldb target is being built.
if (CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|FreeBSD|Linux|NetBSD")
Index: cmake/modules/LLDBConfig.cmake
===
--- cmake/modules/LLDBConfig.cmake
+++ cmake/modules/LLDBConfig.cmake
@@ -346,14 +346,14 @@
endif()
-if (HAVE_LIBPTHREAD)
- list(APPEND system_libs pthread)
-endif(HAVE_LIBPTHREAD)
-
-if (HAVE_LIBDL)
- list(APPEND system_libs ${CMAKE_DL_LIBS})
+if(UNIX)
+ set(CMAKE_THREAD_PREFER_PTHREAD TRUE)
+ find_package(Threads REQUIRED)
+ list(APPEND system_libs ${CMAKE_THREAD_LIBS_INIT})
endif()
+list(APPEND system_libs ${CMAKE_DL_LIBS})
+
# Figure out if lldb could use lldb-server. If so, then we'll
# ensure we build lldb-server when an lldb target is being built.
if (CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|FreeBSD|Linux|NetBSD")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)
labath added inline comments.
Comment at: cmake/modules/LLDBConfig.cmake:354
+
+ check_library_exists(dl dlopen "" HAVE_LIBDL)
+ if (HAVE_LIBDL)
krytarowski wrote:
> krytarowski wrote:
> > labath wrote:
> > > krytarowski wrote:
> > > > mgorny wrote:
> > > > > krytarowski wrote:
> > > > > > A more portable form of this:
> > > > > >
> > > > > > ```
> > > > > > foreach(lib ${CMAKE_DL_LIBS})
> > > > > > list(APPEND system_libs -l${lib})
> > > > > > endforeach()
> > > > > > ```
> > > > > >
> > > > > > Haiku needs 2 libraries, other Unices can use `-lldl` or similar.
> > > > > Didn't this raise the problem of cmake using full paths on some
> > > > > systems?
> > > > No.
> > > Do you have some reference for the portability claim? (mainly because I'm
> > > trying to learn more about how cmake works, but also because I'd rather
> > > let cmake figure out when to prepend -l instead of doing it myself).
> > >
> > > I don't see how the "two libraries" thingy comes into play here. It could
> > > be a problem if we are mismatching lists and strings but I don't see why
> > > that would necessitate manually tagging with -l.
> > dlopen(3)-like functions in `-ldl` is Linux; HPUX uses `-ldl`, Darwin/BSDs
> > no extra libraries, Haiku `-lroot -lbe` etc.
> >
> > https://github.com/Kitware/CMake/search?p=2&q=CMAKE_DL_LIBS&type=&utf8=%E2%9C%93
> >
> > I know that we don't support multiple OSes as of today, but we can make it
> > compatible with every recognizable one with this change.
> >
> > It looks like the proper syntax is to drop `-l` from my first proposal:
> >
> > ```
> > foreach(lib ${CMAKE_DL_LIBS})
> > list(APPEND system_libs ${lib})
> > endforeach()
> > ```
> Or perhaps even unconditional: `list(APPEND system_libs ${CMAKE_DL_LIBS})`
I like that: :D
https://reviews.llvm.org/D44379
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)
krytarowski added inline comments.
Comment at: cmake/modules/LLDBConfig.cmake:354
+
+ check_library_exists(dl dlopen "" HAVE_LIBDL)
+ if (HAVE_LIBDL)
krytarowski wrote:
> labath wrote:
> > krytarowski wrote:
> > > mgorny wrote:
> > > > krytarowski wrote:
> > > > > A more portable form of this:
> > > > >
> > > > > ```
> > > > > foreach(lib ${CMAKE_DL_LIBS})
> > > > > list(APPEND system_libs -l${lib})
> > > > > endforeach()
> > > > > ```
> > > > >
> > > > > Haiku needs 2 libraries, other Unices can use `-lldl` or similar.
> > > > Didn't this raise the problem of cmake using full paths on some
> > > > systems?
> > > No.
> > Do you have some reference for the portability claim? (mainly because I'm
> > trying to learn more about how cmake works, but also because I'd rather let
> > cmake figure out when to prepend -l instead of doing it myself).
> >
> > I don't see how the "two libraries" thingy comes into play here. It could
> > be a problem if we are mismatching lists and strings but I don't see why
> > that would necessitate manually tagging with -l.
> dlopen(3)-like functions in `-ldl` is Linux; HPUX uses `-ldl`, Darwin/BSDs no
> extra libraries, Haiku `-lroot -lbe` etc.
>
> https://github.com/Kitware/CMake/search?p=2&q=CMAKE_DL_LIBS&type=&utf8=%E2%9C%93
>
> I know that we don't support multiple OSes as of today, but we can make it
> compatible with every recognizable one with this change.
>
> It looks like the proper syntax is to drop `-l` from my first proposal:
>
> ```
> foreach(lib ${CMAKE_DL_LIBS})
> list(APPEND system_libs ${lib})
> endforeach()
> ```
Or perhaps even unconditional: `list(APPEND system_libs ${CMAKE_DL_LIBS})`
https://reviews.llvm.org/D44379
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)
krytarowski added inline comments.
Comment at: cmake/modules/LLDBConfig.cmake:354
+
+ check_library_exists(dl dlopen "" HAVE_LIBDL)
+ if (HAVE_LIBDL)
labath wrote:
> krytarowski wrote:
> > mgorny wrote:
> > > krytarowski wrote:
> > > > A more portable form of this:
> > > >
> > > > ```
> > > > foreach(lib ${CMAKE_DL_LIBS})
> > > > list(APPEND system_libs -l${lib})
> > > > endforeach()
> > > > ```
> > > >
> > > > Haiku needs 2 libraries, other Unices can use `-lldl` or similar.
> > > Didn't this raise the problem of cmake using full paths on some systems?
> > No.
> Do you have some reference for the portability claim? (mainly because I'm
> trying to learn more about how cmake works, but also because I'd rather let
> cmake figure out when to prepend -l instead of doing it myself).
>
> I don't see how the "two libraries" thingy comes into play here. It could be
> a problem if we are mismatching lists and strings but I don't see why that
> would necessitate manually tagging with -l.
dlopen(3)-like functions in `-ldl` is Linux; HPUX uses `-ldl`, Darwin/BSDs no
extra libraries, Haiku `-lroot -lbe` etc.
https://github.com/Kitware/CMake/search?p=2&q=CMAKE_DL_LIBS&type=&utf8=%E2%9C%93
I know that we don't support multiple OSes as of today, but we can make it
compatible with every recognizable one with this change.
It looks like the proper syntax is to drop `-l` from my first proposal:
```
foreach(lib ${CMAKE_DL_LIBS})
list(APPEND system_libs ${lib})
endforeach()
```
https://reviews.llvm.org/D44379
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)
labath planned changes to this revision.
labath added a comment.
Right, so this will not work for the BUILD_SHARED_LIBS case, and there doesn't
seem to be an easy way to make it work from this end. I'm going to try fixing
this from the llvm side and come back to this if we still need the pthread/dl
fixes.
Comment at: cmake/modules/LLDBConfig.cmake:354
+
+ check_library_exists(dl dlopen "" HAVE_LIBDL)
+ if (HAVE_LIBDL)
krytarowski wrote:
> mgorny wrote:
> > krytarowski wrote:
> > > A more portable form of this:
> > >
> > > ```
> > > foreach(lib ${CMAKE_DL_LIBS})
> > > list(APPEND system_libs -l${lib})
> > > endforeach()
> > > ```
> > >
> > > Haiku needs 2 libraries, other Unices can use `-lldl` or similar.
> > Didn't this raise the problem of cmake using full paths on some systems?
> No.
Do you have some reference for the portability claim? (mainly because I'm
trying to learn more about how cmake works, but also because I'd rather let
cmake figure out when to prepend -l instead of doing it myself).
I don't see how the "two libraries" thingy comes into play here. It could be a
problem if we are mismatching lists and strings but I don't see why that would
necessitate manually tagging with -l.
https://reviews.llvm.org/D44379
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)
krytarowski added inline comments.
Comment at: cmake/modules/LLDBConfig.cmake:354
+
+ check_library_exists(dl dlopen "" HAVE_LIBDL)
+ if (HAVE_LIBDL)
mgorny wrote:
> krytarowski wrote:
> > A more portable form of this:
> >
> > ```
> > foreach(lib ${CMAKE_DL_LIBS})
> > list(APPEND system_libs -l${lib})
> > endforeach()
> > ```
> >
> > Haiku needs 2 libraries, other Unices can use `-lldl` or similar.
> Didn't this raise the problem of cmake using full paths on some systems?
No.
https://reviews.llvm.org/D44379
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)
mgorny added inline comments.
Comment at: cmake/modules/LLDBConfig.cmake:354
+
+ check_library_exists(dl dlopen "" HAVE_LIBDL)
+ if (HAVE_LIBDL)
krytarowski wrote:
> A more portable form of this:
>
> ```
> foreach(lib ${CMAKE_DL_LIBS})
> list(APPEND system_libs -l${lib})
> endforeach()
> ```
>
> Haiku needs 2 libraries, other Unices can use `-lldl` or similar.
Didn't this raise the problem of cmake using full paths on some systems?
https://reviews.llvm.org/D44379
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)
krytarowski added inline comments.
Comment at: cmake/modules/LLDBConfig.cmake:354
+
+ check_library_exists(dl dlopen "" HAVE_LIBDL)
+ if (HAVE_LIBDL)
A more portable form of this:
```
foreach(lib ${CMAKE_DL_LIBS})
list(APPEND system_libs -l${lib})
endforeach()
```
Haiku needs 2 libraries, other Unices can use `-lldl` or similar.
https://reviews.llvm.org/D44379
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)
mgorny added a comment. Yes, full standalone. https://reviews.llvm.org/D44379 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)
labath added a comment. Ah, yes, another build mode :/. Are you doing a standalone build or not? I am pretty sure this will be fine for an integrated build, but I have no idea what will it do in the standalone mode... https://reviews.llvm.org/D44379 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)
mgorny added a comment. I can't test this right now but please make sure not to break linking to split shared libraries without dylib. https://reviews.llvm.org/D44379 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)
labath created this revision.
labath added reviewers: krytarowski, zturner.
Herald added a subscriber: mgorny.
This patch consists of two relatively independent changes:
- first, I teach the standalone build to detect the case when llvm was built in
the shared mode and set the appropriate cmake variables to make the
add_llvm_*** family of functions link against the shared library. Without
these, the we would end up linking against the constituent .a files while other
parts of the build (e.g. clang) would pull in the libllvm.so, which resulted in
multiply defined symbols.
- second, I add detection code for pthread and dl libraries. This is necessary,
because we have direct calls to these libraries (instead of going through llvm)
and in the standalone build we cannot rely on llvm to detect these for us. In a
standalone non-dylib build this was accidentaly working because these libraries
were pulled in as an interface dependency of the .a files, but in a dylib build
these are no longer part of the link interface, and so we need to add them
explicitly.
https://reviews.llvm.org/D44379
Files:
cmake/modules/LLDBConfig.cmake
cmake/modules/LLDBStandalone.cmake
Index: cmake/modules/LLDBStandalone.cmake
===
--- cmake/modules/LLDBStandalone.cmake
+++ cmake/modules/LLDBStandalone.cmake
@@ -17,7 +17,9 @@
"--includedir"
"--prefix"
"--src-root"
- "--cmakedir")
+ "--cmakedir"
+ "--shared-mode"
+ )
execute_process(
COMMAND ${CONFIG_COMMAND}
RESULT_VARIABLE HAD_ERROR
@@ -44,6 +46,7 @@
list(GET CONFIG_OUTPUT 4 LLVM_OBJ_ROOT)
list(GET CONFIG_OUTPUT 5 MAIN_SRC_DIR)
list(GET CONFIG_OUTPUT 6 LLVM_CMAKE_PATH)
+ list(GET CONFIG_OUTPUT 7 SHARED_MODE)
if(NOT MSVC_IDE)
set(LLVM_ENABLE_ASSERTIONS ${ENABLE_ASSERTIONS}
@@ -72,6 +75,11 @@
message(FATAL_ERROR "Not found: ${LLVMCONFIG_FILE}")
endif()
+ if(SHARED_MODE STREQUAL "shared")
+set(LLVM_LINK_LLVM_DYLIB ON)
+set(LLVM_DYLIB_COMPONENTS all)
+ endif()
+
# They are used as destination of target generators.
set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)
set(LLVM_LIBRARY_OUTPUT_INTDIR
${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX})
Index: cmake/modules/LLDBConfig.cmake
===
--- cmake/modules/LLDBConfig.cmake
+++ cmake/modules/LLDBConfig.cmake
@@ -346,12 +346,15 @@
endif()
-if (HAVE_LIBPTHREAD)
- list(APPEND system_libs pthread)
-endif(HAVE_LIBPTHREAD)
-
-if (HAVE_LIBDL)
- list(APPEND system_libs ${CMAKE_DL_LIBS})
+if(UNIX)
+ set(CMAKE_THREAD_PREFER_PTHREAD TRUE)
+ find_package(Threads REQUIRED)
+ list(APPEND system_libs ${CMAKE_THREAD_LIBS_INIT})
+
+ check_library_exists(dl dlopen "" HAVE_LIBDL)
+ if (HAVE_LIBDL)
+list(APPEND system_libs ${CMAKE_DL_LIBS})
+ endif()
endif()
# Figure out if lldb could use lldb-server. If so, then we'll
Index: cmake/modules/LLDBStandalone.cmake
===
--- cmake/modules/LLDBStandalone.cmake
+++ cmake/modules/LLDBStandalone.cmake
@@ -17,7 +17,9 @@
"--includedir"
"--prefix"
"--src-root"
- "--cmakedir")
+ "--cmakedir"
+ "--shared-mode"
+ )
execute_process(
COMMAND ${CONFIG_COMMAND}
RESULT_VARIABLE HAD_ERROR
@@ -44,6 +46,7 @@
list(GET CONFIG_OUTPUT 4 LLVM_OBJ_ROOT)
list(GET CONFIG_OUTPUT 5 MAIN_SRC_DIR)
list(GET CONFIG_OUTPUT 6 LLVM_CMAKE_PATH)
+ list(GET CONFIG_OUTPUT 7 SHARED_MODE)
if(NOT MSVC_IDE)
set(LLVM_ENABLE_ASSERTIONS ${ENABLE_ASSERTIONS}
@@ -72,6 +75,11 @@
message(FATAL_ERROR "Not found: ${LLVMCONFIG_FILE}")
endif()
+ if(SHARED_MODE STREQUAL "shared")
+set(LLVM_LINK_LLVM_DYLIB ON)
+set(LLVM_DYLIB_COMPONENTS all)
+ endif()
+
# They are used as destination of target generators.
set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)
set(LLVM_LIBRARY_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX})
Index: cmake/modules/LLDBConfig.cmake
===
--- cmake/modules/LLDBConfig.cmake
+++ cmake/modules/LLDBConfig.cmake
@@ -346,12 +346,15 @@
endif()
-if (HAVE_LIBPTHREAD)
- list(APPEND system_libs pthread)
-endif(HAVE_LIBPTHREAD)
-
-if (HAVE_LIBDL)
- list(APPEND system_libs ${CMAKE_DL_LIBS})
+if(UNIX)
+ set(CMAKE_THREAD_PREFER_PTHREAD TRUE)
+ find_package(Threads REQUIRED)
+ list(APPEND system_libs ${CMAKE_THREAD_LIBS_INIT})
+
+ check_library_exists(dl dlopen "" HAVE_LIBDL)
+ if (HAVE_LIBDL)
+list(APPEND system_libs ${CMAKE_DL_LIBS})
+ endif()
endif()
# Figure out if lldb could use lldb-server. If so, then we'll
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/m
