Rewrite readBitsInt{Be,Le}() - fix known issues, speed up a bit

See kaitai-io/kaitai_struct#949

Fix #20, fix #22

Author: generalmimon

KaitaiStream.js

@@ -412,40 +412,53 @@ KaitaiStream.prototype.readF8le = function() {
 // ------------------------------------------------------------------------
 
 KaitaiStream.prototype.alignToByte = function() {
-  this.bits = 0;
   this.bitsLeft = 0;
+  this.bits = 0;
 };
 
+/*
+ bitsLeft = 3
+      \  \  bitsNeeded = 10 -> bytesNeeded = 2
+       \  \ /         \
+  |01101xxx|xxxxxxxx|xx......|
+        \             /\     \
+         \__ n = 13 _/  \     \
+                       new bitsLeft = 6
+ */
 KaitaiStream.prototype.readBitsIntBe = function(n) {
   // JS only supports bit operations on 32 bits
   if (n > 32) {
     throw new Error("readBitsIntBe: the maximum supported bit length is 32 (tried to read " + n + " bits)");
   }
+  var res = 0;
+
   var bitsNeeded = n - this.bitsLeft;
+  this.bitsLeft = -bitsNeeded & 7; // `-bitsNeeded mod 8`
+
   if (bitsNeeded > 0) {
     // 1 bit  => 1 byte
     // 8 bits => 1 byte
     // 9 bits => 2 bytes
-    var bytesNeeded = Math.ceil(bitsNeeded / 8);
+    var bytesNeeded = ((bitsNeeded - 1) >> 3) + 1; // `ceil(bitsNeeded / 8)` (NB: `x >> 3` is `floor(x / 8)`)
     var buf = this.readBytes(bytesNeeded);
     for (var i = 0; i < bytesNeeded; i++) {
-      this.bits <<= 8;
-      this.bits |= buf[i];
-      this.bitsLeft += 8;
+      res = res << 8 | buf[i];
     }
+
+    var newBits = res;
+    res = res >>> this.bitsLeft | this.bits << bitsNeeded; // `x << 32` is defined as `x << 0` in JS, but only `0 << 32`
+                                                           // can occur here (`n = 32` and `bitsLeft = 0`, this implies
+                                                           // `bits = 0` unless changed externally)
+    this.bits = newBits; // will be masked at the end of the function
+  } else {
+    res = this.bits >>> -bitsNeeded; // shift unneeded bits out
   }
 
-  // raw mask with required number of 1s, starting from lowest bit
-  var mask = n === 32 ? 0xffffffff : (1 << n) - 1;
-  // shift this.bits to align the highest bits with the mask & derive reading result
-  var shiftBits = this.bitsLeft - n;
-  var res = (this.bits >>> shiftBits) & mask;
-  // clear top bits that we've just read => AND with 1s
-  this.bitsLeft -= n;
-  mask = (1 << this.bitsLeft) - 1;
+  var mask = (1 << this.bitsLeft) - 1; // `bitsLeft` is in range 0..7, so `(1 << 32)` does not have to be considered
   this.bits &= mask;
 
-  return res;
+  // always return an unsigned 32-bit integer
+  return res >>> 0;
 };
 
 /**
@@ -455,32 +468,59 @@ KaitaiStream.prototype.readBitsIntBe = function(n) {
  */
 KaitaiStream.prototype.readBitsInt = KaitaiStream.prototype.readBitsIntBe;
 
+/*
+    n = 13       bitsNeeded = 10
+                    /       \
+ bitsLeft = 3  ______       __
+    \  \      /      \      \ \
+    |xxx01101|xxxxxxxx|......xx|
+                       \    /
+                    new bitsLeft = 6
+
+          bitsLeft = 7
+             \      \
+    |01101100|..xxxxx1|........|
+                \___/
+                n = 5
+ */
 KaitaiStream.prototype.readBitsIntLe = function(n) {
   // JS only supports bit operations on 32 bits
   if (n > 32) {
     throw new Error("readBitsIntLe: the maximum supported bit length is 32 (tried to read " + n + " bits)");
   }
+  var res = 0;
   var bitsNeeded = n - this.bitsLeft;
+
   if (bitsNeeded > 0) {
-      // 1 bit  => 1 byte
-      // 8 bits => 1 byte
-      // 9 bits => 2 bytes
-      var bytesNeeded = Math.ceil(bitsNeeded / 8);
-      var buf = this.readBytes(bytesNeeded);
-      for (var i = 0; i < bytesNeeded; i++) {
-          this.bits |= (buf[i] << this.bitsLeft);
-          this.bitsLeft += 8;
-      }
+    // 1 bit  => 1 byte
+    // 8 bits => 1 byte
+    // 9 bits => 2 bytes
+    var bytesNeeded = ((bitsNeeded - 1) >> 3) + 1; // `ceil(bitsNeeded / 8)` (NB: `x >> 3` is `floor(x / 8)`)
+    var buf = this.readBytes(bytesNeeded);
+    for (var i = 0; i < bytesNeeded; i++) {
+      res |= buf[i] << (i * 8);
+    }
+
+    // NB: in JavaScript, bit shift operators always shift by modulo 32 of the right-hand operand (see
+    // https://tc39.es/ecma262/multipage/ecmascript-data-types-and-values.html#sec-numeric-types-number-unsignedRightShift),
+    // so `res >>> 32` is equivalent to `res >>> 0` (but we don't want that)
+    var newBits = bitsNeeded < 32 ? res >>> bitsNeeded : 0;
+    res = res << this.bitsLeft | this.bits;
+    this.bits = newBits;
+  } else {
+    res = this.bits;
+    this.bits >>>= n;
   }
 
-  // raw mask with required number of 1s, starting from lowest bit
-  var mask = n === 32 ? 0xffffffff : (1 << n) - 1;
-  // derive reading result
-  var res = this.bits & mask;
-  // remove bottom bits that we've just read by shifting
-  this.bits >>>= n;
-  this.bitsLeft -= n;
+  this.bitsLeft = -bitsNeeded & 7; // `-bitsNeeded mod 8`
 
+  // always return an unsigned 32-bit integer
+  if (n < 32) {
+    var mask = (1 << n) - 1;
+    res &= mask; // this produces a signed 32-bit int, but the sign bit is cleared
+  } else {
+    res >>>= 0;
+  }
   return res;
 };