Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide visibility of non-public symbols #1644

Merged
merged 16 commits into from
Aug 15, 2024
Merged
6 changes: 5 additions & 1 deletion ci/build_wheel_python.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ source rapids-date-string

rapids-generate-version > ./VERSION

cd "${package_dir}"
pushd "${package_dir}"

RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})"
CPP_WHEELHOUSE=$(RAPIDS_PY_WHEEL_NAME="rmm_${RAPIDS_PY_CUDA_SUFFIX}" rapids-download-wheels-from-s3 cpp /tmp/librmm_dist)
Expand All @@ -29,3 +29,7 @@ mkdir -p final_dist
python -m auditwheel repair -w final_dist dist/*

RAPIDS_PY_WHEEL_NAME="${package_name}_${RAPIDS_PY_CUDA_SUFFIX}" rapids-upload-wheels-to-s3 python final_dist

# switch back to the root of the repo and check symbol visibility
popd
ci/check_symbols.sh "$(echo ${package_dir}/final_dist/rmm_*.whl)"
82 changes: 82 additions & 0 deletions ci/check_symbols.sh
vyasr marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
#!/bin/bash
# Copyright (c) 2024, NVIDIA CORPORATION.

set -eEuo pipefail

echo "checking for symbol visibility issues"

WHEEL_FILE=${1}

raise-symbols-found-error() {
local pattern="${1}"

err_msg="ERROR: Found some exported symbols matching the pattern '${pattern}'.

These should be marked with 'hidden' visibility.
See https://cmake.org/cmake/help/latest/prop_tgt/LANG_VISIBILITY_PRESET.html and https://gcc.gnu.org/wiki/Visibility for details.
"

echo ""
echo "${err_msg}"
exit 1
}

WHEEL_EXPORT_DIR="$(mktemp -d)"

unzip \
-d "${WHEEL_EXPORT_DIR}" \
"${WHEEL_FILE}"

dso_files=$(
find \
"${WHEEL_EXPORT_DIR}" \
-type f \
\( -name '*.so' -o -name '*.so.*' \)
vyasr marked this conversation as resolved.
Show resolved Hide resolved
)

for dso_file in ${dso_files}; do
echo ""
echo "checking exported symbols in '${dso_file}'"
symbol_file="./syms.txt"
readelf --symbols --wide "${dso_file}" \
| c++filt \
> "${symbol_file}"

echo "symbol counts by type"
echo " * GLOBAL: $(grep --count -E ' GLOBAL ' < ${symbol_file})"
echo " * WEAK: $(grep --count -E ' WEAK ' < ${symbol_file})"
echo " * LOCAL: $(grep --count -E ' LOCAL ' < ${symbol_file})"

# Explanation for '-v' uses here:
#
# * 'format_error' symbols are intentionally exported, that type of error
# can be thrown across library boundaries. See "Problems with C++ exceptions"
# at https://gcc.gnu.org/wiki/Visibility.
echo "checking for 'fmt::' symbols..."
if grep -E 'fmt\:\:' < "${symbol_file}" \
| grep -v 'format_error'
then
raise-symbols-found-error 'fmt::'
fi

# Explanation for '-v' uses here:
#
# * trivially-destructible objects sometimes get an entry in the symbol table
# for a specialization of `std::_Destroy_aux()` called to destroy them.
# There is one for `spdlog::details::log_msg_buffer like that:
#
# 'std::_Destroy_aux<false>::__destroy<spdlog::details::log_msg_buffer*>'
#
# That should be safe to export.
#
echo "checking for 'spdlog::' symbols..."
if grep -E 'spdlog\:\:' < "${symbol_file}" \
| grep -v 'std\:\:_Destroy_aux'
then
raise-symbols-found-error 'spdlog::'
fi
echo "No symbol visibility issues found"
done

echo ""
echo "No symbol visibility issues found in any DSOs"
9 changes: 8 additions & 1 deletion python/rmm/rmm/_cuda/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# =============================================================================
# Copyright (c) 2022, NVIDIA CORPORATION.
# Copyright (c) 2022-2024, NVIDIA CORPORATION.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
# in compliance with the License. You may obtain a copy of the License at
Expand All @@ -17,3 +17,10 @@ set(linked_libraries rmm::rmm)

rapids_cython_create_modules(SOURCE_FILES "${cython_sources}" LINKED_LIBRARIES "${linked_libraries}"
CXX)

# mark all symbols in these Cython targets "hidden" by default, so they won't collide with symbols
# loaded from other DSOs
foreach(_cython_target IN LISTS RAPIDS_CYTHON_CREATED_TARGETS)
set_target_properties(${_cython_target} PROPERTIES C_VISIBILITY_PRESET hidden
CXX_VISIBILITY_PRESET hidden)
vyasr marked this conversation as resolved.
Show resolved Hide resolved
endforeach()
9 changes: 8 additions & 1 deletion python/rmm/rmm/_lib/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# =============================================================================
# Copyright (c) 2022, NVIDIA CORPORATION.
# Copyright (c) 2022-2024, NVIDIA CORPORATION.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
# in compliance with the License. You may obtain a copy of the License at
Expand All @@ -19,6 +19,13 @@ set(linked_libraries rmm::rmm)
rapids_cython_create_modules(SOURCE_FILES "${cython_sources}" LINKED_LIBRARIES "${linked_libraries}"
CXX)

# mark all symbols in these Cython targets "hidden" by default, so they won't collide with symbols
# loaded from other DSOs
foreach(_cython_target IN LISTS RAPIDS_CYTHON_CREATED_TARGETS)
set_target_properties(${_cython_target} PROPERTIES C_VISIBILITY_PRESET hidden
CXX_VISIBILITY_PRESET hidden)
vyasr marked this conversation as resolved.
Show resolved Hide resolved
endforeach()

add_library(_torch_allocator SHARED _torch_allocator.cpp)
# Want the output to be called _torch_allocator.so
set_target_properties(_torch_allocator PROPERTIES PREFIX "" SUFFIX ".so")
Expand Down
Loading