On Sat, Dec 06, 2014 at 01:47:52AM +0300, Solar Designer wrote: > Another suspect is C strict aliasing rules violations, which I think are > present in scrypt's current -nosse and -sse code. I was able to trigger > misbehavior in -nosse with either unusual gcc options (aggressive > function inlining) or slight source code changes. I was not able to > trigger misbehavior in -sse, but per my understanding the violations are > in there. Another compiler might expose these. (I fixed both in yescrypt.)
BTW, when you fix this, beware of icc 14.0.0 (other versions?) bug where it sometimes preloads elements of a union into registers despite of writes into other elements, at least when the union consists of __m128i and uint32_t fields. union is a standard way to meet C strict aliasing rules, yet there's this bug to trap us. Adding volatile works around it, and also it does not show up with the accesses being in different functions (would show up with more aggressive function inlining anyway? not sure). Miscompiles are a thing. This is a reason why I think runtime self-test of the full scrypt is desirable. (I am planning on adding that to yescrypt as well.) One aspect I haven't decided on yet is whether it's a good idea to have a self-test even in -ref code or not (since this goes against the simplicity goal for -ref). Alexander
