Skip to content

fix: << overflow check for large shift amounts (multiples of 64)#969

Open
He-Pin wants to merge 1 commit into
databricks:masterfrom
He-Pin:fix/leftshift-overflow-large-amount
Open

fix: << overflow check for large shift amounts (multiples of 64)#969
He-Pin wants to merge 1 commit into
databricks:masterfrom
He-Pin:fix/leftshift-overflow-large-amount

Conversation

@He-Pin

@He-Pin He-Pin commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix << operator falsely error when shift amount is a multiple of 64.

Motivation

The << operator in sjsonnet incorrectly throws an overflow error when the shift amount is a multiple of 64 (e.g., 1 << 64, 0 << 128, 1 << 192). Both go-jsonnet and jrsonnet handle these cases correctly by masking the shift amount modulo 64 (matching Java's native << behavior on long).

Root cause: The overflow check math.abs(ll) >= (1L << (63 - rr)) breaks when rr >= 64 and rr % 64 == 0. Java's << masks the shift count to 6 bits, so 1L << (63 - 64) becomes 1L << 63 = Long.MIN_VALUE (negative). Since math.abs(ll) >= Long.MIN_VALUE is always true, a false overflow error is raised.

Modification

Mask the shift amount to rr % 64 before applying the overflow check, in both tryInlineArith and visitBinaryOp in Evaluator.scala. This matches the semantics of go-jsonnet and jrsonnet.

Result

Expressions like 1 << 64, 0 << 128, 1 << 192 now return correct results instead of false errors. Genuine overflows like 3 << 126 still error correctly.

Behavior Comparison

Expression sjsonnet (before) sjsonnet (after) go-jsonnet v0.22.0 jrsonnet 0.5.0-pre99 jsonnet-cpp
1 << 64 ERROR (bug) 1 1 1 N/A (not available locally)
0 << 64 ERROR (bug) 0 0 0 N/A
0 << 1000 ERROR (bug) 0 0 0 N/A
1 << 128 ERROR (bug) 1 1 1 N/A
1 << 192 ERROR (bug) 1 1 1 N/A
4.5 << 66 16 16 16 16 N/A
3 << 126 error (correct) error (correct) -4611686018427387904 error N/A
1 << 63 error (correct) error (correct) -9223372036854775808 error N/A

Note: go-jsonnet allows signed overflow for 3 << 126 and 1 << 63, producing negative results via two's complement wraparound. Both sjsonnet and jrsonnet reject these as overflow. This PR aligns sjsonnet with jrsonnet's stricter behavior, which is the safer choice.

jsonnet-cpp was not available locally for comparison.

Test plan

  • 1 << 64 returns 1 (was false error)
  • 0 << 64 returns 0 (was false error)
  • 0 << 1000 returns 0 (was false error)
  • 1 << 128 returns 1 (was false error)
  • 1 << 192 returns 1 (was false error)
  • 4.5 << 66 returns 16
  • 3 << 126 still correctly errors on overflow
  • All FileTests and EvaluatorTests pass
  • Code passes scalafmt

Motivation:
The << operator falsely errors when the shift amount is a multiple of 64
(e.g. 1 << 64, 0 << 128). Root cause: Java's << masks shift count to 6
bits, so 1L << (63 - 64) becomes 1L << 63 = Long.MIN_VALUE (negative),
making math.abs(ll) >= Long.MIN_VALUE always true — a false overflow.

Modification:
Mask shift amount to rr % 64 before applying the overflow check, in both
tryInlineArith and visitBinaryOp in Evaluator.scala. This matches the
semantics of go-jsonnet and jrsonnet.

Result:
Expressions like 1 << 64, 0 << 128, 1 << 192 now return correct results
instead of false errors. Genuine overflows like 3 << 126 still error
correctly.

| Expression | Before  | After | go-jsonnet | jrsonnet |
|-----------|---------|-------|------------|----------|
| 1 << 64   | ERROR   | 1     | 1          | 1        |
| 0 << 128  | ERROR   | 0     | 0          | 0        |
| 1 << 192  | ERROR   | 1     | 1          | 1        |
| 3 << 126  | error   | error | wraps      | error    |
| 4.5 << 66 | 16      | 16    | 16         | 16       |
@He-Pin He-Pin force-pushed the fix/leftshift-overflow-large-amount branch from adf07c6 to 3e84390 Compare June 18, 2026 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant