-
Notifications
You must be signed in to change notification settings - Fork 160
fix: blacklist symbols from glibc 2.18 in earlier policies #655
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
Conversation
GLIBC 2.17 on aarch64 does expose some symbols marked GLIBC_2.18 so the policy allowing `GLIBC_2.18` is correct. However, symbols that were really introduced in GLIBC 2.18 (e.g. `__cxa_thread_atexit_impl@@GLIBC_2.18`) should be blacklisted in the manylinux_2_17 policy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes issue #647 by blacklisting GLIBC 2.18 symbols that should not be allowed in manylinux_2_17 and earlier policies. While GLIBC 2.17 on aarch64 legitimately exposes some symbols marked as GLIBC_2.18, certain symbols (like __cxa_thread_atexit_impl@@GLIBC_2.18) were truly introduced in GLIBC 2.18 and should be rejected for manylinux_2_17 compatibility.
- Added blacklist entries for GLIBC 2.18-specific symbols (
__cxa_thread_atexit_impl,__issignaling*,pthread_getattr_default_np,pthread_setattr_default_np) to manylinux_2_5, manylinux_2_12, and manylinux_2_17 policies - Created comprehensive integration test to verify blacklist enforcement with test binaries that use these symbols
- Test validates that wheels with blacklisted symbols are properly detected and can still be repaired for compatible platforms
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/auditwheel/policy/manylinux-policy.json | Added GLIBC 2.18 symbol blacklists for libc.so.6, libm.so.6, and libpthread.so.0 in manylinux_2_5, manylinux_2_12, and manylinux_2_17 policies |
| tests/integration/test_manylinux.py | Added test_glibc_blacklist test for aarch64 to verify blacklisted GLIBC 2.18 symbols are properly detected and wheels can be repaired |
| tests/integration/test_aarch64_glibc_blacklist/tst-tls-atexit-lib.c | Test library using __cxa_thread_atexit_impl to trigger blacklist detection |
| tests/integration/test_aarch64_glibc_blacklist/signaling.cpp | Test executables using issignaling functions (double, float, long double variants) |
| tests/integration/test_aarch64_glibc_blacklist/pthread_default_attr.c | Test executable using pthread_getattr_default_np and pthread_setattr_default_np functions |
| tests/integration/test_aarch64_glibc_blacklist/CMakeLists.txt | CMake build configuration for test binaries |
| tests/integration/test_aarch64_glibc_blacklist/pyproject.toml | Python package configuration for test wheel |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #655 +/- ##
==========================================
+ Coverage 95.09% 95.20% +0.11%
==========================================
Files 22 22
Lines 1814 1814
Branches 340 340
==========================================
+ Hits 1725 1727 +2
+ Misses 49 48 -1
+ Partials 40 39 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Wow, thanks so much for the quick fix! TIL 2.17 has 2.18 symbols :-) |
GLIBC 2.17 on aarch64 does expose some symbols marked GLIBC_2.18 so the policy allowing
GLIBC_2.18is correct. However, symbols that were really introduced in GLIBC 2.18 (e.g.__cxa_thread_atexit_impl@@GLIBC_2.18) should be blacklisted in the manylinux_2_17 policy.fix #647