blake2 hashes don't work on older CPUs if cryptonite was built with support_aesni

[pvgoran]

I have a machine with an AMD Phenom II (AMD Phenom(tm) II X3 720 Processor as per /proc/cpuinfo).

When I build cryptonite on it without specifying any configuration options, it fails the Blake2 tests:

$ git clone https://github.com/haskell-crypto/cryptonite.git
$ cd cryptonite
$ cabal build -j
$ cabal test -j --test-options='-p Blake2'
...
Running 1 test suites...
Test suite test-cryptonite: RUNNING...
cryptonite
  hash
    KATs
      Blake2b-160
        1:         test-cryptonite: too many pending signals
Test suite test-cryptonite: FAIL
Test suite logged to:
/home/paul/build/cryptonite/dist-newstyle/build/x86_64-linux/ghc-8.6.5/cryptonite-0.26/t/test-cryptonite/test/cryptonite-0.26-test-cryptonite.log
0 of 1 test suites (0 of 1 test cases) passed.
cabal: Tests failed for test:test-cryptonite from cryptonite-0.26.

Also, when I build git-annex (https://git-annex.branchable.com/, uses cryptonite) and run its tests, they also fail, with a more specific message:

$ git clone git://git-annex.branchable.com/ git-annex
$ cd git-annex
$ cabal install -j --only-dependencies
$ cabal configure
$ cabal build -j
$ ./dist-newstyle/build/x86_64-linux/ghc-8.6.5/git-annex-8.20200226/build/git-annex/git-annex test -v -p Tests.QuickCheck.blake2s_160
Illegal instruction (core dumped)

(See https://git-annex.branchable.com/bugs/__34__Illegal_instruction__34___when_running_certain_tests_on_AMD_Phenom_II for where this came from.)

When I disable support_aesni by passing --constraint="cryptonite -support_aesni" to cabal, both of these tests pass.

This means that AES-NI is used in Blake2 implementation based on purely compile-time options. Because of this, any binary distribution of cryptonite (or a software which employs cryptonite for Blake2 hashes) will be crippled: if built with AES-NI support, it will fail hard on older CPUs; if built without AES-NI support, it will be slowed than it could be on newer CPUs.

Ideally, AES-NI should only be used after checking that it is actually available at runtime. Barring this, support_aesni should probably be disabled by default.

[ocheron]

Thanks for sharing the details. Maybe it's possible to remove global cc-options like -mssse3 and use per-file pragmas at appropriate locations instead. But need to be careful if trading toolchain support for CPU support.

[codygman]

Is it possible to just disable sss33 with CPP in the cabal file for CPU's that don't support it?

[pvgoran]

Is it possible to just disable sss33 with CPP in the cabal file for CPU's that don't support it?

Even if it's possible, it won't help: the problem manifests at run-time, and compile-time checks won't prevent it because the code may be run on a different machine after being compiled.

[ocheron]

I came up with this but it may decrease compiler support: ocheron:aesni-per-file.

[pvgoran]

@ocheron Will this somehow allow the code in x86ni.c to run on older CPUs?

[ocheron]

I hope it will allow the code in blake2/ref to run on older CPUs. Normally x86ni.c is not run, this is consistent with your report being a Blake2 failure, not AES.

EDIT: Actually this is because of the code in blake2/sse, not blake2/ref. On amd64 at least SSE2 is available, and the blake2 condition is OR, not AND.

[pvgoran]

Ok, this seems to work for me. Even if the test suite is compiled on another machine and copied to the old-CPU machine, it now passes successfully.

(Although I still don't understand how changing the compilation options and pragmas for AES code allows the Blake2 code to run.)

[ocheron]

Yes it's a bit convoluted. In the AES code the library already has what is necessary to use AES-NI at runtime when the CPU supports it. But in order to compile the AES-NI variant, we need an option like -mssse3. With a Makefile-based project the option would be enabled only on needed files easily, but with Cabal we can't do that and enable globally. When enabled, the C compiler can emit SSSE3 intructions to optimize things, including code not related to AES-NI and having a single variant at runtime. For some reason it tends to show up preferably in Blake2 but can be actually anywhere.

Thanks for giving it a try, it's useful to know the approach works with no other cause. Now it's just a matter of reading compiler release notes and deciding how/if/when we can do something like this.

[ocheron]

Solution applied in #316 with optional package flag to restore previous behavior. Switching to function attributes instead of file pragma gives better compatibility with clang.