Fix symbol visibility issues, add test for it

.cirrus.yml

@@ -29,6 +29,8 @@ env:
   BENCH: yes
   SECP256K1_BENCH_ITERS: 2
   CTIMETESTS: yes
+  SYMBOL_CHECK: yes
+  VIRTUAL_ENV: /root/venv
   # Compile and run the tests
   EXAMPLES: yes
 
@@ -53,6 +55,7 @@ cat_logs_snippet: &CAT_LOGS
 
 linux_arm64_container_snippet: &LINUX_ARM64_CONTAINER
   env_script:
+    - export PATH="$VIRTUAL_ENV/bin:$PATH"
     - env | tee /tmp/env
   build_script:
     - DOCKER_BUILDKIT=1 docker build --file "ci/linux-debian.Dockerfile" --tag="ci_secp256k1_arm"

.github/workflows/ci.yml

@@ -40,6 +40,7 @@ env:
   BENCH: 'yes'
   SECP256K1_BENCH_ITERS: 2
   CTIMETESTS: 'yes'
+  SYMBOL_CHECK: 'yes'
   # Compile and run the examples.
   EXAMPLES: 'yes'
 
@@ -461,6 +462,7 @@ jobs:
       ASAN_OPTIONS: 'strict_string_checks=1:detect_stack_use_after_return=1:detect_leaks=1'
       LSAN_OPTIONS: 'use_unaligned=1'
       SECP256K1_TEST_ITERS: 32
+      SYMBOL_CHECK: 'no'
 
     steps:
       - name: Checkout
@@ -525,6 +527,7 @@ jobs:
       SECP256K1_TEST_ITERS: 32
       ASM: 'no'
       WITH_VALGRIND: 'no'
+      SYMBOL_CHECK: 'no'
 
     steps:
       - name: Checkout
@@ -620,6 +623,7 @@ jobs:
       CC: 'clang'
       HOMEBREW_NO_AUTO_UPDATE: 1
       HOMEBREW_NO_INSTALL_CLEANUP: 1
+      SYMBOL_CHECK: 'no'
 
     strategy:
       fail-fast: false
@@ -652,6 +656,11 @@ jobs:
         env: ${{ matrix.env_vars }}
         run: ./ci/ci.sh
 
+      - name: Symbol check
+        run: |
+          python3 -m pip install lief
+          python3 ./tools/symbol-check.py .libs/libsecp256k1.dylib
+
       - run: cat tests.log || true
         if: ${{ always() }}
       - run: cat noverify_tests.log || true
@@ -681,6 +690,7 @@ jobs:
       HOMEBREW_NO_INSTALL_CLEANUP: 1
       WITH_VALGRIND: 'no'
       CTIMETESTS: 'no'
+      SYMBOL_CHECK: 'no'
 
     strategy:
       fail-fast: false
@@ -708,6 +718,15 @@ jobs:
         env: ${{ matrix.env_vars }}
         run: ./ci/ci.sh
 
+      - name: Symbol check
+        env:
+          VIRTUAL_ENV: '${{ github.workspace }}/venv'
+        run: |
+          python3 -m venv $VIRTUAL_ENV
+          export PATH="$VIRTUAL_ENV/bin:$PATH"
+          python3 -m pip install lief
+          python3 ./tools/symbol-check.py .libs/libsecp256k1.dylib
+
       - run: cat tests.log || true
         if: ${{ always() }}
       - run: cat noverify_tests.log || true
@@ -763,6 +782,12 @@ jobs:
         run: |
           cd build/bin/RelWithDebInfo && file *tests.exe bench*.exe libsecp256k1-*.dll || true
 
+      - name: Symbol check
+        if: ${{ matrix.configuration.cmake_options != '-A x64 -DBUILD_SHARED_LIBS=OFF' }}
+        run: |
+          python -m pip install lief
+          python .\tools\symbol-check.py build\bin\RelWithDebInfo\libsecp256k1-2.dll
+
       - name: Check
         run: |
           ctest -C RelWithDebInfo --test-dir build -j ([int]$env:NUMBER_OF_PROCESSORS + 1)

CMakeLists.txt

@@ -267,8 +267,6 @@ else()
   try_append_c_flags(-Wundef)
 endif()
 
-set(CMAKE_C_VISIBILITY_PRESET hidden)
-
 set(print_msan_notice)
 if(SECP256K1_BUILD_CTIME_TESTS)
   include(CheckMemorySanitizer)

Makefile.am

@@ -47,6 +47,7 @@ noinst_HEADERS += src/assumptions.h
 noinst_HEADERS += src/checkmem.h
 noinst_HEADERS += src/testutil.h
 noinst_HEADERS += src/util.h
+noinst_HEADERS += src/util_local_visibility.h
 noinst_HEADERS += src/int128.h
 noinst_HEADERS += src/int128_impl.h
 noinst_HEADERS += src/int128_native.h

ci/ci.sh

@@ -14,7 +14,7 @@ print_environment() {
     for var in WERROR_CFLAGS MAKEFLAGS BUILD \
             ECMULTWINDOW ECMULTGENKB ASM WIDEMUL WITH_VALGRIND EXTRAFLAGS \
             EXPERIMENTAL ECDH RECOVERY EXTRAKEYS MUSIG SCHNORRSIG ELLSWIFT \
-            SECP256K1_TEST_ITERS BENCH SECP256K1_BENCH_ITERS CTIMETESTS\
+            SECP256K1_TEST_ITERS BENCH SECP256K1_BENCH_ITERS CTIMETESTS SYMBOL_CHECK \
             EXAMPLES \
             HOST WRAPPER_CMD \
             CC CFLAGS CPPFLAGS AR NM \
@@ -107,6 +107,19 @@ file *tests* || true
 file bench* || true
 file .libs/* || true
 
+if [ "$SYMBOL_CHECK" = "yes" ]
+then
+    case "$HOST" in
+        *mingw*)
+            ls -l .libs
+            python3 ./tools/symbol-check.py .libs/libsecp256k1-2.dll
+            ;;
+        *)
+            python3 ./tools/symbol-check.py .libs/libsecp256k1.so
+            ;;
+    esac
+fi
+
 # This tells `make check` to wrap test invocations.
 export LOG_COMPILER="$WRAPPER_CMD"
 

ci/linux-debian.Dockerfile

@@ -32,7 +32,7 @@ RUN apt-get update && apt-get install --no-install-recommends -y \
         gcc-powerpc64le-linux-gnu libc6-dev-ppc64el-cross libc6-dbg:ppc64el \
         gcc-mingw-w64-x86-64-win32 wine64 wine \
         gcc-mingw-w64-i686-win32 wine32 \
-        python3 && \
+        python3-full && \
         if ! ( dpkg --print-architecture | grep --quiet "arm64" ) ; then \
          apt-get install --no-install-recommends -y \
          gcc-aarch64-linux-gnu libc6-dev-arm64-cross libc6-dbg:arm64 ;\
@@ -77,3 +77,7 @@ RUN \
     apt-get autoremove -y wget && \
     apt-get clean && rm -rf /var/lib/apt/lists/*
 
+ENV VIRTUAL_ENV=/root/venv
+RUN python3 -m venv $VIRTUAL_ENV
+ENV PATH="$VIRTUAL_ENV/bin:$PATH"
+RUN pip install lief

configure.ac

@@ -111,7 +111,6 @@ AC_DEFUN([SECP_TRY_APPEND_DEFAULT_CFLAGS], [
       SECP_TRY_APPEND_CFLAGS([-Wcast-align=strict], $1) # GCC >= 8.0
       SECP_TRY_APPEND_CFLAGS([-Wconditional-uninitialized], $1) # Clang >= 3.0 only
       SECP_TRY_APPEND_CFLAGS([-Wreserved-identifier], $1) # Clang >= 13.0 only
-      SECP_TRY_APPEND_CFLAGS([-fvisibility=hidden], $1) # GCC >= 4.0
 
       CFLAGS="$SECP_TRY_APPEND_DEFAULT_CFLAGS_saved_CFLAGS"
     fi

src/modules/musig/session_impl.h

@@ -392,7 +392,7 @@ static void secp256k1_nonce_function_musig(secp256k1_scalar *k, const unsigned c
     memset(&sha, 0, sizeof(sha));
 }
 
-int secp256k1_musig_nonce_gen_internal(const secp256k1_context* ctx, secp256k1_musig_secnonce *secnonce, secp256k1_musig_pubnonce *pubnonce, const unsigned char *input_nonce, const unsigned char *seckey, const secp256k1_pubkey *pubkey, const unsigned char *msg32, const secp256k1_musig_keyagg_cache *keyagg_cache, const unsigned char *extra_input32) {
+static int secp256k1_musig_nonce_gen_internal(const secp256k1_context* ctx, secp256k1_musig_secnonce *secnonce, secp256k1_musig_pubnonce *pubnonce, const unsigned char *input_nonce, const unsigned char *seckey, const secp256k1_pubkey *pubkey, const unsigned char *msg32, const secp256k1_musig_keyagg_cache *keyagg_cache, const unsigned char *extra_input32) {
     secp256k1_scalar k[2];
     secp256k1_ge nonce_pts[2];
     int i;

src/precomputed_ecmult.h

@@ -26,9 +26,10 @@ extern "C" {
 static secp256k1_ge_storage secp256k1_pre_g[ECMULT_TABLE_SIZE(WINDOW_G)];
 static secp256k1_ge_storage secp256k1_pre_g_128[ECMULT_TABLE_SIZE(WINDOW_G)];
 #else /* !defined(EXHAUSTIVE_TEST_ORDER) */
+#include "util_local_visibility.h"
 #    define WINDOW_G ECMULT_WINDOW_SIZE
-extern const secp256k1_ge_storage secp256k1_pre_g[ECMULT_TABLE_SIZE(WINDOW_G)];
-extern const secp256k1_ge_storage secp256k1_pre_g_128[ECMULT_TABLE_SIZE(WINDOW_G)];
+SECP256K1_LOCAL_VAR const secp256k1_ge_storage secp256k1_pre_g[ECMULT_TABLE_SIZE(WINDOW_G)];
+SECP256K1_LOCAL_VAR const secp256k1_ge_storage secp256k1_pre_g_128[ECMULT_TABLE_SIZE(WINDOW_G)];
 #endif /* defined(EXHAUSTIVE_TEST_ORDER) */
 
 #ifdef __cplusplus

src/precomputed_ecmult_gen.h

@@ -16,7 +16,8 @@ extern "C" {
 #ifdef EXHAUSTIVE_TEST_ORDER
 static secp256k1_ge_storage secp256k1_ecmult_gen_prec_table[COMB_BLOCKS][COMB_POINTS];
 #else
-extern const secp256k1_ge_storage secp256k1_ecmult_gen_prec_table[COMB_BLOCKS][COMB_POINTS];
+#include "util_local_visibility.h"
+SECP256K1_LOCAL_VAR const secp256k1_ge_storage secp256k1_ecmult_gen_prec_table[COMB_BLOCKS][COMB_POINTS];
 #endif /* defined(EXHAUSTIVE_TEST_ORDER) */
 
 #ifdef __cplusplus

src/util_local_visibility.h

@@ -0,0 +1,12 @@
+#ifndef SECP256K1_LOCAL_VISIBILITY_H
+#define SECP256K1_LOCAL_VISIBILITY_H
+
+/* Global variable visibility */
+/* See: https://github.com/bitcoin-core/secp256k1/issues/1181 */
+#if !defined(_WIN32) && defined(__GNUC__) && (__GNUC__ >= 4)
+# define SECP256K1_LOCAL_VAR extern __attribute__ ((visibility ("hidden")))
+#else
+# define SECP256K1_LOCAL_VAR extern
+#endif
+
+#endif /* SECP256K1_LOCAL_VISIBILITY_H */

tools/symbol-check.py

@@ -0,0 +1,98 @@
+#!/usr/bin/env python3
+'''
+A script to check that a secp256k1 shared library
+exports only expected symbols.
+
+Example usage:
+
+- when building with Autotools:
+
+    ./tools/symbol-check.py .libs/libsecp256k1.so
+or
+    ./tools/symbol-check.py .libs/libsecp256k1-<V>.dll
+or
+    ./tools/symbol-check.py .libs/libsecp256k1.dylib
+
+- when building with CMake:
+
+    ./tools/symbol-check.py build/lib/libsecp256k1.so
+or
+    ./tools/symbol-check.py build/bin/libsecp256k1-<V>.dll
+or
+    ./tools/symbol-check.py build/lib/libsecp256k1.dylib
+'''
+import re
+import sys
+import subprocess
+from typing import List
+
+import lief
+
+
+def grep_exported_symbols() -> List[str]:
+    grep_output = subprocess.check_output(["git", "grep", "^SECP256K1_API", "--", "include"], universal_newlines=True, encoding="utf8")
+    lines = grep_output.split("\n")
+    exports: List[str] = []
+    pattern = re.compile(r'\bsecp256k1_\w+')
+    for line in lines:
+        if line.strip():
+            function_name = pattern.findall(line)[-1]
+            exports.append(function_name)
+    return exports
+
+
+def check_ELF_exported_symbols(library, expected_exports) -> bool:
+    ok: bool = True
+    elf_lib: lief.ELF.Binary = library.concrete
+
+    for symbol in elf_lib.exported_symbols:
+        name: str = symbol.name
+        if name in expected_exports:
+            continue
+        print(f'{filename}: export of symbol {name} not expected')
+        ok = False
+    return ok
+
+
+def check_PE_exported_functions(library, expected_exports) -> bool:
+    ok: bool = True
+    pe_lib: lief.PE.Binary = library.concrete
+
+    for function in pe_lib.exported_functions:
+        name: str = function.name
+        if name in expected_exports:
+            continue
+        print(f'{filename}: export of function {name} not expected')
+        ok = False
+    return ok
+
+
+def check_MACHO_exported_functions(library, expected_exports) -> bool:
+    ok: bool = True
+    macho_lib: lief.MACHO.Binary = library.concrete
+
+    for function in macho_lib.exported_functions:
+        name: str = function.name[1:]
+        if name in expected_exports:
+            continue
+        print(f'{filename}: export of function {name} not expected')
+        ok = False
+    return ok
+
+
+if __name__ == '__main__':
+    filename: str = sys.argv[1]
+    library: lief.Binary = lief.parse(filename)
+    exe_format: lief.Binary.FORMATS = library.format
+    if exe_format == lief.Binary.FORMATS.ELF:
+        success = check_ELF_exported_symbols(library, grep_exported_symbols())
+    elif exe_format == lief.Binary.FORMATS.PE:
+        success = check_PE_exported_functions(library, grep_exported_symbols())
+    elif exe_format == lief.Binary.FORMATS.MACHO:
+        success = check_MACHO_exported_functions(library, grep_exported_symbols())
+    else:
+        print(f'{filename}: unknown executable format')
+        sys.exit(1)
+
+    if not success:
+        sys.exit(1)