Skip to content

Comments

build: fix clang -Wshorten-64-to-32 warnings#1801

Open
csjones wants to merge 4 commits intobitcoin-core:masterfrom
21-DOT-DEV:CLANG-WARN-WSHORTEN-64-TO-32
Open

build: fix clang -Wshorten-64-to-32 warnings#1801
csjones wants to merge 4 commits intobitcoin-core:masterfrom
21-DOT-DEV:CLANG-WARN-WSHORTEN-64-TO-32

Conversation

@csjones
Copy link

@csjones csjones commented Jan 15, 2026

Addressing most of the remaining build warnings from issue thread #1617.

I used master branch @ 4721e07 with command:
cmake -B build \
  -DCMAKE_C_COMPILER=clang \
  -DSECP256K1_ENABLE_MODULE_ECDH=ON \
  -DSECP256K1_ENABLE_MODULE_RECOVERY=ON \
  -DSECP256K1_ENABLE_MODULE_EXTRAKEYS=ON \
  -DSECP256K1_ENABLE_MODULE_SCHNORRSIG=ON \
  -DSECP256K1_ENABLE_MODULE_MUSIG=ON \
  -DSECP256K1_ENABLE_MODULE_ELLSWIFT=ON \
  -DSECP256K1_BUILD_TESTS=OFF \
  -DSECP256K1_BUILD_EXHAUSTIVE_TESTS=OFF \
  -DSECP256K1_BUILD_CTIME_TESTS=OFF \
  -DSECP256K1_BUILD_BENCHMARK=OFF \
  -DSECP256K1_BUILD_EXAMPLES=OFF \
  -DCMAKE_C_FLAGS="-Wall -Wextra -Wshorten-64-to-32 -Werror=shorten-64-to-32"
  
cmake --build build --verbose
which produced these 14 errors:
/Users/csjones/Developer/secp256k1/src/modinv64_impl.h:279:39: error: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'uint32_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
  279 |             w = (f * g * (f * f - 2)) & m;
      |               ~ ~~~~~~~~~~~~~~~~~~~~~~^~~
/Users/csjones/Developer/secp256k1/src/modinv64_impl.h:289:19: error: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'uint32_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
  289 |             w = f + (((f + 1) & 4) << 1);
      |               ~ ~~^~~~~~~~~~~~~~~~~~~~~~
/Users/csjones/Developer/secp256k1/src/modinv64_impl.h:290:26: error: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'uint32_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
  290 |             w = (-w * g) & m;
      |               ~ ~~~~~~~~~^~~
/Users/csjones/Developer/secp256k1/src/modinv64_impl.h:370:39: error: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'uint32_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
  370 |             w = (f * g * (f * f - 2)) & m;
      |               ~ ~~~~~~~~~~~~~~~~~~~~~~^~~
/Users/csjones/Developer/secp256k1/src/modinv64_impl.h:380:19: error: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'uint32_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
  380 |             w = f + (((f + 1) & 4) << 1);
      |               ~ ~~^~~~~~~~~~~~~~~~~~~~~~
/Users/csjones/Developer/secp256k1/src/modinv64_impl.h:381:26: error: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'uint32_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
  381 |             w = (-w * g) & m;
      |               ~ ~~~~~~~~~^~~
In file included from /Users/csjones/Developer/secp256k1/src/secp256k1.c:28:
In file included from /Users/csjones/Developer/secp256k1/src/scalar_impl.h:20:
/Users/csjones/Developer/secp256k1/src/scalar_4x64_impl.h:112:42: error: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'int' [-Werror,-Wshorten-64-to-32]
  112 |     overflow = secp256k1_u128_to_u64(&t) + secp256k1_scalar_check_overflow(r);
      |              ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/csjones/Developer/secp256k1/src/scalar_4x64_impl.h:637:10: error: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'uint32_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
  637 |     m6 = c0;
      |        ~ ^~
/Users/csjones/Developer/secp256k1/src/scalar_4x64_impl.h:657:13: error: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'uint32_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
  657 |     p4 = c0 + m6;
      |        ~ ~~~^~~~
