Skip to content

Commit a7a5117

Browse files
Merge #1359: Fix symbol visibility issues, add test for it
d147876 build: Drop no longer needed `-fvisibility=hidden` compiler option (Hennadii Stepanov) 8ed1d83 ci: Run `tools/symbol-check.py` (Hennadii Stepanov) 41d32ab test: Add `tools/symbol-check.py` (Hennadii Stepanov) 8854805 Introduce `SECP256K1_LOCAL_VAR` macro (Hennadii Stepanov) Pull request description: Closes #1181. [Catches](#1359 (comment)) the actual symbol visibility issue. Replaces #1135. ACKs for top commit: real-or-random: reACK d147876 Tree-SHA512: 4d39f3c4cd32afa2b4418ca79331c72827c76a49a5422afa7c85e60d00a750b91b1b1ab10d91ba578f5817dd938016751168758461fb89de8da56f7d005ae2da
2 parents 13ed6f6 + d147876 commit a7a5117

11 files changed

+144
-8
lines changed

.cirrus.yml

+3
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ env:
2929
BENCH: yes
3030
SECP256K1_BENCH_ITERS: 2
3131
CTIMETESTS: yes
32+
SYMBOL_CHECK: yes
33+
VIRTUAL_ENV: /root/venv
3234
# Compile and run the tests
3335
EXAMPLES: yes
3436

@@ -53,6 +55,7 @@ cat_logs_snippet: &CAT_LOGS
5355

5456
linux_arm64_container_snippet: &LINUX_ARM64_CONTAINER
5557
env_script:
58+
- export PATH="$VIRTUAL_ENV/bin:$PATH"
5659
- env | tee /tmp/env
5760
build_script:
5861
- DOCKER_BUILDKIT=1 docker build --file "ci/linux-debian.Dockerfile" --tag="ci_secp256k1_arm"

.github/workflows/ci.yml

+29
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ env:
4040
BENCH: 'yes'
4141
SECP256K1_BENCH_ITERS: 2
4242
CTIMETESTS: 'yes'
43+
SYMBOL_CHECK: 'yes'
4344
# Compile and run the examples.
4445
EXAMPLES: 'yes'
4546

@@ -365,6 +366,7 @@ jobs:
365366
ASAN_OPTIONS: 'strict_string_checks=1:detect_stack_use_after_return=1:detect_leaks=1'
366367
LSAN_OPTIONS: 'use_unaligned=1'
367368
SECP256K1_TEST_ITERS: 32
369+
SYMBOL_CHECK: 'no'
368370

369371
steps:
370372
- name: Checkout
@@ -415,6 +417,7 @@ jobs:
415417
SECP256K1_TEST_ITERS: 32
416418
ASM: 'no'
417419
WITH_VALGRIND: 'no'
420+
SYMBOL_CHECK: 'no'
418421

419422
steps:
420423
- name: Checkout
@@ -483,6 +486,7 @@ jobs:
483486
CC: 'clang'
484487
HOMEBREW_NO_AUTO_UPDATE: 1
485488
HOMEBREW_NO_INSTALL_CLEANUP: 1
489+
SYMBOL_CHECK: 'no'
486490

487491
strategy:
488492
fail-fast: false
@@ -515,6 +519,12 @@ jobs:
515519
env: ${{ matrix.env_vars }}
516520
run: ./ci/ci.sh
517521

522+
- name: Symbol check
523+
run: |
524+
python3 --version
525+
python3 -m pip install lief
526+
python3 ./tools/symbol-check.py .libs/libsecp256k1.dylib
527+
518528
- name: Print logs
519529
uses: ./.github/actions/print-logs
520530
if: ${{ !cancelled() }}
@@ -530,6 +540,7 @@ jobs:
530540
HOMEBREW_NO_INSTALL_CLEANUP: 1
531541
WITH_VALGRIND: 'no'
532542
CTIMETESTS: 'no'
543+
SYMBOL_CHECK: 'no'
533544

534545
strategy:
535546
fail-fast: false
@@ -557,6 +568,16 @@ jobs:
557568
env: ${{ matrix.env_vars }}
558569
run: ./ci/ci.sh
559570

