Skip to content

Commit

Permalink
Fix readBitsInt{Le,Be} methods - use zero-fill right shift, fix 63-bi…
Browse files Browse the repository at this point in the history
…t mask of '1's

As in Java (kaitai-io/kaitai_struct_java_runtime@e1168b0) and JavaScript (kaitai-io/kaitai_struct_javascript_runtime@1f8dd95),
using the zero-fill right shift instead of the sign-propagating right shift
is crucial for the little-endian procedure to work properly if the "$this->bits"
property has the most significant bit (sign bit) set (1) - such number will
be negative.

PHP does not have it built-in, so we need to supply a tiny method that does this.
  • Loading branch information
generalmimon committed Mar 2, 2021
1 parent a73ce95 commit c0cc3e2
Showing 1 changed file with 21 additions and 4 deletions.
25 changes: 21 additions & 4 deletions lib/Kaitai/Struct/Stream.php
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ public function readBitsIntBe(int $n): int {
}

// raw mask with required number of 1s, starting from lowest bit
$mask = (1 << $n) - 1;
$mask = $this->getMaskOnes($n);
// shift $this->bits to align the highest bits with the mask & derive reading result
$shiftBits = $this->bitsLeft - $n;
$res = ($this->bits >> $shiftBits) & $mask;
Expand Down Expand Up @@ -277,15 +277,25 @@ public function readBitsIntLe(int $n): int {
}

// raw mask with required number of 1s, starting from lowest bit
$mask = (1 << $n) - 1;
$mask = $this->getMaskOnes($n);
// derive reading result
$res = $this->bits & $mask;
// remove bottom bits that we've just read by shifting
$this->bits >>= $n;
$this->bits = $this->zeroFillRightShift($this->bits, $n);
$this->bitsLeft -= $n;

return $res;
}
}

private static function getMaskOnes(int $n): int {
// 1. (1 << 63) === PHP_INT_MIN (and yes, it is negative, because PHP uses signed 64-bit ints on 64-bit system),
// so (1 << 63) - 1 gets converted to float and loses precision (leading to incorrect result)
// 2. (1 << 64) - 1 works fine, because (1 << 64) === 0 (it overflows) and -1 is exactly what we want
// (`php -r "var_dump(decbin(-1));"` => string(64) "111...11")
$bit = 1 << $n;
return $bit === PHP_INT_MIN ? ~$bit : $bit - 1;
}


/**************************************************************************
* Byte arrays
Expand Down Expand Up @@ -471,6 +481,13 @@ private static function decodeSignedInt(int $x, int $mask): int {
return ($x & ~$mask) - ($x & $mask);
}

// From https://stackoverflow.com/a/14428473, modified
private static function zeroFillRightShift(int $a, int $b): int {
$res = $a >> $b;
if ($a >= 0 || $b === 0) return $res;
return $res & ~(PHP_INT_MIN >> ($b - 1));
}

private function decodeSinglePrecisionFloat(int $bits): float {
$fractionToFloat = function (int $fraction): float {
$val = 0;
Expand Down

0 comments on commit c0cc3e2

Please sign in to comment.