mirror of
https://github.com/open-policy-agent/opa.git
synced 2026-08-28 03:05:04 -06:00
topdown: Fix false modulo by zero for multiples of 2^64 (#8989)
### Why the changes in this PR are needed? `x % y` raises a spurious `modulo by zero` error whenever `y` is a nonzero multiple of 2^64: ```rego 10 % 18446744073709551616 # 2^64 -> error "modulo by zero", want 10 5 % 55340232221128654848 # 3*2^64 -> error, want 5 7 % 340282366920938463463374607431768211456 # 2^128 -> error, want 7 100 % -18446744073709551616 # -2^64 -> error, want 100 ``` `arithRem` checks for a zero divisor with `b.Int64() == 0`. `big.Int.Int64()` returns the low 64 bits when the value does not fit in an int64, and those bits are zero for any multiple of 2^64, so a clearly nonzero divisor is read as zero. Big-integer modulo itself is already correct — `10 % 18446744073709551617` (2^64+1) returns 10 today — so this is the zero check misfiring, not the modulo semantics that #8887 deliberately left out of scope. ### What are the changes in this PR? - `arithRem` tests `b.Sign() == 0` instead of `b.Int64() == 0`. `Sign()` is zero only for an actual zero, so genuine `x % 0` still errors and nonzero divisors of any magnitude go through `big.Int.Rem`. - A golden case in `test-arithmetic-bignum.yaml` covering 2^64, a multiple of 2^64, 2^128, a negative multiple of 2^64, and the 2^64+1 control that already passed. It fails on `main` and passes with this change. - A WASM exception for the new case, matching the existing >64-bit arithmetic cases, since WASM cannot represent integers larger than 64 bits (#3711). ### Notes to assist PR review: `go test ./v1/topdown/` passes. The divide path is unaffected: `arithDivide` operates on a `big.Float` and already guards with `acc == big.Exact && i == 0`, so `10 / 18446744073709551616` does not hit the same issue. Signed-off-by: Sueun Cho <sueun.dev@gmail.com>
This commit is contained in:
@@ -4,3 +4,4 @@
|
||||
"strings/format_int: bignum exact (>64-bit integer, all bases + negative)": "WASM cannot represent integers larger than 64 bits (see https://github.com/open-policy-agent/opa/issues/3711); this change fixes the Go topdown builtin."
|
||||
"arithmetic/bignum exact (>64-bit integers through plus, minus, multiply)": "WASM cannot represent integers larger than 64 bits (see https://github.com/open-policy-agent/opa/issues/3711); this change fixes the Go topdown builtins."
|
||||
"aggregates/bignum exact (>64-bit integers through sum and product)": "WASM cannot represent integers larger than 64 bits (see https://github.com/open-policy-agent/opa/issues/3711); this change fixes the Go topdown builtins."
|
||||
"arithmetic/bignum modulo (divisor a nonzero multiple of 2^64)": "WASM cannot represent integers larger than 64 bits (see https://github.com/open-policy-agent/opa/issues/3711); this change fixes the Go topdown builtin."
|
||||
|
||||
@@ -34,3 +34,28 @@ cases:
|
||||
float_add: "4"
|
||||
mixed_add: "3.5"
|
||||
float_mul: "5"
|
||||
- note: "arithmetic/bignum modulo (divisor a nonzero multiple of 2^64)"
|
||||
query: data.generated.p = x
|
||||
modules:
|
||||
- |
|
||||
package generated
|
||||
|
||||
p := result if {
|
||||
result := {
|
||||
"pow64": sprintf("%v", [10 % 18446744073709551616]),
|
||||
"pow64_multiple": sprintf("%v", [5 % 55340232221128654848]),
|
||||
"pow128": sprintf("%v", [7 % 340282366920938463463374607431768211456]),
|
||||
"neg_pow64": sprintf("%v", [100 % -18446744073709551616]),
|
||||
"big_dividend": sprintf("%v", [18446744073709551617 % 18446744073709551616]),
|
||||
"nonzero_control": sprintf("%v", [10 % 18446744073709551617]),
|
||||
}
|
||||
}
|
||||
data: {}
|
||||
want_result:
|
||||
- x:
|
||||
pow64: "10"
|
||||
pow64_multiple: "5"
|
||||
pow128: "7"
|
||||
neg_pow64: "100"
|
||||
big_dividend: "1"
|
||||
nonzero_control: "10"
|
||||
|
||||
@@ -130,7 +130,10 @@ func arithDivide(a, b *big.Float) (*big.Float, error) {
|
||||
}
|
||||
|
||||
func arithRem(a, b *big.Int) (*big.Int, error) {
|
||||
if b.Int64() == 0 {
|
||||
// Sign, not Int64: Int64 returns the low 64 bits when b does not fit in an
|
||||
// int64, so any nonzero multiple of 2^64 (e.g. 10 % 18446744073709551616)
|
||||
// would be misreported as modulo by zero.
|
||||
if b.Sign() == 0 {
|
||||
return nil, errors.New("modulo by zero")
|
||||
}
|
||||
return new(big.Int).Rem(a, b), nil
|
||||
|
||||
Reference in New Issue
Block a user