/Users/csjones/Developer/secp256k1/src/scalar_4x64_impl.h:677:34: error: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'unsigned int' [-Werror,-Wshorten-64-to-32]
  677 |     secp256k1_scalar_reduce(r, c + secp256k1_scalar_check_overflow(r));
      |     ~~~~~~~~~~~~~~~~~~~~~~~    ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /Users/csjones/Developer/secp256k1/src/secp256k1.c:30:
/Users/csjones/Developer/secp256k1/src/ecmult_impl.h:536:21: error: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'int' [-Werror,-Wshorten-64-to-32]
  536 |     for (i = n_wnaf - 1; i >= 0; i--) {
      |            ~ ~~~~~~~^~~
/Users/csjones/Developer/secp256k1/src/ecmult_impl.h:580:52: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32]
  580 |         for(j = ECMULT_TABLE_SIZE(bucket_window+2) - 1; j > 0; j--) {
      |               ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
In file included from /Users/csjones/Developer/secp256k1/src/secp256k1.c:32:
In file included from /Users/csjones/Developer/secp256k1/src/ecmult_gen_impl.h:14:
/Users/csjones/Developer/secp256k1/src/hash_impl.h:151:52: error: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'uint32_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
  151 |     secp256k1_write_be32(&sizedesc[0], hash->bytes >> 29);
      |     ~~~~~~~~~~~~~~~~~~~~               ~~~~~~~~~~~~^~~~~
/Users/csjones/Developer/secp256k1/src/hash_impl.h:152:52: error: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'uint32_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
  152 |     secp256k1_write_be32(&sizedesc[4], hash->bytes << 3);
      |     ~~~~~~~~~~~~~~~~~~~~               ~~~~~~~~~~~~^~~~
14 errors generated.

I applied most of the suggested fixes from #1617 (comment) and used an explicit cast of (uint32_t) in scalar_4x64_impl.h for the two warnings not mentioned in the original issue thread. The warnings in ecmult_impl.h were skipped.

Only two warnings remain after applying the changes from this PR.
/Users/csjones/Developer/secp256k1/src/ecmult_impl.h:536:21: error: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'int' [-Werror,-Wshorten-64-to-32]
  536 |     for (i = n_wnaf - 1; i >= 0; i--) {
      |            ~ ~~~~~~~^~~
/Users/csjones/Developer/secp256k1/src/ecmult_impl.h:580:52: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32]
  580 |         for(j = ECMULT_TABLE_SIZE(bucket_window+2) - 1; j > 0; j--) {
      |               ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
2 errors generated.

EDIT: Investigating failing CI Fixed.

@real-or-random
Copy link
Contributor

Can you squash the last two commits?

…ion' warnings in hash_impl.h

fix: correct cast placement in hash_impl.h to preserve full 64-bit shift operations
@csjones csjones force-pushed the CLANG-WARN-WSHORTEN-64-TO-32 branch from 98749c3 to 4a84fb7 Compare January 21, 2026 16:52
@csjones
Copy link
Author

csjones commented Jan 21, 2026

@real-or-random just pushed the squashed commit.

Copy link
Contributor

@real-or-random real-or-random left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @sipa for the w change... Is this correct even? Even if it's correct, do we want to keep the smaller uint32_t?

Comment on lines +657 to 658
p4 = (uint32_t)c0 + m6;
VERIFY_CHECK(p4 <= 2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
p4 = (uint32_t)c0 + m6;
VERIFY_CHECK(p4 <= 2);
VERIFY_CHECK(c0 <= 1 && m6 <= 1);
p4 = (uint32_t)c0 + m6;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change made

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up removing m6 <= 1 because it's already verified at line 637. The CI failed with c0 <= 1 and after running the ci script locally a few times, I changed it to c0 <= 2, which is consistent with the original p4 <= 2 check (since p4 = c0 + m6 and m6 <= 1). I also restored the p4 <= 2 check.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I see. I have no idea how the algorithm works. Still, this is a bit surprising to me because that means m6 == 0 always (but then it probably wouldn't be there in the first place) or m6 == 0 iff c0 == 2, and I don't see why the latter should hold.

Perhaps @sipa can provide some insights..

@csjones csjones force-pushed the CLANG-WARN-WSHORTEN-64-TO-32 branch from 9e3dc93 to da9db68 Compare January 22, 2026 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants