Arithmetic operations choose arch-dependant type and overflow in 32bit

[diegommm]

Hi, thank you for your work and for reading. I found that arithmetic operations choose architecture-dependant types in some circumstances and can cause overflows in 32bit architectures.

Example

Given a value a of type int64 and a value b of any integer type, if we run the expression a+b then we will get int(a)+int(b), which can overflow in 32bit architectures.

Reproduction steps

First, save the following file as 32bit_test.go:

package main

import (
        "fmt"
        "math"
        "testing"

        "github.com/expr-lang/expr"
)

func TestOverflows32bit(t *testing.T) {
        p, err := expr.Compile("val64 + 0")
        if err != nil {
                t.Fatalf("compile error: %v", err)
        }
        val, err := expr.Run(p, map[string]any{
                "val64": int64(math.MaxInt32 + 1),
        })
        if err != nil {
                t.Fatalf("run error: %v", err)
        }
        str := fmt.Sprint(val)
        if str != "2147483648" {
                t.Fatalf("expected %v, got %v", "2147483648", val)
        }
}

Second, run it in a 32bit architecture docker container:

docker run --name go32 --rm -dit --platform linux/386 i386/golang:latest bash
docker exec go32 mkdir -p /go/src/test
docker cp 32bit_test.go go32:/go/src/test/main_test.go
docker exec -it go32 bash
> cd /go/src/test/
> go mod init
> go get github.com/expr-lang/expr
> go test
--- FAIL: TestOverflows32bit (0.01s)
    main_test.go:24: expected 2147483648, got -2147483648
FAIL
exit status 1
FAIL    test    0.043s

[antonmedv]

I don’t get, what results do you expect?

[diegommm]

I apologize for not clarifying my expectations. I would have expected the sum to not overflow. The issue is that there is no int64 builtin sum operation when running in 32bit architectures because if I sum two int64 values then they get converted to int, which in 32bit architectures is an int32, so it unnecessarily overflows.

I think the following example is more illustrative:

package main // run this in 32bit, you can use the docker commands from the description

import (
        "fmt"
        "math"
        "testing"

        "github.com/expr-lang/expr"
)

func TestOverflows32bit(t *testing.T) {
        p, err := expr.Compile("maxInt32AsInt64 + oneAsInt64") // int64 + int64 overflows in 32bit
        if err != nil {
                t.Fatalf("compile error: %v", err)
        }
        val, err := expr.Run(p, map[string]any{
                "maxInt32AsInt64": int64(math.MaxInt32),
                "oneAsInt64":      int64(1),
        })
        if err != nil {
                t.Fatalf("run error: %v", err)
        }
        str := fmt.Sprint(val)
        if str != "2147483648" { // succeeds in 64bit, overflows in 32bit, producing -2147483648
                t.Fatalf("expected %v, got %v", "2147483648", val)
        }
}

[diegommm]

In the code for Add in vm/runtime/helpers[generated].go it shows (roughly):

func Add(a, b interface{}) interface{} {
	switch x := a.(type) {
	//...
	case int64:
		switch y := b.(type) {
		// ...
		case int64:
			return int(x) + int(y)
		}
	}
}

The same happens for other arithmetic and comparison operations.

[antonmedv]

Let be get it right:

maxInt32AsInt64 := int64(math.MaxInt32)
oneAsInt64  :=   int64(1)

So we do in Expr:

int(maxInt32AsInt64) + int(oneAsInt64)

On 32 word arch this is:

int32(maxInt32AsInt64) + int32(oneAsInt64)

But what you want is:

int64(maxInt32AsInt64) + int64(oneAsInt64)

Right?

Question: why you don't convert int64 to int32 on env creation?

[diegommm]

Question: why you don't convert int64 to int32 on env creation?

The goal is that I can have 64bit arithmetic on 32bit systems. It doesn't make sense to convert to int32 because then I can't use 64bit arithmetic.

[antonmedv]

So this is kind of of a bug. I think we can address this one in upcoming v1.18.