From 0d8221e053595dca54aa928c9d8f3357a44882c9 Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Fri, 24 Apr 2020 00:01:08 +0000 Subject: [PATCH 1/5] Hide C++ symbols in libxgboost.so when building Python wheel --- CMakeLists.txt | 20 ++++++++++++++++---- include/xgboost/c_api.h | 2 +- rabit | 2 +- src/CMakeLists.txt | 6 ++++++ 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 61cde7f20c87..2eb445e884bf 100644 --- 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};$") #-- 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 $ diff --git a/include/xgboost/c_api.h b/include/xgboost/c_api.h index 52b837e26e9d..237858bdce1d 100644 --- 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/rabit b/rabit index 2f7fcff4d770..7e103e75e7c1 160000 --- a/rabit +++ b/rabit @@ -1 +1 @@ -Subproject commit 2f7fcff4d770a3eb4fba6b25ded74b45e196ccd6 +Subproject commit 7e103e75e7c16f12672db9a775015249e2b86680 diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 8329835d6f0f..9a60603168a4 100644 --- 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 + $<$:-Xcompiler=-fvisibility=hidden> + ) + endif (HIDE_CXX_SYMBOLS) + set_target_properties(objxgboost PROPERTIES CUDA_SEPARABLE_COMPILATION OFF) else (USE_CUDA) From 4a24c153b66bba7e819ae7ea72ba9a7d57f02690 Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Fri, 24 Apr 2020 00:11:11 +0000 Subject: [PATCH 2/5] Update Jenkinsfile --- Jenkinsfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Jenkinsfile b/Jenkinsfile index f3733d9cb281..f6f3723d880a 100644 --- 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} manylinux1_x86_64 """ From 07a14a13d4b3befef894c7a0a50064a04cea628a Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Fri, 24 Apr 2020 02:19:30 +0000 Subject: [PATCH 3/5] Add test --- tests/cpp/c_api/test_c_api.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/cpp/c_api/test_c_api.cc b/tests/cpp/c_api/test_c_api.cc index 281231c39f6c..98046105dfe3 100644 --- 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 From bc22002955817f04a7b9607af4f0c2c53c118ed4 Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Thu, 23 Apr 2020 22:59:59 -0700 Subject: [PATCH 4/5] Upgrade rabit --- rabit | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rabit b/rabit index 7e103e75e7c1..4fb34a008db6 160000 --- a/rabit +++ b/rabit @@ -1 +1 @@ -Subproject commit 7e103e75e7c16f12672db9a775015249e2b86680 +Subproject commit 4fb34a008db6437c84d1877635064e09a55c8553 From ebd7ff234ce0a8ebdbd0e3661db51da43746133f Mon Sep 17 00:00:00 2001 From: fis Date: Fri, 24 Apr 2020 22:19:24 +0800 Subject: [PATCH 5/5] Add setup.py option. --- python-package/setup.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/python-package/setup.py b/python-package/setup.py index 8c09f4b3e508..0e135b22216a 100644 --- 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