-
Notifications
You must be signed in to change notification settings - Fork 218
Symbol visibility controls and exports (remove-internal approach) #1625
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
Open
ramakrishnap-nv
wants to merge
33
commits into
main
Choose a base branch
from
fix-symbol-visibility-remove-internal
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
20ad11d
refactor(tests): link internal tests against cuopt_static
ramakrishnap-nv de93a4d
refactor(tests): consolidate internal numopt tests into single binary
ramakrishnap-nv e6589c1
fix(tests): make combined internal test binaries link and run
ramakrishnap-nv a59bd8c
style(tests): remove stale comments from empty CMakeLists stubs
ramakrishnap-nv bd27745
refactor(tests): remove empty dual_simplex and socp CMakeLists stubs
ramakrishnap-nv 0e5689a
fix(build): link PSLP directly on cuopt shared target
ramakrishnap-nv 90e5d79
refactor(tests): delete CUOPT_TEST_PROGRAM_MAIN from combined-binary …
ramakrishnap-nv 3e58684
refactor(tests): replace runtime cuSPARSE check with compile-time macro
ramakrishnap-nv c9476b2
fix(tests): require error when mixed precision is unsupported
ramakrishnap-nv 1f52c5e
fix(tests): fix invalid CSR in problem_test create_problem helper
ramakrishnap-nv 545ce45
fix(test): check runtime cuSPARSE version in pdlp_precision_mixed test
ramakrishnap-nv 0effbce
fix(tests): restore RMM setup in ROUTING_UNIT_TEST
ramakrishnap-nv 755ec5d
fix(tests): fix malformed SPDX block comment in internal/main.cu
ramakrishnap-nv 8f23153
fix(build): add OpenMP::OpenMP_CUDA to cuopt_objs PRIVATE deps
ramakrishnap-nv c159896
fix(build): add simde::simde to cuopt_objs PRIVATE deps
ramakrishnap-nv 3a55206
fix(tests): add missing symmetric QCMATRIX entry in fast parser edge …
ramakrishnap-nv 5ec9425
Merge branch 'main' into refactor/test-static-link-for-internal-tests
ramakrishnap-nv 6610d44
check symbols script
arhag23 2ca4e80
Symbol visibility controls and exports
arhag23 b1644c1
Merge remote-tracking branch 'upstream/main' into fix-symbol-visibili…
ramakrishnap-nv 6f3f476
fix(tests): link internal-symbol tests against cuopt_static
ramakrishnap-nv 63402ae
Merge remote-tracking branch 'upstream/main' into fix-symbol-visibili…
ramakrishnap-nv 0db00c3
Merge branch 'main' into fix-symbol-visibility-remove-internal
ramakrishnap-nv 969511b
style(tests): drop stray blank comment line in base_fixture.hpp
ramakrishnap-nv b0ce83e
fix(ci): make check_symbols.sh executable
ramakrishnap-nv d51eaed
Merge branch 'main' into fix-symbol-visibility-remove-internal
ramakrishnap-nv 2980061
refactor(tests): consolidate L0 routing tests into ROUTING_INTERNAL_TEST
ramakrishnap-nv e9d92d7
refactor(tests): combine disabled L1 routing tests into one binary
ramakrishnap-nv eed31e1
style(tests): bump copyright year to 2026 on L1 routing test files
ramakrishnap-nv b5cfd94
fix(tests): keep combined L1 routing binary out of the CI gtest glob
ramakrishnap-nv 1f0dd51
Merge branch 'main' into fix-symbol-visibility-remove-internal
ramakrishnap-nv 97c786b
fix(tests): drop duplicate standalone PDLP/MPS tests, make PDLP_MG_TE…
ramakrishnap-nv f380d96
ci(check-symbols): assert core public C API symbols stay exported
ramakrishnap-nv File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| #!/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. | ||
|
|
||
| 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' | ||
| 'cuopt::detail' | ||
| 'grpc::' | ||
| 'google::protobuf' | ||
| 'tbb::' | ||
| 'absl::' | ||
| 'dejavu::' | ||
| 'papilo::' | ||
| 'boost::' | ||
| ) | ||
|
|
||
| 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 | ||
|
|
||
| # Required public API symbols that must stay exported. This is a small stability | ||
| # anchor (core C API lifecycle entrypoints), not an exhaustive list: without it, | ||
| # a library whose visibility was over-tightened so the public API is entirely | ||
| # hidden would still pass the forbidden-symbol checks above while being unusable. | ||
| # Keep this set minimal and limited to entrypoints guaranteed to exist. | ||
| required_symbols=( | ||
| cuOptReadProblem | ||
| cuOptCreateProblem | ||
| cuOptSolve | ||
| cuOptDestroyProblem | ||
| ) | ||
|
|
||
| exported_funcs="$(readelf --dyn-syms --wide "${LIBRARY}" | awk '$7 != "UND" && $4 == "FUNC" { print $8 }')" | ||
|
|
||
| for sym in "${required_symbols[@]}"; do | ||
| echo "Checking that required symbol '${sym}' is exported..." | ||
| if ! grep -qxF "${sym}" <<< "${exported_funcs}"; then | ||
| echo "ERROR: Required public API symbol '${sym}' is not exported from ${LIBRARY}." | ||
| echo "ERROR: Symbol visibility may be over-restricted and hiding the public API." | ||
| failed=1 | ||
| fi | ||
| done | ||
|
|
||
| if [[ "${failed}" -ne 0 ]]; then | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "No symbol visibility issues found in ${LIBRARY}" | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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 | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| #if defined(__GNUC__) || defined(__clang__) | ||
| #define CUOPT_EXPORT __attribute__((visibility("default"))) | ||
| #else | ||
| #define CUOPT_EXPORT | ||
| #endif | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.