571+
- name: Symbol check
572+
env:
573+
VIRTUAL_ENV: '${{ github.workspace }}/venv'
574+
run: |
575+
python3 --version
576+
python3 -m venv $VIRTUAL_ENV
577+
export PATH="$VIRTUAL_ENV/bin:$PATH"
578+
python3 -m pip install lief
579+
python3 ./tools/symbol-check.py .libs/libsecp256k1.dylib
580+
560581
- name: Print logs
561582
uses: ./.github/actions/print-logs
562583
if: ${{ !cancelled() }}
@@ -573,6 +594,7 @@ jobs:
573594
configuration:
574595
- job_name: 'x64 (MSVC): Windows (VS 2022, shared)'
575596
cmake_options: '-A x64 -DBUILD_SHARED_LIBS=ON'
597+
symbol_check: 'true'
576598
- job_name: 'x64 (MSVC): Windows (VS 2022, static)'
577599
cmake_options: '-A x64 -DBUILD_SHARED_LIBS=OFF'
578600
- job_name: 'x64 (MSVC): Windows (VS 2022, int128_struct)'
@@ -601,6 +623,13 @@ jobs:
601623
run: |
602624
cd build/bin/RelWithDebInfo && file *tests.exe bench*.exe libsecp256k1-*.dll || true
603625
626+
- name: Symbol check
627+
if: ${{ matrix.configuration.symbol_check }}
628+
run: |
629+
py -3 --version
630+
py -3 -m pip install lief
631+
py -3 .\tools\symbol-check.py build\bin\RelWithDebInfo\libsecp256k1-5.dll
632+
604633
- name: Check
605634
run: |
606635
ctest -C RelWithDebInfo --test-dir build -j ([int]$env:NUMBER_OF_PROCESSORS + 1)

CMakeLists.txt

-2
Original file line numberDiff line numberDiff line change
@@ -271,8 +271,6 @@ else()
271271
try_append_c_flags(-Wundef)
272272
endif()
273273

274-
set(CMAKE_C_VISIBILITY_PRESET hidden)
275-
276274
set(print_msan_notice)
277275
if(SECP256K1_BUILD_CTIME_TESTS)
278276
include(CheckMemorySanitizer)

Makefile.am

+1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ noinst_HEADERS += src/assumptions.h
4747
noinst_HEADERS += src/checkmem.h
4848
noinst_HEADERS += src/testutil.h
4949
noinst_HEADERS += src/util.h
50+
noinst_HEADERS += src/util_local_visibility.h
5051
noinst_HEADERS += src/int128.h
5152
noinst_HEADERS += src/int128_impl.h
5253
noinst_HEADERS += src/int128_native.h

ci/ci.sh

+15-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ print_environment() {
1414
for var in WERROR_CFLAGS MAKEFLAGS BUILD \
1515
ECMULTWINDOW ECMULTGENKB ASM WIDEMUL WITH_VALGRIND EXTRAFLAGS \
1616
EXPERIMENTAL ECDH RECOVERY EXTRAKEYS MUSIG SCHNORRSIG ELLSWIFT \
17-
SECP256K1_TEST_ITERS BENCH SECP256K1_BENCH_ITERS CTIMETESTS\
17+
SECP256K1_TEST_ITERS BENCH SECP256K1_BENCH_ITERS CTIMETESTS SYMBOL_CHECK \
1818
EXAMPLES \
1919
HOST WRAPPER_CMD \
2020
CC CFLAGS CPPFLAGS AR NM \
@@ -107,6 +107,20 @@ file *tests* || true
107107
file bench* || true
108108
file .libs/* || true
109109

110+
if [ "$SYMBOL_CHECK" = "yes" ]
111+
then
112+
python3 --version
113+
case "$HOST" in
114+
*mingw*)
115+
ls -l .libs
116+
python3 ./tools/symbol-check.py .libs/libsecp256k1-5.dll
117+
;;
118+
*)
119+
python3 ./tools/symbol-check.py .libs/libsecp256k1.so
120+
;;
121+
esac
122+
fi
123+
110124
# This tells `make check` to wrap test invocations.
111125
export LOG_COMPILER="$WRAPPER_CMD"
112126

ci/linux-debian.Dockerfile

+5-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ RUN apt-get update && apt-get install --no-install-recommends -y \
3232
gcc-powerpc64le-linux-gnu libc6-dev-ppc64el-cross libc6-dbg:ppc64el \
3333
gcc-mingw-w64-x86-64-win32 wine64 wine \
3434
gcc-mingw-w64-i686-win32 wine32 \
35-
python3 && \
35+
python3-full && \
3636
if ! ( dpkg --print-architecture | grep --quiet "arm64" ) ; then \
3737
apt-get install --no-install-recommends -y \
3838
gcc-aarch64-linux-gnu libc6-dev-arm64-cross libc6-dbg:arm64 ;\
@@ -77,3 +77,7 @@ RUN \
7777
apt-get autoremove -y wget && \
7878
apt-get clean && rm -rf /var/lib/apt/lists/*
7979

80+
ENV VIRTUAL_ENV=/root/venv
81+
RUN python3 -m venv $VIRTUAL_ENV
82+
ENV PATH="$VIRTUAL_ENV/bin:$PATH"
83+
RUN pip install lief

configure.ac

-1
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,6 @@ AC_DEFUN([SECP_TRY_APPEND_DEFAULT_CFLAGS], [
111111
SECP_TRY_APPEND_CFLAGS([-Wcast-align=strict], $1) # GCC >= 8.0
112112
SECP_TRY_APPEND_CFLAGS([-Wconditional-uninitialized], $1) # Clang >= 3.0 only
113113
SECP_TRY_APPEND_CFLAGS([-Wreserved-identifier], $1) # Clang >= 13.0 only
114-
SECP_TRY_APPEND_CFLAGS([-fvisibility=hidden], $1) # GCC >= 4.0
115114
116115
CFLAGS="$SECP_TRY_APPEND_DEFAULT_CFLAGS_saved_CFLAGS"
117116
fi

src/precomputed_ecmult.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ extern "C" {
1313

1414
#include "ecmult.h"
1515
#include "group.h"
16+
#include "util_local_visibility.h"
17+
1618
#if defined(EXHAUSTIVE_TEST_ORDER)
1719
# if EXHAUSTIVE_TEST_ORDER == 7
1820
# define WINDOW_G 3
@@ -27,8 +29,8 @@ static secp256k1_ge_storage secp256k1_pre_g[ECMULT_TABLE_SIZE(WINDOW_G)];
2729
static secp256k1_ge_storage secp256k1_pre_g_128[ECMULT_TABLE_SIZE(WINDOW_G)];
2830
#else /* !defined(EXHAUSTIVE_TEST_ORDER) */
2931
# define WINDOW_G ECMULT_WINDOW_SIZE
30-
extern const secp256k1_ge_storage secp256k1_pre_g[ECMULT_TABLE_SIZE(WINDOW_G)];
31-
extern const secp256k1_ge_storage secp256k1_pre_g_128[ECMULT_TABLE_SIZE(WINDOW_G)];
32+
SECP256K1_LOCAL_VAR const secp256k1_ge_storage secp256k1_pre_g[ECMULT_TABLE_SIZE(WINDOW_G)];
33+
SECP256K1_LOCAL_VAR const secp256k1_ge_storage secp256k1_pre_g_128[ECMULT_TABLE_SIZE(WINDOW_G)];
3234
#endif /* defined(EXHAUSTIVE_TEST_ORDER) */
3335

3436
#ifdef __cplusplus

src/precomputed_ecmult_gen.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,12 @@ extern "C" {
1313

1414
#include "group.h"
1515
#include "ecmult_gen.h"
16+
#include "util_local_visibility.h"
17+
1618
#ifdef EXHAUSTIVE_TEST_ORDER
1719
static secp256k1_ge_storage secp256k1_ecmult_gen_prec_table[COMB_BLOCKS][COMB_POINTS];
1820
#else
19-
extern const secp256k1_ge_storage secp256k1_ecmult_gen_prec_table[COMB_BLOCKS][COMB_POINTS];
21+
SECP256K1_LOCAL_VAR const secp256k1_ge_storage secp256k1_ecmult_gen_prec_table[COMB_BLOCKS][COMB_POINTS];
2022
#endif /* defined(EXHAUSTIVE_TEST_ORDER) */
2123

2224
#ifdef __cplusplus

src/util_local_visibility.h

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#ifndef SECP256K1_LOCAL_VISIBILITY_H
2+
#define SECP256K1_LOCAL_VISIBILITY_H
3+
4+
/* Global variable visibility */
5+
/* See: https://github.com/bitcoin-core/secp256k1/issues/1181 */
6+
#if !defined(_WIN32) && defined(__GNUC__) && (__GNUC__ >= 4)
7+
# define SECP256K1_LOCAL_VAR extern __attribute__ ((visibility ("hidden")))
8+
#else
9+
# define SECP256K1_LOCAL_VAR extern
10+
#endif
11+
12+
#endif /* SECP256K1_LOCAL_VISIBILITY_H */

tools/symbol-check.py

+72
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
#!/usr/bin/env python3
2+
"""Check that a libsecp256k1 shared library exports only expected symbols.
3+
4+
Usage examples:
5+
- When building with Autotools:
6+
./tools/symbol-check.py .libs/libsecp256k1.so
7+
./tools/symbol-check.py .libs/libsecp256k1-<V>.dll
8+
./tools/symbol-check.py .libs/libsecp256k1.dylib
9+
10+
- When building with CMake:
11+
./tools/symbol-check.py build/lib/libsecp256k1.so
12+
./tools/symbol-check.py build/bin/libsecp256k1-<V>.dll
13+
./tools/symbol-check.py build/lib/libsecp256k1.dylib"""
14+
15+
import re
16+
import sys
17+
import subprocess
18+
19+
import lief
20+
21+
22+
class UnexpectedExport(RuntimeError):
23+
pass
24+
25+
26+
def get_exported_exports(library) -> list[str]:
27+
"""Adapter function to get exported symbols based on the library format."""
28+
if library.format == lief.Binary.FORMATS.ELF:
29+
return [symbol.name for symbol in library.exported_symbols]
30+
elif library.format == lief.Binary.FORMATS.PE:
31+
return [entry.name for entry in library.get_export().entries]
32+
elif library.format == lief.Binary.FORMATS.MACHO:
33+
return [symbol.name[1:] for symbol in library.exported_symbols]
34+
raise NotImplementedError(f"Unsupported format: {library.format}")
35+
36+
37+
def grep_expected_symbols() -> list[str]:
38+
"""Guess the list of expected exported symbols from the source code."""
39+
grep_output = subprocess.check_output(
40+
["git", "grep", r"^\s*SECP256K1_API", "--", "include"],
41+
universal_newlines=True,
42+
encoding="utf-8"
43+
)
44+
lines = grep_output.split("\n")
45+
pattern = re.compile(r'\bsecp256k1_\w+')
46+
exported: list[str] = [pattern.findall(line)[-1] for line in lines if line.strip()]
47+
return exported
48+
49+
50+
def check_symbols(library, expected_exports) -> None:
51+
"""Check that the library exports only the expected symbols."""
52+
actual_exports = get_exported_exports(library)
53+
unexpected_exports = set(actual_exports) - set(expected_exports)
54+
if unexpected_exports != set():
55+
raise UnexpectedExport(f"Unexpected exported symbols: {unexpected_exports}")
56+
57+
def main():
58+
if len(sys.argv) != 2:
59+
print(__doc__)
60+
return 1
61+
library = lief.parse(sys.argv[1])
62+
expected_exports = grep_expected_symbols()
63+
try:
64+
check_symbols(library, expected_exports)
65+
except UnexpectedExport as e:
66+
print(f"{sys.argv[0]}: In {sys.argv[1]}: {e}")
67+
return 1
68+
return 0
69+
70+
71+
if __name__ == "__main__":
72+
sys.exit(main())

0 commit comments

Comments
 (0)