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.

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

runtime: total allocation stats are managed in a uintptr which can quickly wrap around on 32-bit architectures [1.17 backport] #52688

Closed
gopherbot opened this issue May 3, 2022 · 4 comments
Closed

runtime: total allocation stats are managed in a uintptr which can quickly wrap around on 32-bit architectures [1.17 backport] #52688

gopherbot opened this issue May 3, 2022 · 4 comments
Labels
CherryPickApproved Used during the release process for point releases
Milestone
Go1.17.12

Comments

@gopherbot
Copy link

@mknyszek requested issue #52680 to be considered for backport to the next 1.17 minor release.

@gopherbot Please open backport issues for Go 1.17 and Go 1.18.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label May 3, 2022
@gopherbot gopherbot mentioned this issue May 3, 2022
Closed
@gopherbot gopherbot added this to the Go1.17.10 milestone May 3, 2022
@mknyszek
Copy link
Contributor

mknyszek commented May 3, 2022

Basically, on 32-bit platforms, allocation stats can end up completely wrong. I'm surprised this flew under the radar for so long. There's no workaround.

@dr2chase dr2chase added the CherryPickApproved Used during the release process for point releases label May 4, 2022
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label May 4, 2022
@heschi heschi modified the milestones: Go1.17.10, Go1.17.11 May 10, 2022
@toothrot
Copy link
Contributor

Just checking in. The next minor release is coming up soon, and this issue has already been bumped once. Is there any new progress?

@dmitshur dmitshur modified the milestones: Go1.17.11, Go1.17.12 Jun 1, 2022
@gopherbot
Copy link
Author

Change https://go.dev/cl/411496 mentions this issue: [release-branch.go1.17] runtime: store consistent total allocation stats as uint64

@gopherbot
Copy link
Author

Closed by merging b1be664 to release-branch.go1.17.

gopherbot pushed a commit that referenced this issue Jul 6, 2022
@mknyszek @heschi
[release-branch.go1.17] runtime: store consistent total allocation st…
b1be664 
…ats as uint64

Currently the consistent total allocation stats are managed as uintptrs,
which means they can easily overflow on 32-bit systems. Fix this by
storing these stats as uint64s. This will cause some minor performance
degradation on 32-bit systems, but there really isn't a way around this,
and it affects the correctness of the metrics we export.

For #52680.
Fixes #52688.

Change-Id: I8b1926116e899ae9f03d58e0320bcb9264945b3e
Reviewed-on: https://go-review.googlesource.com/c/go/+/411496
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases
Projects
None yet
Milestone
Go1.17.12
Development

No branches or pull requests
6 participants
@toothrot @mknyszek @dmitshur @dr2chase @gopherbot @heschi