Repository navigation
Fix crash on x86 CPUs without AES support in clang-cl builds - #1
Merged
Merged
Conversation
When built with clang-cl, the library could pick an implementation that uses AES instructions even on a CPU that doesn't have them, which crashes on the first call. It happened because different source files disagreed about whether the plain software version was available, and the picker had a fallback that assumed AES was always present. This reorganizes the code the same way libaegis does it: the compiler and CPU checks now live in one shared header that every file uses, so they all agree, and an implementation is only chosen when the CPU really supports it. Keeping the two projects consistent should make them easier to maintain. Also lets clang-cl use the fastest HiAE implementation, which was needlessly turned off for it, and adds a clang-cl build to CI.
jedisct1
force-pushed
the
clang-cl-dispatch
branch
from
September 14, 2026 18:55
7e072cf to
db9cb4c
Compare
Collaborator
Author
|
AI slop made by Perplexity Computer while I was AFK but it doesn't touch any of the crypto implementations. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When the library is built with clang-cl, it could end up using an implementation that relies on AES instructions even on a CPU that doesn't have them. The first call then crashes. This was reported against a shipping Windows build.
The cause was that different source files disagreed about whether the plain software version had been built, so the code that picks an implementation didn't know it was there, and fell back to assuming AES was always available. The CPU check itself was correct, its answer was just ignored.
This reorganizes things the way libaegis already does it: the compiler and CPU checks live in one shared header that every file uses, so they all agree, and an implementation is only picked when the CPU really supports it. Having the two projects match should make both easier to maintain.
It also lets clang-cl use the fastest HiAE implementation, which was turned off for it with no real reason, and adds a clang-cl build to CI so this doesn't come back.
Tested on Linux with the normal build and with AES/AVX-512 enabled at compile time, plus the single-file amalgamated build. All tests pass and forcing the software implementation still works.