Hide C++ symbols in libxgboost.so when building Python wheel (#5590)

* Hide C++ symbols in libxgboost.so when building Python wheel * Update Jenkinsfile * Add test * Upgrade rabit * Add setup.py option. Co-authored-by: fis <[email protected]>
dmlc · Apr 24, 2020 · ef26bc4 · ef26bc4
1 parent 660be66
commit ef26bc4
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 7 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -4,6 +4,7 @@ include(cmake/Utils.cmake)
 list(APPEND CMAKE_MODULE_PATH "${xgboost_SOURCE_DIR}/cmake/modules")
 cmake_policy(SET CMP0022 NEW)
 cmake_policy(SET CMP0079 NEW)
+cmake_policy(SET CMP0063 NEW)
 
 if ((${CMAKE_VERSION} VERSION_GREATER 3.13) OR (${CMAKE_VERSION} VERSION_EQUAL 3.13))
   cmake_policy(SET CMP0077 NEW)
@@ -36,6 +37,7 @@ option(USE_DMLC_GTEST "Use google tests bundled with dmlc-core submodule" OFF)
 option(USE_NVTX "Build with cuda profiling annotations. Developers only." OFF)
 set(NVTX_HEADER_DIR "" CACHE PATH "Path to the stand-alone nvtx header")
 option(RABIT_MOCK "Build rabit with mock" OFF)
+option(HIDE_CXX_SYMBOLS "Build shared library and hide all C++ symbols" OFF)
 ## CUDA
 option(USE_CUDA  "Build with GPU acceleration" OFF)
 option(USE_NCCL  "Build with NCCL to enable distributed GPU support." OFF)
@@ -131,6 +133,9 @@ foreach(lib rabit rabit_base rabit_empty rabit_mock rabit_mock_static)
   # from dmlc is correctly applied to rabit.
   if (TARGET ${lib})
     target_link_libraries(${lib} dmlc ${CMAKE_THREAD_LIBS_INIT})
+    if (HIDE_CXX_SYMBOLS)  # Hide all C++ symbols from Rabit
+      set_target_properties(${lib} PROPERTIES CXX_VISIBILITY_PRESET hidden)
+    endif (HIDE_CXX_SYMBOLS)
   endif (TARGET ${lib})
 endforeach()
 
@@ -148,10 +153,17 @@ set(XGBOOST_OBJ_SOURCES "${XGBOOST_OBJ_SOURCES};$<TARGET_OBJECTS:objxgboost>")
 
 #-- library
 if (BUILD_STATIC_LIB)
-    add_library(xgboost STATIC ${XGBOOST_OBJ_SOURCES})
-else()
-    add_library(xgboost SHARED ${XGBOOST_OBJ_SOURCES})
-endif(BUILD_STATIC_LIB)
+  add_library(xgboost STATIC ${XGBOOST_OBJ_SOURCES})
+else (BUILD_STATIC_LIB)
+  add_library(xgboost SHARED ${XGBOOST_OBJ_SOURCES})
+endif (BUILD_STATIC_LIB)
+
+#-- Hide all C++ symbols
+if (HIDE_CXX_SYMBOLS)
+  set_target_properties(objxgboost PROPERTIES CXX_VISIBILITY_PRESET hidden)
+  set_target_properties(xgboost PROPERTIES CXX_VISIBILITY_PRESET hidden)
+endif (HIDE_CXX_SYMBOLS)
+
 target_include_directories(xgboost
   INTERFACE
   $<INSTALL_INTERFACE:${CMAKE_INSTALL_PREFIX}/include>

diff --git a/Jenkinsfile b/Jenkinsfile
@@ -247,7 +247,7 @@ def BuildCUDA(args) {
     def docker_binary = "docker"
     def docker_args = "--build-arg CUDA_VERSION=${args.cuda_version}"
     sh """
-    ${dockerRun} ${container_type} ${docker_binary} ${docker_args} tests/ci_build/build_via_cmake.sh -DUSE_CUDA=ON -DUSE_NCCL=ON -DOPEN_MP:BOOL=ON
+    ${dockerRun} ${container_type} ${docker_binary} ${docker_args} tests/ci_build/build_via_cmake.sh -DUSE_CUDA=ON -DUSE_NCCL=ON -DOPEN_MP:BOOL=ON -DHIDE_CXX_SYMBOLS=ON
     ${dockerRun} ${container_type} ${docker_binary} ${docker_args} bash -c "cd python-package && rm -rf dist/* && python setup.py bdist_wheel --universal"
     ${dockerRun} ${container_type} ${docker_binary} ${docker_args} python3 tests/ci_build/rename_whl.py python-package/dist/*.whl ${commit_id} manylinux2010_x86_64
     """

diff --git a/include/xgboost/c_api.h b/include/xgboost/c_api.h
@@ -20,7 +20,7 @@
 #if defined(_MSC_VER) || defined(_WIN32)
 #define XGB_DLL XGB_EXTERN_C __declspec(dllexport)
 #else
-#define XGB_DLL XGB_EXTERN_C
+#define XGB_DLL XGB_EXTERN_C __attribute__ ((visibility ("default")))
 #endif  // defined(_MSC_VER) || defined(_WIN32)
 
 // manually define unsigned long

diff --git a/python-package/setup.py b/python-package/setup.py
@@ -14,11 +14,14 @@
 CURRENT_DIR = os.path.abspath(os.path.dirname(__file__))
 sys.path.insert(0, CURRENT_DIR)
 
+# Options only effect `python setup.py install`, building `bdist_wheel`
+# requires using CMake directly.
 USER_OPTIONS = {
     'use-openmp': (None, 'Build with OpenMP support.', 1),
     'use-cuda':   (None, 'Build with GPU acceleration.', 0),
     'use-nccl':   (None, 'Build with NCCL to enable distributed GPU support.', 0),
     'build-with-shared-nccl': (None, 'Build with shared NCCL library.', 0),
+    'hide-cxx-symbols':       (None, 'Hide all C++ symbols during build.', 1),
     'use-hdfs':   (None, 'Build with HDFS support', 0),
     'use-azure':  (None, 'Build with AZURE support.', 0),
     'use-s3':     (None, 'Build with S3 support', 0),
@@ -103,6 +106,7 @@ def build(self, src_dir, build_dir, generator, build_tool=None, use_omp=1):
             if k == 'USE_OPENMP' and use_omp == 0:
                 continue
 
+        self.logger.info('Run CMake command: %s', str(cmake_cmd))
         subprocess.check_call(cmake_cmd, cwd=build_dir)
 
         if system() != 'Windows':
@@ -236,6 +240,7 @@ def initialize_options(self):
         self.use_cuda = 0
         self.use_nccl = 0
         self.build_with_shared_nccl = 0
+        self.hide_cxx_symbols = 1
 
         self.use_hdfs = 0
         self.use_azure = 0

diff --git a/rabit b/rabit
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -39,6 +39,12 @@ if (USE_CUDA)
     )
   endif (MSVC)
 
+  if (HIDE_CXX_SYMBOLS)
+    target_compile_options(objxgboost PRIVATE
+      $<$<COMPILE_LANGUAGE:CUDA>:-Xcompiler=-fvisibility=hidden>
+    )
+  endif (HIDE_CXX_SYMBOLS)
+
   set_target_properties(objxgboost PROPERTIES
     CUDA_SEPARABLE_COMPILATION OFF)
 else (USE_CUDA)

diff --git a/tests/cpp/c_api/test_c_api.cc b/tests/cpp/c_api/test_c_api.cc
@@ -141,4 +141,11 @@ TEST(CAPI, JsonModelIO) {
   ASSERT_EQ(model_str_0.front(), '{');
   ASSERT_EQ(model_str_0, model_str_1);
 }
+
+TEST(CAPI, CatchDMLCError) {
+  DMatrixHandle out;
+  ASSERT_EQ(XGDMatrixCreateFromFile("foo", 0, &out), -1);
+  EXPECT_THROW({ dmlc::Stream::Create("foo", "r"); },  dmlc::Error);
+}
+
 }  // namespace xgboost