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

Crash when compiled as a universal binary (x86_64 + arm64) for macOS #118

Open
felipefarinon opened this issue May 31, 2024 · 18 comments
Open

Comments

@felipefarinon
Copy link
Contributor

felipefarinon commented May 31, 2024

When we compile cryptopp as a universal binary (x86_64 + arm64) using this CMake script, running the cryptest validation test fails. It also crashes my application that relies on this, even though compilation succeeds.

The same universal binary compiled using the GNUMakeFile provided by cryptopp passes all validation tests without issue. To compile using the GNUMakeFile you can enter: CXXFLAGS="-DNDEBUG -g2 -O3 -stdlib=libc++ -arch x86_64 -arch arm64" make -j 4 on a fresh cryptopp install.

I've tried to further reduce the problem, and finally the problem isn't with the universal binary per se, but rather with how the CMake script handles the CRYPTOPP_DISABLE_ASM flag. If you compile with cmake -G Ninja .. -DCRYPTOPP_DISABLE_ASM=1 you'll have the same issue.

Steps

  1. Download this repo.
  2. Run:
mkdir build
cd build
cmake -G Ninja .. -DCMAKE_OSX_ARCHITECTURES="x86_64;arm64"
...
-- [cryptopp] CMake version 3.29.3
-- [cryptopp] System Darwin
-- [cryptopp] Processor x86_64
-- [cryptopp]     CMAKE_OSX_ARCHITECTURES : x86_64;arm64
-- [cryptopp] CMAKE_HOST_SYSTEM_PROCESSOR : x86_64
-- [cryptopp]      CMAKE_SYSTEM_PROCESSOR : x86_64
-- [cryptopp] Target architecture detected as: x86_64;arm64 -> CRYPTOPP_AMD64
-- [cryptopp] Target architecture detected as: x86_64;arm64 -> CRYPTOPP_ARMV8
-- [cryptopp] Performing Test CRYPTOPP_X86_WAQ
-- [cryptopp] Performing Test CRYPTOPP_X86_WAQ - Failed
-- [cryptopp] Performing Test CRYPTOPP_HAVE_SSE2
-- [cryptopp] Performing Test CRYPTOPP_HAVE_SSE2 - Failed
-- [cryptopp] Performing Test CRYPTOPP_ARM_NEON_HEADER
-- [cryptopp] Performing Test CRYPTOPP_ARM_NEON_HEADER - Failed
-- [cryptopp] Performing Test CRYPTOPP_ARM_ACLE_HEADER
-- [cryptopp] Performing Test CRYPTOPP_ARM_ACLE_HEADER - Failed
-- [cryptopp] Performing Test CRYPTOPP_ARM_SIMD
-- [cryptopp] Performing Test CRYPTOPP_ARM_SIMD - Failed
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE
-- [cryptopp] Generating cmake package config files
-- [cryptopp] Generating pkgconfig files
-- [cryptopp] Platform: x86_64
-- [cryptopp] Compiler definitions:  CRYPTOPP_DISABLE_ASM=1;CRYPTOPP_DISABLE_ASM=1
-- [cryptopp] Compiler options:   
-- [cryptopp] Build type: Release
  1. Compilation succeeds, but running validation tests fail:
cd cryptopp
./cryptest v
Using seed: 1717168127      

Testing Settings...

passed:  Your machine is little endian.
passed:  Your machine is 64-bit.
passed:  sizeof(byte) == 1
passed:  sizeof(word16) == 2
passed:  sizeof(word32) == 4
passed:  sizeof(word64) == 8
passed:  sizeof(word128) == 16
passed:  sizeof(hword) == 4, sizeof(word) == 8, sizeof(dword) == 16
passed:  cacheLineSize == 64
hasSSE2 == 0, hasSSSE3 == 0, hasSSE4.1 == 0, hasSSE4.2 == 0, hasAVX == 0, hasAVX2 == 0, hasAESNI == 0, hasCLMUL == 0, hasRDRAND == 0, hasRDSEED == 0, hasSHA == 0, isP4 == 0
...
Testing RandomPool generator...

passed:  10240 generated bytes compressed to 10245 bytes by DEFLATE
passed:  IncorporateEntropy with 128 bytes
passed:  GenerateWord32 and Crop
passed:  DiscardBytes with 1024 bytes
zsh: segmentation fault  ./cryptest v

OS: macOS 12.7.5
CPU: 1.6 GHz Dual-Core Intel Core i5
Compiler: Apple clang version 14.0.0 (clang-1400.0.29.202)
Version: master, most recent commit on Mar 11, 2024

@abdes
Copy link
Owner

abdes commented May 31, 2024 via email

@felipefarinon
Copy link
Contributor Author

Sure, give me a few minutes.

@felipefarinon
Copy link
Contributor Author

I diff'ed the files that were compiled with GNUMakeFile and cmake:

GNUMakeFile: +adhoc.cpp
cmake: +sse_simd.cpp

I saw there's an explicit line on the cmake script that adds sse_simd.cpp, so I tried removing it, leaving the only file difference the adhoc.cpp. The compilation succeeded, but the crash still happens. Now checking the compilation flags and adhoc.cpp.

@felipefarinon
Copy link
Contributor Author

adhoc.cpp is part of the test project, not of the main library. I included it in the cryptest compilation for CMake, but to no effect. Now checking the compilation flags.

@felipefarinon
Copy link
Contributor Author

felipefarinon commented May 31, 2024

I've sampled the compilation for one file below, since the other files seem to follow the same pattern. Anyway, the complete compilation log is attached.

GNUMakeFile.txt
cmake.txt

GNUMakeFile sha.cpp:

c++ -DCRYPTOPP_DISABLE_ASM -fPIC -pthread -fno-common -pipe -DNDEBUG -g2 -O3 -stdlib=libc++ -arch x86_64 -arch arm64 -c sha.cpp

CMake sha.cpp:

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++  -DCRYPTOPP_DISABLE_ASM=1 -I/Users/felipefarinon/downloads/cryptopp-cmake-master/build/cryptopp -I/Users/felipefarinon/downloads/cryptopp-cmake-master/build -O3 -DNDEBUG -arch x86_64 -arch arm64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.1.sdk -mmacosx-version-min=12.7 -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -MD -MT cryptopp/CMakeFiles/cryptopp.dir/sha.cpp.o -MF CMakeFiles/cryptopp.dir/sha.cpp.o.d -o CMakeFiles/cryptopp.dir/sha.cpp.o -c /Users/felipefarinon/downloads/cryptopp-cmake-master/build/cryptopp/sha.cpp

There's a difference in the visibility settings and the way the DISABLED_ASM is set. Also, cmake creates the library with ar while GNUMakeFile uses libtool, but nothing stands out yet.

@abdes
Copy link
Owner

abdes commented May 31, 2024 via email

@felipefarinon
Copy link
Contributor Author

felipefarinon commented May 31, 2024

The call stack for cryptest:

Testing RandomPool generator...

passed:  10240 generated bytes compressed to 10245 bytes by DEFLATE
passed:  IncorporateEntropy with 128 bytes
passed:  GenerateWord32 and Crop
passed:  DiscardBytes with 1024 bytes
Process 24581 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x20)
    frame #0: 0x000000010029d0a9 cryptest`CryptoPP::IteratedHashBase<unsigned int, CryptoPP::HashTransformation>::TruncatedFinal(unsigned char*, unsigned long) + 1001
cryptest`CryptoPP::IteratedHashBase<unsigned int, CryptoPP::HashTransformation>::TruncatedFinal:
->  0x10029d0a9 <+1001>: movdqu (%rbx,%rsi,4), %xmm0
    0x10029d0ae <+1006>: movdqu 0x10(%rbx,%rsi,4), %xmm1
    0x10029d0b4 <+1012>: movdqa 0x1cace4(%rip), %xmm2     ; typeinfo name for CryptoPP::InvertibleRabinFunction* + 46
    0x10029d0bc <+1020>: pshufb %xmm2, %xmm0
Target 0: (cryptest) stopped.
(lldb) backtrace
error: 'backtrace' is not a valid command.
(lldb) thread backtrace
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x20)
  * frame #0: 0x000000010029d0a9 cryptest`CryptoPP::IteratedHashBase<unsigned int, CryptoPP::HashTransformation>::TruncatedFinal(unsigned char*, unsigned long) + 1001
    frame #1: 0x00000001002f948d cryptest`CryptoPP::RandomPool::IncorporateEntropy(unsigned char const*, unsigned long) + 141
    frame #2: 0x00000001002e504c cryptest`CryptoPP::AutoSeededRandomPool::Reseed(bool, unsigned int) + 76
    frame #3: 0x0000000100071af1 cryptest`CryptoPP::Test::TestRandomPool() + 145
    frame #4: 0x000000010006ff29 cryptest`CryptoPP::Test::ValidateAll(bool) + 41
    frame #5: 0x000000010000b419 cryptest`CryptoPP::Test::Validate(int, bool) + 89
    frame #6: 0x000000010000373c cryptest`CryptoPP::Test::scoped_main(int, char**) + 8892
    frame #7: 0x0000000100a7152e dyld`start + 462

As for the compilation, I managed to reduce the differences between both commands lines, but the crash persists. I will keep reducing it further.

GNUMakeFile:

c++ -DCRYPTOPP_DISABLE_ASM -fPIC -pthread -fno-common -pipe -DNDEBUG -g2 -O3 -stdlib=libc++ -arch x86_64 -arch arm64 -c sha.cpp

CMake

c++  -DCRYPTOPP_DISABLE_ASM -I/Users/felipefarinon/downloads/cryptopp-cmake-master/build/cryptopp -I/Users/felipefarinon/downloads/cryptopp-cmake-master/build -pthread -fno-common -O3 -DNDEBUG -arch x86_64 -arch arm64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.1.sdk -mmacosx-version-min=12.7 -fPIC -MD -MT cryptopp/CMakeFiles/cryptopp.dir/sha.cpp.o -MF CMakeFiles/cryptopp.dir/sha.cpp.o.d -o CMakeFiles/cryptopp.dir/sha.cpp.o -c /Users/felipefarinon/downloads/cryptopp-cmake-master/build/cryptopp/sha.cpp

@felipefarinon
Copy link
Contributor Author

Ending my investigation for today: I've reduced further the compilation differences, with no success. I also called objdump on the randpool, iterhash and osrng object files, and they match 100%.

Tomorrow I'll check if I have the same bug with DISABLE_ASM on my Windows machine, where I'm more proficient with the debugger. I'm also intrigued that GNUMakeFile uses libtool instead of ar, like CMake.

@abdes
Copy link
Owner

abdes commented Jun 1, 2024 via email

@felipefarinon
Copy link
Contributor Author

Can confirm that the issue also happens on Windows, at the same step of cryptest (RandomPool generator test). I've compiled the dev-windows config, on Windows 10, with CRYPTOPP_DISABLE_ASM=ON, latest master commit, with CRYPTOPP_USE_MASTER_BRANCH=TRUE (to avoid #101). It's a stack corruption problem detected by a stack guard. I'll try running with a sanitizer to see if we get the exact code that's corrupting the stack.

image

@felipefarinon
Copy link
Contributor Author

It's a stack overflow in the SecBlock ctor. The tests pass if I define CRYPTOPP_BOOL_ALIGN16=1, which seems to add some padding to the buffer in the allocator of SecBlock. There are some interesting interactions between CRYPTOPP_BOOL_ALIGN16 and CRYPTOPP_DISABLE_ASM.

image

@abdes
Copy link
Owner

abdes commented Jun 3, 2024

Tagging @noloader

@felipefarinon
Copy link
Contributor Author

felipefarinon commented Jun 4, 2024

Found it! The CMake script is overriding the CRYPTOPP compilation flags for certain source files, not for all. This makes specific classes have different sizes/offsets depending on the compilation unit they're instantiated. I'm opening a PR with the fix for cryptest.

That being said, I believe the compilation flags should be part of the public definition of the library, so that users of the library are required to compile with the right defines. Otherwise, a change in the automatic configuration of the library might create this kind of hard to diagnose issues.

felipefarinon added a commit to felipefarinon/cryptopp-cmake that referenced this issue Jun 4, 2024
@abdes
Copy link
Owner

abdes commented Jun 4, 2024

Good catch. Agree that we may need to review the library public interface to check if it's missing any flags or definitions.

Feel free to also submit a PR for that change.

abdes pushed a commit that referenced this issue Jun 4, 2024
@noloader
Copy link
Collaborator

noloader commented Jun 4, 2024

Hi Everyone. Sorry for the late reply. I was locked out of GitHub again.

When you see -DCRYPTOPP_DISABLE_ASM, something is sideways. In this case, -DCRYPTOPP_DISABLE_ASM is a symptom and the underlying problem is Clang. Clang likes to misreport the architecture and features. (This problem was not present with GCC, back when it was available in XCode).

If you want to build fat binaries, then take a look at https://www.cryptopp.com/wiki/Universal_Binaries.

@felipefarinon
Copy link
Contributor Author

felipefarinon commented Jun 5, 2024

No worries. If to build fat binaries we have to patch both config_asm.h and config_cxx.h, does this mean that the CMake script doesn't support fat binaries yet? Otherwise, I'd expect that setting CMAKE_OSX_ARCHITECTURES would patch the headers accordingly.

That being said, after having a look at config_asm.h, it seems it just specializes the assembly use to a specific processor. If we disable ASM entirely, the library becomes slower, but will it become wrong? For my application it'd be a very acceptable compromise to make the lib less performant in exchange for ease of compilation.

@felipefarinon
Copy link
Contributor Author

Any positioning on using CRYPTOPP_DISABLE_ASM for compiling fat binaries, if performance is acceptable, @noloader?

@felipefarinon
Copy link
Contributor Author

Hello. Looking forward to the reply, this is an important issue for my project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants