Fix symbol visibility issues, add test for it

.cirrus.yml

@@ -27,6 +27,8 @@ env:
   BENCH: yes
   SECP256K1_BENCH_ITERS: 2
   CTIMETESTS: yes
+  SYMBOL_CHECK: yes
+  VIRTUAL_ENV: /root/venv
   # Compile and run the tests
   EXAMPLES: yes
 
@@ -51,6 +53,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

@@ -38,6 +38,7 @@ env:
   BENCH: 'yes'
   SECP256K1_BENCH_ITERS: 2
   CTIMETESTS: 'yes'
+  SYMBOL_CHECK: 'yes'
   # Compile and run the examples.
   EXAMPLES: 'yes'
 
@@ -445,6 +446,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
@@ -501,6 +503,7 @@ jobs:
       SECP256K1_TEST_ITERS: 32
       ASM: 'no'
       WITH_VALGRIND: 'no'
+      SYMBOL_CHECK: 'no'
 
     steps:
       - name: Checkout
@@ -594,6 +597,7 @@ jobs:
       CC: 'clang'
       HOMEBREW_NO_AUTO_UPDATE: 1
       HOMEBREW_NO_INSTALL_CLEANUP: 1
+      SYMBOL_CHECK: 'no'
 
     strategy:
       fail-fast: false
@@ -626,6 +630,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,14 @@ jobs:
         run: |
           cd build/src/RelWithDebInfo && file *tests.exe bench*.exe libsecp256k1-*.dll || true
 
+      - name: Symbol check
+        if: ${{ matrix.configuration.cmake_options != '-A x64 -DBUILD_SHARED_LIBS=OFF' }}
+        run: |
+          choco install --yes --no-progress python3
+          refreshenv
+          python -m pip install lief
+          python .\tools\symbol-check.py build\src\RelWithDebInfo\libsecp256k1-2.dll
+
       - name: Check
         run: |
           ctest -C RelWithDebInfo --test-dir build -j ([int]$env:NUMBER_OF_PROCESSORS + 1)

CMakeLists.txt

@@ -261,8 +261,6 @@ else()
   try_append_c_flags(-Wundef)
 endif()
 
-set(CMAKE_C_VISIBILITY_PRESET hidden)
-
 # Ask CTest to create a "check" target (e.g., make check) as alias for the "test" target.
 # CTEST_TEST_TARGET_ALIAS is not documented but supposed to be user-facing.
 # See: https://gitlab.kitware.com/cmake/cmake/-/commit/816c9d1aa1f2b42d40c81a991b68c96eb12b6d2

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_defs.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 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 \
@@ -105,6 +105,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/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_defs.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_defs.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_defs.h

@@ -0,0 +1,12 @@
+#ifndef SECP256K1_UTIL_DEFS_H
+#define SECP256K1_UTIL_DEFS_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_UTIL_DEFS_H */

tools/symbol-check.py

@@ -0,0 +1,131 @@
+#!/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/src/libsecp256k1.so
+or
+    ./tools/symbol-check.py build/src/libsecp256k1-<V>.dll
+or
+    ./tools/symbol-check.py build/src/libsecp256k1.dylib
+'''
+import os
+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
+
+CHECKS = {
+lief.Binary.FORMATS.ELF: [
+    ('EXPORTED_SYMBOLS', check_ELF_exported_symbols),
+],
+lief.Binary.FORMATS.PE: [
+    ('EXPORTED_FUNCTIONS', check_PE_exported_functions),
+],
+lief.Binary.FORMATS.MACHO: [
+    ('EXPORTED_FUNCTIONS', check_MACHO_exported_functions),
+]
+}
+
+USAGE = """
+symbol-check.py is a script to check that a secp256k1 shared library
+exports only expected symbols.
+
+Usage:
+    ./tools/symbol-check.py <library>
+
+"""
+
+if __name__ == '__main__':
+    if len(sys.argv) != 2:
+        sys.exit(USAGE)
+
+    filename: str = sys.argv[1]
+    if not os.path.isfile(filename):
+        print(f'{filename}: file does not exist')
+        sys.exit(1)
+
+    try:
+        library: lief.Binary = lief.parse(filename)
+        exe_format: lief.Binary.FORMATS = library.format
+        if exe_format == lief.Binary.FORMATS.UNKNOWN:
+            print(f'{filename}: unknown executable format')
+            sys.exit(1)
+
+        obj_type = library.abstract.header.object_type
+        if obj_type != lief.OBJECT_TYPES.LIBRARY:
+            print(f'{filename}: unsupported object type, only LIBRARY type is supported')
+            sys.exit(1)
+
+        expected_exports = grep_exported_symbols()
+        failed: List[str] = []
+        for (name, func) in CHECKS[exe_format]:
+            if not func(library, expected_exports):
+                failed.append(name)
+        if failed:
+            print(f'{filename}: failed {" ".join(failed)}')
+            sys.exit(1)
+    except IOError:
+        print(f'{filename}: cannot open')
+        sys.exit(1)