From d5164d893d9feaefc5b5b46ee4ed5906517cea40 Mon Sep 17 00:00:00 2001 From: Sueun Cho Date: Tue, 11 Aug 2026 23:40:50 +0900 Subject: [PATCH] topdown: fix sum overflow when integer elements fit int64 but the sum does not (#8987) ## Description `sum` has an integer fast path that accumulates elements into a plain Go `int`. Any element that fits a machine int takes this path, so a running total that exceeds int64 wraps silently: ```rego sum([9223372036854775807, 1]) # -9223372036854775808 (should be 9223372036854775808) sum({9223372036854775807, 1, 2}) # -9223372036854775806 (should be 9223372036854775810) ``` `plus` is correct for the same values (`9223372036854775807 + 1` is `9223372036854775808`), so `sum` and `+` disagree. #8887 (fixes #6281) added exact `big.Int` accumulation for elements that are individually larger than 64 bits, but that fallback only runs when an element does not fit a machine int (`n.Int()` fails). When every element fits int64 and only the running total overflows, the fast path is still taken and wraps. ## Fix Guard the fast-path addition and fall back to the existing `exactIntAccumulate` big.Int path on overflow, the accumulator `product` already uses. Small-int, float, mixed, and >64-bit-element inputs are unchanged. ## Test Extended `v1/test/cases/testdata/v1/aggregates/test-aggregates-bignum.yaml` with a case where each element fits int64 but the sum does not: array and set overflow, negative overflow, an at-limit value that must stay on the fast path, and a `+` control. Results are rendered with `sprintf` because the golden-case loader parses expected numbers as float64. The case fails on `main` and passes with this change. Added the matching WASM exception (#3711), as #8887 did, since the result exceeds 64 bits. `go test ./v1/topdown/` passes. Signed-off-by: Sueun Cho --- internal/wasm/sdk/test/e2e/exceptions.yaml | 1 + .../v1/aggregates/test-aggregates-bignum.yaml | 26 ++++++++++++++ v1/topdown/aggregates.go | 34 ++++++++++++++----- 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/internal/wasm/sdk/test/e2e/exceptions.yaml b/internal/wasm/sdk/test/e2e/exceptions.yaml index 3b58aa6a16..d6650e5360 100644 --- a/internal/wasm/sdk/test/e2e/exceptions.yaml +++ b/internal/wasm/sdk/test/e2e/exceptions.yaml @@ -5,3 +5,4 @@ "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." +"aggregates/sum integer fast-path overflow (elements fit int64, sum does not)": "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." diff --git a/v1/test/cases/testdata/v1/aggregates/test-aggregates-bignum.yaml b/v1/test/cases/testdata/v1/aggregates/test-aggregates-bignum.yaml index afd758380b..30e2d09f89 100644 --- a/v1/test/cases/testdata/v1/aggregates/test-aggregates-bignum.yaml +++ b/v1/test/cases/testdata/v1/aggregates/test-aggregates-bignum.yaml @@ -32,3 +32,29 @@ cases: sum_float: "4" sum_mixed: "3.5" product_float: "3" + # Every element fits in a machine int, but the running sum does not. The sum + # fast path accumulated into a plain int and wrapped silently; it now falls + # back to exact big.Int accumulation, matching the plus builtin. + - note: "aggregates/sum integer fast-path overflow (elements fit int64, sum does not)" + query: data.generated.p = x + modules: + - | + package generated + + p := result if { + result := { + "sum_array": sprintf("%v", [sum([9223372036854775807, 1])]), + "sum_set": sprintf("%v", [sum({9223372036854775807, 1, 2})]), + "sum_negative": sprintf("%v", [sum([-9223372036854775808, -1])]), + "sum_at_limit": sprintf("%v", [sum([9223372036854775806, 1])]), + "plus_control": sprintf("%v", [9223372036854775807 + 1]), + } + } + data: {} + want_result: + - x: + sum_array: "9223372036854775808" + sum_set: "9223372036854775810" + sum_negative: "-9223372036854775809" + sum_at_limit: "9223372036854775807" + plus_control: "9223372036854775808" diff --git a/v1/topdown/aggregates.go b/v1/topdown/aggregates.go index f018057213..65fda29bce 100644 --- a/v1/topdown/aggregates.go +++ b/v1/topdown/aggregates.go @@ -5,6 +5,7 @@ package topdown import ( + "math" "math/big" "github.com/open-policy-agent/opa/v1/ast" @@ -63,25 +64,38 @@ func exactIntAccumulate(a termIterable, init int64, op func(z, x, y *big.Int) *b return builtins.IntToNumber(acc), true } +// addInt returns x+y, reporting false if the sum overflows an int so the caller +// can fall back to exact big.Int accumulation instead of wrapping silently. +func addInt(x, y int) (int, bool) { + if (y > 0 && x > math.MaxInt-y) || (y < 0 && x < math.MinInt-y) { + return 0, false + } + return x + y, true +} + func builtinSum(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Term) error) error { switch a := operands[0].Value.(type) { case *ast.Array: // Fast path for arrays of integers is := 0 - nonInts := a.Until(func(x *ast.Term) bool { + bail := a.Until(func(x *ast.Term) bool { if n, ok := x.Value.(ast.Number); ok { if i, ok := n.Int(); ok { - is += i - return false + if s, ok := addInt(is, i); ok { + is = s + return false + } } } return true }) - if !nonInts { + if !bail { return iter(ast.InternedTerm(is)) } - // Non-integer values found, so we need to sum as floats. + // A non-integer element, or an integer sum that would overflow the + // machine int: accumulate on exact big.Ints, falling back to floats for + // genuinely non-integer input. if n, ok := exactIntAccumulate(a, 0, (*big.Int).Add); ok { return iter(ast.NewTerm(n)) } @@ -103,16 +117,18 @@ func builtinSum(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Term) err case ast.Set: // Fast path for sets of integers is := 0 - nonInts := a.Until(func(x *ast.Term) bool { + bail := a.Until(func(x *ast.Term) bool { if n, ok := x.Value.(ast.Number); ok { if i, ok := n.Int(); ok { - is += i - return false + if s, ok := addInt(is, i); ok { + is = s + return false + } } } return true }) - if !nonInts { + if !bail { return iter(ast.InternedTerm(is)) }