I imagine you intended to mail the nettle-bugs list, rather than nettle-bugs-owner administrative address? Feel free to reply on list.
Ugh, sorry, cut and paste...
That's unexpected. Hmm. The NEWS file for 2.7.1 claims that this problem was fixed in that release, but appearantly it wasn't. You can check if the error disappears if you replace the definition of ROT32 in macros.h by
#define ROTL32(n,x) (((x)<<(n)) | ((x)>>((-(n)&31))))
I'll check this tonight.
blowfish.c:353:19: runtime error: left shift of 244 by 24 places cannot be represented in type 'int' twofish.c:196:54: runtime error: left shift of 184 by 24 places cannot be represented in type 'int'
These should have some casts to uint32_t, I think. Unlikely to cause real problems.
Right, I don't know of any compiler that exploits these, and in fact in C++ (but not C) this particular case is covered by a defect report that makes it defined.
Thanks,
John
Note that the safer rotate macro can be compiled into a rotate instruction just like the current macro. But LLVM and GCC don't know this. I've made requests to both compiler teams to implement this optimization, but I have no idea if they'll go for it.
http://llvm.org/bugs/show_bug.cgi?id=17904 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59100
John Regehr
On 11/12/2013 08:55 PM, John Regehr wrote:
#define ROTL32(n,x) (((x)<<(n)) | ((x)>>((-(n)&31))))
The problem is n==0, not n==32. So this fixes the undefined behavior:
#define ROTL32(n,x) ((n)==0?(x):(((x)<<(n)) | ((x)>>(32-(n)))))
John
nettle-bugs mailing list nettle-bugs@lists.lysator.liu.se http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
John Regehr regehr@cs.utah.edu writes:
#define ROTL32(n,x) (((x)<<(n)) | ((x)>>((-(n)&31))))
The problem is n==0, not n==32.
Exactly. The old rotation macro (still in nettle-2.7.1, it seems) used the subexpression x >> (32 - n), which gives undefined behavior for n == 0. And with cast, this rotation macro is used with key-dependent n in the range 0 <= n < 32.
But doesn't the above version of the rotation macro work fine for all n in the needed range? The intention is that for n == 0, it should boil down to (x << 0) | (x >> 0), which is perfectly well defined C. I really want to avoid conditionals here.
About gcc optimizations, see my corresponding bug report, http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57157. As far as I understand, recognition of rotates has been considerably improved since.
Regards, /Niels
Niels, sorry, of course you're right that your updated macro fixes the n==0 case.
It sounds like both the GCC and LLVM people are interested in optimizing the safe rotate idioms, nice.
John
On 11/13/2013 12:27 AM, Niels Möller wrote:
John Regehr regehr@cs.utah.edu writes:
#define ROTL32(n,x) (((x)<<(n)) | ((x)>>((-(n)&31))))The problem is n==0, not n==32.
Exactly. The old rotation macro (still in nettle-2.7.1, it seems) used the subexpression x >> (32 - n), which gives undefined behavior for n == 0. And with cast, this rotation macro is used with key-dependent n in the range 0 <= n < 32.
But doesn't the above version of the rotation macro work fine for all n in the needed range? The intention is that for n == 0, it should boil down to (x << 0) | (x >> 0), which is perfectly well defined C. I really want to avoid conditionals here.
About gcc optimizations, see my corresponding bug report, http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57157. As far as I understand, recognition of rotates has been considerably improved since.
Regards, /Niels
nettle-bugs@lists.lysator.liu.se
-
John Regehr -
nisse＠lysator.liu.se