Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions ci/check_symbols.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
#!/bin/bash
# SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0

set -eEuo pipefail

echo "checking for symbol visibility issues"

LIBRARY="${1}"

echo ""
echo "Checking exported symbols in '${LIBRARY}'"
symbol_file="$(mktemp)"
match_file="$(mktemp)"
trap 'rm -f "${symbol_file}" "${match_file}"' EXIT

# Ignore WEAK and UNIQUE symbols since UNIQUE symbols should be exported and
# WEAK symbols may come from template instantiations.
# Ignore symbols containing "_error" since these are likely exception types
# and should be exported.
Comment on lines +19 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

readelf --dyn-syms symbol visibility naming convention exception types "_error" suffix

💡 Result:

There is no single, universal naming convention in the ELF specification that mandates an "_error" suffix for symbols [1][2]. When you encounter a symbol with an "_error" suffix in the output of readelf --dyn-syms, it is typically a result of project-specific internal naming conventions rather than an ELF-level standard [3][4][5]. Naming Conventions and Exception Types: In large software projects like GNU Binutils or GDB, the "_error" suffix is frequently used as a naming convention to identify functions or variables responsible for handling, reporting, or defining error conditions [3][6][4][5]. For instance, GDB uses an "_ERROR" suffix for various exception types within its common-exceptions.h header to maintain naming consistency, even when the specific symbol is not strictly a fatal error [4][5]. Similarly, internal BFD (Binary File Descriptor) library functions—such as _bfd_x86_elf_link_report_tls_invalid_section_error—use this suffix to clearly denote their purpose in error reporting during the linking process [6]. Visibility and readelf --dyn-syms: The presence of such symbols in the dynamic symbol table (viewable via readelf --dyn-syms) indicates that these symbols are exported or required for dynamic linking [1]. In many cases, developers intentionally mark these internal symbols as hidden (using attributes like ATTRIBUTE_HIDDEN) to prevent them from being visible or preemptable outside of the library itself, even if they appear in the object file [3][6][7][8]. This practice reduces the size of the dynamic symbol table and improves encapsulation, effectively making them private to the DSO (Dynamic Shared Object) while keeping them accessible for internal build-time operations [3][7][8]. In summary, the "_error" suffix is a developer-chosen label for error-handling code, and its visibility in your dynamic symbol table is a separate characteristic managed by ELF symbol visibility attributes (such as STV_HIDDEN, STV_PROTECTED, or STV_DEFAULT) [6][1].

Citations:


🏁 Script executed:

#!/bin/bash
set -eEuo pipefail

# Cleanup on exit.
cleanup() {
    if [[ -d "${TEMP_DIR:-}" ]]; then
        rm -rf "${TEMP_DIR}"
    fi
}
trap cleanup EXIT

LIB_NAME="${1}"

# Ignore symbols containing "_error" since these are likely exception types
# and should be exported.
readelf --dyn-syms --wide "${LIB_NAME}" \
    | c++filt \
    | awk '$0 !~ /_error/' \
    | sed -n 's/.*\[\(.*\)\]/\1/p' \
    | sort \
    | uniq > "${TEMP_DIR}/exported_symbols.txt"

diff -u "${TEMP_DIR}/exported_symbols.txt" "${TEMP_DIR}/expected_symbols.txt"

Repository: NVIDIA/cuopt

Length of output: 185


🏁 Script executed:

rg -n --hidden --glob '!**/.git/**' "_error|check_symbols\.sh|expected_symbols" .

Repository: NVIDIA/cuopt

Length of output: 50368


🏁 Script executed:

#!/bin/bash
set -eEuo pipefail

rg -n --hidden --glob '!**/.git/**' "_error|check_symbols\.sh|expected_symbols" .

Repository: NVIDIA/cuopt

Length of output: 50368


