Skip to content

Test failure in test/finprofiled.go --- integer divide by zero #64153

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

Closed
RuinanSun opened this issue Nov 15, 2023 · 5 comments
Closed

Test failure in test/finprofiled.go --- integer divide by zero #64153

RuinanSun opened this issue Nov 15, 2023 · 5 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone

Comments

@RuinanSun
Copy link
Contributor

RuinanSun commented Nov 15, 2023

What version of Go are you using (go version)?

$ go version
go version devel go1.22-12051f7d95 Wed Nov 15 04:10:05 2023 +0000 linux/arm64

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/home/ruisun01/.cache/go-build'
GOENV='/home/ruisun01/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/ruisun01/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/ruisun01/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/ruisun01/work/aarch64/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/ruisun01/work/aarch64/go/pkg/tool/linux_arm64'
GOVCS=''
GOVERSION='devel go1.22-12051f7d95 Wed Nov 15 04:10:05 2023 +0000'
GCCGO='gccgo'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/ruisun01/work/aarch64/go/src/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3780787530=/tmp/go-build -gno-record-gcc-switches'

What did you do?

run all.bash, or run go test cmd/internal/testdir -run Test/finprofiled.go

What did you expect to see?

All tests pass

What did you see instead?

--- FAIL: Test (0.08s)
    --- FAIL: Test/finprofiled.go (2.13s)
        testdir_test.go:142: exit status 2
            panic: runtime error: integer divide by zero

            goroutine 1 [running]:
            main.main()
            	/home/ruisun01/work/aarch64/go/test/finprofiled.go:60 +0x2d4

FAIL
FAIL	cmd/internal/testdir	2.240s
FAIL

Some thoughts:

I'm not very familiar with GC and memory profiler, but in finprofiled.go:60, the code truly does not check the value of nobj:

// finprofiled.go
...
for _, p := range prof {
	bytes := p.AllocBytes - p.FreeBytes
	nobj := p.AllocObjects - p.FreeObjects
	// println("bytes: ", bytes, "\t\tnobj: ", nobj)
	size := bytes / nobj // can nobj be zero?
	if size == tinyBlockSize {
		totalBytes += bytes
	}
}
...

I try to reproduce it in other machines(x86, darwin/arm64) but it only occurs on my arm64 container. So I have to say I can't make sure whether it's caused by my container or it's just a simple bug. Here are some info I just collected:

  • I git bisect and it points at CL 538217

  • Here is the log if I uncomment the println:

--- FAIL: Test (0.05s)
    --- FAIL: Test/finprofiled.go (2.89s)
        testdir_test.go:142: exit status 2
            bytes:  24 		nobj:  3
            bytes:  16 		nobj:  2
            bytes:  80 		nobj:  1
            bytes:  16 		nobj:  1
            bytes:  576 		nobj:  6
            bytes:  416 		nobj:  1
            bytes:  416 		nobj:  1
            bytes:  1400 		nobj:  1
            bytes:  6240 		nobj:  15
            bytes:  6240 		nobj:  15
            bytes:  21000 		nobj:  15
            bytes:  4194320 		nobj:  262145
            bytes:  5824 		nobj:  14
            bytes:  5824 		nobj:  14
            bytes:  19600 		nobj:  14
            bytes:  2296 		nobj:  1
            bytes:  -8 		nobj:  0 # Is that valid?
            panic: runtime error: integer divide by zero

            goroutine 1 [running]:
            main.main()
            	/home/ruisun01/work/aarch64/go/test/finprofiled.go:61 +0x340

FAIL
FAIL	cmd/internal/testdir	2.947s
FAIL
@bcmills bcmills added the compiler/runtime Issues related to the Go compiler and/or runtime. label Nov 15, 2023
@bcmills
Copy link
Contributor

bcmills commented Nov 15, 2023

I git bisect and it points at CL 538217

Marking as release-blocker since that is a significant change during the Go 1.22 development cycle.

@bcmills bcmills added this to the Go1.22 milestone Nov 15, 2023
@bcmills bcmills added release-blocker NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Nov 15, 2023
@mknyszek mknyszek self-assigned this Nov 15, 2023
@mknyszek
Copy link
Contributor

This is odd. The negative bytes suggests an accounting error for the alloc header. I'll take a look.

@mknyszek
Copy link
Contributor

OK there's a fairly clear (to me) accounting error for memory profiles specifically. That explains the -8 and it's easy to fix. However, I do think that this code can correctly end up having 0 objects for an individual memprofile record (which represents allocations for a particular stack trace), so there's an issue with the test.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/542735 mentions this issue: runtime: use span.elemsize for accounting in mallocgc

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/542736 mentions this issue: test: ignore MemProfileRecords with no live objects in finprofiled.go

@github-project-automation github-project-automation bot moved this from Todo to Done in Go Compiler / Runtime Nov 16, 2023
gopherbot pushed a commit that referenced this issue Nov 17, 2023
Currently the final size computed for an object in mallocgc excludes the
allocation header. This is correct in a number of cases, but definitely
wrong for memory profiling because the "free" side accounts for the full
allocation slot.

This change makes an explicit distinction between the parts of mallocgc
that care about the full allocation slot size ("the GC's accounting")
and those that don't (pointer+len should always be valid). It then
applies the appropriate size to the different forms of accounting in
mallocgc.

For #64153.

Change-Id: I481b34b2bb9ff923b59e8408ab2b8fb9025ba944
Reviewed-on: https://go-review.googlesource.com/c/go/+/542735
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Michael Knyszek <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
@golang golang locked and limited conversation to collaborators Nov 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
None yet
Development

No branches or pull requests

4 participants