diff --git a/common/types/int.go b/common/types/int.go index 28abedfa..4b76ce26 100644 --- a/common/types/int.go +++ b/common/types/int.go @@ -16,6 +16,7 @@ package types import ( "fmt" + "math" "reflect" "strconv" "time" @@ -63,6 +64,9 @@ func (i Int) Add(other ref.Val) ref.Val { if !ok { return ValOrErr(other, "no such overload") } + if (otherInt > 0 && i > math.MaxInt64-otherInt) || (otherInt < 0 && i < math.MinInt64-otherInt) { + return NewErr("integer overflow") + } return i + otherInt } @@ -179,6 +183,10 @@ func (i Int) Divide(other ref.Val) ref.Val { if otherInt == IntZero { return NewErr("divide by zero") } + // In twos complement, negating MinInt64 would result in a valid of MaxInt64+1. + if i == math.MinInt64 && otherInt == -1 { + return NewErr("integer overflow") + } return i / otherInt } @@ -200,6 +208,10 @@ func (i Int) Modulo(other ref.Val) ref.Val { if otherInt == IntZero { return NewErr("modulus by zero") } + // In twos complement, negating MinInt64 would result in a valid of MaxInt64+1. + if i == math.MinInt64 && otherInt == -1 { + return NewErr("integer overflow") + } return i % otherInt } @@ -209,11 +221,23 @@ func (i Int) Multiply(other ref.Val) ref.Val { if !ok { return ValOrErr(other, "no such overload") } + // Detecting multiplication overflow is more complicated than the others. The first two detect + // attempting to negate MinInt64, which would result in MaxInt64+1. The other four detect normal + // overflow conditions. + if (i == -1 && otherInt == math.MinInt64) || (otherInt == -1 && i == math.MinInt64) || + (i > 0 && otherInt > 0 && i > math.MaxInt64/otherInt) || (i > 0 && otherInt < 0 && otherInt < math.MinInt64/i) || + (i < 0 && otherInt > 0 && i < math.MinInt64/otherInt) || (i < 0 && otherInt < 0 && otherInt < math.MaxInt64/i) { + return NewErr("integer overflow") + } return i * otherInt } // Negate implements traits.Negater.Negate. func (i Int) Negate() ref.Val { + // In twos complement, negating MinInt64 would result in a valid of MaxInt64+1. + if i == math.MinInt64 { + return NewErr("integer overflow") + } return -i } @@ -223,6 +247,9 @@ func (i Int) Subtract(subtrahend ref.Val) ref.Val { if !ok { return ValOrErr(subtrahend, "no such overload") } + if (subtraInt < 0 && i > math.MaxInt64+subtraInt) || (subtraInt > 0 && i < math.MinInt64+subtraInt) { + return NewErr("integer overflow") + } return i - subtraInt } diff --git a/common/types/int_test.go b/common/types/int_test.go index d0c840d6..52db4ec2 100644 --- a/common/types/int_test.go +++ b/common/types/int_test.go @@ -34,6 +34,18 @@ func TestIntAdd(t *testing.T) { if !IsError(Int(-1).Add(String("-1"))) { t.Error("Adding non-int to int was not an error.") } + if lhs, rhs := math.MaxInt64, 1; !IsError(Int(lhs).Add(Int(rhs))) { + t.Errorf("Expected adding %d and %d to result in overflow.", lhs, rhs) + } + if lhs, rhs := math.MinInt64, -1; !IsError(Int(lhs).Add(Int(rhs))) { + t.Errorf("Expected adding %d and %d to result in overflow.", lhs, rhs) + } + if lhs, rhs := math.MaxInt64-1, 1; !Int(lhs).Add(Int(rhs)).Equal(Int(math.MaxInt64)).(Bool) { + t.Errorf("Expected adding %d and %d to yield %d", lhs, rhs, math.MaxInt64) + } + if lhs, rhs := math.MinInt64+1, -1; !Int(lhs).Add(Int(rhs)).Equal(Int(math.MinInt64)).(Bool) { + t.Errorf("Expected adding %d and %d to yield %d", lhs, rhs, math.MaxInt64) + } } func TestIntCompare(t *testing.T) { @@ -195,6 +207,9 @@ func TestIntDivide(t *testing.T) { if !IsError(Int(1).Divide(Double(-1))) { t.Error("Division permitted without express type-conversion.") } + if lhs, rhs := math.MinInt64, -1; !IsError(Int(lhs).Divide(Int(rhs))) { + t.Errorf("Expected dividing %d and %d result in overflow.", lhs, rhs) + } } func TestIntEqual(t *testing.T) { @@ -213,6 +228,9 @@ func TestIntModulo(t *testing.T) { if !IsError(Int(21).Modulo(uintZero)) { t.Error("Modulus permitted between different types without type conversion.") } + if lhs, rhs := math.MinInt64, -1; !IsError(Int(lhs).Modulo(Int(rhs))) { + t.Errorf("Expected modulo %d and %d result in overflow.", lhs, rhs) + } } func TestIntMultiply(t *testing.T) { @@ -222,12 +240,33 @@ func TestIntMultiply(t *testing.T) { if !IsError(Int(1).Multiply(Double(-4.0))) { t.Error("Multiplication permitted without express type-conversion.") } + if lhs, rhs := math.MaxInt64/2, 3; !IsError(Int(lhs).Multiply(Int(rhs))) { + t.Errorf("Expected multiplying %d and %d to result in overflow.", lhs, rhs) + } + if lhs, rhs := math.MinInt64/2, 3; !IsError(Int(lhs).Multiply(Int(rhs))) { + t.Errorf("Expected multiplying %d and %d to result in overflow.", lhs, rhs) + } + if lhs, rhs := math.MaxInt64/2, 2; !Int(lhs).Multiply(Int(rhs)).Equal(Int(math.MaxInt64 - 1)).(Bool) { + t.Errorf("Expected multiplying %d and %d to yield %d", lhs, rhs, math.MaxInt64-1) + } + if lhs, rhs := math.MinInt64/2, 2; !Int(lhs).Multiply(Int(rhs)).Equal(Int(math.MinInt64)).(Bool) { + t.Errorf("Expected multiplying %d and %d to yield %d", lhs, rhs, math.MaxInt64) + } + if lhs, rhs := math.MinInt64, -1; !IsError(Int(lhs).Multiply(Int(rhs))) { + t.Errorf("Expected multiplying %d and %d result in overflow.", lhs, rhs) + } } func TestIntNegate(t *testing.T) { if !Int(1).Negate().Equal(Int(-1)).(Bool) { t.Error("Negating int value did not succeed") } + if v := math.MinInt64; !IsError(Int(v).Negate()) { + t.Errorf("Expected negating %d to result in overflow.", v) + } + if v := math.MaxInt64; !Int(v).Negate().Equal(Int(math.MinInt64 + 1)).(Bool) { + t.Errorf("Expected negating %d to yeild %d", v, math.MinInt64+1) + } } func TestIntSubtract(t *testing.T) { @@ -237,4 +276,16 @@ func TestIntSubtract(t *testing.T) { if !IsError(Int(1).Subtract(Uint(1))) { t.Error("Subtraction permitted without express type-conversion.") } + if lhs, rhs := math.MaxInt64, -1; !IsError(Int(lhs).Subtract(Int(rhs))) { + t.Errorf("Expected subtracting %d and %d to result in overflow.", lhs, rhs) + } + if lhs, rhs := math.MinInt64, 1; !IsError(Int(lhs).Subtract(Int(rhs))) { + t.Errorf("Expected subtracting %d and %d to result in overflow.", lhs, rhs) + } + if lhs, rhs := math.MaxInt64-1, -1; !Int(lhs).Subtract(Int(rhs)).Equal(Int(math.MaxInt64)).(Bool) { + t.Errorf("Expected subtracting %d and %d to yield %d", lhs, rhs, math.MaxInt64) + } + if lhs, rhs := math.MinInt64+1, 1; !Int(lhs).Subtract(Int(rhs)).Equal(Int(math.MinInt64)).(Bool) { + t.Errorf("Expected subtracting %d and %d to yield %d", lhs, rhs, math.MaxInt64) + } } diff --git a/common/types/uint.go b/common/types/uint.go index ffa316a0..a313d917 100644 --- a/common/types/uint.go +++ b/common/types/uint.go @@ -57,6 +57,9 @@ func (i Uint) Add(other ref.Val) ref.Val { if !ok { return ValOrErr(other, "no such overload") } + if otherUint > 0 && i > math.MaxUint64-otherUint { + return NewErr("unsigned integer overflow") + } return i + otherUint } @@ -183,6 +186,9 @@ func (i Uint) Multiply(other ref.Val) ref.Val { if !ok { return ValOrErr(other, "no such overload") } + if otherUint != 0 && i > math.MaxUint64/otherUint { + return NewErr("unsigned integer overflow") + } return i * otherUint } @@ -192,6 +198,9 @@ func (i Uint) Subtract(subtrahend ref.Val) ref.Val { if !ok { return ValOrErr(subtrahend, "no such overload") } + if subtraUint > i { + return NewErr("unsigned integer overflow") + } return i - subtraUint } diff --git a/common/types/uint_test.go b/common/types/uint_test.go index 84ea1974..7f37c1bf 100644 --- a/common/types/uint_test.go +++ b/common/types/uint_test.go @@ -33,6 +33,12 @@ func TestUint_Add(t *testing.T) { if !IsError(Uint(1).Add(String("-1"))) { t.Error("Adding non-uint to uint was not an error.") } + if lhs, rhs := uint64(math.MaxUint64), 1; !IsError(Uint(lhs).Add(Uint(rhs))) { + t.Errorf("Expected adding %d and %d to result in overflow.", lhs, rhs) + } + if lhs, rhs := uint64(math.MaxUint64-1), 1; !Uint(lhs).Add(Uint(rhs)).Equal(Uint(math.MaxUint64)).(Bool) { + t.Errorf("Expected adding %d and %d to yield %d", lhs, rhs, uint64(math.MaxUint64)) + } } func TestUint_Compare(t *testing.T) { @@ -193,6 +199,12 @@ func TestUint_Multiply(t *testing.T) { if !IsError(Uint(1).Multiply(Double(-4.0))) { t.Error("Multiplication permitted without express type-conversion.") } + if lhs, rhs := uint64(math.MaxUint64/2), 3; !IsError(Uint(lhs).Multiply(Uint(rhs))) { + t.Errorf("Expected multiplying %d and %d to result in overflow.", lhs, rhs) + } + if lhs, rhs := uint64(math.MaxUint64/2), 2; !Uint(lhs).Multiply(Uint(rhs)).Equal(Uint(uint64(math.MaxUint64 - 1))).(Bool) { + t.Errorf("Expected multiplying %d and %d to yield %d", lhs, rhs, uint64(math.MaxUint64-1)) + } } func TestUint_Subtract(t *testing.T) { @@ -202,4 +214,10 @@ func TestUint_Subtract(t *testing.T) { if !IsError(Uint(1).Subtract(Int(1))) { t.Error("Subtraction permitted without express type-conversion.") } + if lhs, rhs := uint64(math.MaxUint64-1), uint64(math.MaxUint64); !IsError(Uint(lhs).Subtract(Uint(rhs))) { + t.Errorf("Expected subtracting %d and %d to result in overflow.", lhs, rhs) + } + if lhs, rhs := uint64(math.MaxUint64), uint64(math.MaxUint64); !Uint(lhs).Subtract(Uint(rhs)).Equal(Uint(0)).(Bool) { + t.Errorf("Expected subtracting %d and %d to yield %d", lhs, rhs, 0) + } } diff --git a/conformance/BUILD.bazel b/conformance/BUILD.bazel index e9e9bc6b..e2d5bfbd 100644 --- a/conformance/BUILD.bazel +++ b/conformance/BUILD.bazel @@ -15,7 +15,6 @@ sh_test( "--skip_test=enums/strong_proto2", "--skip_test=enums/strong_proto3", "--skip_test=fields/qualified_identifier_resolution/map_key_float,map_key_null,map_value_repeat_key", - "--skip_test=integer_math/int64_math/int64_overflow_positive,int64_overflow_negative,uint64_overflow_positive,uint64_overflow_negative,int64_min_negate", "--skip_test=timestamps/timestamp_range/from_string_under,add_duration_under,add_duration_over", "$(location @com_google_cel_spec//tests/simple:testdata/plumbing.textproto)", "$(location @com_google_cel_spec//tests/simple:testdata/basic.textproto)",