Skip to content

Commit 0e5cd60

Browse files
committed
Merge bitcoin/bitcoin#30415: contrib: fix check-deps.sh to check for weak symbols
3ae35b4 ci: run check-deps.sh as part of clang-tidy job (Ryan Ofsky) 0aaa129 contrib: fix check-deps.sh when libraries do not import symbols (Ryan Ofsky) 3c99f5a contrib: fix check-deps.sh to check for weak symbols (Ryan Ofsky) 86c80e9 contrib: make check-deps.sh script work with cmake (Ryan Ofsky) Pull request description: Fix check-deps.sh to check for weak symbols so it can detect when an exported template function like is used from another library. Also update the script to work with cmake and configure it to run as part of CI. Problem was reported by hebasto in bitcoin/bitcoin#29015 (comment) ACKs for top commit: TheCharlatan: Re-ACK 3ae35b4 hebasto: ACK 3ae35b4, I have reviewed the code and it looks OK. Also I've tested it locally. Tree-SHA512: c3b58175450b675e6e848549b81bcfe42930ea9bcd693063867ce3f0ac3999c98cd2c3e961f163ff06641e8288f3a4e81530936a296a83d45d33364f27489521
2 parents 118b55c + 3ae35b4 commit 0e5cd60

File tree

3 files changed

+21
-19
lines changed

3 files changed

+21
-19
lines changed

ci/test/00_setup_env_native_tidy.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export NO_DEPENDS=1
1414
export RUN_UNIT_TESTS=false
1515
export RUN_FUNCTIONAL_TESTS=false
1616
export RUN_FUZZ_TESTS=false
17+
export RUN_CHECK_DEPS=true
1718
export RUN_TIDY=true
1819
export GOAL="install"
1920
export BITCOIN_CONFIG="\

ci/test/03_test_script.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,10 @@ if [ -n "$USE_VALGRIND" ]; then
137137
"${BASE_ROOT_DIR}/ci/test/wrap-valgrind.sh"
138138
fi
139139

140+
if [ "$RUN_CHECK_DEPS" = "true" ]; then
141+
"${BASE_ROOT_DIR}/contrib/devtools/check-deps.sh" .
142+
fi
143+
140144
if [ "$RUN_UNIT_TESTS" = "true" ]; then
141145
DIR_UNIT_TEST_DATA="${DIR_UNIT_TEST_DATA}" LD_LIBRARY_PATH="${DEPENDS_DIR}/${HOST}/lib" CTEST_OUTPUT_ON_FAILURE=ON ctest "${MAKEJOBS}"
142146
fi

contrib/devtools/check-deps.sh

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,10 @@ declare -A LIBS
88
LIBS[cli]="libbitcoin_cli.a"
99
LIBS[common]="libbitcoin_common.a"
1010
LIBS[consensus]="libbitcoin_consensus.a"
11-
LIBS[crypto]="crypto/.libs/libbitcoin_crypto_base.a crypto/.libs/libbitcoin_crypto_x86_shani.a crypto/.libs/libbitcoin_crypto_sse41.a crypto/.libs/libbitcoin_crypto_avx2.a"
11+
LIBS[crypto]="crypto/libbitcoin_crypto.a crypto/libbitcoin_crypto_x86_shani.a crypto/libbitcoin_crypto_sse41.a crypto/libbitcoin_crypto_avx2.a"
1212
LIBS[node]="libbitcoin_node.a"
13-
LIBS[util]="libbitcoin_util.a"
14-
LIBS[wallet]="libbitcoin_wallet.a"
15-
LIBS[wallet_tool]="libbitcoin_wallet_tool.a"
13+
LIBS[util]="util/libbitcoin_util.a"
14+
LIBS[wallet]="wallet/libbitcoin_wallet.a"
1615

1716
# Declare allowed dependencies "X Y" where X is allowed to depend on Y. This
1817
# list is taken from doc/design/libraries.md.
@@ -32,32 +31,28 @@ ALLOWED_DEPENDENCIES=(
3231
"wallet common"
3332
"wallet crypto"
3433
"wallet util"
35-
"wallet_tool util"
36-
"wallet_tool wallet"
3734
)
3835

3936
# Add minor dependencies omitted from doc/design/libraries.md to keep the
4037
# dependency diagram simple.
4138
ALLOWED_DEPENDENCIES+=(
4239
"wallet consensus"
43-
"wallet_tool common"
44-
"wallet_tool crypto"
4540
)
4641

4742
# Declare list of known errors that should be suppressed.
4843
declare -A SUPPRESS
4944
# init.cpp file currently calls Berkeley DB sanity check function on startup, so
5045
# there is an undocumented dependency of the node library on the wallet library.
51-
SUPPRESS["libbitcoin_node_a-init.o libbitcoin_wallet_a-bdb.o _ZN6wallet27BerkeleyDatabaseSanityCheckEv"]=1
46+
SUPPRESS["init.cpp.o bdb.cpp.o _ZN6wallet27BerkeleyDatabaseSanityCheckEv"]=1
5247
# init/common.cpp file calls InitError and InitWarning from interface_ui which
5348
# is currently part of the node library. interface_ui should just be part of the
5449
# common library instead, and is moved in
5550
# https://github.com/bitcoin/bitcoin/issues/10102
56-
SUPPRESS["libbitcoin_common_a-common.o libbitcoin_node_a-interface_ui.o _Z11InitWarningRK13bilingual_str"]=1
57-
SUPPRESS["libbitcoin_common_a-common.o libbitcoin_node_a-interface_ui.o _Z9InitErrorRK13bilingual_str"]=1
51+
SUPPRESS["common.cpp.o interface_ui.cpp.o _Z11InitWarningRK13bilingual_str"]=1
52+
SUPPRESS["common.cpp.o interface_ui.cpp.o _Z9InitErrorRK13bilingual_str"]=1
5853
# rpc/external_signer.cpp adds defines node RPC methods but is built as part of the
5954
# common library. It should be moved to the node library instead.
60-
SUPPRESS["libbitcoin_common_a-external_signer.o libbitcoin_node_a-server.o _ZN9CRPCTable13appendCommandERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEPK11CRPCCommand"]=1
55+
SUPPRESS["external_signer.cpp.o server.cpp.o _ZN9CRPCTable13appendCommandERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEPK11CRPCCommand"]=1
6156

6257
usage() {
6358
echo "Usage: $(basename "${BASH_SOURCE[0]}") [BUILD_DIR]"
@@ -67,8 +62,10 @@ usage() {
6762
lib_targets() {
6863
for lib in "${!LIBS[@]}"; do
6964
for lib_path in ${LIBS[$lib]}; do
70-
# shellcheck disable=SC2001
71-
sed 's:/.libs/\(.*\)\.a$:/\1.la:g' <<<"$lib_path"
65+
local name="${lib_path##*/}"
66+
name="${name#lib}"
67+
name="${name%.a}"
68+
echo "$name"
7269
done
7370
done
7471
}
@@ -78,8 +75,8 @@ extract_symbols() {
7875
local temp_dir="$1"
7976
for lib in "${!LIBS[@]}"; do
8077
for lib_path in ${LIBS[$lib]}; do
81-
nm -o "$lib_path" | grep ' T ' | awk '{print $3, $1}' >> "${temp_dir}/${lib}_exports.txt"
82-
nm -o "$lib_path" | grep ' U ' | awk '{print $3, $1}' >> "${temp_dir}/${lib}_imports.txt"
78+
nm -o "$lib_path" | { grep ' T \| W ' || true; } | awk '{print $3, $1}' >> "${temp_dir}/${lib}_exports.txt"
79+
nm -o "$lib_path" | { grep ' U ' || true; } | awk '{print $3, $1}' >> "${temp_dir}/${lib}_imports.txt"
8380
awk '{print $1}' "${temp_dir}/${lib}_exports.txt" | sort -u > "${temp_dir}/${lib}_exported_symbols.txt"
8481
awk '{print $1}' "${temp_dir}/${lib}_imports.txt" | sort -u > "${temp_dir}/${lib}_imported_symbols.txt"
8582
done
@@ -175,7 +172,7 @@ check_not_suppressed() {
175172

176173
# Check arguments.
177174
if [ "$#" = 0 ]; then
178-
BUILD_DIR="$(dirname "${BASH_SOURCE[0]}")/../../src"
175+
BUILD_DIR="$(dirname "${BASH_SOURCE[0]}")/../../build"
179176
elif [ "$#" = 1 ]; then
180177
BUILD_DIR="$1"
181178
else
@@ -190,10 +187,10 @@ if [ ! -f "$BUILD_DIR/Makefile" ]; then
190187
fi
191188

192189
# Build libraries and run checks.
193-
cd "$BUILD_DIR"
194190
# shellcheck disable=SC2046
195-
make -j"$(nproc)" $(lib_targets)
191+
cmake --build "$BUILD_DIR" -j"$(nproc)" -t $(lib_targets)
196192
TEMP_DIR="$(mktemp -d)"
193+
cd "$BUILD_DIR/src"
197194
extract_symbols "$TEMP_DIR"
198195
if check_libraries "$TEMP_DIR"; then
199196
echo "Success! No unexpected dependencies were detected."

0 commit comments

Comments
 (0)