Date: Fri, 25 Oct 2013 15:45:17 0500
Richard Smith <richardsmith_at_[hidden]> writes:
 On Fri, Oct 25, 2013 at 1:10 PM, Jeffrey Yasskin <jyasskin_at_[hidden]> wrote:

 On Fri, Oct 25, 2013 at 12:50 PM, Jens Maurer <Jens.Maurer_at_[hidden]> wrote:
 > On 10/25/2013 09:36 PM, John Regehr wrote:
 >>> What reason do you have to believe that crypto is using any signed
 >>> arithmetic? I would not.
 >>
 >> Here's an example that's at least slightly interesting, from the latest
 >> version of LibTomCrypt:
 >>
 >> kappa[i] =
 >> (key[pos ] << 24) ^
 >> (key[pos + 1] << 16) ^
 >> (key[pos + 2] << 8) ^
 >> (key[pos + 3] );
 >>
 >> key is a pointer to unsigned char. Of course, the array element becomes
 >> signed after promotion. The shift by 24 then executes an undefined
 >> behavior whenever the shifted value is >127.
 >>
 >> So the interesting thing is that the developer is basically doing things
 >> right and getting hosed by the arithmetic conversions.
 >
 > If I'm reading 5p10 correctly, this should help (and is consistently
 > expressing intent):
 >
 > kappa[i] =
 > (key[pos ] << 24u) ^
 > (key[pos + 1] << 16u) ^
 > (key[pos + 2] << 8u) ^
 > (key[pos + 3] );
 >
 > Jens

 Nope: [expr.shift]p1 says, "The type of the result is that of the
 promoted left operand."


 I believe John is correct. 5.8/1 says we perform integral promotions on the
 operands, not the usual arithmetic conversions. Clang, g++, and EDG agree 
 the type of (unsigned char)0 << 0u is int.
Yes, John correctly pointed out the integral promotion, and yes the type
of the expression you show is 'int'  and it has been so for decades.

 However, I think p2 saves our intrepid developer in C++14: "Otherwise,
 if E1 has a signed type and nonnegative value, and E1 × 2^E2 is
 representable in the corresponding unsigned type of the result type,
 then that value, converted to the result type, is the
 resulting value;"


 We gave this defined behavior as a DR, so I view this code has having de facto
 defined behavior in C++11 and C++98 too. But it's UB in C.

 In other words, we've already fixed this one (for some value of "fixed").
 On Fri, Oct 25, 2013 at 1:10 PM, Jeffrey Yasskin <jyasskin_at_[hidden]> wrote:

 On Fri, Oct 25, 2013 at 12:50 PM, Jens Maurer <Jens.Maurer_at_[hidden]> wrote:
 > On 10/25/2013 09:36 PM, John Regehr wrote:
 >>> What reason do you have to believe that crypto is using any signed
 >>> arithmetic? I would not.
 >>
 >> Here's an example that's at least slightly interesting, from the latest
 >> version of LibTomCrypt:
 >>
 >> kappa[i] =
 >> (key[pos ] << 24) ^
 >> (key[pos + 1] << 16) ^
 >> (key[pos + 2] << 8) ^
 >> (key[pos + 3] );
 >>
 >> key is a pointer to unsigned char. Of course, the array element becomes
 >> signed after promotion. The shift by 24 then executes an undefined
 >> behavior whenever the shifted value is >127.
 >>
 >> So the interesting thing is that the developer is basically doing things
 >> right and getting hosed by the arithmetic conversions.
 >
 > If I'm reading 5p10 correctly, this should help (and is consistently
 > expressing intent):
 >
 > kappa[i] =
 > (key[pos ] << 24u) ^
 > (key[pos + 1] << 16u) ^
 > (key[pos + 2] << 8u) ^
 > (key[pos + 3] );
 >
 > Jens

 Nope: [expr.shift]p1 says, "The type of the result is that of the
 promoted left operand."


 I believe John is correct. 5.8/1 says we perform integral promotions on the
 operands, not the usual arithmetic conversions. Clang, g++, and EDG agree 
 the type of (unsigned char)0 << 0u is int.
Yes, John correctly pointed out the integral promotion, and yes the type
of the expression you show is 'int'  and it has been so for decades.

 However, I think p2 saves our intrepid developer in C++14: "Otherwise,
 if E1 has a signed type and nonnegative value, and E1 × 2^E2 is
 representable in the corresponding unsigned type of the result type,
 then that value, converted to the result type, is the
 resulting value;"


 We gave this defined behavior as a DR, so I view this code has having de facto
 defined behavior in C++11 and C++98 too. But it's UB in C.

 In other words, we've already fixed this one (for some value of "fixed").
Received on 20131025 22:45:33