Repository navigation
Fix build with NOSSE2 - #1891
Conversation
Disable AESNI and SHANI code that depends on SSE2 availability if NOSSE2 is enabled.
idrassi
left a comment
There was a problem hiding this comment.
Thanks for picking this up. The unmodified patch's 32 bit crypto libraries passed EncryptionTest::TestAll on my machine and under QEMU Pentium III model with SSE2 unavailable. There's useful progress here, but I found some issues that need addressing before merging:
-
The amd64 build still fails: Disabling SHA removes the indirect
wchar_tdeclaration from<immintrin.h>.Camellia.cthen fails when includingCommon/Crypto.h. Adding<stddef.h>before that include fixes the build; the algorithm tests passed. -
Please keep runtime AES acceleration:
-mno-sse2already disables AES intrinsics.Aes_hw_cpu.asmis built separately, soCRYPTOPP_DISABLE_AESNIunnecessarily disables AES acceleration and the userspace Camellia fast path. Removing it passed the tests after the header fix. Please also keep object selection and callers consistent forNOSSE2=1 NOAESNI=1. -
The SHA guards don't agree: With GCC's
-msha -mno-sse2,__SHA__still enables theTrySHA256call whileCRYPTOPP_DISABLE_SHANIremoves its implementation, causing a link failure. Please align the guards, preserving the NOASM and UEFI exclusions from #1896.
Keep AES/Camellia acceleration enabled and honor NOSSE2 with NOAESNI. Include stddef.h outside UEFI to provide wchar_t for Camellia. Match the SHA probe guard to its implementation, keeping NOASM and UEFI exclusions.
idrassi
left a comment
There was a problem hiding this comment.
I have pushed a small followup addressing the points from my review:
- Add the missing header for wchar_t in the amd64 build.
- Preserve runtime AES/Camellia acceleration and handle NOSSE2=1 NOAESNI=1 consistently.
- Align the SHA probe guards with its implementation, retaining the NOASM and UEFI exclusions.
Tested under QEMU Pentium III without SSE2.
Merging. Thank you for your contribution.
Disable AESNI and SHANI code that depends on SSE2 availability if NOSSE2 is enabled.