ci/check_symbols.sh:25 ignores too much. awk '$0 !~ /_error/' drops any demangled symbol containing _error, so public API names like get_error_status, get_error_message, or last_error never reach the diff. Narrow this to the specific exception symbols you want to exempt.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/check_symbols.sh` around lines 19 - 20, Narrow the symbol filtering in
ci/check_symbols.sh so it exempts only the intended exception-type symbols
rather than any demangled name containing “_error”. Update the awk condition
used by the symbol comparison flow to match the specific exception symbols,
while allowing public API names such as get_error_status, get_error_message, and
last_error to remain in the diff.


readelf --dyn-syms --wide "${LIBRARY}" \
| awk '$7 != "UND" && $5 != "WEAK" && $5 != "UNIQUE"' \
| c++filt --no-params \
| awk '$0 !~ /_error/' \
> "${symbol_file}"

patterns=(
'cub::'
'thrust::'
'raft::'
'rmm::'
'cuopt::mathematical_optimization::detail'
'cuopt::routing::detail'
'grpc::'
'google::protobuf'
'tbb::'
'absl::'
'dejavu::'
'papilo::'
'boost::'
'pslp_'
)

failed=0

for pattern in "${patterns[@]}"; do
echo "Checking for '${pattern}' symbols..."

awk -v pattern="${pattern}" '
BEGIN { has_trailing_scope = (substr(pattern, length(pattern) - 1) == "::") }
$1 ~ /^[0-9]+:/ {
symbol = ""
for (i = 8; i <= NF; ++i) {
symbol = symbol (i == 8 ? "" : " ") $i
}

sub(/<.*/, "", symbol)
sub(/^.*[[:space:]](for|to)[[:space:]]+/, "", symbol)

if (has_trailing_scope) {
matched = (index(symbol, pattern) == 1)
} else {
matched = (symbol == pattern || index(symbol, pattern "::") == 1)
}

if (matched) { print }
}
' "${symbol_file}" > "${match_file}"

matches=$(awk 'END { print NR }' "${match_file}")
if [[ "${matches}" -ne 0 ]]; then
sed -n '1,20p' "${match_file}"
echo "ERROR: Found exported symbols in ${LIBRARY} matching the pattern ${pattern}."
echo "ERROR: Total matching symbols: ${matches}"
failed=1
fi
done

if [[ "${failed}" -ne 0 ]]; then
exit 1
fi

echo "No symbol visibility issues found in ${LIBRARY}"
4 changes: 3 additions & 1 deletion conda/recipes/libcuopt/recipe.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION.
# SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0
schema_version: 1

Expand Down Expand Up @@ -107,6 +107,7 @@ outputs:
script:
content: |
cmake --install cpp/build
./ci/check_symbols.sh cpp/build/libcuopt.so
dynamic_linking:
overlinking_behavior: "error"
prefix_detection:
Expand All @@ -118,6 +119,7 @@ outputs:
build:
- cmake ${{ cmake_version }}
- ${{ stdlib("c") }}
- binutils
host:
- libboost-devel
- cuda-version =${{ cuda_version }}
Expand Down
150 changes: 121 additions & 29 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -481,58 +481,48 @@ if (NOT SKIP_GRPC_BUILD)
# at runtime with "undefined symbol: absl::…::Mutex::Dtor".
set_property(SOURCE ${GRPC_INFRA_FILES} DIRECTORY ${CMAKE_SOURCE_DIR}
APPEND PROPERTY COMPILE_OPTIONS "-DNDEBUG")
set_property(SOURCE ${PROTO_SRCS} ${GRPC_PROTO_SRCS} ${GRPC_SERVICE_SRCS} ${DATA_PROTO_SRCS} DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
APPEND PROPERTY COMPILE_OPTIONS "$<$<COMPILE_LANGUAGE:CXX>:-fvisibility=default>")
endif (NOT SKIP_GRPC_BUILD)

add_library(cuopt SHARED
add_library(cuopt_objs OBJECT
${CUOPT_SRC_FILES}
)

set_target_properties(cuopt
PROPERTIES BUILD_RPATH "\$ORIGIN"
INSTALL_RPATH "\$ORIGIN"
INTERFACE_POSITION_INDEPENDENT_CODE ON
set_target_properties(cuopt_objs
PROPERTIES POSITION_INDEPENDENT_CODE ON
CXX_VISIBILITY_PRESET hidden
CUDA_VISIBILITY_PRESET hidden
VISIBILITY_INLINES_HIDDEN ON
CXX_SCAN_FOR_MODULES OFF
)

target_compile_definitions(cuopt
target_compile_definitions(cuopt_objs
PUBLIC "CUOPT_LOG_ACTIVE_LEVEL=RAPIDS_LOGGER_LOG_LEVEL_${LIBCUOPT_LOGGING_LEVEL}"
PUBLIC CUSPARSE_ENABLE_EXPERIMENTAL_API
)

target_compile_options(cuopt
target_compile_options(cuopt_objs
PRIVATE "$<$<COMPILE_LANGUAGE:CXX>:${CUOPT_CXX_FLAGS}>"
"$<$<COMPILE_LANGUAGE:CUDA>:${CUOPT_CUDA_FLAGS}>"
)

if (WRITE_FATBIN)
file(WRITE "${CUOPT_BINARY_DIR}/fatbin.ld"
[=[
SECTIONS
{
.nvFatBinSegment : { *(.nvFatBinSegment) }
.nv_fatbin : { *(.nv_fatbin) }
}
]=])
target_link_options(cuopt PRIVATE "${CUOPT_BINARY_DIR}/fatbin.ld")
endif ()

add_library(cuopt::cuopt ALIAS cuopt)
# ##################################################################################################
# - include paths ---------------------------------------------------------------------------------
message(STATUS "target include directories CUDSS_INCLUDES = ${CUDSS_INCLUDE}")

# Adding Papilo as a system include messes up clang's include resolution if papilo is already installed as a conda package
target_include_directories(cuopt PRIVATE
target_include_directories(cuopt_objs PRIVATE
"${papilo_SOURCE_DIR}/src"
"${papilo_BINARY_DIR}"
)

target_include_directories(cuopt SYSTEM PRIVATE
target_include_directories(cuopt_objs SYSTEM PRIVATE
"${pslp_SOURCE_DIR}/include"
"${dejavu_SOURCE_DIR}"
)

target_include_directories(cuopt
target_include_directories(cuopt_objs
PRIVATE
"${CMAKE_CURRENT_SOURCE_DIR}/../thirdparty"
"${CMAKE_CURRENT_SOURCE_DIR}/src"
Expand All @@ -547,13 +537,12 @@ target_include_directories(cuopt
PUBLIC
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
"$<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/include>"
INTERFACE
"$<INSTALL_INTERFACE:include>"
)

# Link PSLP by file to avoid export dependency tracking
target_link_libraries(cuopt PRIVATE $<TARGET_FILE:PSLP>)
add_dependencies(cuopt PSLP)
target_link_libraries(cuopt_objs PRIVATE $<TARGET_FILE:PSLP>)
add_dependencies(cuopt_objs PSLP)

# ##################################################################################################
# - link libraries --------------------------------------------------------------------------------
Expand All @@ -568,7 +557,7 @@ list(PREPEND CUOPT_PRIVATE_CUDA_LIBS CUDA::cublasLt)

# Pass CUDSS_MT_LIB_FILE_NAME as a compile definition
get_filename_component(CUDSS_MT_LIB_FILE_NAME "${CUDSS_MT_LIB_FILE}" NAME)
target_compile_definitions(cuopt PRIVATE CUDSS_MT_LIB_FILE_NAME="${CUDSS_MT_LIB_FILE_NAME}")
target_compile_definitions(cuopt_objs PRIVATE CUDSS_MT_LIB_FILE_NAME="${CUDSS_MT_LIB_FILE_NAME}")

execute_process(
COMMAND git rev-parse --short HEAD
Expand All @@ -587,13 +576,101 @@ configure_file(
)

# Add the generated include directory
target_include_directories(cuopt PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/include)
target_include_directories(cuopt_objs PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/include)

list(JOIN CMAKE_CUDA_ARCHITECTURES "," JOINED_CUDA_ARCHITECTURES)
target_compile_definitions(cuopt PUBLIC
target_compile_definitions(cuopt_objs PUBLIC
CUOPT_CUDA_ARCHITECTURES="${JOINED_CUDA_ARCHITECTURES}"
CUOPT_CPU_ARCHITECTURE="${CMAKE_SYSTEM_PROCESSOR}")

target_link_libraries(cuopt_objs
PUBLIC
CUDA::cublas
CUDA::cusparse
rmm::rmm
rapids_logger::rapids_logger
CCCL::CCCL
raft::raft
${CUDSS_LIB_FILE}
PRIVATE
${CUOPT_PRIVATE_CUDA_LIBS}
$<$<BOOL:${CUOPT_ENABLE_GRPC}>:protobuf::libprotobuf>
$<$<BOOL:${CUOPT_ENABLE_GRPC}>:gRPC::grpc++>
)
Comment on lines +582 to +599

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Triplicated build config across cuopt_objs/cuopt_static/cuopt risks drift.

Because OBJECT-library usage requirements don't propagate through $<TARGET_OBJECTS:cuopt_objs>, the same target_compile_definitions/target_link_libraries blocks (CUOPT_CUDA_ARCHITECTURES, CUOPT_CPU_ARCHITECTURE, CUOPT_LOG_ACTIVE_LEVEL, CUSPARSE_ENABLE_EXPERIMENTAL_API, CUDA::cublas/cusparse, rmm, raft, CCCL, CUDSS, CUOPT_PRIVATE_CUDA_LIBS, protobuf/grpc++) are copy-pasted across cuopt_objs (582-599), cuopt_static (604-639), and later cuopt itself. A future dependency/define change applied to only one target will silently desync the other two, potentially causing subtle ABI/link mismatches between the static-test build and the shipped shared library.

Consider hoisting these into reusable CMake list/variable(s) (e.g. CUOPT_PUBLIC_LINK_LIBS, CUOPT_COMMON_COMPILE_DEFS) and reusing them in all three target_link_libraries/target_compile_definitions calls, or wrap the common setup in a small function()/macro applied to each target.

♻️ Sketch of a shared-variable approach
+set(CUOPT_COMMON_COMPILE_DEFS
+    "CUOPT_LOG_ACTIVE_LEVEL=RAPIDS_LOGGER_LOG_LEVEL_${LIBCUOPT_LOGGING_LEVEL}"
+    CUSPARSE_ENABLE_EXPERIMENTAL_API
+    "CUOPT_CUDA_ARCHITECTURES=${JOINED_CUDA_ARCHITECTURES}"
+    "CUOPT_CPU_ARCHITECTURE=${CMAKE_SYSTEM_PROCESSOR}")
+set(CUOPT_PUBLIC_LINK_LIBS CUDA::cublas CUDA::cusparse rmm::rmm
+    rapids_logger::rapids_logger CCCL::CCCL raft::raft ${CUDSS_LIB_FILE})

 target_compile_definitions(cuopt_objs
-  PUBLIC "CUOPT_LOG_ACTIVE_LEVEL=RAPIDS_LOGGER_LOG_LEVEL_${LIBCUOPT_LOGGING_LEVEL}"
-  PUBLIC CUSPARSE_ENABLE_EXPERIMENTAL_API
+  PUBLIC ${CUOPT_COMMON_COMPILE_DEFS}
 )
 ...
 target_link_libraries(cuopt_static
-        PUBLIC
-        CUDA::cublas
-        CUDA::cusparse
-        rmm::rmm
-        rapids_logger::rapids_logger
-        CCCL::CCCL
-        raft::raft
-        ${CUDSS_LIB_FILE}
+        PUBLIC ${CUOPT_PUBLIC_LINK_LIBS}
         PRIVATE
         ${CUOPT_PRIVATE_CUDA_LIBS}
         ...
 )

Also applies to: 604-639

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/CMakeLists.txt` around lines 582 - 599, The build configuration for
cuopt_objs, cuopt_static, and cuopt is duplicated and can drift. Extract the
shared compile definitions and link libraries—including CUDA architecture
definitions, logging/CUSPARSE definitions, CUDA, RMM, RAFT, CCCL, CUDSS, private
CUDA libraries, and conditional protobuf/gRPC dependencies—into reusable
variables or a helper function, then apply them consistently to all three
targets while preserving their existing public/private visibility.


if (BUILD_TESTS)
add_library(cuopt_static STATIC $<TARGET_OBJECTS:cuopt_objs>)

target_link_libraries(cuopt_static
PUBLIC
CUDA::cublas
CUDA::cusparse
rmm::rmm
rapids_logger::rapids_logger
CCCL::CCCL
raft::raft
${CUDSS_LIB_FILE}
PRIVATE
${CUOPT_PRIVATE_CUDA_LIBS}
$<$<BOOL:${CUOPT_ENABLE_GRPC}>:protobuf::libprotobuf>
$<$<BOOL:${CUOPT_ENABLE_GRPC}>:gRPC::grpc++>
)

target_include_directories(cuopt_static
PUBLIC
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
"$<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/include>"
"$<INSTALL_INTERFACE:include>"
)

target_compile_options(cuopt_static
PRIVATE "$<$<COMPILE_LANGUAGE:CXX>:${CUOPT_CXX_FLAGS}>"
"$<$<COMPILE_LANGUAGE:CUDA>:${CUOPT_CUDA_FLAGS}>"
)

target_compile_definitions(cuopt_static PUBLIC
CUOPT_CUDA_ARCHITECTURES="${JOINED_CUDA_ARCHITECTURES}"
CUOPT_CPU_ARCHITECTURE="${CMAKE_SYSTEM_PROCESSOR}"
"CUOPT_LOG_ACTIVE_LEVEL=RAPIDS_LOGGER_LOG_LEVEL_${LIBCUOPT_LOGGING_LEVEL}"
CUSPARSE_ENABLE_EXPERIMENTAL_API
)

target_link_libraries(cuopt_static PRIVATE $<TARGET_FILE:PSLP>)
add_dependencies(cuopt_static PSLP)
endif ()

add_library(cuopt SHARED $<TARGET_OBJECTS:cuopt_objs>)
add_library(cuopt::cuopt ALIAS cuopt)

set_target_properties(cuopt
PROPERTIES BUILD_RPATH "\$ORIGIN"
INSTALL_RPATH "\$ORIGIN"
INTERFACE_POSITION_INDEPENDENT_CODE ON
CXX_SCAN_FOR_MODULES OFF
LINKER_LANGUAGE CUDA
)

target_include_directories(cuopt
PUBLIC
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
"$<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/include>"
"$<INSTALL_INTERFACE:include>"
)

target_compile_options(cuopt
PRIVATE "$<$<COMPILE_LANGUAGE:CXX>:${CUOPT_CXX_FLAGS}>"
"$<$<COMPILE_LANGUAGE:CUDA>:${CUOPT_CUDA_FLAGS}>"
)

target_compile_definitions(cuopt PUBLIC
CUOPT_CUDA_ARCHITECTURES="${JOINED_CUDA_ARCHITECTURES}"
CUOPT_CPU_ARCHITECTURE="${CMAKE_SYSTEM_PROCESSOR}"
"CUOPT_LOG_ACTIVE_LEVEL=RAPIDS_LOGGER_LOG_LEVEL_${LIBCUOPT_LOGGING_LEVEL}"
CUSPARSE_ENABLE_EXPERIMENTAL_API
)

target_link_options(cuopt PRIVATE "LINKER:--exclude-libs=ALL")

target_link_libraries(cuopt
PUBLIC
CUDA::cublas
Expand All @@ -609,6 +686,21 @@ target_link_libraries(cuopt
$<$<BOOL:${CUOPT_ENABLE_GRPC}>:gRPC::grpc++>
)

target_link_libraries(cuopt PRIVATE $<TARGET_FILE:PSLP>)
add_dependencies(cuopt PSLP)

if (WRITE_FATBIN)
file(WRITE "${CUOPT_BINARY_DIR}/fatbin.ld"
[=[
SECTIONS
{
.nvFatBinSegment : { *(.nvFatBinSegment) }
.nv_fatbin : { *(.nv_fatbin) }
}
]=])
target_link_options(cuopt PRIVATE "${CUOPT_BINARY_DIR}/fatbin.ld")
endif ()


# ##################################################################################################
# - generate tests --------------------------------------------------------------------------------
Expand Down
5 changes: 3 additions & 2 deletions cpp/include/cuopt/error.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@
/* clang-format on */
#pragma once

#include <cuopt/export.hpp>
#include "cuopt/mathematical_optimization/constants.h"

#include <stdarg.h>

#include <raft/core/error.hpp>

namespace cuopt {
namespace CUOPT_EXPORT cuopt {

/**
* @brief Indicates different type of exceptions which cuOpt might throw
Expand Down Expand Up @@ -168,4 +169,4 @@ void execute_cuopt_fail(Args... args)
throw cuopt::logic_error(msg, error_type_t::RuntimeError);
}

} // namespace cuopt
} // namespace CUOPT_EXPORT cuopt
14 changes: 14 additions & 0 deletions cpp/include/cuopt/export.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/* clang-format off */
/*
* SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/
/* clang-format on */

#pragma once

#if defined(__GNUC__) || defined(__clang__)
#define CUOPT_EXPORT __attribute__((visibility("default")))
#else
#define CUOPT_EXPORT
#endif
7 changes: 5 additions & 2 deletions cpp/include/cuopt/grpc/cython_grpc_client.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#pragma once

#include <cuopt/export.hpp>
#include <cuopt/mathematical_optimization/utilities/cython_solve.hpp>

#include <cstddef>
Expand All @@ -22,7 +23,8 @@ class data_model_view_t;
} // namespace io
} // namespace cuopt::mathematical_optimization

namespace cuopt::cython {
namespace cuopt {
namespace CUOPT_EXPORT cython {

/** Mirrors cuopt::mathematical_optimization::job_status_t for the Python bindings. */
enum class grpc_job_status_t : int {
Expand Down Expand Up @@ -166,4 +168,5 @@ class grpc_python_client_t {
std::unique_ptr<impl_t> impl_;
};

} // namespace cuopt::cython
} // namespace CUOPT_EXPORT cython
} // namespace cuopt
Loading
Loading