Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clang 14 fails to compile trezor-crypto #1919

Closed
PiRK opened this issue Jan 12, 2022 · 1 comment
Closed

Clang 14 fails to compile trezor-crypto #1919

PiRK opened this issue Jan 12, 2022 · 1 comment
Labels
bug Something isn't working

Comments

@PiRK
Copy link
Contributor

PiRK commented Jan 12, 2022

Describe the bug

Clang 14 now raises a warning when bitwise operations are performed with boolean operands


...
[ 15%] Building C object trezor-crypto/CMakeFiles/TrezorCrypto.dir/crypto/ed25519-donna/modm-donna-32bit.c.o
/home/pierre/dev/wallet-core/trezor-crypto/crypto/sha3.c:291:7: error: performing pointer subtraction with a null pointer has undefined behavior [-Werror,-Wnull-pointer-subtraction]
                if (IS_ALIGNED_64(msg)) {
                    ^~~~~~~~~~~~~~~~~~
/home/pierre/dev/wallet-core/trezor-crypto/crypto/sha3.c:29:55: note: expanded from macro 'IS_ALIGNED_64'
#define IS_ALIGNED_64(p) (0 == (7 & ((const char*)(p) - (const char*)0)))
                                                      ^ ~~~~~~~~~~~~~~
1 error generated.
make[3]: *** [trezor-crypto/CMakeFiles/TrezorCrypto.dir/build.make:277: trezor-crypto/CMakeFiles/TrezorCrypto.dir/crypto/sha3.c.o] Error 1
make[3]: *** Waiting for unfinished jobs....
/home/pierre/dev/wallet-core/trezor-crypto/crypto/bignum.c:274:10: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
  assert((cond == 1) | (cond == 0));
         ^~~~~~~~~~~~~~~~~~~~~~~~~
                     ||
/usr/include/assert.h:109:11: note: expanded from macro 'assert'
      if (expr)                                                         \
          ^~~~
/home/pierre/dev/wallet-core/trezor-crypto/crypto/bignum.c:274:10: note: cast one or both operands to int to silence this warning
/home/pierre/dev/wallet-core/trezor-crypto/crypto/bignum.c:293:10: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
  assert((cond == 1) | (cond == 0));
         ^~~~~~~~~~~~~~~~~~~~~~~~~
                     ||
/usr/include/assert.h:109:11: note: expanded from macro 'assert'
      if (expr)                                                         \
          ^~~~
/home/pierre/dev/wallet-core/trezor-crypto/crypto/bignum.c:293:10: note: cast one or both operands to int to silence this warning
2 errors generated.

To Reproduce
Install clang 14 and try to build.

Expected behavior
Fix build to work with clang 14

Additional context

See a similar error recently fixed by bitcoin core:
bitcoin/bitcoin#23573

And another one in libsecp256k1:
bitcoin-core/secp256k1#1009

The solution is to cast the operands to int. This has been done upstream (in two steps, because the first commit was the wrong solution):
trezor/trezor-firmware@0c48217
trezor/trezor-firmware@d1d3558

I tried to follow the instructions in trezor-crypto/setup_from_upstream.sh to update trezor-crypto, but it is a bit too complex a solution just to fix a simple compiler warning. If you want to use this solution, you will probably want someone you trust to do the update, because it makes for a very large PR to review.

The simple alternative is too pick just the two relevant commits.

On my side, I will simply revert to clang 13 to work around the issue:

echo "#### Building... ####"
cmake -H. -Bbuild -DCMAKE_BUILD_TYPE=Debug \
    -DCMAKE_C_COMPILER=clang-13 -DCMAKE_CXX_COMPILER=clang++-13
make -Cbuild -j12 tests TrezorCryptoTests
@PiRK PiRK added the bug Something isn't working label Jan 12, 2022
@hewigovens
Copy link
Contributor

Is Clang 14 released already? we will wait until Apple upgrades Xcode:

~ clang --version
Apple clang version 13.0.0 (clang-1300.0.29.30)
Target: arm64-apple-darwin21.2.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

PiRK added a commit to PiRK/wallet-core that referenced this issue Jan 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants