correct safe integer range for bitwise ops (#381)

Fixes #317 #316 #315

Author: stephenamar-db

bench/src/main/scala/sjsonnet/ProfilingEvaluator.scala

@@ -162,7 +162,7 @@ class ProfilingEvaluator(
     all.foreach { b =>
       val cl = b.expr match {
         case _: Val => classOf[Val]
-        case ee      => ee.getClass
+        case ee     => ee.getClass
       }
       val n = cl.getName.replaceAll("^sjsonnet\\.", "").replace('$', '.')
       val eb = m.getOrElseUpdate(n, new ExprTypeBox(n))

sjsonnet/src/sjsonnet/Evaluator.scala

@@ -549,14 +549,21 @@ class Evaluator(
 
       case Expr.BinaryOp.OP_<< =>
         (l, r) match {
-          case (Val.Num(_, l), Val.Num(_, r)) => Val.Num(pos, (l.toLong << r.toLong).toDouble)
-          case _                              => fail()
+          case (l: Val.Num, r: Val.Num) =>
+            val ll = l.asSafeLong(pos)
+            val rr = r.asSafeLong(pos)
+            if (rr >= 1 && ll >= (1L << (63 - rr)))
+              Error.fail("numeric value outside safe integer range for bitwise operation", pos)
+            else
+              Val.Num(pos, (ll << rr).toDouble)
+          case _ => fail()
         }
 
       case Expr.BinaryOp.OP_>> =>
         (l, r) match {
-          case (Val.Num(_, l), Val.Num(_, r)) => Val.Num(pos, (l.toLong >> r.toLong).toDouble)
-          case _                              => fail()
+          case (l: Val.Num, r: Val.Num) =>
+            Val.Num(pos, (l.asSafeLong(pos) >> r.asSafeLong(pos)).toDouble)
+          case _ => fail()
         }
 
       case Expr.BinaryOp.OP_in =>
@@ -567,20 +574,23 @@ class Evaluator(
 
       case Expr.BinaryOp.OP_& =>
         (l, r) match {
-          case (Val.Num(_, l), Val.Num(_, r)) => Val.Num(pos, (l.toLong & r.toLong).toDouble)
-          case _                              => fail()
+          case (l: Val.Num, r: Val.Num) =>
+            Val.Num(pos, (l.asSafeLong(pos) & r.asSafeLong(pos)).toDouble)
+          case _ => fail()
         }
 
       case Expr.BinaryOp.OP_^ =>
         (l, r) match {
-          case (Val.Num(_, l), Val.Num(_, r)) => Val.Num(pos, (l.toLong ^ r.toLong).toDouble)
-          case _                              => fail()
+          case (l: Val.Num, r: Val.Num) =>
+            Val.Num(pos, (l.asSafeLong(pos) ^ r.asSafeLong(pos)).toDouble)
+          case _ => fail()
         }
 
       case Expr.BinaryOp.OP_| =>
         (l, r) match {
-          case (Val.Num(_, l), Val.Num(_, r)) => Val.Num(pos, (l.toLong | r.toLong).toDouble)
-          case _                              => fail()
+          case (l: Val.Num, r: Val.Num) =>
+            Val.Num(pos, (l.asSafeLong(pos) | r.asSafeLong(pos)).toDouble)
+          case _ => fail()
         }
 
       case _ => fail()

sjsonnet/src/sjsonnet/Val.scala

@@ -81,6 +81,10 @@ object PrettyNamed {
   implicit val nullName: PrettyNamed[Val.Null] = new PrettyNamed("null")
 }
 object Val {
+  // Constants for safe double-to-int conversion
+  // IEEE 754 doubles precisely represent integers up to 2^53, beyond which precision is lost
+  private val DOUBLE_MAX_SAFE_INTEGER = (1L << 53) - 1
+  private val DOUBLE_MIN_SAFE_INTEGER = -((1L << 53) - 1)
 
   abstract class Literal extends Val with Expr {
     final override private[sjsonnet] val _tag = ExprTags.`Val.Literal`
@@ -108,6 +112,17 @@ object Val {
     def prettyName = "number"
     override def asInt: Int = value.toInt
     override def asLong: Long = value.toLong
+
+    def asSafeLong(pos: Position)(implicit ev: EvalErrorScope): Long = {
+      if (value.isInfinite || value.isNaN) {
+        Error.fail("numeric value is not finite", pos)
+      }
+
+      if (value < DOUBLE_MIN_SAFE_INTEGER || value > DOUBLE_MAX_SAFE_INTEGER) {
+        Error.fail("numeric value outside safe integer range for bitwise operation", pos)
+      }
+      value.toLong
+    }
     override def asDouble: Double = value
   }
 

sjsonnet/test/resources/test_suite/error.integer_conversion.jsonnet

@@ -0,0 +1,3 @@
+// Value just beyond MAX_SAFE_INTEGER (2^53 - 1)
+local beyond_max = 9007199254740992;  // 2^53
+beyond_max << 1  // Should throw error "numeric value outside safe integer range for bitwise operation"

sjsonnet/test/resources/test_suite/error.integer_left_shift.jsonnet

@@ -0,0 +1,3 @@
+// Test that left-shifting a value that would exceed int64_t range throws an error
+local large_value = 1 << 62;  // 2^62
+large_value << 2  // Would be 2^64, exceeding signed 64-bit integer range

sjsonnet/test/resources/test_suite/error.integer_left_shift_runtime.jsonnet

@@ -0,0 +1,2 @@
+local large_value = 4503599627370496;
+large_value << 11

sjsonnet/test/resources/test_suite/safe_integer_conversion.jsonnet

@@ -1,11 +1,11 @@
 // Test values at boundary of safe integer range
-local max_safe = 9007199254740992; // 2^53
-local min_safe = -9007199254740992; // -2^53
+local max_safe = 9007199254740991;  // 2^53 - 1
+local min_safe = -9007199254740991;  // -(2^53 - 1)
 
-std.assertEqual(max_safe & 1, 0) && // Check 2^53
-std.assertEqual(min_safe & 1, 0) && // Check -2^53
-std.assertEqual((max_safe - 1) & 1, 1) && // Check 2^53 - 1
-std.assertEqual((min_safe + 1) & 1, 1) && // Check -2^53 + 1
+std.assertEqual(max_safe & 1, 1) &&  // Check 2^53
+std.assertEqual(min_safe & 1, 1) &&  // Check -2^53
+std.assertEqual((max_safe - 1) & 1, 0) &&  // Check 2^53 - 1
+std.assertEqual((min_safe + 1) & 1, 0) &&  // Check -2^53
 
 std.assertEqual(~(max_safe - 1), min_safe) &&  // ~(2^53 - 1) == -2^53
 std.assertEqual(~(min_safe + 1), max_safe - 2) &&  // ~(-2^53 + 1) == 2^53 - 2
@@ -17,18 +17,18 @@ std.assertEqual(~(-1), 0) &&
 
 // Test shift operations with large values at safe boundary
 // (2^53 - 1) right shift by 4 bits
-std.assertEqual((max_safe - 1) >> 4, 562949953421311) &&
-// MAX_SAFE_INTEGER (2^53) right shift by 1 bit
-std.assertEqual(max_safe >> 1, 4503599627370496) && // 2^52
-// MIN_SAFE_INTEGER (-2^53) right shift by 1 bit
-std.assertEqual(min_safe >> 1, -4503599627370496) && // -2^52
+std.assertEqual(max_safe >> 4, 562949953421311) &&
+// MAX_SAFE_INTEGER (2^53 - 1) right shift by 1 bit is (2^52 - 1)
+std.assertEqual(max_safe >> 1, 4503599627370495) &&  // 2^52
+// MIN_SAFE_INTEGER -(2^53 - 1) right shift by 1 bit is (-2^52)
+std.assertEqual(min_safe >> 1, -4503599627370496) &&  // -2^52
 
 // Cannot left shift 2^53 without potential overflow/loss of precision issues
 // depending on the shift amount, but can shift smaller numbers up to it.
 // (2^52) left shift by 1 bit (result is 2^53)
-std.assertEqual((max_safe >> 1) << 1, max_safe) &&
-// (-2^52) left shift by 1 bit (result is -2^53)
-std.assertEqual((min_safe >> 1) << 1, min_safe) &&
+std.assertEqual(4503599627370495 << 1, max_safe - 1) &&
+// (-(2^52-1)) left shift by 1 bit (result is -(2^53-2))
+std.assertEqual((-4503599627370495) << 1, min_safe + 1) &&
 
 // Test larger values within safe range
 std.assertEqual(~123456789, -123456790) &&
@@ -41,4 +41,4 @@ std.assertEqual(123 ^ 456, 435) &&
 std.assertEqual(123 << 2, 492) &&
 std.assertEqual(123 >> 2, 30) &&
 
-true
+true

sjsonnet/test/src-js/sjsonnet/ErrorTests.scala

@@ -355,5 +355,23 @@ object ErrorTests extends TestSuite {
          |    at [std.manifestTomlEx].(sjsonnet/test/resources/test_suite/error.manifest_toml_null_value.jsonnet:17:19)
          |""".stripMargin
     )
+
+    test("integer_conversion") - check(
+      """|sjsonnet.Error: numeric value outside safe integer range for bitwise operation
+         |    at [BinaryOp <<].(sjsonnet/test/resources/test_suite/error.integer_conversion.jsonnet:3:12)
+         |""".stripMargin
+    )
+
+    test("integer_left_shift") - check(
+      """|sjsonnet.Error: numeric value outside safe integer range for bitwise operation
+         |    at [BinaryOp <<].(sjsonnet/test/resources/test_suite/error.integer_left_shift.jsonnet:3:13)
+         |""".stripMargin
+    )
+
+    test("integer_left_shift_runtime") - check(
+      """|sjsonnet.Error: numeric value outside safe integer range for bitwise operation
+         |    at [BinaryOp <<].(sjsonnet/test/resources/test_suite/error.integer_left_shift_runtime.jsonnet:2:13)
+         |""".stripMargin
+    )
   }
 }

sjsonnet/test/src-jvm-native/ErrorTests.scala

@@ -342,5 +342,23 @@ object ErrorTests extends TestSuite {
          |    at [std.manifestTomlEx].(sjsonnet/test/resources/test_suite/error.manifest_toml_null_value.jsonnet:17:19)
          |""".stripMargin
     )
+
+    test("integer_conversion") - check(
+      """|sjsonnet.Error: numeric value outside safe integer range for bitwise operation
+         |    at [BinaryOp <<].(sjsonnet/test/resources/test_suite/error.integer_conversion.jsonnet:3:12)
+         |""".stripMargin
+    )
+
+    test("integer_left_shift") - check(
+      """|sjsonnet.Error: numeric value outside safe integer range for bitwise operation
+         |    at [BinaryOp <<].(sjsonnet/test/resources/test_suite/error.integer_left_shift.jsonnet:3:13)
+         |""".stripMargin
+    )
+
+    test("integer_left_shift_runtime") - check(
+      """|sjsonnet.Error: numeric value outside safe integer range for bitwise operation
+         |    at [BinaryOp <<].(sjsonnet/test/resources/test_suite/error.integer_left_shift_runtime.jsonnet:2:13)
+         |""".stripMargin
+    )
   }
 }

sjsonnet/test/src/sjsonnet/Std0150FunctionsTests.scala

@@ -158,7 +158,6 @@ object Std0150FunctionsTests extends TestSuite {
       ujson.Str("j s o n n e t this is")
       eval("std.foldr(function(v, i) i + v + v, 'bcd', 'a')") ==> ujson.Str("addccbb")
 
-
       eval("""std.foldl(function (acc, it) acc + " " + it, "jsonnet", "this is")""") ==>
       ujson.Str("this is j s o n n e t")
     }