Add assertions for dangerous bit shift that could trigger undefined

behaviour

Change-Id: I051ed229d407eafcae1ea4b5fc745176a751e036

ion/include/ion/keyboard.h

@@ -3,6 +3,7 @@
 
 extern "C" {
 #include <stdint.h>
+#include <assert.h>
 }
 
 namespace Ion {
@@ -37,10 +38,14 @@ public:
   constexpr State(uint64_t s = 0) :
     m_bitField(s)
   {}
+  /* "Shift behavior is undefined if the right operand is negative, or greater
+   * than or equal to the length in bits of the promoted left operand" according
+   * to C++ spec but k is always in [0:52]. */
   explicit constexpr State(Key k) :
     m_bitField((uint64_t)1 << (uint8_t)k)
   {}
   inline bool keyDown(Key k) const {
+    assert((uint8_t)k < 64);
     return (m_bitField>>(uint8_t)k) & 1;
   }
   operator uint64_t() const { return m_bitField; }

ion/src/device/regs/register.h

@@ -2,6 +2,7 @@
 #define REGS_REGISTER_H
 
 #include <stdint.h>
+#include <assert.h>
 
 template <typename T>
 class Register {
@@ -21,13 +22,19 @@ public:
     m_value = bit_range_set_value(high, low, m_value, value);
   }
   T getBitRange(uint8_t high, uint8_t low) volatile {
+    /* "Shift behavior is undefined if the right operand is negative, or greater
+     * than or equal to the length in bits of the promoted left operand" according
+     * to C++ spec. */
+    assert(low < 8*sizeof(T));
     return (m_value & bit_range_mask(high,low)) >> low;
   }
 protected:
   static constexpr T bit_range_mask(uint8_t high, uint8_t low) {
+    // Same comment as for getBitRange: we should assert (high-low+1) < 8*sizeof(T)
     return ((((T)1)<<(high-low+1))-1)<<low;
   }
   static constexpr T bit_range_value(T value, uint8_t high, uint8_t low) {
+    // Same comment as for getBitRange: we should assert low < 8*sizeof(T))
     return (value<<low) & bit_range_mask(high,low);
   }
   static constexpr T bit_range_set_value(uint8_t high, uint8_t low, T originalValue, T targetValue) {

liba/src/aeabi-rt/llsl.c

@@ -1,10 +1,17 @@
 /* See the "Run-time ABI for the ARM Architecture", Section 4.2 */
+#include <assert.h>
 
 typedef unsigned int uint32_t;
 
 long long __aeabi_llsl(long long value, int shift) {
   uint32_t low = (uint32_t)value << shift;
+  /* "Shift behavior is undefined if the right operand is negative, or greater
+   * than or equal to the length in bits of the promoted left operand" according
+   * to C++ spec. However, arm compiler fill the vacated bits with 0 */
+  assert(shift < 32 || low == 0);
   uint32_t high = ((uint32_t)(value >> 32) << shift);
+  // Same comment
+  assert(shift < 32 || high == 0);
   if (shift < 32) {
     high |= ((uint32_t)value >> (32 - shift));
   } else {

liba/src/aeabi-rt/llsr.c

@@ -1,14 +1,21 @@
 /* See the "Run-time ABI for the ARM Architecture", Section 4.2 */
+#include <assert.h>
 
 typedef unsigned int uint32_t;
 
 long long __aeabi_llsr(long long value, int shift) {
   uint32_t low = ((uint32_t)value >> shift);
+  /* "Shift behavior is undefined if the right operand is negative, or greater
+   * than or equal to the length in bits of the promoted left operand" according
+   * to C++ spec. However, arm compiler fill the vacated bits with 0 */
+  assert(shift < 32 || low == 0);
   if (shift < 32) {
     low |= ((uint32_t)(value >> 32) << (32 - shift));
   } else {
     low |= ((uint32_t)(value >> 32) >> (shift - 32));
   }
   uint32_t high = (uint32_t)(value >> 32) >> shift;
+  // Same comment
+  assert(shift < 32 || high == 0);
   return ((long long)high << 32) | low;
 }

poincare/src/integer.cpp

@@ -515,13 +515,15 @@ T Integer::approximate() const {
   /* Shift the most significant int to the left of the mantissa. The most
    * significant 1 will be ignore at the end when inserting the mantissa in
    * the resulting uint64_t (as required by IEEE754). */
+  assert(totalNumberOfBits-numberOfBitsInLastDigit > 0 && totalNumberOfBits-numberOfBitsInLastDigit < 64); // Shift operator behavior is undefined if the right operand is negative, or greater than or equal to the length in bits of the promoted left operand
   mantissa |= ((uint64_t)lastDigit << (totalNumberOfBits-numberOfBitsInLastDigit));
   int digitIndex = 2;
   int numberOfBits = numberOfBitsInLastDigit;
   /* Complete the mantissa by inserting, from left to right, every digit of the
    * Integer from the most significant one to the last from. We break when
-   * the mantissa is complete to avoid undefined right shifting (when the shift
-   * width is wider than the length of the digit in bits). */
+   * the mantissa is complete to avoid undefined right shifting (Shift operator
+   * behavior is undefined if the right operand is negative, or greater than or
+   * equal to the length in bits of the promoted left operand). */
   while (m_numberOfDigits >= digitIndex) {
     lastDigit = digit(m_numberOfDigits-digitIndex);
     numberOfBits += 32;
@@ -529,6 +531,7 @@ T Integer::approximate() const {
       break;
     }
     if (totalNumberOfBits > numberOfBits) {
+      assert(totalNumberOfBits-numberOfBits > 0 && totalNumberOfBits-numberOfBits < 64);
       mantissa |= ((uint64_t)lastDigit << (totalNumberOfBits-numberOfBits));
     } else {
       mantissa |= ((uint64_t)lastDigit >> (numberOfBits-totalNumberOfBits));