Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

TestProcStatLimits: Int overflow on 32 bits arches #432

Open
eclipseo opened this issue Jan 14, 2022 · 2 comments
Open

TestProcStatLimits: Int overflow on 32 bits arches #432

eclipseo opened this issue Jan 14, 2022 · 2 comments
Labels
bug help wanted

Comments

@eclipseo
Copy link
Contributor

On both i686 and armv7hl, TestProcStatLimits overflows:

# github.com/prometheus/procfs [github.com/prometheus/procfs.test]
./proc_stat_test.go:97:49: cannot use math.MinInt64 (untyped int constant -9223372036854775808) as int value in struct literal (overflows)
./proc_stat_test.go:98:51: cannot use math.MaxInt64 (untyped int constant 9223372036854775807) as int value in struct literal (overflows)
TimRots added a commit to TimRots/procfs that referenced this issue Mar 9, 2022
@TimRots
proc_stat_test.go: architecture dependent MinInt and MaxInt constants
b608f80 
to avoid `math.MinInt64` and `math.MaxInt64` overflowing `int` type
when running `TestProcStatLimits` on 32bit architectures:
```
./proc_stat_test.go:97:49: cannot use math.MinInt64 (untyped int constant -9223372036854775808) as int value in struct literal (overflows)
./proc_stat_test.go:98:51: cannot use math.MaxInt64 (untyped int constant 9223372036854775807) as int value in struct literal (overflows)
```

Fixes: prometheus#432

Signed-off-by: Tim Rots <tim.rots@protonmail.ch>
@TimRots TimRots mentioned this issue Mar 9, 2022
Closed
@dswarbrick
Copy link
Contributor

Linking to PR #404 which introduced this regression.

The crux of the issue is that the shipped testdata fixtures will overflow ints in the ProcStat struct on 32-bit archs.

@dswarbrick
Copy link
Contributor

In terms of fixing this, we ran into a pretty similar situation before, which I fixed in #314, i.e. split the tests into 32-bit and 64-bit variants, using Go build tags to conditionally perform the right test on the right arch.

I guess it boils down to how imperative it is that we ship testdata fixtures which exercise the full range of machine-native ints / uints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

proc_stat_test.go: architecture dependent MinInt and MaxInt constants TimRots/procfs
3 participants
@discordianfish @dswarbrick @eclipseo