Bug 939157 - RotateLeft with shift of zero gives undefined behavior. r=Waldo
authorSean Stangl <sstangl@mozilla.com>
Tue, 03 Nov 2015 14:25:48 -0800
changeset 271229 980aebe436cf782cba8c65eeff46ec3dec4fc23f
parent 271228 ea7b16628b48ebfe1561798930c7a3ad793e2cbf
child 271230 0d2feb34ceb86f38ce21d4c32f08f1efff071068
push id29634
push usercbook@mozilla.com
push dateThu, 05 Nov 2015 10:59:26 +0000
treeherdermozilla-central@59c648a3f955 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo
bugs939157
milestone45.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 939157 - RotateLeft with shift of zero gives undefined behavior. r=Waldo
js/src/jit/arm/Assembler-arm.h
mfbt/MathAlgorithms.h
--- a/js/src/jit/arm/Assembler-arm.h
+++ b/js/src/jit/arm/Assembler-arm.h
@@ -642,19 +642,23 @@ class Imm8 : public Operand2
 {
   public:
     explicit Imm8(uint32_t imm)
       : Operand2(EncodeImm(imm))
     { }
 
   public:
     static datastore::Imm8mData EncodeImm(uint32_t imm) {
+        // RotateLeft below may not be called with a shift of zero.
+        if (imm <= 0xFF)
+            return datastore::Imm8mData(imm, 0);
+
         // An encodable integer has a maximum of 8 contiguous set bits,
         // with an optional wrapped left rotation to even bit positions.
-        for (int rot = 0; rot < 16; rot++) {
+        for (int rot = 1; rot < 16; rot++) {
             uint32_t rotimm = mozilla::RotateLeft(imm, rot*2);
             if (rotimm <= 0xFF)
                 return datastore::Imm8mData(rotimm, rot);
         }
         return datastore::Imm8mData();
     }
 
     // Pair template?
--- a/mfbt/MathAlgorithms.h
+++ b/mfbt/MathAlgorithms.h
@@ -479,28 +479,38 @@ RoundUpPow2(size_t aValue)
 /**
  * Rotates the bits of the given value left by the amount of the shift width.
  */
 template<typename T>
 inline T
 RotateLeft(const T aValue, uint_fast8_t aShift)
 {
   MOZ_ASSERT(aShift < sizeof(T) * CHAR_BIT, "Shift value is too large!");
+  MOZ_ASSERT(aShift > 0,
+             "Rotation by value length is undefined behavior, but compilers "
+             "do not currently fold a test into the rotate instruction. "
+             "Please remove this restriction when compilers optimize the "
+             "zero case (http://blog.regehr.org/archives/1063).");
   static_assert(IsUnsigned<T>::value, "Rotates require unsigned values");
   return (aValue << aShift) | (aValue >> (sizeof(T) * CHAR_BIT - aShift));
 }
 
 /**
  * Rotates the bits of the given value right by the amount of the shift width.
  */
 template<typename T>
 inline T
 RotateRight(const T aValue, uint_fast8_t aShift)
 {
   MOZ_ASSERT(aShift < sizeof(T) * CHAR_BIT, "Shift value is too large!");
+  MOZ_ASSERT(aShift > 0,
+             "Rotation by value length is undefined behavior, but compilers "
+             "do not currently fold a test into the rotate instruction. "
+             "Please remove this restriction when compilers optimize the "
+             "zero case (http://blog.regehr.org/archives/1063).");
   static_assert(IsUnsigned<T>::value, "Rotates require unsigned values");
   return (aValue >> aShift) | (aValue << (sizeof(T) * CHAR_BIT - aShift));
 }
 
 /**
  * Returns true if |x| is a power of two.
  * Zero is not an integer power of two. (-Inf is not an integer)
  */