google / google/gapid

core/math/sint: Abs(-2147483648) returns negative value

Open
#3,866 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
2.2k
Forks
329
PR merge metrics
No merged PRs in 30d

Description

**Environment information:**
- GAPID version: latest commit (bee6c18117577f871a4737a6be77af04c6f11577)

**Bug description**
The `core/math/sint.Abs` function returns a negative value (`-2147483648`) when given the input `-2147483648`.

From [core/math/sint/sint.go](https://github.com/google/gapid/blob/bee6c18117577f871a4737a6be77af04c6f11577/core/math/sint/sint.go#L17):
```go
func Abs(a int) int {
if a < 0 {
return -a
}
return a
}
```

**Reproduction steps**
Steps to reproduce the behavior:

```go
package main

import (
"fmt"

"github.com/google/gapid/core/math/sint"
)

func main() {
fmt.Println("sint.Abs result:", sint.Abs(-2147483648))
// Outputs: -2147483648
}
```

**Discussion:**

As the absolute value of `-2147483648` is outside of the range of positive values of 32-bit signed integers, the value overflows back into the negative range.

There are a few potential solutions to this problem, none entirely preferable.
1. add a doc comment to `Abs` stating that this is expected behaviour and let users handle the edge case
2. panic in `Abs` if the output value is negative
3. change the signature of `Abs` to `func Abs(int) (int, bool)` where the boolean return value states whether the operation was successful
4. return a value other than `-2147483648` (e.g. `0`), however this may be the most confusion of all potential solutions, so not really relevant as a solution.

I am not expecting a fix, but would welcome a discussion on this topic, and perhaps we can update the documentation of `Abs` based on the outcome discussion.

Cheers,
Robin

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.