Add Symbol Versioning Support#3096
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3096 +/- ##
==========================================
- Coverage 78.14% 78.12% -0.03%
==========================================
Files 689 689
Lines 123526 123563 +37
Branches 17196 17184 -12
==========================================
+ Hits 96531 96535 +4
- Misses 26078 26111 +33
Partials 917 917 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
[IGNORE] Use different symbol versions for libcrypto and libssl. ie AWS_LC_SSL_1_0 and AWS_LC_CRYPTO_1_0. Otherwise the RPM provides/requires will get confused esp if one changes and not the other [UPDATE] I am wrong here. It works fine to use the same version symbol, rpm namespaces it by soname anyways and that's how glibc etc... do it (all the sub libs have the same version symbol). |
|
|
||
| ```bash | ||
| # Generate new baseline | ||
| ./util/extract_symbols.sh build/crypto/libcrypto-awslc.so.0 \ |
There was a problem hiding this comment.
I don't see an extract_symbols.sh in this PR. Does it need to be added?
| # Version scripts are derived from the symbol registry files and have stable | ||
| # filenames (crypto/libcrypto.map, ssl/libssl.map). The version node names are | ||
| # recorded in the registry (crypto/libcrypto.txt, ssl/libssl.txt). | ||
| if(SET_LIB_SONAME AND BUILD_SHARED_LIBS AND UNIX AND NOT APPLE) |
There was a problem hiding this comment.
SET_LIB_SONAME is also set in the default elseif(NOT ENABLE_PRE_SONAME_BUILD AND BUILD_SHARED_LIBS AND UNIX AND NOT APPLE) branch (L105), so this enables symbol versioning for every standard Linux shared build, and not just ENABLE_DIST_PKG?
That's broader than the PR description/docs state. If dist-pkg-only is intended:
if(ENABLE_DIST_PKG AND BUILD_SHARED_LIBS AND UNIX AND NOT APPLE)Otherwise, we should document that versioning now applies to all SONAME shared builds.
| # GNU ld version script for AWS-LC | ||
| # Auto-generated from symbol registry. Do not edit manually. | ||
| # To add new symbols: util/update_symbol_version.sh <new_version> | ||
| # To regenerate: util/generate_initial_version_scripts.sh | ||
|
|
There was a problem hiding this comment.
# Auto-generated from symbol registry. Do not edit manually.
Should we add a CI step that runs generate_version_script on both registries and git diff --exit-codes the maps to avoid drift?
| awk '$2 == "T" && $3 !~ /@/ && $3 !~ /^_/ { print $3 }' | \ | ||
| wc -l) | ||
|
|
||
| if [[ ${SSL_UNVERSIONED} -eq 0 ]]; then |
There was a problem hiding this comment.
This validates that exported symbols are versioned, but not that all symbols exported before this change are still exported. (Since local: *; will hide any OPENSSL_EXPORT symbol the generator missed, a gap in the registry is a silent symbol drop.)
Are we relying on the abidiff job to catch this problem? Is it running against the versioned .so?
| # 2. All exported symbols have proper version tags | ||
| # 3. No unversioned symbols leak | ||
| # 4. Version definitions are present in shared libraries | ||
| # 5. Libraries can be linked and used correctly |
There was a problem hiding this comment.
The registry is generated from a non-FIPS build (no FIPS define in the target sets), but this applies it to a FIPS build? (FIPS-gated OPENSSL_EXPORT symbols won't be registered and will be localized.)
Can we confirm this job actually exercises those exports rather than just linking?
| REGISTRY_PATH="${CHECK_LIB}/lib${CHECK_LIB}.txt" | ||
|
|
||
| if [[ "${CHECK_LIB}" == "crypto" ]]; then | ||
| READ_FLAGS=(-exclude ssl.h -internal-dirs crypto) |
There was a problem hiding this comment.
The generation scripts use -internal-dirs crypto,third_party/jitterentropy, so the JENT_* symbols are registered but never scanned here and will permanently surface under the "registered but not in headers" warning. Aligning the flags avoids the standing noise:
| READ_FLAGS=(-exclude ssl.h -internal-dirs crypto) | |
| READ_FLAGS=(-exclude ssl.h -internal-dirs crypto,third_party/jitterentropy) |
| -source-root . \ | ||
| -emit-visibility \ | ||
| "${READ_FLAGS[@]}" \ | ||
| -out /tmp/header_syms.txt 2>/dev/null |
There was a problem hiding this comment.
Suppressing stderr here means a broken extraction produces an empty header_syms.txt, which makes the baseline check pass trivially (no unregistered symbols, everything just "missing").
Suggest dropping 2>/dev/null (or teeing to a log) so extraction failures are visible.
| This produces: | ||
| - `build/crypto/libcrypto-awslc.so.0.0.0` - with AWS_LC_1.0 versioned symbols | ||
| - `build/ssl/libssl-awslc.so.0.0.0` - with AWS_LC_1.0 versioned symbols |
There was a problem hiding this comment.
Several stale references:
- The build output is now
.so.1.x.x(ABI_VERSION bumped to 1). - The version scripts are
crypto/libcrypto.map/ssl/libssl.map(L93-94). util/extract_symbols.shdoesn't exist (L145, L242, L257-260 -- the tooling isread_public_symbols/generate_version_script/update_symbol_version.sh).tests/ci/baselines/symbols/*.txthas moved tocrypto/+ssl/.
| # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 OR ISC | ||
|
|
||
| set -exo pipefail |
There was a problem hiding this comment.
Nit:
| set -exo pipefail | |
| set -exuo pipefail |
| name: libcrypto symbol check (incremental) | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 |
There was a problem hiding this comment.
Nit: these new jobs use actions/checkout@v3 while the rest of the repo is now on v5/v6.
4 participants
Summary
.mapfiles) tolibcrypto-awslcandlibssl-awslcshared libraries, automatically enabled when building with-DENABLE_DIST_PKG=ONcrypto/libcrypto.txt,ssl/libssl.txt) as the source of truth, with Go-based tooling (util/read_public_symbols/,util/generate_version_script/) to generate and maintain version scriptsabidiff.yml) and multi-arch dist-pkg tests (Ubuntu 24.04, AL2, AL2023 × gcc/clang × x86_64/aarch64)Details
Build system changes:
cmake/GenerateVersionScript.cmakeprovidesapply_version_script()to attach a.mapfile to a targetCMakeLists.txtprobes for--undefined-versionlinker support (needed for lld compatibility) and setsENABLE_SYMBOL_VERSIONINGwhenSET_LIB_SONAMEandBUILD_SHARED_LIBSare both truecrypto/CMakeLists.txtandssl/CMakeLists.txtcallapply_version_script()on their respective targetsSymbol management tooling:
util/read_public_symbols/main.go— parses headers to extract public API symbols and classify visibility (PUBLIC, PRIVATE, PRIVATE_CXX)util/generate_version_script/main.go— generates.mapfiles from registry.txtfilesutil/generate_initial_version_scripts.sh— bootstraps registries and maps from scratchutil/update_symbol_version.sh— adds new symbols to a new version node with inheritanceCI:
abidiff.yml: incremental and baseline symbol checks for both librarieslinux_x86_omnibus.yml(single job) tolinux-multi-arch-omnibus.yml(full matrix with install/run split).github/docker_images/symbol_check/) and check script (check_symbols.sh)Testing:
tests/ci/run_symbol_version_test.sh— comprehensive validation that versioned symbols are applied, no unversioned symbols leak, libraries link correctly, and version definitions are presenttests/ci/run_dist_pkg_run_tests.sh— runtime tests for installed dist-pkg librariesDocumentation:
docs/SymbolVersioning.md— full developer guide covering version evolution, CI policy, and troubleshootingBUILDING.mdandREADME.mdupdated with distribution packaging sectionTest plan
-DENABLE_DIST_PKG=ON -DBUILD_SHARED_LIBS=ONand verifyreadelf --version-infoshowsAWS_LC_1.0on both.sofilestests/ci/run_symbol_version_test.shlocallynm -D libcrypto-awslc.so | grep ' T ' | grep -v '@'By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.