dreal/dreal3

BUILD_SHARED_LIB=ON → disaster

Closed this issue · 18 comments

mwoehlke-kitware commented

I'm trying to build dReal with BUILD_SHARED_LIB=ON. It fails horribly. Rather than try to include the myriad link errors, here are some brief examples:

linking lib/libdop.so (cc)
CMakeFiles/dop.dir/tools/dop/print_latex.cpp.o: In function `operator<<':
.../dreal/src/opensmt/egraph/Enode.h:380: undefined reference to `Enode::print(std::ostream&) const'
CMakeFiles/dop.dir/tools/dop/process.cpp.o: In function `operator()<__gnu_cxx::__normal_iterator<std::pair<std::basic_string<char>, Enode*>*, std::vector<std::pair<std::basic_string<char>, Enode*> > >, __gnu_cxx::__normal_iterator<std::pair<std::basic_string<char>, Enode*>*, std::vector<std::pair<std::basic_string<char>, Enode*> > > >':
.../dreal/src/tools/dop/process.cpp:175: undefined reference to `dreal::starts_with(std::string const&, std::string const&)'

.../lib/libdreal.so: undefined reference to `filib::fp_traits_base<double>::nan_val'
.../lib/libdreal.so: undefined reference to `ibex::IntervalVector::mid() const'
.../lib/libdreal.so: undefined reference to `glp_ipt_status'

This is happening on multiple machines (so far, on Fedora 21 and Ubuntu 14.04, but I suspect the problem is universal), as of @e6d1d49.

CMake arguments:

USE_NLOPT=OFF
BUILD_TESTING=OFF
BUILD_SHARED_LIB=ON
CMAKE_SHARED_LINKER_FLAGS=-Wl,--no-undefined

Possibly related to #51. In particular, #51 (comment) suggests that this is a known issue that for some reason was not fixed, despite that #51 was closed.

mwoehlke-kitware commented

This seems to fix the problem, but I'm not sure if it is TRTTD... dReal's build system seems... odd...

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 7c2bb55..13a56a0 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -808,6 +808,11 @@ else()
   add_library(dop ${DOP_SRCS})
 endif()

+set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ${DREAL_EXTRA_LINKER_FLAGS} ${EXTERNAL_LD_FLAGS}")
+
+target_link_libraries(dreal ${EXTRA_LIBS})
+target_link_libraries(dop dreal ${PYTHON_LIBRARIES} ${EXTRA_LIBS})
+
 add_dependencies(dop PICOSAT)
 add_dependencies(dop IBEX)
 add_dependencies(dop ADEPT)
mwoehlke-kitware commented

Amendment: the above is only sufficient to build dReal... trying to use it:

/usr/bin/ld: .../lib/dreal/libglpk.a(libglpk_la-glpapi01.o): relocation R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared object; recompile with -fPIC
.../lib/dreal/libglpk.a: error adding symbols: Bad value

It looks like dReal also needs to pass -DBUILD_SHARED_LIBS (or at least -DCMAKE_POSITION_INDEPENDENT_CODE) to its externals. (Also, what's with BUILD_SHARED_LIB? Why not use the standard BUILD_SHARED_LIBS?)

soonho-tri commented

Hi @mwoehlke-kitware, thanks for the report. This is a known issue and something I've wanted to fix for a long time. I'll take a look at this soon and get back to you. Thanks again.

mwoehlke-kitware commented

I think I have things working between the above patch and #317... but the above patch feels like a hack; I suspect there are missing subtleties...

soonho-tri commented

@mwoehlke-kitware, thanks for the help!

mwoehlke-kitware commented

I'm still getting undefined references...

soonho-tri commented

On Ubuntu?

mwoehlke-kitware commented

On Linux. I don't have an Ubuntu machine.

soonho-tri commented

I see. I just triggered a fresh build on a Linux machine (Ubuntu).

soonho-tri commented

FYI, it works on Mac (static/shared) and Ubuntu (static). I forgot to check the shared + Ubuntu combo..

mwoehlke-kitware commented

It seems to be "working" because dreal.pc has a bunch of spam about other libraries. However, you are doing it wrong; libdreal itself uses ibex, capd, etc....; it should link to them. Consumers should _not_¹ link to them directly. This is what causes bugs (and yes, it's a bug) like in #51 (comment). This would also be more obviously broken if you were using CMake exported targets, as replicating the existing behavior would require adding all of these as interface link libraries, which is obviously wrong (they should be private or maybe public).

(¹ Probably. I don't know dReal's API well enough to know if they're part of its public API, but I suspect not all of them are.)

Please add -DCMAKE_SHARED_LINKER_FLAGS=-Wl,--no-undefined to your build and you will see the problem. The fix is roughly the patch provided above. (I may submit a PR in a little bit, but I'm only guessing at how to deal with dReal's esoteric build "correctly".)

soonho-tri commented

However, you are doing it wrong; libdreal itself uses ibex, capd, etc....; it should link to them. Consumers should not¹ link to them directly.

Do you mean, libdreal should be self-contained by including its dependencies (i.e. ibex, capd)?

soonho-tri commented

I was able to reproduce the undefined references by adding -DCMAKE_SHARED_LINKER_FLAGS=-Wl,--no-undefined (on a Ubuntu-14 machine). Having the following patch (that you provided above) seems to solve the problem:

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 7c2bb55..13a56a0 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -808,6 +808,11 @@ else()
   add_library(dop ${DOP_SRCS})
 endif()

+set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ${DREAL_EXTRA_LINKER_FLAGS} ${EXTERNAL_LD_FLAGS}")
+
+target_link_libraries(dreal ${EXTRA_LIBS})
+target_link_libraries(dop dreal ${PYTHON_LIBRARIES} ${EXTRA_LIBS})
+
 add_dependencies(dop PICOSAT)
 add_dependencies(dop IBEX)
 add_dependencies(dop ADEPT)
soonho-tri commented

It seems to be "working" because dreal.pc has a bunch of spam about other libraries.

  1. Speaking of dReal APIs, users should not know about the external APIs that we're using.
  2. Does that mean I need to remove all the external libraries from dreal.pc by cleaning up the following lines

    dreal3/src/CMakeLists.txt

    Lines 951 to 973 in e1aafe6

    ########################################
    # PKG CONFIG
    ########################################
    SET(PKG_CONFIG_LIBDIR "\${prefix}/lib/dreal")
    SET(PKG_CONFIG_INCLUDEDIR "\${prefix}/include/dreal")
    SET(PROJECT_DESCRIPTION "nonlinear SMT Solver")
    SET(PKG_CONFIG_LIBS "-L\${libdir} -ldreal -libex -lcapd -ladept")
    if(USE_CLP)
    foreach(CLP_LIB ${CLP_LIBRARIES})
    SET(PKG_CONFIG_LIBS "${PKG_CONFIG_LIBS} -l${CLP_LIB}")
    endforeach()
    endif()
    if(USE_NLOPT)
    SET(PKG_CONFIG_LIBS "${PKG_CONFIG_LIBS} -lnlopt")
    endif()
    if(USE_GLPK)
    SET(PKG_CONFIG_LIBS "${PKG_CONFIG_LIBS} -lglpk")
    endif()
    SET(PKG_CONFIG_LIBS "${PKG_CONFIG_LIBS} -lprim -lpicosat ${CMAKE_THREAD_LIBS_INIT} -lm")
    SET(PKG_CONFIG_CFLAGS "-I\${includedir}")
    CONFIGURE_FILE("${CMAKE_CURRENT_SOURCE_DIR}/pkg-config.pc.cmake"
    "${CMAKE_CURRENT_BINARY_DIR}/dreal.pc")
    ?
soonho-tri commented

Sorry for the questions and thanks for the help!

mwoehlke-kitware commented

Do you mean, libdreal should be self-contained by including its dependencies (i.e. ibex, capd)?

Exactly. That goes for any library, too, not just dreal 😄.

Does that mean I need to remove all the external libraries from dreal.pc [...]

Well... it's complicated. If dreal is built static, then you need those (no such thing as private linking for static libraries). If it's shared, you may be able to remove them, in which case maybe you should, but opinions might vary on that matter. However, given that you also said:

users should not know about the external APIs that we're using

...this makes me guess that probably you don't want to leak the list of link libraries when not necessary.

If the patch above isn't striking you as some crazy backward approach, I'll submit a PR shortly. Please don't use the version above; it is quick-and-dirty and works with my current configuration, but at least one bit should be configuration-dependent.

soonho-tri commented

@mwoehlke-kitware, thanks for all the answers! TIL a lot!

If the patch above isn't striking you as some crazy backward approach, I'll submit a PR shortly. Please don't use the version above; it is quick-and-dirty and works with my current configuration, but at least one bit should be configuration-dependent.

Please submit the PR whenever it's ready. Thanks!!

mwoehlke-kitware commented

BTW, can you re-open this so the PR can close it properly? TIA!

  • 👍1